[Impeller] implements new blur tile mode #48805
Conversation
|
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. |
| Entity::TileMode tile_mode) { | ||
| switch (tile_mode) { | ||
| case Entity::TileMode::kDecal: | ||
| if (renderer.GetDeviceCapabilities().SupportsDecalSamplerAddressMode()) { |
There was a problem hiding this comment.
Could you add a todo/issue for emulating this when it's not supported? The halo won't render correctly on some devices w/ the current technique without this.
We already have a specialized pipeline that the old blur uses for this w/ the same vertex/uniform layouts (GaussianBlurDecalPipeline).
There was a problem hiding this comment.
Done: flutter/flutter#139773
How do we look up availability on gpuinfo.org? I had a hard time finding it. It's just available to everyone on Vulkan right? The check is just for OpenGLES?
There was a problem hiding this comment.
Yeah decal is supported for Metal (via MTLSamplerAddressModeClampToZero) and our min Vulkan profile without any extensions. For OpenGLES we check for GL_EXT_texture_border_clamp or GL_NV_texture_border_clamp.
This looks worse than the situation actually is, though. I'm pretty sure "coverage" on gpuinfo.org is just "percentage of devices in the database" and doesn't weight for an estimation of actual usage. And of course, there are ancient devices on the list that Flutter doesn't support.
There was a problem hiding this comment.
Oh yea, good point. Over time the usefulness of gpuinfo.org will wane if they don't have some sort of filter for old devices. Something like filtering based on operating system would probably work since the OS developers will start dropping support.
So, first we attempt to create the gutter by expanding the clip coverage to get the snapshot. That may or may not contain all the content for the gutter we need in the snapshot. Then when we downsample we blow out the uvs of the snapshot to create the gutter. One place where this may not work is if we draw an image filter on an image whose "source rect" is smaller than the full image. I guess that may be possible with |
| @@ -82,21 +108,22 @@ std::shared_ptr<Texture> MakeDownsampleSubpass( | |||
| // creates a halo effect. This compensates for when the expanded clip | |||
| // region can't give us the full gutter we want. | |||
| Vector2 texture_size = Vector2(input_texture->GetSize()); | |||
| Quad vertices = | |||
| Quad guttered_uvs = | |||
There was a problem hiding this comment.
This math may assume that the centroid of the uv's is 0.5, 0.5 too so it may not work if that isn't the case? @bdero
|
I'm not certain if this handles all cases yet. This passes all of our existing tests and passes 1 new one so it's a step forward. The broken cases may be rare enough to ingore for now. I'm going to land it, implement clamping the sigma then poke around to see if I can break this. |
…139781) flutter/engine@89ad167...6a687be 2023-12-08 30870216+gaaclarke@users.noreply.github.com [Impeller] implements new blur tile mode (flutter/engine#48805) 2023-12-07 skia-flutter-autoroll@skia.org Roll Skia from dd7e37c0e2bd to 9e89d96899f4 (2 revisions) (flutter/engine#48809) 2023-12-07 matanlurey@users.noreply.github.com Add a note that rolling clang_version manually is dangerous. (flutter/engine#48808) 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 chinmaygarde@google.com,rmistry@google.com,zra@google.com 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://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
…lutter#139781) flutter/engine@89ad167...6a687be 2023-12-08 30870216+gaaclarke@users.noreply.github.com [Impeller] implements new blur tile mode (flutter/engine#48805) 2023-12-07 skia-flutter-autoroll@skia.org Roll Skia from dd7e37c0e2bd to 9e89d96899f4 (2 revisions) (flutter/engine#48809) 2023-12-07 matanlurey@users.noreply.github.com Add a note that rolling clang_version manually is dangerous. (flutter/engine#48808) 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 chinmaygarde@google.com,rmistry@google.com,zra@google.com 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://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
fixes flutter/flutter#139165
before
after
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.