Skip to content

Commit 205ed68

Browse files
authored
[Impeller]: new blur - adds mips for backdrop filters (flutter#49607)
If the new blur flag is on and a blur is used as a backdrop filter, the rendered texture will now have mipmaps which will make the downscaling step of the blur have more signal (and thus be less shimmery). issue: flutter/flutter#140193 fixes: flutter/flutter#140949 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat
1 parent b8e5d47 commit 205ed68

28 files changed

+335
-42
lines changed

ci/licenses_golden/licenses_flutter

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5553,6 +5553,8 @@ ORIGIN: ../../../flutter/impeller/renderer/snapshot.h + ../../../flutter/LICENSE
55535553
ORIGIN: ../../../flutter/impeller/renderer/stroke.comp + ../../../flutter/LICENSE
55545554
ORIGIN: ../../../flutter/impeller/renderer/surface.cc + ../../../flutter/LICENSE
55555555
ORIGIN: ../../../flutter/impeller/renderer/surface.h + ../../../flutter/LICENSE
5556+
ORIGIN: ../../../flutter/impeller/renderer/texture_mipmap.cc + ../../../flutter/LICENSE
5557+
ORIGIN: ../../../flutter/impeller/renderer/texture_mipmap.h + ../../../flutter/LICENSE
55565558
ORIGIN: ../../../flutter/impeller/renderer/threadgroup_sizing_test.comp + ../../../flutter/LICENSE
55575559
ORIGIN: ../../../flutter/impeller/renderer/vertex_buffer_builder.cc + ../../../flutter/LICENSE
55585560
ORIGIN: ../../../flutter/impeller/renderer/vertex_buffer_builder.h + ../../../flutter/LICENSE
@@ -8381,6 +8383,8 @@ FILE: ../../../flutter/impeller/renderer/snapshot.h
83818383
FILE: ../../../flutter/impeller/renderer/stroke.comp
83828384
FILE: ../../../flutter/impeller/renderer/surface.cc
83838385
FILE: ../../../flutter/impeller/renderer/surface.h
8386+
FILE: ../../../flutter/impeller/renderer/texture_mipmap.cc
8387+
FILE: ../../../flutter/impeller/renderer/texture_mipmap.h
83848388
FILE: ../../../flutter/impeller/renderer/threadgroup_sizing_test.comp
83858389
FILE: ../../../flutter/impeller/renderer/vertex_buffer_builder.cc
83868390
FILE: ../../../flutter/impeller/renderer/vertex_buffer_builder.h

impeller/aiks/aiks_context.cc

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,18 @@ namespace impeller {
1212

1313
AiksContext::AiksContext(
1414
std::shared_ptr<Context> context,
15-
std::shared_ptr<TypographerContext> typographer_context)
15+
std::shared_ptr<TypographerContext> typographer_context,
16+
std::optional<std::shared_ptr<RenderTargetAllocator>>
17+
render_target_allocator)
1618
: context_(std::move(context)) {
1719
if (!context_ || !context_->IsValid()) {
1820
return;
1921
}
2022

2123
content_context_ = std::make_unique<ContentContext>(
22-
context_, std::move(typographer_context));
24+
context_, std::move(typographer_context),
25+
render_target_allocator.has_value() ? render_target_allocator.value()
26+
: nullptr);
2327
if (!content_context_->IsValid()) {
2428
return;
2529
}

impeller/aiks/aiks_context.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,12 @@ class AiksContext {
2828
/// `nullptr` is supplied, then attempting to draw
2929
/// text with Aiks will result in validation
3030
/// errors.
31+
/// @param render_target_allocator Injects a render target allocator or
32+
/// allocates its own if none is supplied.
3133
AiksContext(std::shared_ptr<Context> context,
32-
std::shared_ptr<TypographerContext> typographer_context);
34+
std::shared_ptr<TypographerContext> typographer_context,
35+
std::optional<std::shared_ptr<RenderTargetAllocator>>
36+
render_target_allocator = std::nullopt);
3337

3438
~AiksContext();
3539

impeller/aiks/aiks_unittests.cc

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
#include "flutter/impeller/aiks/aiks_unittests.h"
66

7+
#include <algorithm>
78
#include <array>
89
#include <cmath>
910
#include <cstdlib>
@@ -26,6 +27,7 @@
2627
#include "impeller/entity/contents/radial_gradient_contents.h"
2728
#include "impeller/entity/contents/solid_color_contents.h"
2829
#include "impeller/entity/contents/sweep_gradient_contents.h"
30+
#include "impeller/entity/render_target_cache.h"
2931
#include "impeller/geometry/color.h"
3032
#include "impeller/geometry/constants.h"
3133
#include "impeller/geometry/geometry_asserts.h"
@@ -3716,5 +3718,81 @@ TEST_P(AiksTest, SubpassWithClearColorOptimization) {
37163718
// will be filled with NaNs and may produce a magenta texture on macOS or iOS.
37173719
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
37183720
}
3721+
3722+
TEST_P(AiksTest, GuassianBlurUpdatesMipmapContents) {
3723+
// This makes sure if mip maps are recycled across invocations of blurs the
3724+
// contents get updated each frame correctly. If they aren't updated the color
3725+
// inside the blur and outside the blur will be different.
3726+
//
3727+
// If there is some change to render target caching this could display a false
3728+
// positive in the future. Also, if the LOD that is rendered is 1 it could
3729+
// present a false positive.
3730+
int32_t count = 0;
3731+
auto callback = [&](AiksContext& renderer) -> std::optional<Picture> {
3732+
Canvas canvas;
3733+
if (count++ == 0) {
3734+
canvas.DrawCircle({100, 100}, 50, {.color = Color::CornflowerBlue()});
3735+
} else {
3736+
canvas.DrawCircle({100, 100}, 50, {.color = Color::Chartreuse()});
3737+
}
3738+
canvas.ClipRRect(Rect::MakeLTRB(75, 50, 375, 275), {20, 20});
3739+
canvas.SaveLayer({.blend_mode = BlendMode::kSource}, std::nullopt,
3740+
ImageFilter::MakeBlur(Sigma(30.0), Sigma(30.0),
3741+
FilterContents::BlurStyle::kNormal,
3742+
Entity::TileMode::kClamp));
3743+
canvas.Restore();
3744+
return canvas.EndRecordingAsPicture();
3745+
};
3746+
3747+
ASSERT_TRUE(OpenPlaygroundHere(callback));
3748+
}
3749+
3750+
TEST_P(AiksTest, GaussianBlurSetsMipCountOnPass) {
3751+
Canvas canvas;
3752+
canvas.DrawCircle({100, 100}, 50, {.color = Color::CornflowerBlue()});
3753+
canvas.SaveLayer({}, std::nullopt,
3754+
ImageFilter::MakeBlur(Sigma(3), Sigma(3),
3755+
FilterContents::BlurStyle::kNormal,
3756+
Entity::TileMode::kClamp));
3757+
canvas.Restore();
3758+
3759+
Picture picture = canvas.EndRecordingAsPicture();
3760+
3761+
int32_t max_mip_count = 0;
3762+
picture.pass->IterateAllElements([&](EntityPass::Element& element) -> bool {
3763+
if (auto subpass = std::get_if<std::unique_ptr<EntityPass>>(&element)) {
3764+
max_mip_count =
3765+
std::max(max_mip_count, subpass->get()->GetRequiredMipCount());
3766+
}
3767+
return true;
3768+
});
3769+
3770+
EXPECT_EQ(1, max_mip_count);
3771+
}
3772+
3773+
TEST_P(AiksTest, GaussianBlurAllocatesCorrectMipCountRenderTarget) {
3774+
Canvas canvas;
3775+
canvas.DrawCircle({100, 100}, 50, {.color = Color::CornflowerBlue()});
3776+
canvas.SaveLayer({}, std::nullopt,
3777+
ImageFilter::MakeBlur(Sigma(3), Sigma(3),
3778+
FilterContents::BlurStyle::kNormal,
3779+
Entity::TileMode::kClamp));
3780+
canvas.Restore();
3781+
3782+
Picture picture = canvas.EndRecordingAsPicture();
3783+
std::shared_ptr<RenderTargetCache> cache =
3784+
std::make_shared<RenderTargetCache>(GetContext()->GetResourceAllocator());
3785+
AiksContext aiks_context(GetContext(), nullptr, cache);
3786+
picture.ToImage(aiks_context, {100, 100});
3787+
3788+
size_t max_mip_count = 0;
3789+
for (auto it = cache->GetTextureDataBegin(); it != cache->GetTextureDataEnd();
3790+
++it) {
3791+
max_mip_count =
3792+
std::max(it->texture->GetTextureDescriptor().mip_count, max_mip_count);
3793+
}
3794+
EXPECT_EQ(max_mip_count, 1lu);
3795+
}
3796+
37193797
} // namespace testing
37203798
} // namespace impeller

