[Impeller] Fix sampling management problems (#39483)
diff --git a/impeller/display_list/display_list_dispatcher.cc b/impeller/display_list/display_list_dispatcher.cc index 0bb9330..58e826a 100644 --- a/impeller/display_list/display_list_dispatcher.cc +++ b/impeller/display_list/display_list_dispatcher.cc
@@ -136,6 +136,9 @@ desc.label = "Nearest Sampler"; break; case flutter::DlImageSampling::kLinear: + // Impeller doesn't support cubic sampling, but linear is closer to correct + // than nearest for this case. + case flutter::DlImageSampling::kCubic: desc.min_filter = desc.mag_filter = impeller::MinMagFilter::kLinear; desc.label = "Linear Sampler"; break; @@ -144,8 +147,6 @@ desc.mip_filter = impeller::MipFilter::kLinear; desc.label = "Mipmap Linear Sampler"; break; - default: - break; } return desc; }
diff --git a/impeller/entity/contents/contents.cc b/impeller/entity/contents/contents.cc index 5de83a1..f6f52cd 100644 --- a/impeller/entity/contents/contents.cc +++ b/impeller/entity/contents/contents.cc
@@ -41,6 +41,7 @@ std::optional<Snapshot> Contents::RenderToSnapshot( const ContentContext& renderer, const Entity& entity, + const std::optional<SamplerDescriptor>& sampler_descriptor, bool msaa_enabled) const { auto coverage = GetCoverage(entity); if (!coverage.has_value()) { @@ -64,8 +65,15 @@ return std::nullopt; } - return Snapshot{.texture = texture, - .transform = Matrix::MakeTranslation(coverage->origin)}; + auto snapshot = Snapshot{ + .texture = texture, + .transform = Matrix::MakeTranslation(coverage->origin), + }; + if (sampler_descriptor.has_value()) { + snapshot.sampler_descriptor = sampler_descriptor.value(); + } + + return snapshot; } bool Contents::ShouldRender(const Entity& entity,
diff --git a/impeller/entity/contents/contents.h b/impeller/entity/contents/contents.h index 78ccc3f..ec99190 100644 --- a/impeller/entity/contents/contents.h +++ b/impeller/entity/contents/contents.h
@@ -10,6 +10,7 @@ #include "flutter/fml/macros.h" #include "impeller/geometry/rect.h" +#include "impeller/renderer/sampler_descriptor.h" #include "impeller/renderer/snapshot.h" #include "impeller/renderer/texture.h" @@ -61,6 +62,7 @@ virtual std::optional<Snapshot> RenderToSnapshot( const ContentContext& renderer, const Entity& entity, + const std::optional<SamplerDescriptor>& sampler_descriptor = std::nullopt, bool msaa_enabled = true) const; virtual bool ShouldRender(const Entity& entity,
diff --git a/impeller/entity/contents/filters/blend_filter_contents.cc b/impeller/entity/contents/filters/blend_filter_contents.cc index e363158..ddfc723 100644 --- a/impeller/entity/contents/filters/blend_filter_contents.cc +++ b/impeller/entity/contents/filters/blend_filter_contents.cc
@@ -14,6 +14,7 @@ #include "impeller/entity/contents/solid_color_contents.h" #include "impeller/entity/entity.h" #include "impeller/geometry/path_builder.h" +#include "impeller/renderer/formats.h" #include "impeller/renderer/render_pass.h" #include "impeller/renderer/sampler_library.h" @@ -107,8 +108,9 @@ typename FS::BlendInfo blend_info; - auto sampler = renderer.GetContext()->GetSamplerLibrary()->GetSampler({}); - FS::BindTextureSamplerDst(cmd, dst_snapshot->texture, sampler); + auto dst_sampler = renderer.GetContext()->GetSamplerLibrary()->GetSampler( + dst_snapshot->sampler_descriptor); + FS::BindTextureSamplerDst(cmd, dst_snapshot->texture, dst_sampler); blend_info.dst_y_coord_scale = dst_snapshot->texture->GetYCoordScale(); blend_info.dst_input_alpha = absorb_opacity ? dst_snapshot->opacity : 1.0; @@ -118,10 +120,12 @@ // This texture will not be sampled from due to the color factor. But // this is present so that validation doesn't trip on a missing // binding. - FS::BindTextureSamplerSrc(cmd, dst_snapshot->texture, sampler); + FS::BindTextureSamplerSrc(cmd, dst_snapshot->texture, dst_sampler); } else { + auto src_sampler = renderer.GetContext()->GetSamplerLibrary()->GetSampler( + src_snapshot->sampler_descriptor); blend_info.color_factor = 0; - FS::BindTextureSamplerSrc(cmd, src_snapshot->texture, sampler); + FS::BindTextureSamplerSrc(cmd, src_snapshot->texture, src_sampler); blend_info.src_y_coord_scale = src_snapshot->texture->GetYCoordScale(); } auto blend_uniform = host_buffer.EmplaceUniform(blend_info); @@ -145,7 +149,10 @@ return Snapshot{.texture = out_texture, .transform = Matrix::MakeTranslation(coverage.origin), - .sampler_descriptor = dst_snapshot->sampler_descriptor, + // Since we absorbed the transform of the inputs and used the + // respective snapshot sampling modes when blending, pass on + // the default NN clamp sampler. + .sampler_descriptor = {}, .opacity = (absorb_opacity ? 1.0f : dst_snapshot->opacity) * alpha.value_or(1.0)}; } @@ -168,8 +175,6 @@ RenderPass& pass) { auto& host_buffer = pass.GetTransientsBuffer(); - auto sampler = renderer.GetContext()->GetSamplerLibrary()->GetSampler({}); - Command cmd; cmd.label = "Pipeline Blend Filter"; auto options = OptionsFromPass(pass); @@ -183,6 +188,8 @@ return false; } + auto sampler = renderer.GetContext()->GetSamplerLibrary()->GetSampler( + input->sampler_descriptor); FS::BindTextureSamplerSrc(cmd, input->texture, sampler); auto size = input->texture->GetSize(); @@ -262,13 +269,14 @@ } out_texture->SetLabel("Pipeline Blend Filter Texture"); - return Snapshot{ - .texture = out_texture, - .transform = Matrix::MakeTranslation(coverage.origin), - .sampler_descriptor = - inputs[0]->GetSnapshot(renderer, entity)->sampler_descriptor, - .opacity = (absorb_opacity ? 1.0f : dst_snapshot->opacity) * - alpha.value_or(1.0)}; + return Snapshot{.texture = out_texture, + .transform = Matrix::MakeTranslation(coverage.origin), + // Since we absorbed the transform of the inputs and used the + // respective snapshot sampling modes when blending, pass on + // the default NN clamp sampler. + .sampler_descriptor = {}, + .opacity = (absorb_opacity ? 1.0f : dst_snapshot->opacity) * + alpha.value_or(1.0)}; } #define BLEND_CASE(mode) \ @@ -346,6 +354,7 @@ foreground_color_, GetAbsorbOpacity(), GetAlpha()); } + FML_UNREACHABLE(); }
diff --git a/impeller/entity/contents/filters/filter_contents.cc b/impeller/entity/contents/filters/filter_contents.cc index d1d76f3..cf10504 100644 --- a/impeller/entity/contents/filters/filter_contents.cc +++ b/impeller/entity/contents/filters/filter_contents.cc
@@ -237,6 +237,7 @@ std::optional<Snapshot> FilterContents::RenderToSnapshot( const ContentContext& renderer, const Entity& entity, + const std::optional<SamplerDescriptor>& sampler_descriptor, bool msaa_enabled) const { Entity entity_with_local_transform = entity; entity_with_local_transform.SetTransformation(
diff --git a/impeller/entity/contents/filters/filter_contents.h b/impeller/entity/contents/filters/filter_contents.h index 779cfb0..74c1039 100644 --- a/impeller/entity/contents/filters/filter_contents.h +++ b/impeller/entity/contents/filters/filter_contents.h
@@ -119,6 +119,7 @@ std::optional<Snapshot> RenderToSnapshot( const ContentContext& renderer, const Entity& entity, + const std::optional<SamplerDescriptor>& sampler_descriptor = std::nullopt, bool msaa_enabled = true) const override; virtual Matrix GetLocalTransform(const Matrix& parent_transform) const;
diff --git a/impeller/entity/contents/filters/inputs/contents_filter_input.cc b/impeller/entity/contents/filters/inputs/contents_filter_input.cc index 7495157..e6e4022 100644 --- a/impeller/entity/contents/filters/inputs/contents_filter_input.cc +++ b/impeller/entity/contents/filters/inputs/contents_filter_input.cc
@@ -4,6 +4,7 @@ #include "impeller/entity/contents/filters/inputs/contents_filter_input.h" +#include <optional> #include <utility> namespace impeller { @@ -22,7 +23,8 @@ const ContentContext& renderer, const Entity& entity) const { if (!snapshot_.has_value()) { - snapshot_ = contents_->RenderToSnapshot(renderer, entity, msaa_enabled_); + snapshot_ = contents_->RenderToSnapshot(renderer, entity, std::nullopt, + msaa_enabled_); } return snapshot_; }
diff --git a/impeller/entity/contents/texture_contents.cc b/impeller/entity/contents/texture_contents.cc index f5b44e5..fc127a2 100644 --- a/impeller/entity/contents/texture_contents.cc +++ b/impeller/entity/contents/texture_contents.cc
@@ -67,6 +67,7 @@ std::optional<Snapshot> TextureContents::RenderToSnapshot( const ContentContext& renderer, const Entity& entity, + const std::optional<SamplerDescriptor>& sampler_descriptor, bool msaa_enabled) const { auto bounds = path_.GetBoundingBox(); if (!bounds.has_value()) { @@ -78,14 +79,16 @@ if (is_rect_ && source_rect_ == Rect::MakeSize(texture_->GetSize()) && (opacity_ >= 1 - kEhCloseEnough || defer_applying_opacity_)) { auto scale = Vector2(bounds->size / Size(texture_->GetSize())); - return Snapshot{.texture = texture_, - .transform = entity.GetTransformation() * - Matrix::MakeTranslation(bounds->origin) * - Matrix::MakeScale(scale), - .sampler_descriptor = sampler_descriptor_, - .opacity = opacity_}; + return Snapshot{ + .texture = texture_, + .transform = entity.GetTransformation() * + Matrix::MakeTranslation(bounds->origin) * + Matrix::MakeScale(scale), + .sampler_descriptor = sampler_descriptor.value_or(sampler_descriptor_), + .opacity = opacity_}; } - return Contents::RenderToSnapshot(renderer, entity); + return Contents::RenderToSnapshot( + renderer, entity, sampler_descriptor.value_or(sampler_descriptor_)); } bool TextureContents::Render(const ContentContext& renderer,
diff --git a/impeller/entity/contents/texture_contents.h b/impeller/entity/contents/texture_contents.h index 06ef924..fec23fb 100644 --- a/impeller/entity/contents/texture_contents.h +++ b/impeller/entity/contents/texture_contents.h
@@ -55,6 +55,7 @@ std::optional<Snapshot> RenderToSnapshot( const ContentContext& renderer, const Entity& entity, + const std::optional<SamplerDescriptor>& sampler_descriptor = std::nullopt, bool msaa_enabled = true) const override; // |Contents|
diff --git a/impeller/renderer/sampler_descriptor.cc b/impeller/renderer/sampler_descriptor.cc index 3525184..b2e10ca 100644 --- a/impeller/renderer/sampler_descriptor.cc +++ b/impeller/renderer/sampler_descriptor.cc
@@ -3,9 +3,19 @@ // found in the LICENSE file. #include "impeller/renderer/sampler_descriptor.h" +#include "fml/logging.h" namespace impeller { -// +SamplerDescriptor::SamplerDescriptor() = default; + +SamplerDescriptor::SamplerDescriptor(std::string label, + MinMagFilter min_filter, + MinMagFilter mag_filter, + MipFilter mip_filter) + : min_filter(min_filter), + mag_filter(mag_filter), + mip_filter(mip_filter), + label(std::move(label)) {} } // namespace impeller
diff --git a/impeller/renderer/sampler_descriptor.h b/impeller/renderer/sampler_descriptor.h index 6ebb96e..01d0f29 100644 --- a/impeller/renderer/sampler_descriptor.h +++ b/impeller/renderer/sampler_descriptor.h
@@ -26,6 +26,13 @@ std::string label = "NN Clamp Sampler"; + SamplerDescriptor(); + + SamplerDescriptor(std::string label, + MinMagFilter min_filter, + MinMagFilter mag_filter, + MipFilter mip_filter); + // Comparable<SamplerDescriptor> std::size_t GetHash() const override { return fml::HashCombine(min_filter, mag_filter, mip_filter,
diff --git a/impeller/renderer/snapshot.h b/impeller/renderer/snapshot.h index 26fe49d..41ad407 100644 --- a/impeller/renderer/snapshot.h +++ b/impeller/renderer/snapshot.h
@@ -11,6 +11,7 @@ #include "flutter/fml/macros.h" #include "impeller/geometry/matrix.h" #include "impeller/geometry/rect.h" +#include "impeller/renderer/formats.h" #include "impeller/renderer/sampler_descriptor.h" #include "impeller/renderer/texture.h" @@ -25,7 +26,11 @@ /// The transform that should be applied to this texture for rendering. Matrix transform; - SamplerDescriptor sampler_descriptor; + SamplerDescriptor sampler_descriptor = + SamplerDescriptor("Default Snapshot Sampler", + MinMagFilter::kLinear, + MinMagFilter::kLinear, + MipFilter::kLinear); Scalar opacity = 1.0f;