Make the circle.frag Anti Aliasing continuous around 2px#183348
Make the circle.frag Anti Aliasing continuous around 2px#183348Hari-07 wants to merge 2 commits intoflutter:masterfrom
Conversation
github-actions
bot
added
engine
| ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); | ||
| } | ||
|
|
||
| TEST_P(AiksTest, DrawThinStrokedCircle) { |
There was a problem hiding this comment.
This test was copied from #183184 and should be removed before merging
![gemini-code-assist[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to fix a visual discontinuity in the anti-aliasing of thin stroked circles by making the fade width dependent on the stroke width. A new playground test has been added for visual verification. My review identified a potential logic error in the shader where values with different units are compared, which could result in incorrect anti-aliasing under certain scaling conditions. Additionally, I found a minor issue in the new test case where an alpha slider has an incorrect range, limiting its usability.
| ImGui::SliderFloat("Stroked Radius", &stroked_radius, 0, 500); | ||
| ImGui::SliderFloat("Stroked Width", &stroke_width, 0, 500); | ||
| ImGui::SliderFloat("Stroked Width Fine", &stroke_width_fine, 0, 5); | ||
| ImGui::SliderFloat("Stroked Alpha", &stroked_alpha, 0, 10.0); |
There was a problem hiding this comment.
The stroked_alpha variable is initialized to 255.0f and used with DlColor::withAlpha(), which expects a value in the range [0, 255]. However, the slider for this variable has a range of [0, 10.0]. This will cause the alpha value to be clamped to 10 as soon as the slider is interacted with, making the circle nearly transparent and the control hard to use as intended. The slider range should be adjusted to match the expected alpha range.
| ImGui::SliderFloat("Stroked Alpha", &stroked_alpha, 0, 10.0); | |
| ImGui::SliderFloat("Stroked Alpha", &stroked_alpha, 0, 255.0); |
gaaclarke
left a comment
gaaclarke
left a comment
There was a problem hiding this comment.
Thanks for taking the time write this up. I'm still having some discussions with the team about what we want to do with #183184. That requires not just figuring out what we want to do for a fix but also what do we want to make cherry-picked into stable, etc.
Hari-07
force-pushed
the
circle-frag-2px-bug
branch
from
26d986f to
ceb8bf6
Compare
|
Walley has addressed this issue in #183536 Thanks for the contribution, it's much appreciated. In this case Walley had a bit more background and we had some meetings around how to fix it so we let him address the root problem and a few others. Give that a shot if you want to verify it meets your requirements. I'll close this out in lieu of his. |
2 participants
Currently Circle.frag has a hard limit of 2 pixels below which it gives full opacity to the ring
So
float alpha = 1.0 - smoothstep(-fade_width, 0.0, sdf_distance);was discontinuous at that border sincesdf_distancejumped abruptly.Fixes #183339
With this update fade width varies smoothly with stroke width, until 2px at which point both are equal, eliminating a discontinuity
Screen.Recording.2026-03-07.at.1.46.23.PM.mov
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.