-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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] implements new blur tile mode #48805
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
f7ca7ef
[Impeller] made blur respect the tile mode
gaaclarke f23bbe1
format
gaaclarke cf50fcb
moved back to a static test
gaaclarke 5a7881e
removed hack to turn it on
gaaclarke a16e915
a bit of cleanup
gaaclarke c65e3a1
removed hack
gaaclarke File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,6 +54,31 @@ Matrix MakeAnchorScale(const Point& anchor, Vector2 scale) { | |
Matrix::MakeTranslation({-anchor.x, -anchor.y, 0}); | ||
} | ||
|
||
void SetTileMode(SamplerDescriptor* descriptor, | ||
const ContentContext& renderer, | ||
Entity::TileMode tile_mode) { | ||
switch (tile_mode) { | ||
case Entity::TileMode::kDecal: | ||
if (renderer.GetDeviceCapabilities().SupportsDecalSamplerAddressMode()) { | ||
descriptor->width_address_mode = SamplerAddressMode::kDecal; | ||
descriptor->height_address_mode = SamplerAddressMode::kDecal; | ||
} | ||
break; | ||
case Entity::TileMode::kClamp: | ||
descriptor->width_address_mode = SamplerAddressMode::kClampToEdge; | ||
descriptor->height_address_mode = SamplerAddressMode::kClampToEdge; | ||
break; | ||
case Entity::TileMode::kMirror: | ||
descriptor->width_address_mode = SamplerAddressMode::kMirror; | ||
descriptor->height_address_mode = SamplerAddressMode::kMirror; | ||
break; | ||
case Entity::TileMode::kRepeat: | ||
descriptor->width_address_mode = SamplerAddressMode::kRepeat; | ||
descriptor->height_address_mode = SamplerAddressMode::kRepeat; | ||
break; | ||
} | ||
} | ||
|
||
/// Makes a subpass that will render the scaled down input and add the | ||
/// transparent gutter required for the blur halo. | ||
std::shared_ptr<Texture> MakeDownsampleSubpass( | ||
|
@@ -62,7 +87,8 @@ std::shared_ptr<Texture> MakeDownsampleSubpass( | |
const SamplerDescriptor& sampler_descriptor, | ||
const Quad& uvs, | ||
const ISize& subpass_size, | ||
const Vector2 padding) { | ||
const Vector2 padding, | ||
Entity::TileMode tile_mode) { | ||
ContentContext::SubpassCallback subpass_callback = | ||
[&](const ContentContext& renderer, RenderPass& pass) { | ||
HostBuffer& host_buffer = pass.GetTransientsBuffer(); | ||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
MakeAnchorScale({0.5, 0.5}, | ||
texture_size / (texture_size + padding * 2)) | ||
.Transform( | ||
{Point(0, 0), Point(1, 0), Point(0, 1), Point(1, 1)}); | ||
|
||
BindVertices<TextureFillVertexShader>(cmd, host_buffer, | ||
{ | ||
{vertices[0], uvs[0]}, | ||
{vertices[1], uvs[1]}, | ||
{vertices[2], uvs[2]}, | ||
{vertices[3], uvs[3]}, | ||
}); | ||
(texture_size + padding * 2) / texture_size) | ||
.Transform(uvs); | ||
|
||
BindVertices<TextureFillVertexShader>( | ||
cmd, host_buffer, | ||
{ | ||
{Point(0, 0), guttered_uvs[0]}, | ||
{Point(1, 0), guttered_uvs[1]}, | ||
{Point(0, 1), guttered_uvs[2]}, | ||
{Point(1, 1), guttered_uvs[3]}, | ||
}); | ||
|
||
SamplerDescriptor linear_sampler_descriptor = sampler_descriptor; | ||
SetTileMode(&linear_sampler_descriptor, renderer, tile_mode); | ||
linear_sampler_descriptor.mag_filter = MinMagFilter::kLinear; | ||
linear_sampler_descriptor.min_filter = MinMagFilter::kLinear; | ||
TextureFillVertexShader::BindFrameInfo( | ||
|
@@ -165,8 +192,10 @@ std::shared_ptr<Texture> MakeBlurSubpass( | |
|
||
} // namespace | ||
|
||
GaussianBlurFilterContents::GaussianBlurFilterContents(Scalar sigma) | ||
: sigma_(sigma) {} | ||
GaussianBlurFilterContents::GaussianBlurFilterContents( | ||
Scalar sigma, | ||
Entity::TileMode tile_mode) | ||
: sigma_(sigma), tile_mode_(tile_mode) {} | ||
|
||
// This value was extracted from Skia, see: | ||
// * https://github.com/google/skia/blob/d29cc3fe182f6e8a8539004a6a4ee8251677a6fd/src/gpu/ganesh/GrBlurUtils.cpp#L2561-L2576 | ||
|
@@ -264,7 +293,7 @@ std::optional<Entity> GaussianBlurFilterContents::RenderFilter( | |
|
||
std::shared_ptr<Texture> pass1_out_texture = MakeDownsampleSubpass( | ||
renderer, input_snapshot->texture, input_snapshot->sampler_descriptor, | ||
uvs, subpass_size, padding); | ||
uvs, subpass_size, padding, tile_mode_); | ||
|
||
Vector2 pass1_pixel_size = 1.0 / Vector2(pass1_out_texture->GetSize()); | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
orGL_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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.