impeller/aiks/canvas.cc

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,37 @@ void Canvas::Save() {
133133
Save(false);
134134
}
135135

136+
namespace {
137+
class MipCountVisitor : public ImageFilterVisitor {
138+
public:
139+
virtual void Visit(const BlurImageFilter& filter) {
140+
required_mip_count_ = FilterContents::kBlurFilterRequiredMipCount;
141+
}
142+
virtual void Visit(const LocalMatrixImageFilter& filter) {
143+
required_mip_count_ = 1;
144+
}
145+
virtual void Visit(const DilateImageFilter& filter) {
146+
required_mip_count_ = 1;
147+
}
148+
virtual void Visit(const ErodeImageFilter& filter) {
149+
required_mip_count_ = 1;
150+
}
151+
virtual void Visit(const MatrixImageFilter& filter) {
152+
required_mip_count_ = 1;
153+
}
154+
virtual void Visit(const ComposeImageFilter& filter) {
155+
required_mip_count_ = 1;
156+
}
157+
virtual void Visit(const ColorImageFilter& filter) {
158+
required_mip_count_ = 1;
159+
}
160+
int32_t GetRequiredMipCount() const { return required_mip_count_; }
161+
162+
private:
163+
int32_t required_mip_count_ = -1;
164+
};
165+
} // namespace
166+
136167
void Canvas::Save(bool create_subpass,
137168
BlendMode blend_mode,
138169
const std::shared_ptr<ImageFilter>& backdrop_filter) {
@@ -156,6 +187,9 @@ void Canvas::Save(bool create_subpass,
156187
return filter;
157188
};
158189
subpass->SetBackdropFilter(backdrop_filter_proc);
190+
MipCountVisitor mip_count_visitor;
191+
backdrop_filter->Visit(mip_count_visitor);
192+
subpass->SetRequiredMipCount(mip_count_visitor.GetRequiredMipCount());
159193
}
160194
subpass->SetBlendMode(blend_mode);
161195
current_pass_ = GetCurrentPass().AddSubpass(std::move(subpass));

