Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Impeller] Implement drawShadow #35426

Merged
merged 2 commits into from Aug 17, 2022
Merged

Conversation

bdero
Copy link
Member

@bdero bdero commented Aug 16, 2022

Doesn't include the ambient occlusion part (which has a comparatively tiny impact on the shadow color).
The accuracy of these shadows will improve a bit more when I land improvements to the rrect approximation.

Skia:

image

Impeller:

image

Fixes flutter/flutter#104715

@bdero bdero added the impeller label Aug 16, 2022
@bdero bdero requested review from chinmaygarde and zanderso Aug 16, 2022
@bdero bdero self-assigned this Aug 16, 2022
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

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

@bdero
Copy link
Member Author

bdero commented Aug 16, 2022

(Still needs a display list test)

@chinmaygarde chinmaygarde added the Work in progress (WIP) Not ready (yet) for review! label Aug 16, 2022
@bdero
Copy link
Member Author

bdero commented Aug 16, 2022

I expect a low impact to gallery raster time benchmarks.

@bdero
Copy link
Member Author

bdero commented Aug 16, 2022

Added a display list playground to demonstrate the behavior (and problems with the approximation):

image

@bdero bdero added autosubmit Merge PR when tree becomes green via auto submit App and removed Work in progress (WIP) Not ready (yet) for review! labels Aug 16, 2022
@auto-submit auto-submit bot merged commit 246f245 into flutter:main Aug 17, 2022
24 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App impeller needs tests
Projects
Status: Done
2 participants