UberSDF thin roundrect handling#189224
Merged
Merged
Conversation
github-actions
Bot
added
engine
Contributor
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
b-luk
force-pushed
the
ubersdfthinroundrect
branch
from
July 9, 2026 23:16
804ef70 to
7fc010b
Compare
Contributor
Author
|
/gemini review |
Contributor
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
flar
previously approved these changes
Jul 10, 2026
flar
left a comment
flar
left a comment
Contributor
There was a problem hiding this comment.
Mainly a comment nit and something I think deserves a separate issue where we'll have to ask hard questions...
| return rect; | ||
| } | ||
|
|
||
| // Convert expanded rectangle from pixel size back to local size. |
Contributor
There was a problem hiding this comment.
"device size" seems a better term here than "pixel size". Since "device" refers to a coordinate space and "pixel" refers to a specific measurement (the device rectangle might be a pixel in size, but it also will usually be much bigger).
| } | ||
| } | ||
|
|
||
| TEST_P(AiksTest, UpscaledRectReturnsEmptyForEmptyRect) { |
Contributor
There was a problem hiding this comment.
These tests will be run with a variety of backends, but they really just test a non-backend-y method on Canvas.
Maybe if the method was static and took the transform, then these methods could be in a test suite that wasn't backend-specific?
Contributor
There was a problem hiding this comment.
Ugh, there are a ton of methods in here that do that. I'll file an issue.
Contributor
Author
There was a problem hiding this comment.
I think making the method be static and take the transform is a good idea, and I think it's something I might need to do anyway to get UberSDF lines working. I'll try to figure out a good way to do this in my UberSDF lines PR.
|
|
||
| // Rect's device size is (0.2 * 2, 10.0 * 4) = (0.4, 40.0) pixels. | ||
| // It's expanded to (1.0, 40.0) pixels. | ||
| // In local space, this corresponds to (1.0 / 2.0, 40.0 / 4.0) = (0.5, 10.0). |
Contributor
|
Goldens look pretty good. |
Contributor
Author
|
PTAL. Needs re-approval after updating the comment :( |
|
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. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
flutter-dashboard
Bot
removed
the
autosubmit
This was referenced Jul 11, 2026
auto-submit Bot
pushed a commit
to flutter/packages
that referenced
this pull request
Jul 12, 2026
flutter/flutter@f7b66f3...cf9e8af 2026-07-12 [email protected] Flip from deprecated strict analysis modes to lint rules (flutter/flutter#187692) 2026-07-12 [email protected] Roll Skia from 6ecffccd32d5 to 8bf65996caba (5 revisions) (flutter/flutter#189333) 2026-07-11 [email protected] Roll Fuchsia Linux SDK from czpzDg9ABY2oKLAOY... to vhIlDkWIy21IrlB9E... (flutter/flutter#189309) 2026-07-11 [email protected] iOS: clean up swiftc.py after migration to target triples (flutter/flutter#189240) 2026-07-10 [email protected] Increase number of 'Mac tool_integration_tests' subshards from 5 to 10 (flutter/flutter#189040) 2026-07-10 [email protected] Fix PopupWindowControllerLinux missing WindowControllerLinux implementation (flutter/flutter#189189) 2026-07-10 [email protected] [android_hardware_smoke_test] Synchronize platform view compositing (flutter/flutter#189151) 2026-07-10 [email protected] UberSDF thin roundrect handling (flutter/flutter#189224) 2026-07-10 [email protected] Roll Skia from ab3a7b98c94d to 6ecffccd32d5 (15 revisions) (flutter/flutter#189270) 2026-07-10 [email protected] [skwasm][canvaskit] Honor Paint.filterQuality in Canvas.drawAtlas (flutter/flutter#186108) 2026-07-10 [email protected] Android_hardware_smoke_test: Cache flutter engine when running vulkan backend instrumented tests (flutter/flutter#189026) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: 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
10 tasks
auto-submit Bot
pushed a commit
that referenced
this pull request
Jul 15, 2026
This pull request is created by [automatic cherry pick workflow](https://github.com/flutter/flutter/blob/main/docs/releases/Flutter-Cherrypick-Process.md#automatically-creates-a-cherry-pick-request) Please fill in the form below, and a flutter domain expert will evaluate this cherry pick request. This is the first in a set of 3 PRs (#188821, #189224, and #188514) that will be cherry picked in order to fix a set of line-related issues when Impeller with SDF rendering is enabled. ### Issue Link: What is the link to the issue this cherry-pick is addressing? #188329 #188911 #188593 ### Impact Description: What is the impact (ex. visual jank on Samsung phones, app crash, cannot ship an iOS app)? Does it impact development (ex. flutter doctor crashes when Android Studio is installed), or the shipping of production apps (the app crashes on launch). This information is for domain experts and release engineers to understand the consequences of saying yes or no to the cherry pick. Thin ines (including lines rendered with `canvas.drawLine`, thin line-like rectangles rendered with `canvas.drawRect`, and paths drawn with `canvas.drawPath` that consist of a single line or thin rectangle) have various highly visible rendering issues: - opacity is ignored - lines with stroke width less than 1 pixel are incorrectly clamped up to be ~2 pixels wide - lines with stroke width more than 1 pixel are overly thick, with an added thickness proportional to their intended width This issue is present when impeller is enabled Impeller with SDF rendering. In the beta channel, Impeller with SDF rendering is the default when building apps for desktop platforms: macOS, Linux, and Windows. ### Changelog Description: Explain this cherry pick: * In one line that is accessible to most Flutter developers. * That describes the state prior to the fix. * That includes which platforms are impacted. See [best practices](https://github.com/flutter/flutter/blob/main/docs/releases/Hotfix-Documentation-Best-Practices.md) for examples. [flutter/188329](#188329), [flutter/188911](#188911), [flutter/188593](#188593): Multiple line rendering issues when Impeller with SDF rendering is enabled (default enabled on desktop platforms) ### Workaround: Is there a workaround for this issue? Disable Impeller, falling back to Skia rendering. ### Risk: What is the risk level of this cherry-pick? ### Test Coverage: Are you confident that your fix is well-tested by automated tests? ### Validation Steps: What are the steps to validate that this fix works? After all 3 PRs are cherry picked, verify the sample apps in the relevant issues (#188329 and #188911) render with expected opacity and with expected line widths.
b-luk
added
the
cp: beta
auto-submit Bot
pushed a commit
that referenced
this pull request
Jul 15, 2026
This pull request is created by [automatic cherry pick workflow](https://github.com/flutter/flutter/blob/main/docs/releases/Flutter-Cherrypick-Process.md#automatically-creates-a-cherry-pick-request) Please fill in the form below, and a flutter domain expert will evaluate this cherry pick request. This is the second in a set of 3 PRs (#188821, #189224, and #188514) that will be cherry picked in order to fix a set of line-related issues when Impeller with SDF rendering is enabled. The first PR was cherry picked and merged with #189517 ### Issue Link: What is the link to the issue this cherry-pick is addressing? #188329 #188911 #188593 ### Impact Description: What is the impact (ex. visual jank on Samsung phones, app crash, cannot ship an iOS app)? Does it impact development (ex. flutter doctor crashes when Android Studio is installed), or the shipping of production apps (the app crashes on launch). This information is for domain experts and release engineers to understand the consequences of saying yes or no to the cherry pick. Thin ines (including lines rendered with `canvas.drawLine`, thin line-like rectangles rendered with `canvas.drawRect`, and paths drawn with `canvas.drawPath` that consist of a single line or thin rectangle) have various highly visible rendering issues: - opacity is ignored - lines with stroke width less than 1 pixel are incorrectly clamped up to be ~2 pixels wide - lines with stroke width more than 1 pixel are overly thick, with an added thickness proportional to their intended width This issue is present when impeller is enabled Impeller with SDF rendering. In the beta channel, Impeller with SDF rendering is the default when building apps for desktop platforms: macOS, Linux, and Windows. ### Changelog Description: Explain this cherry pick: * In one line that is accessible to most Flutter developers. * That describes the state prior to the fix. * That includes which platforms are impacted. See [best practices](https://github.com/flutter/flutter/blob/main/docs/releases/Hotfix-Documentation-Best-Practices.md) for examples. [flutter/188329](#188329), [flutter/188911](#188911), [flutter/188593](#188593): Multiple line rendering issues when Impeller with SDF rendering is enabled (default enabled on desktop platforms) ### Workaround: Is there a workaround for this issue? Disable Impeller, falling back to Skia rendering. ### Risk: What is the risk level of this cherry-pick? ### Test Coverage: Are you confident that your fix is well-tested by automated tests? ### Validation Steps: What are the steps to validate that this fix works? After all 3 PRs are cherry picked, verify the sample apps in the relevant issues (#188329 and #188911) render with expected opacity and with expected line widths.
auto-submit Bot
pushed a commit
that referenced
this pull request
Jul 16, 2026
…es (#189537) This pull request is created by [automatic cherry pick workflow](https://github.com/flutter/flutter/blob/main/docs/releases/Flutter-Cherrypick-Process.md#automatically-creates-a-cherry-pick-request) Please fill in the form below, and a flutter domain expert will evaluate this cherry pick request. This is the third in a set of 3 PRs (#188821, #189224, and #188514) that will be cherry picked in order to fix a set of line-related issues when Impeller with SDF rendering is enabled. The first PR was cherry picked and merged with #189517 The second PR was cherry picked and merged with #189529 ### Issue Link: What is the link to the issue this cherry-pick is addressing? #188329 #188911 #188593 ### Impact Description: What is the impact (ex. visual jank on Samsung phones, app crash, cannot ship an iOS app)? Does it impact development (ex. flutter doctor crashes when Android Studio is installed), or the shipping of production apps (the app crashes on launch). This information is for domain experts and release engineers to understand the consequences of saying yes or no to the cherry pick. Thin ines (including lines rendered with `canvas.drawLine`, thin line-like rectangles rendered with `canvas.drawRect`, and paths drawn with `canvas.drawPath` that consist of a single line or thin rectangle) have various highly visible rendering issues: - opacity is ignored - lines with stroke width less than 1 pixel are incorrectly clamped up to be ~2 pixels wide - lines with stroke width more than 1 pixel are overly thick, with an added thickness proportional to their intended width This issue is present when impeller is enabled Impeller with SDF rendering. In the beta channel, Impeller with SDF rendering is the default when building apps for desktop platforms: macOS, Linux, and Windows. ### Changelog Description: Explain this cherry pick: * In one line that is accessible to most Flutter developers. * That describes the state prior to the fix. * That includes which platforms are impacted. See [best practices](https://github.com/flutter/flutter/blob/main/docs/releases/Hotfix-Documentation-Best-Practices.md) for examples. [flutter/188329](#188329), [flutter/188911](#188911), [flutter/188593](#188593): Multiple line rendering issues when Impeller with SDF rendering is enabled (default enabled on desktop platforms) ### Workaround: Is there a workaround for this issue? Disable Impeller, falling back to Skia rendering. ### Risk: What is the risk level of this cherry-pick? ### Test Coverage: Are you confident that your fix is well-tested by automated tests? ### Validation Steps: What are the steps to validate that this fix works? After all 3 PRs are cherry picked, verify the sample apps in the relevant issues (#188329 and #188911) render with expected opacity and with expected line widths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes thin UberSDF roundrect drawing, in the same way that #188821 fixed it for normal rects.
Prerequisite for using UberSDF for lines.
Screenshot
DrawLinesWithFilledRoundRects
Before:
After:
Video
DrawLinesWithFilledRoundRects
Before:
roundrect.ubersdf.video.before.mov
After:
roundrect.ubersdf.video.after.mov
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
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.