Apply the drawShadow bounds workaround to the Web HTML backend #42304
Conversation
flutter-dashboard
bot
added
the
platform-web
|
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. |
jason-simmons
force-pushed
the
web_adjust_shadow_bounds
branch
from
b71122f to
c15ddfb
Compare
|
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. |
| // calculation. This duplicates a workaround that had been previously used | ||
| // in the Flutter framework to adjust the bounds for shadows drawn by Skia. | ||
| // (See https://github.com/flutter/flutter/pull/127052) | ||
| return bounds.inflate(20); |
There was a problem hiding this comment.
In the original workaround, the bounds computed was the join of the inflated shadow shape and the computed shadow bounds. It was basically enforcing a minimum padding of 20 pixels, but did not otherwise pad shadows larger than that.
Here you are returning an inflated value of the shadow bounds which pads all bounds even those that were originally larger than 20 pixels of shadow.
I'm not saying that is a bad thing, but it isn't the same thing...
There was a problem hiding this comment.
Changed the comment so it does not say that this workaround is identical to the old one
Previously the Flutter framework had been inflating the rectangle computed for the bounds of a drawShadow operation in order to work around potential inaccuracies in the SkPicture's bounds calculation. That workaround is now obsolete for most platforms and was removed from the framework (see flutter/flutter#127052). But the Web HTML backend is using different code for computing shadow bounds. This PR restores the workaround for Web HTML for consistency with the old behavior.
jason-simmons
force-pushed
the
web_adjust_shadow_bounds
branch
from
c15ddfb to
2b768b0
Compare
|
Golden file changes are available for triage from new commit, Click here to view. |
jason-simmons
added
the
autosubmit
…ions) (#127676) Manual roll requested by [email protected] flutter/engine@8573f3b...84f2fc1 2023-05-26 [email protected] Revert Dart SDK to 398e847c0b24ad2e57194844410613b7dadcb74f (flutter/engine#42347) 2023-05-26 [email protected] Apply the drawShadow bounds workaround to the Web HTML backend (flutter/engine#42304) 2023-05-26 [email protected] Roll Dart SDK from f17b745f5c3c to 398e847c0b24 (1 revision) (flutter/engine#42345) 2023-05-26 [email protected] Roll Fuchsia Mac SDK from AgIKKqtBtb6RJnyI9... to c6-zqvlx2YtyGwGzJ... (flutter/engine#42344) 2023-05-26 [email protected] Roll Fuchsia Linux SDK from DNwJQMQZladAsKTjv... to 9aZkaKhX4GslPUnbX... (flutter/engine#42343) 2023-05-26 [email protected] Roll Skia from 7e20e6679785 to 1622df035714 (1 revision) (flutter/engine#42342) 2023-05-26 [email protected] Roll Dart SDK from bc65e3f5efa0 to f17b745f5c3c (1 revision) (flutter/engine#42341) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from DNwJQMQZladA to 9aZkaKhX4Gsl fuchsia/sdk/core/mac-amd64 from AgIKKqtBtb6R to c6-zqvlx2Yty If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
3 participants
Previously the Flutter framework had been inflating the rectangle computed for the bounds of a drawShadow operation in order to work around potential inaccuracies in the SkPicture's bounds calculation.
That workaround is now obsolete for most platforms and was removed from the framework (see flutter/flutter#127052). But the Web HTML backend is using different code for computing shadow bounds. This PR restores the workaround for Web HTML for consistency with the old behavior.