[Impeller] Add an API for sampling strictly within the bounds of the source rect in DrawImageRect (#49696)

jason-simmons · web-flow · commit 041ae355e7fe · 2024-01-16T21:56:05.000Z
Fixes flutter/flutter#140393
diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter
@@ -5261,6 +5261,7 @@ ORIGIN: ../../../flutter/impeller/entity/shaders/sweep_gradient_ssbo_fill.frag +
 ORIGIN: ../../../flutter/impeller/entity/shaders/texture_fill.frag + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/entity/shaders/texture_fill.vert + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/entity/shaders/texture_fill_external.frag + ../../../flutter/LICENSE
+ORIGIN: ../../../flutter/impeller/entity/shaders/texture_fill_strict_src.frag + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/entity/shaders/tiled_texture_fill.frag + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/entity/shaders/tiled_texture_fill_external.frag + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/entity/shaders/vertices.frag + ../../../flutter/LICENSE
@@ -8090,6 +8091,7 @@ FILE: ../../../flutter/impeller/entity/shaders/sweep_gradient_ssbo_fill.frag
 FILE: ../../../flutter/impeller/entity/shaders/texture_fill.frag
 FILE: ../../../flutter/impeller/entity/shaders/texture_fill.vert
 FILE: ../../../flutter/impeller/entity/shaders/texture_fill_external.frag
+FILE: ../../../flutter/impeller/entity/shaders/texture_fill_strict_src.frag
 FILE: ../../../flutter/impeller/entity/shaders/tiled_texture_fill.frag
 FILE: ../../../flutter/impeller/entity/shaders/tiled_texture_fill_external.frag
 FILE: ../../../flutter/impeller/entity/shaders/vertices.frag
diff --git a/impeller/aiks/canvas.cc b/impeller/aiks/canvas.cc
@@ -662,7 +662,8 @@ void Canvas::DrawImageRect(const std::shared_ptr<Image>& image,
                            Rect source,
                            Rect dest,
                            const Paint& paint,
-                           SamplerDescriptor sampler) {
+                           SamplerDescriptor sampler,
+                           SourceRectConstraint src_rect_constraint) {
   if (!image || source.IsEmpty() || dest.IsEmpty()) {
     return;
   }
@@ -676,6 +677,8 @@ void Canvas::DrawImageRect(const std::shared_ptr<Image>& image,
   auto contents = TextureContents::MakeRect(dest);
   contents->SetTexture(image->GetTexture());
   contents->SetSourceRect(source);
+  contents->SetStrictSourceRect(src_rect_constraint ==
+                                SourceRectConstraint::kStrict);
   contents->SetSamplerDescriptor(std::move(sampler));
   contents->SetOpacity(paint.color.alpha);
   contents->SetDeferApplyingOpacity(paint.HasColorFilter());
diff --git a/impeller/aiks/canvas.h b/impeller/aiks/canvas.h
@@ -45,6 +45,15 @@ enum class PointStyle {
   kSquare,
 };
 
+/// Controls the behavior of the source rectangle given to DrawImageRect.
+enum class SourceRectConstraint {
+  /// @brief Faster, but may sample outside the bounds of the source rectangle.
+  kFast,
+
+  /// @brief Sample only within the source rectangle. May be slower.
+  kStrict,
+};
+
 class Canvas {
  public:
   struct DebugOptions {
@@ -123,11 +132,13 @@ class Canvas {
                  const Paint& paint,
                  SamplerDescriptor sampler = {});
 
-  void DrawImageRect(const std::shared_ptr<Image>& image,
-                     Rect source,
-                     Rect dest,
-                     const Paint& paint,
-                     SamplerDescriptor sampler = {});
+  void DrawImageRect(
+      const std::shared_ptr<Image>& image,
+      Rect source,
+      Rect dest,
+      const Paint& paint,
+      SamplerDescriptor sampler = {},
+      SourceRectConstraint src_rect_constraint = SourceRectConstraint::kFast);
 
   void ClipPath(
       Path path,
diff --git a/impeller/aiks/canvas_recorder.h b/impeller/aiks/canvas_recorder.h
@@ -237,13 +237,16 @@ class CanvasRecorder {
                                offset, paint, sampler);
   }
 
-  void DrawImageRect(const std::shared_ptr<Image>& image,
-                     Rect source,
-                     Rect dest,
-                     const Paint& paint,
-                     SamplerDescriptor sampler = {}) {
+  void DrawImageRect(
+      const std::shared_ptr<Image>& image,
+      Rect source,
+      Rect dest,
+      const Paint& paint,
+      SamplerDescriptor sampler = {},
+      SourceRectConstraint src_rect_constraint = SourceRectConstraint::kFast) {
     return ExecuteAndSerialize(FLT_CANVAS_RECORDER_OP_ARG(DrawImageRect), image,
-                               source, dest, paint, sampler);
+                               source, dest, paint, sampler,
+                               src_rect_constraint);
   }
 
   void ClipPath(
diff --git a/impeller/aiks/canvas_recorder_unittests.cc b/impeller/aiks/canvas_recorder_unittests.cc
@@ -54,6 +54,8 @@ class Serializer {
 
   void Write(const std::vector<Color>& matrices) {}
 
+  void Write(const SourceRectConstraint& src_rect_constraint) {}
+
   CanvasRecorderOp last_op_;
 };
 }  // namespace
@@ -196,7 +198,7 @@ TEST(CanvasRecorder, DrawImage) {
 
 TEST(CanvasRecorder, DrawImageRect) {
   CanvasRecorder<Serializer> recorder;
-  recorder.DrawImageRect({}, {}, {}, {}, {});
+  recorder.DrawImageRect({}, {}, {}, {}, {}, SourceRectConstraint::kFast);
   ASSERT_EQ(recorder.GetSerializer().last_op_,
             CanvasRecorderOp::kDrawImageRect);
 }
diff --git a/impeller/aiks/trace_serializer.cc b/impeller/aiks/trace_serializer.cc
@@ -254,4 +254,9 @@ void TraceSerializer::Write(const std::vector<Rect>& matrices) {
 void TraceSerializer::Write(const std::vector<Color>& matrices) {
   buffer_ << "[std::vector<Color>] ";
 }
+
+void TraceSerializer::Write(const SourceRectConstraint& src_rect_constraint) {
+  buffer_ << "[SourceRectConstraint] ";
+}
+
 }  // namespace impeller
diff --git a/impeller/aiks/trace_serializer.h b/impeller/aiks/trace_serializer.h
@@ -58,6 +58,8 @@ class TraceSerializer {
 
   void Write(const std::vector<Color>& matrices);
 
+  void Write(const SourceRectConstraint& src_rect_constraint);
+
  private:
   std::stringstream buffer_;
 };
diff --git a/impeller/display_list/dl_unittests.cc b/impeller/display_list/dl_unittests.cc
@@ -801,6 +801,18 @@ TEST_P(DisplayListTest, CanDrawNinePatchImageCornersScaledDown) {
   ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
 }
 
+TEST_P(DisplayListTest, NinePatchImagePrecision) {
+  // Draw a nine patch image with colored corners and verify that the corner
+  // color does not leak outside the intended region.
+  auto texture = CreateTextureForFixture("nine_patch_corners.png");
+  flutter::DisplayListBuilder builder;
+  builder.DrawImageNine(DlImageImpeller::Make(texture),
+                        SkIRect::MakeXYWH(10, 10, 1, 1),
+                        SkRect::MakeXYWH(0, 0, 200, 100),
+                        flutter::DlFilterMode::kNearest, nullptr);
+  ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
+}
+
 TEST_P(DisplayListTest, CanDrawPoints) {
   flutter::DisplayListBuilder builder;
   SkPoint points[7] = {
diff --git a/impeller/display_list/nine_patch_converter.cc b/impeller/display_list/nine_patch_converter.cc
@@ -86,7 +86,7 @@ void NinePatchConverter::DrawNinePatch(const std::shared_ptr<Image>& image,
       // DrawImageAtlas.
       canvas->DrawImageRect(image, Rect::MakeLTRB(srcX0, srcY0, srcX1, srcY1),
                             Rect::MakeLTRB(dstX0, dstY0, dstX1, dstY1), *paint,
-                            sampler);
+                            sampler, SourceRectConstraint::kStrict);
     }
   }
 }
diff --git a/impeller/entity/BUILD.gn b/impeller/entity/BUILD.gn
@@ -52,6 +52,7 @@ impeller_shaders("entity_shaders") {
     "shaders/texture_fill.frag",
     "shaders/texture_fill.vert",
     "shaders/texture_fill_external.frag",
+    "shaders/texture_fill_strict_src.frag",
     "shaders/tiled_texture_fill.frag",
     "shaders/tiled_texture_fill_external.frag",
     "shaders/vertices.frag",
diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc
@@ -332,6 +332,7 @@ ContentContext::ContentContext(
   rrect_blur_pipelines_.CreateDefault(*context_, options_trianglestrip);
   texture_blend_pipelines_.CreateDefault(*context_, options);
   texture_pipelines_.CreateDefault(*context_, options);
+  texture_strict_src_pipelines_.CreateDefault(*context_, options);
   position_uv_pipelines_.CreateDefault(*context_, options);
   tiled_texture_pipelines_.CreateDefault(*context_, options);
   gaussian_blur_noalpha_decal_pipelines_.CreateDefault(*context_,
diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h
@@ -59,6 +59,7 @@
 #include "impeller/entity/sweep_gradient_fill.frag.h"
 #include "impeller/entity/texture_fill.frag.h"
 #include "impeller/entity/texture_fill.vert.h"
+#include "impeller/entity/texture_fill_strict_src.frag.h"
 #include "impeller/entity/tiled_texture_fill.frag.h"
 #include "impeller/entity/uv.comp.h"
 #include "impeller/entity/vertices.frag.h"
@@ -130,6 +131,9 @@ using RRectBlurPipeline =
 using BlendPipeline = RenderPipelineT<BlendVertexShader, BlendFragmentShader>;
 using TexturePipeline =
     RenderPipelineT<TextureFillVertexShader, TextureFillFragmentShader>;
+using TextureStrictSrcPipeline =
+    RenderPipelineT<TextureFillVertexShader,
+                    TextureFillStrictSrcFragmentShader>;
 using PositionUVPipeline =
     RenderPipelineT<TextureFillVertexShader, TiledTextureFillFragmentShader>;
 using TiledTexturePipeline =
@@ -418,6 +422,11 @@ class ContentContext {
     return GetPipeline(texture_pipelines_, opts);
   }
 
+  std::shared_ptr<Pipeline<PipelineDescriptor>> GetTextureStrictSrcPipeline(
+      ContentContextOptions opts) const {
+    return GetPipeline(texture_strict_src_pipelines_, opts);
+  }
+
 #ifdef IMPELLER_ENABLE_OPENGLES
   std::shared_ptr<Pipeline<PipelineDescriptor>> GetTextureExternalPipeline(
       ContentContextOptions opts) const {
@@ -871,6 +880,7 @@ class ContentContext {
   mutable Variants<RRectBlurPipeline> rrect_blur_pipelines_;
   mutable Variants<BlendPipeline> texture_blend_pipelines_;
   mutable Variants<TexturePipeline> texture_pipelines_;
+  mutable Variants<TextureStrictSrcPipeline> texture_strict_src_pipelines_;
 #ifdef IMPELLER_ENABLE_OPENGLES
   mutable Variants<TextureExternalPipeline> texture_external_pipelines_;
   mutable Variants<TiledTextureExternalPipeline>
diff --git a/impeller/entity/contents/texture_contents.cc b/impeller/entity/contents/texture_contents.cc
@@ -111,6 +111,7 @@ bool TextureContents::Render(const ContentContext& renderer,
 
   using VS = TextureFillVertexShader;
   using FS = TextureFillFragmentShader;
+  using FSStrictSrc = TextureFillStrictSrcFragmentShader;
   using FSExternal = TextureFillExternalFragmentShader;
 
   if (destination_rect_.IsEmpty() || source_rect_.IsEmpty() ||
@@ -121,11 +122,9 @@ bool TextureContents::Render(const ContentContext& renderer,
   bool is_external_texture =
       texture_->GetTextureDescriptor().type == TextureType::kTextureExternalOES;
 
-  // Expand the source rect by half a texel, which aligns sampled texels to the
-  // pixel grid if the source rect is the same size as the destination rect.
+  auto source_rect = capture.AddRect("Source rect", source_rect_);
   auto texture_coords =
-      Rect::MakeSize(texture_->GetSize())
-          .Project(capture.AddRect("Source rect", source_rect_).Expand(0.5));
+      Rect::MakeSize(texture_->GetSize()).Project(source_rect);
 
   VertexBufferBuilder<VS::PerVertexData> vertex_builder;
 
@@ -160,16 +159,22 @@ bool TextureContents::Render(const ContentContext& renderer,
   }
   pipeline_options.primitive_type = PrimitiveType::kTriangleStrip;
 
+  std::shared_ptr<Pipeline<PipelineDescriptor>> pipeline;
 #ifdef IMPELLER_ENABLE_OPENGLES
   if (is_external_texture) {
-    pass.SetPipeline(renderer.GetTextureExternalPipeline(pipeline_options));
-  } else {
-    pass.SetPipeline(renderer.GetTexturePipeline(pipeline_options));
+    pipeline = renderer.GetTextureExternalPipeline(pipeline_options);
   }
-#else
-  pass.SetPipeline(renderer.GetTexturePipeline(pipeline_options));
 #endif  // IMPELLER_ENABLE_OPENGLES
 
+  if (!pipeline) {
+    if (strict_source_rect_enabled_) {
+      pipeline = renderer.GetTextureStrictSrcPipeline(pipeline_options);
+    } else {
+      pipeline = renderer.GetTexturePipeline(pipeline_options);
+    }
+  }
+  pass.SetPipeline(pipeline);
+
   pass.SetStencilReference(entity.GetClipDepth());
   pass.SetVertexBuffer(vertex_builder.CreateVertexBuffer(host_buffer));
   VS::BindFrameInfo(pass, host_buffer.EmplaceUniform(frame_info));
@@ -178,6 +183,20 @@ bool TextureContents::Render(const ContentContext& renderer,
         pass, texture_,
         renderer.GetContext()->GetSamplerLibrary()->GetSampler(
             sampler_descriptor_));
+  } else if (strict_source_rect_enabled_) {
+    // For a strict source rect, shrink the texture coordinate range by half a
+    // texel to ensure that linear filtering does not sample anything outside
+    // the source rect bounds.
+    auto strict_texture_coords =
+        Rect::MakeSize(texture_->GetSize()).Project(source_rect.Expand(-0.5));
+
+    FSStrictSrc::FragInfo frag_info;
+    frag_info.source_rect = Vector4(strict_texture_coords.GetLTRB());
+    FSStrictSrc::BindFragInfo(pass, host_buffer.EmplaceUniform(frag_info));
+    FSStrictSrc::BindTextureSampler(
+        pass, texture_,
+        renderer.GetContext()->GetSamplerLibrary()->GetSampler(
+            sampler_descriptor_));
   } else {
     FS::BindTextureSampler(
         pass, texture_,
@@ -195,6 +214,14 @@ const Rect& TextureContents::GetSourceRect() const {
   return source_rect_;
 }
 
+void TextureContents::SetStrictSourceRect(bool strict) {
+  strict_source_rect_enabled_ = strict;
+}
+
+bool TextureContents::GetStrictSourceRect() const {
+  return strict_source_rect_enabled_;
+}
+
 void TextureContents::SetSamplerDescriptor(SamplerDescriptor desc) {
   sampler_descriptor_ = std::move(desc);
 }
diff --git a/impeller/entity/contents/texture_contents.h b/impeller/entity/contents/texture_contents.h
@@ -45,6 +45,10 @@ class TextureContents final : public Contents {
 
   const Rect& GetSourceRect() const;
 
+  void SetStrictSourceRect(bool strict);
+
+  bool GetStrictSourceRect() const;
+
   void SetOpacity(Scalar opacity);
 
   Scalar GetOpacity() const;
@@ -85,6 +89,7 @@ class TextureContents final : public Contents {
   std::shared_ptr<Texture> texture_;
   SamplerDescriptor sampler_descriptor_ = {};
   Rect source_rect_;
+  bool strict_source_rect_enabled_ = false;
   Scalar opacity_ = 1.0f;
   Scalar inherited_opacity_ = 1.0f;
   bool defer_applying_opacity_ = false;
diff --git a/impeller/entity/shaders/texture_fill_strict_src.frag b/impeller/entity/shaders/texture_fill_strict_src.frag
@@ -0,0 +1,30 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+precision mediump float;
+
+#include <impeller/constants.glsl>
+#include <impeller/types.glsl>
+
+uniform f16sampler2D texture_sampler;
+
+uniform FragInfo {
+  vec4 source_rect;
+}
+frag_info;
+
+in highp vec2 v_texture_coords;
+IMPELLER_MAYBE_FLAT in float16_t v_alpha;
+
+out f16vec4 frag_color;
+
+void main() {
+  vec2 texture_coords = vec2(clamp(v_texture_coords.x, frag_info.source_rect.x,
+                                   frag_info.source_rect.z),
+                             clamp(v_texture_coords.y, frag_info.source_rect.y,
+                                   frag_info.source_rect.w));
+  f16vec4 sampled =
+      texture(texture_sampler, texture_coords, kDefaultMipBiasHalf);
+  frag_color = sampled * v_alpha;
+}
diff --git a/impeller/fixtures/BUILD.gn b/impeller/fixtures/BUILD.gn
@@ -89,6 +89,7 @@ test_fixtures("file_fixtures") {
     "flutter_logo_baked.glb",
     "kalimba.jpg",
     "multiple_stages.hlsl",
+    "nine_patch_corners.png",
     "resources_limit.vert",
     "sample.comp",
     "sample.frag",
diff --git a/impeller/fixtures/nine_patch_corners.png b/impeller/fixtures/nine_patch_corners.png
diff --git a/impeller/tools/malioc.json b/impeller/tools/malioc.json

Original file line number	Diff line number	Diff line change
`@@ -86,7 +86,7 @@ void NinePatchConverter::DrawNinePatch(const std::shared_ptr<Image>& image,`
`86`	`86`	`// DrawImageAtlas.`
`87`	`87`	`canvas->DrawImageRect(image, Rect::MakeLTRB(srcX0, srcY0, srcX1, srcY1),`
`88`	`88`	`Rect::MakeLTRB(dstX0, dstY0, dstX1, dstY1), *paint,`
`89`		`- sampler);`
	`89`	`+ sampler, SourceRectConstraint::kStrict);`
`90`	`90`	`}`
`91`	`91`	`}`
`92`	`92`	`}`