impeller/aiks/picture.cc

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,16 +64,18 @@ std::shared_ptr<Texture> Picture::RenderToTexture(
6464
*impeller_context, // context
6565
render_target_allocator, // allocator
6666
size, // size
67+
/*mip_count=*/1,
6768
"Picture Snapshot MSAA", // label
6869
RenderTarget::
6970
kDefaultColorAttachmentConfigMSAA, // color_attachment_config
7071
std::nullopt // stencil_attachment_config
7172
);
7273
} else {
7374
target = RenderTarget::CreateOffscreen(
74-
*impeller_context, // context
75-
render_target_allocator, // allocator
76-
size, // size
75+
*impeller_context, // context
76+
render_target_allocator, // allocator
77+
size, // size
78+
/*mip_count=*/1,
7779
"Picture Snapshot", // label
7880
RenderTarget::kDefaultColorAttachmentConfig, // color_attachment_config
7981
std::nullopt // stencil_attachment_config

impeller/core/texture.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ class Texture {
4545

4646
virtual Scalar GetYCoordScale() const;
4747

48+
/// Returns true if mipmaps have never been generated.
49+
/// The contents of the mipmap may be out of date if the root texture has been
50+
/// modified and the mipmaps hasn't been regenerated.
4851
bool NeedsMipmapGeneration() const;
4952

5053
protected:

impeller/entity/contents/checkerboard_contents_unittests.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ TEST_P(EntityTest, RendersWithoutError) {
3535
auto buffer = content_context->GetContext()->CreateCommandBuffer();
3636
auto render_target = RenderTarget::CreateOffscreenMSAA(
3737
*content_context->GetContext(),
38-
*GetContentContext()->GetRenderTargetCache(), {100, 100});
38+
*GetContentContext()->GetRenderTargetCache(), {100, 100},
39+
/*mip_count=*/1);
3940
auto render_pass = buffer->CreateRenderPass(render_target);
4041
Entity entity;
4142

impeller/entity/contents/content_context.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -418,14 +418,14 @@ fml::StatusOr<RenderTarget> ContentContext::MakeSubpass(
418418
RenderTarget subpass_target;
419419
if (context->GetCapabilities()->SupportsOffscreenMSAA() && msaa_enabled) {
420420
subpass_target = RenderTarget::CreateOffscreenMSAA(
421-
*context, *GetRenderTargetCache(), texture_size,
421+
*context, *GetRenderTargetCache(), texture_size, /*mip_count=*/1,
422422
SPrintF("%s Offscreen", label.c_str()),
423423
RenderTarget::kDefaultColorAttachmentConfigMSAA,
424424
std::nullopt // stencil_attachment_config
425425
);
426426
} else {
427427
subpass_target = RenderTarget::CreateOffscreen(
428-
*context, *GetRenderTargetCache(), texture_size,
428+
*context, *GetRenderTargetCache(), texture_size, /*mip_count=*/1,
429429
SPrintF("%s Offscreen", label.c_str()),
430430
RenderTarget::kDefaultColorAttachmentConfig, //
431431
std::nullopt // stencil_attachment_config

impeller/entity/contents/filters/filter_contents.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ std::shared_ptr<FilterContents> FilterContents::MakeDirectionalGaussianBlur(
5050
return blur;
5151
}
5252

53+
#ifdef IMPELLER_ENABLE_NEW_GAUSSIAN_FILTER
54+
const int32_t FilterContents::kBlurFilterRequiredMipCount = 4;
55+
#else
56+
const int32_t FilterContents::kBlurFilterRequiredMipCount = 1;
57+
#endif
58+
5359
std::shared_ptr<FilterContents> FilterContents::MakeGaussianBlur(
5460
const FilterInput::Ref& input,
5561
Sigma sigma_x,

0 commit comments

Comments
 (0)