[Impeller] skip lineTo for empty contours. #52290
Conversation
|
Doesn't this also skip them if the path creator manually closed the path before calling Maybe add a test that verifies this happens in addition to the empty path test? |
|
Okay, we determined goldens didn't run for about a week. Rebasing and hoping to see some diffs on this.
Good idea. Will add a few of these as non-golden unit tests |
|
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. |
jonahwilliams
added
the
autosubmit
| EXPECT_EQ(path.GetComponentCount(Path::ComponentType::kLinear), 2u); | ||
| EXPECT_EQ(path.GetComponentCount(Path::ComponentType::kContour), 2u); | ||
| } | ||
|
|
There was a problem hiding this comment.
I don't see the test I was looking for, namely:
MoveTo(0, 0)
LineTo(10, 0)
LineTo(10, 10)
LineTo(0, 10)
LineTo(0, 0)
Close()
should have 4 lines in it because the last LineTo returned the pen to the starting position.
There was a problem hiding this comment.
Look at PathBuilder::AddOval for an example of this that is less degenerate. Drawing the full circle requires the path definition to return to the original point. An oval/circle cannot rely on the implicit Close() segment to finish the path, but previously it got a zero-length segment on the close. With this change that useless segment is no longer added, but we have no tests that ensure this improved behavior.
…147203) flutter/engine@33c828f...62c9f17 2024-04-22 jonahwilliams@google.com [Impeller] skip lineTo for empty contours. (flutter/engine#52290) 2024-04-22 skia-flutter-autoroll@skia.org Roll Skia from 3b32e3280b67 to e6de04b82b57 (6 revisions) (flutter/engine#52304) 2024-04-22 jonahwilliams@google.com [Impeller] re-enable gold CTL. (flutter/engine#52299) 2024-04-22 skia-flutter-autoroll@skia.org Roll Dart SDK from 0ed66a4d77cb to 95f95b3118fe (1 revision) (flutter/engine#52301) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC jsimmons@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
If we close an empty contour, don't insert a lineTo
Fixes flutter/flutter#146648