Add foundation for uber SDF shader and Contents#183864
Conversation
35593a9 to
5fe1a4c
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces UberSDFContents to render circles and rectangles using Signed Distance Fields (SDFs), integrating a new UberSDFPipeline into the ContentContext. The Canvas::DrawRect and Canvas::DrawCircle methods are updated to leverage these SDF contents under specific conditions, and FillRectGeometry is enhanced with antialiasing padding. New unit tests for UberSDFContents are also added. Review feedback points out a critical use-after-free bug in UberSDFContents due to a raw pointer to a stack-allocated geometry object, recommending a switch to std::shared_ptr. Additionally, it suggests using constexpr for constants and std::make_unique for std::unique_ptr creation for improved code quality.
|
@walley892 I responded to your comments inline. In general I just wanted to say, you aren't wrong. I think we'll have to play it by ear. I think trying to keep ubersdf smushed together as much as possible will help with things like shader codesize and to avoid uniform bloat. Once we organizationally split these things up I think we lose some of the pressure to reuse and minimize. Let's give that a shot and see where it takes us. |
for function of AddRenderEntityWithFiltersToCurrentPass,
refer to the following commit:
72cfdc7("Add foundation for uber SDF shader and Contents (flutter#183864)")
Signed-off-by: Yongqin Liu <yongqin.liu@linaro.org>
issues: flutter#183036 This implements the basic foundation for the UberSDF work. There are improvements we will want to make on it but everything implemented here is behind flags in `main` and has golden tests. Right now it is using a stand-in implementation of the sdf renderer that doesn't incorporate all the work we've done for the circle sdf renderer. It also doesn't render gradients yet in this pr, that is happening in flutter#184090 Design doc: - https://docs.google.com/document/d/166uRQBoKfraIibNKYxrewzVcmflqIWde80uB7L5bJSs/edit?tab=t.0 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. 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](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot 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. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
issues: #183036
This implements the basic foundation for the UberSDF work. There are improvements we will want to make on it but everything implemented here is behind flags in
mainand has golden tests.Right now it is using a stand-in implementation of the sdf renderer that doesn't incorporate all the work we've done for the circle sdf renderer.
It also doesn't render gradients yet in this pr, that is happening in #184090
Design doc:
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.