Skip to content

Add oval drawing support to the SDF uber shader#184903

Merged
auto-submit[bot] merged 9 commits into
flutter:masterfrom
walley892:sdf-ovals
Apr 16, 2026
Merged

Add oval drawing support to the SDF uber shader#184903
auto-submit[bot] merged 9 commits into
flutter:masterfrom
walley892:sdf-ovals

Conversation

@walley892

@walley892 walley892 commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Adds Oval rendering to the UberSDF shader, uses UberSDF oval rendering in the canvas behind the feature flag.

The SDF doesn't actually find the minimum distance to the oval, it find the distance from the test point to the center of the oval along a straight line - visually looks good though.

Fixes #183037.

Before this change:

before

After this change:
after

Diffs highlighted:

diff

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
  • All existing and new tests are passing.

@github-actions github-actions Bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Apr 10, 2026
@walley892

walley892 commented Apr 10, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

gemini-code-assist[bot]
gemini-code-assist Bot reviewed Apr 10, 2026
View reviewed changes

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request adds support for rendering ovals using Signed Distance Fields (SDFs) in Impeller, with changes spanning the canvas, parameter structures, and fragment shaders. Review feedback identifies a potential division by zero in the distanceFromOval shader function, recommends using a final else block in strokedSDF to prevent uninitialized variables, and notes a style guide violation where documentation for the public size member should use triple-slash comments.

Comment thread engine/src/flutter/impeller/entity/shaders/uber_sdf.frag Outdated
Comment thread engine/src/flutter/impeller/entity/shaders/uber_sdf.frag Outdated
Comment thread engine/src/flutter/impeller/entity/contents/uber_sdf_parameters.h Outdated
@walley892 walley892 marked this pull request as ready for review April 10, 2026 23:53
@walley892 walley892 requested review from b-luk and flar April 10, 2026 23:53
@walley892 walley892 added the CICD Run CI/CD label Apr 10, 2026
gemini-code-assist[bot]
gemini-code-assist Bot reviewed Apr 10, 2026
View reviewed changes

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for rendering ovals via the Uber SDF path in Impeller. The changes include adding oval-specific parameters, implementing a new distance function in the uber_sdf.frag shader, and updating the canvas logic to utilize these features when the SDF flag is enabled. Review feedback identified a critical numerical instability and potential division-by-zero in the shader's oval distance calculation, recommending a more robust vector-based approach. Additionally, it was noted that the canvas logic should explicitly check for the absence of a color source, as the current SDF implementation only supports solid colors.

Comment thread engine/src/flutter/impeller/entity/shaders/uber_sdf.frag Outdated
Comment thread engine/src/flutter/impeller/display_list/canvas.cc
b-luk
b-luk reviewed Apr 13, 2026
View reviewed changes

@b-luk b-luk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The SDF doesn't actually find the minimum distance to the oval, it find the distance from the test point to the center of the oval along a straight line - visually looks good though.

There's an ellipse SDF function at https://iquilezles.org/articles/distfunctions2d/. It's much more complicated than I initially would have thought. But apparently it's very complicated to get a true SDF for an ellipse. iquilezles has an entire article just about this: https://iquilezles.org/articles/ellipsedist/.

I made a shadertoy demo that compares your function to the one from iquilezles: https://www.shadertoy.com/view/7ffXDl. It switches between the two every second.

Comparing your function to the true SDF, the contours in the interior of your SDF bulge inwards towards the center at angles close to the minor axis. And on the exterior of the SDF, the contours are more diamond-shaped: at angles near the axes the contours are more sharply rounded, and at angles near the diagonals the contours are flatter. I think these distortions would show up if you draw an oval with a wide stroke width.

Interestingly, the true SDF has contours with sharp angles in the interior, along the long axis. That's not what I'd initially think the SDF would look like, but it makes sense after thinking about it for a while. This means that the interior edge of a stroked oval would have sharp corners rather than being a smooth curve, especially as the stroke width gets larger. Testing this out with a flutter app verifies that this matches what is rendered: https://dartpad.dev/?id=36e089005ba8bc1e2cefd4fc4ad7ca93

Comment thread engine/src/flutter/impeller/entity/shaders/uber_sdf.frag Outdated
float m2 = m * m;

// q is the point along that line that intersects the oval.
vec2 q = vec2(a * b / sqrt((a2 * m2) + b2), a * b * m / sqrt((a2 * m2) + b2));

@b-luk b-luk Apr 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This math is hard for me to follow. Maybe break it down a bit more?

@walley892 walley892 Apr 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added some more comments

@b-luk

b-luk commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

Actually, looking at how this change draws stroked ovals, it draws the interior between two ovals with different sizes:

    outer = distanceFromOval(p, frag_info.size + vec2(half_stroke));
    inner = distanceFromOval(p, frag_info.size - vec2(half_stroke));

So it doesn't actually use the SDF contours for drawing the stroke. So I don't think you'd see the distortions I mentioned in my previous comment. But I do think using this SDF for ovals with wide stroke widths will look significantly different from the current implementation. The SDF draws a stroked shape in which the inner and outer edges are ellipses. But with a real stroked ellipse, its inner and outer edges are not ellipses.

@github-actions github-actions Bot removed the CICD Run CI/CD label Apr 13, 2026
@walley892 walley892 added the CICD Run CI/CD label Apr 13, 2026
@b-luk

b-luk commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

Here's a comparison app showing the current drawOval vs a simulated version of your SDF by drawing ovals with size +/- half_width: https://dartpad.dev/?id=57c1b2dc7af38a63f899fb2d258703e5. I put the canvas in an InteractiveViewer widget, so you can zoom and scroll in it. Try playing around with it.

@github-actions github-actions Bot removed the CICD Run CI/CD label Apr 14, 2026
@walley892

walley892 commented Apr 14, 2026

Copy link
Copy Markdown
Contributor Author

The SDF draws a stroked shape in which the inner and outer edges are ellipses. But with a real stroked ellipse, its inner and outer edges are not ellipses

This is a great point that I didn't consider. I've backed this out to only operate on filled ovals, I think the stroked case will need more consideration

@walley892 walley892 mentioned this pull request Apr 14, 2026
Closed
@b-luk

b-luk commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

This is a great point that I didn't consider. I've backed this out to only operate on filled ovals, I think the stroked case will need more consideration

Any reason we wouldn't want to just use the ellipse SDF from iquilezles? With a true SDF, the inner and outer edges of the stroked shape would just be SDF -/+ half_stroke_width.

@walley892

walley892 commented Apr 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Any reason we wouldn't want to just use the ellipse SDF from iquilezles? With a true SDF, the inner and outer edges of the stroked shape would just be SDF -/+ half_stroke_width.

Both SDFs from iquilezles are approximations with different shortcomings. I don't really want to ctrl-c + ctrl-v without understanding those limitations first.

With a true SDF, the inner and outer edges of the stroked shape would just be SDF -/+ half_stroke_width.

I'm not sure what you mean - you're saying to just add the stroke width to the result of the SDF? I don't think that works - we'd have to do an offset by the normal of the oval, no?

@b-luk

b-luk commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

I'm not sure what you mean - you're saying to just add the stroke width to the result of the SDF? I don't think that works - we'd have to do an offset by the normal of the oval, no?

The normal of the oval doesn't need to be considered for making it stroked.

The SDF gives us the distance to the edge of the shape. So looking at the set of all points where SDF = 1, for example, would give us a shape that is exactly distance 1 outside of the base oval at every point. And at SDF = -1, we get a shape of distance 1 inside the base oval. These are exactly the outer and inner edges of stroking the base oval with stroke width 2.

We get an SDF for the outer edge shape with outer = base_sdf - 1 (so it's 0 where the base_sdf is 1, positive outside of this, and negative inside of it). And similarly for the inner shape is inner = base_sdf + 1.

@flar

flar commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

Computing an ellipse with bounds that are +/- the stroke width doesn't work as mentioned before, but I think what @b-luk is referring to is doing a +/- on the actual distance function. I've suggested elsewhere that most stroke SDFs could be done by taking the filled value and performing abs(filled_sdf) - half_stroke_width to get a value that is <0 inside the stroke and >0 outside the stroke. I think the nature of the SDF will keep this value "normal to the nearest point on the outline", but I'd like to see how it behaves on ovals that get so narrow that they pinch off the inner part of the oval on the narrow ends. That might introduce singularities in the function, but we can try it and see.

@walley892 walley892 added the CICD Run CI/CD label Apr 14, 2026
@walley892

walley892 commented Apr 14, 2026

Copy link
Copy Markdown
Contributor Author

Per discussion with @gaaclarke and @b-luk , I've lifted the SDF code directly from https://iquilezles.org/articles/ellipsedist and threw back in stroked rendering following Flar's comment

flar
flar previously approved these changes Apr 15, 2026
View reviewed changes

@flar flar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks fine, but some suggestions to take or leave...

Comment thread engine/src/flutter/impeller/display_list/canvas.cc
!paint.mask_blur_descriptor.has_value()) {
UberSDFParameters params;

if (paint.style == Paint::Style::kStroke) {

@flar flar Apr 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For simplicity we could add a std::opt<StrokeParams> getter to Paint.

@walley892 walley892 Apr 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion. Will do this in a follow up PR

Comment thread engine/src/flutter/impeller/entity/contents/uber_sdf_parameters.h Outdated
Comment thread engine/src/flutter/impeller/entity/shaders/uber_sdf.frag
// TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
// THE USE OR OTHER DEALINGS IN THE SOFTWARE.
// https://www.youtube.com/c/InigoQuilez
// https://iquilezles.org

@flar flar Apr 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think Aaron said it was acceptable to have these copyright claims in the middle of a source file, but I also like the idea of moving some of these functions to an included source file like we do with the gaussian functions. If we make one for all of these Inigo functions, we'd just need the copyright header in that included file for all of the functions attributed to them.

@walley892 walley892 Apr 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agree. Will also do this in a follow up PR

Comment thread engine/src/flutter/impeller/entity/shaders/uber_sdf.frag
inner = distanceFromRect(p, frag_info.size - half_stroke);
} else {
outer = distanceFromOval(p, frag_info.size) - half_stroke;
inner = distanceFromOval(p, frag_info.size) + half_stroke;

@flar flar Apr 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For this and circle and rect we could do return abs(distanceFromShape(...) - half_stroke

@walley892 walley892 Apr 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will update in a follow up PR (I think the record is scratched)

@github-actions github-actions Bot removed the CICD Run CI/CD label Apr 15, 2026
@walley892 walley892 added the CICD Run CI/CD label Apr 15, 2026
@walley892

walley892 commented Apr 15, 2026

Copy link
Copy Markdown
Contributor Author

Took some time to understand and annotate the Inigo function. Kept the code as-is because it's really optimized.

b-luk
b-luk previously approved these changes Apr 15, 2026
View reviewed changes
@b-luk b-luk mentioned this pull request Apr 15, 2026
Merged
10 tasks
@github-actions github-actions Bot removed the CICD Run CI/CD label Apr 15, 2026
@walley892 walley892 added the CICD Run CI/CD label Apr 15, 2026
flar
flar approved these changes Apr 15, 2026
View reviewed changes

@flar flar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions Bot removed the CICD Run CI/CD label Apr 15, 2026
@walley892 walley892 added the CICD Run CI/CD label Apr 15, 2026
@flutter-dashboard

flutter-dashboard Bot commented Apr 16, 2026

Copy link
Copy Markdown

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 package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #184903 at sha 113fbc3

@flutter-dashboard flutter-dashboard Bot added the will affect goldens Changes to golden files label Apr 16, 2026
@walley892 walley892 added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 16, 2026
@auto-submit auto-submit Bot added this pull request to the merge queue Apr 16, 2026
Merged via the queue into flutter:master with commit e206d86 Apr 16, 2026
203 of 204 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Apr 16, 2026
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 16, 2026
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 16, 2026
@engine-flutter-autoroll
e206d86db Add oval drawing support to the SDF uber shader (flutter/fl…
92d694b 
…utter#184903)
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Apr 16, 2026
@engine-flutter-autoroll
Roll Flutter from c1b14e92dcfb to 31f1802cb859 (46 revisions) (#11518)
25774d6 
Roll Flutter from c1b14e92dcfb to 31f1802cb859 (46 revisions)

flutter/flutter@c1b14e9...31f1802

2026-04-16 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Run all flutter/flutter macOS tests using Xcode 26 and iOS 26 simulator (#185083)" (flutter/flutter#185145)
2026-04-16 [email protected] Add oval drawing support to the SDF uber shader (flutter/flutter#184903)
2026-04-16 [email protected] Run all flutter/flutter macOS tests using Xcode 26 and iOS 26 simulator (flutter/flutter#185083)
2026-04-16 [email protected] Roll Skia from 2c49b3f9c3c2 to 391cdbe3ffe9 (2 revisions) (flutter/flutter#185138)
2026-04-16 [email protected] Roll Dart SDK from 4ee990654146 to fbddcbe0cd96 (1 revision) (flutter/flutter#185137)
2026-04-16 [email protected] Roll Skia from f4e3cd2c2159 to 2c49b3f9c3c2 (14 revisions) (flutter/flutter#185131)
2026-04-16 [email protected] Roll Dart SDK from 87b7c87e7207 to 4ee990654146 (5 revisions) (flutter/flutter#185108)
2026-04-15 [email protected] Use the `flutteractionsbot` token to push the release branch. (flutter/flutter#184833)
2026-04-15 [email protected] Allow period characters in iOS and macOS framework names (flutter/flutter#184335)
2026-04-15 [email protected] Fix SliverResizingHeader semantic focus (flutter/flutter#179690)
2026-04-15 [email protected] ignore avoid_type_to_string lint rule in flutter_tools (flutter/flutter#184766)
2026-04-15 [email protected] Roll Skia from bda7232e6772 to f4e3cd2c2159 (4 revisions) (flutter/flutter#185063)
2026-04-15 [email protected] Add initial AI guidance for issues (flutter/flutter#184885)
2026-04-15 [email protected] Roll Fuchsia Linux SDK from rB8LAuZL_DwHMssTU... to IdBT8fSMYrYSip65j... (flutter/flutter#185064)
2026-04-15 [email protected] Fix an ordering dependency in the services/system_chrome_test.dart test suite (flutter/flutter#185086)
2026-04-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[ios][platform_view]Reland hitTest approach (with a few 2026 update) (#183484)" (flutter/flutter#185082)
2026-04-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Run all flutter/flutter macOS tests using Xcode 26 and iOS 26 simulator (#179810)" (flutter/flutter#185067)
2026-04-15 [email protected] Agent rule: Dart editing (flutter/flutter#185045)
2026-04-15 [email protected] [ios][platform_view]Reland hitTest approach (with a few 2026 update) (flutter/flutter#183484)
2026-04-15 [email protected] Run all flutter/flutter macOS tests using Xcode 26 and iOS 26 simulator (flutter/flutter#179810)
2026-04-15 [email protected] Roll Dart SDK from ee5afcef0596 to 87b7c87e7207 (4 revisions) (flutter/flutter#185060)
2026-04-15 [email protected] Roll Skia from 4c382df6a25a to bda7232e6772 (7 revisions) (flutter/flutter#185057)
2026-04-15 [email protected] Remove material import from toggleable_test.dart + draggable_test.dart + obscured_animated_image_test.dart + sliver_constraints_test.dart (flutter/flutter#181774)
2026-04-15 [email protected] refactor: Remove material imports from Widget tests  (flutter/flutter#184877)
2026-04-14 [email protected] Adds missing `await`s on forgotten cases (flutter/flutter#183466)
2026-04-14 [email protected] Use an if-element in a collection literal instead of a conditional expression (flutter/flutter#184830)
2026-04-14 [email protected] update popular issues documentation (flutter/flutter#183196)
2026-04-14 [email protected] [Android] Add integration test for setting engine flags via the manifest (flutter/flutter#182241)
2026-04-14 [email protected] [fuchsia] Ask for both ambient-replace and VMEX to allow for a softer transition. (flutter/flutter#185042)
2026-04-14 [email protected] Make `multiple_windows` follow repo analyzer rules (flutter/flutter#184753)
2026-04-14 [email protected] Ignore incoming deprecated_web_configuration lint (flutter/flutter#184130)
2026-04-14 [email protected] [AGP 9] Update AGP Error (flutter/flutter#185043)
2026-04-14 [email protected] Move widget_preview_scaffold into pub workspace (flutter/flutter#182627)
2026-04-14 [email protected] Fix gles interactive tests (flutter/flutter#181389)
2026-04-14 [email protected] Update customer tests.version (flutter/flutter#185044)
2026-04-14 [email protected] [SKILL] upgrade-browser (flutter/flutter#184894)
2026-04-14 [email protected] [ci] Split up integration.shard dart_data_asset_test.dart (flutter/flutter#185021)
2026-04-14 [email protected] Hold startup lock until after `pub get` to prevent races (flutter/flutter#184294)
2026-04-14 6226493[email protected] Add `--include-example` flag to `flutter clean` for package example projects (flutter/flutter#183455)
2026-04-14 [email protected] Disable multi-pack-index when calling flutter from Xcode (flutter/flutter#184998)
2026-04-14 [email protected] Fix icon tree shaking when building for desktop (flutter/flutter#184249)
2026-04-14 [email protected] Fix killing wrong xcrun command (flutter/flutter#184831)
2026-04-14 [email protected] Allow Xcode build configuration to not contain flavor name (flutter/flutter#183398)
2026-04-14 [email protected] [web] Async rendering for benchmarks (flutter/flutter#184677)
2026-04-14 [email protected] [ci] Split up integration.shard native_assets_test.dart (flutter/flutter#185020)
2026-04-14 [email protected] Skip flutter widget-preview test that times out frequently (flutter/flutter#184988)
...
@walley892 walley892 mentioned this pull request Apr 17, 2026
Merged
10 tasks
@Piinks Piinks mentioned this pull request Apr 17, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@flar flar flar approved these changes

@b-luk b-luk b-luk approved these changes

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CICD Run CI/CD e: impeller Impeller rendering backend issues and features requests engine flutter/engine related. See also e: labels. will affect goldens Changes to golden files

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add Oval SDF renderer to uber shader

3 participants

@walley892 @b-luk @flar