-
Notifications
You must be signed in to change notification settings - Fork 6k
[Impeller] Directly tessellate stroked circles. #48586
[Impeller] Directly tessellate stroked circles. #48586
Conversation
Note that I updated the filled circle golden test to include a gradient and an image shader to test those cases as the original was not testing them (the DrawLines golden test actually fills circles so they were tested there, but I thought that the test case to actually test filled circles should definitely test those cases). I also modified its background color to White to make the rendering more visible when triaging the goldens. |
: LineGeometry::ComputePixelHalfWidth( | ||
entity.GetTransform(), stroke_width_); | ||
Scalar outer_radius = radius_ + half_width; | ||
Scalar inner_radius = half_width <= 0 ? 0.0 : radius_ - half_width; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking that this might be easier to read as an if/else setting the values more directly in each code block than a series of ternary ?:
operations.
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. |
I'm leaving the golden diffs for reviewers to examine. I didn't realize that I was going to obsolete the golden images for FilledCircles when I changed the name, but I guess that makes sense. Since I updated the test with new (more complete) content, I'm not sure that the golden history matters in that case. |
I think updating the golden names is fine, it would be rare to look through the entire golden history anyway. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…139412) flutter/engine@039439c...43a1598 2023-12-02 [email protected] [Impeller] Directly tessellate stroked circles. (flutter/engine#48586) 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 [email protected],[email protected],[email protected] 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
…lutter#139412) flutter/engine@039439c...43a1598 2023-12-02 [email protected] [Impeller] Directly tessellate stroked circles. (flutter/engine#48586) 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 [email protected],[email protected],[email protected] 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
Similar work as done for filled circles in #48103, stroked circles can also be directly tessellated using the same internal data structures used for filled circles.