Skip to content

Conversation

mgrudzinska
Copy link
Collaborator

One point was skipped during the creation of the offset corner. The error was not visible because the point lies on the line, but it will become apparent if further modifiers are applied to the object (not supported now).

samples not supported now (#3378 used to render the sample):

before this change:
Zrzut ekranu 2025-06-12 o 23 17 35

after this change:
Zrzut ekranu 2025-06-12 o 23 17 17

AE:
Zrzut ekranu 2025-06-12 o 23 18 48

sample:
pucker_rect_op_miter4.json

One point was skipped during the creation of the offset
corner. The error was not visible because the point lies
on the line, but it will become apparent if further
modifiers are applied to the object (not supported now).
@mgrudzinska mgrudzinska self-assigned this Jun 12, 2025
@mgrudzinska mgrudzinska added bug Something isn't working lottie Lottie animation labels Jun 12, 2025
@hermet hermet requested a review from Copilot June 13, 2025 02:05
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Fixes a skipped corner point in the offset path when using miter joins, ensuring subsequent modifiers render correctly.

  • Refactored the miter-join branch to explicitly push an intersection point and then always push the next segment’s start.
  • Broke out the inline conditional into a clear if block followed by separate path commands.
Comments suppressed due to low confidence (3)

src/loaders/lottie/tvgLottieModifier.cpp:123

  • Consider adding a guard to ensure length(norm + nextNorm) is not near zero before normalizing, to avoid potential division by zero.
auto miterDirection = (norm + nextNorm) / length(norm + nextNorm);

src/loaders/lottie/tvgLottieModifier.cpp:128

  • [nitpick] Add a brief comment explaining why two consecutive LineTo commands are issued (first to the miter intersection, then to nextLine.pt1) to improve readability.
out.cmds.push(PathCommand::LineTo);

src/loaders/lottie/tvgLottieModifier.cpp:124

  • Consider adding unit tests for both miter and bevel join scenarios around the miter limit to verify that the intersection and nextLine.pt1 points are correctly included in the generated path.
if (1.0f <= miterLimit * fabsf(miterDirection.x * norm.x + miterDirection.y * norm.y)) {

@hermet hermet merged commit 27e7809 into main Jun 13, 2025
15 checks passed
@hermet hermet deleted the mira/off1 branch June 13, 2025 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lottie Lottie animation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants