[Impeller] Fix sampling management problems

Author: bdero

impeller/display_list/display_list_dispatcher.cc

@@ -136,6 +136,9 @@ static impeller::SamplerDescriptor ToSamplerDescriptor(
       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 @@ static impeller::SamplerDescriptor ToSamplerDescriptor(
       desc.mip_filter = impeller::MipFilter::kLinear;
       desc.label = "Mipmap Linear Sampler";
       break;
-    default:
-      break;
   }
   return desc;
 }

impeller/entity/contents/contents.cc

@@ -41,6 +41,7 @@ Contents::StencilCoverage Contents::GetStencilCoverage(
 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 @@ std::optional<Snapshot> Contents::RenderToSnapshot(
     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,

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 @@ class Contents {
   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,

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 @@ static std::optional<Snapshot> AdvancedBlend(
 
     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 @@ static std::optional<Snapshot> AdvancedBlend(
       // 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 @@ static std::optional<Snapshot> AdvancedBlend(
 
   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 @@ static std::optional<Snapshot> PipelineBlend(
                                                  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 @@ static std::optional<Snapshot> PipelineBlend(
         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 @@ static std::optional<Snapshot> PipelineBlend(
   }
   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 @@ std::optional<Snapshot> BlendFilterContents::RenderFilter(
                                 foreground_color_, GetAbsorbOpacity(),
                                 GetAlpha());
   }
+
   FML_UNREACHABLE();
 }
 

impeller/entity/contents/filters/filter_contents.cc

@@ -237,6 +237,7 @@ std::optional<Rect> FilterContents::GetFilterCoverage(
 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(

impeller/entity/contents/filters/filter_contents.h

@@ -119,6 +119,7 @@ class FilterContents : public Contents {
   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;

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 @@ std::optional<Snapshot> ContentsFilterInput::GetSnapshot(
     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_;
 }

impeller/entity/contents/texture_contents.cc

@@ -67,6 +67,7 @@ std::optional<Rect> TextureContents::GetCoverage(const Entity& entity) const {
 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 @@ std::optional<Snapshot> TextureContents::RenderToSnapshot(
   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,

impeller/entity/contents/texture_contents.h

@@ -55,6 +55,7 @@ class TextureContents final : public Contents {
   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|

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

impeller/renderer/sampler_descriptor.h

@@ -26,6 +26,13 @@ struct SamplerDescriptor final : public Comparable<SamplerDescriptor> {
 
   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,

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 @@ struct Snapshot {
   /// 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;