[Impeller] Reland: Correctly compute UVs in texture fill

impeller/aiks/paint_pass_delegate.cc

@@ -4,6 +4,8 @@
 
 #include "impeller/aiks/paint_pass_delegate.h"
 
+#include "impeller/core/formats.h"
+#include "impeller/core/sampler_descriptor.h"
 #include "impeller/entity/contents/contents.h"
 #include "impeller/entity/contents/texture_contents.h"
 #include "impeller/entity/entity_pass.h"
@@ -46,6 +48,7 @@ std::shared_ptr<Contents> PaintPassDelegate::CreateContentsForSubpassTarget(
   contents->SetSourceRect(Rect::MakeSize(target->GetSize()));
   contents->SetOpacity(paint_.color.alpha);
   contents->SetDeferApplyingOpacity(true);
+
   return paint_.WithFiltersForSubpassTarget(std::move(contents),
                                             effect_transform);
 }
@@ -140,6 +143,7 @@ OpacityPeepholePassDelegate::CreateContentsForSubpassTarget(
   contents->SetSourceRect(Rect::MakeSize(target->GetSize()));
   contents->SetOpacity(paint_.color.alpha);
   contents->SetDeferApplyingOpacity(true);
+
   return paint_.WithFiltersForSubpassTarget(std::move(contents),
                                             effect_transform);
 }

impeller/entity/contents/texture_contents.cc

@@ -26,16 +26,16 @@ TextureContents::~TextureContents() = default;
 
 std::shared_ptr<TextureContents> TextureContents::MakeRect(Rect destination) {
   auto contents = std::make_shared<TextureContents>();
-  contents->rect_ = destination;
+  contents->destination_rect_ = destination;
   return contents;
 }
 
 void TextureContents::SetLabel(std::string label) {
   label_ = std::move(label);
 }
 
-void TextureContents::SetRect(Rect rect) {
-  rect_ = rect;
+void TextureContents::SetDestinationRect(Rect rect) {
+  destination_rect_ = rect;
 }
 
 void TextureContents::SetTexture(std::shared_ptr<Texture> texture) {
@@ -70,7 +70,7 @@ std::optional<Rect> TextureContents::GetCoverage(const Entity& entity) const {
   if (GetOpacity() == 0) {
     return std::nullopt;
   }
-  return rect_.TransformBounds(entity.GetTransformation());
+  return destination_rect_.TransformBounds(entity.GetTransformation());
 };
 
 std::optional<Snapshot> TextureContents::RenderToSnapshot(
@@ -82,7 +82,7 @@ std::optional<Snapshot> TextureContents::RenderToSnapshot(
     const std::string& label) const {
   // Passthrough textures that have simple rectangle paths and complete source
   // rects.
-  auto bounds = rect_;
+  auto bounds = destination_rect_;
   auto opacity = GetOpacity();
   if (source_rect_ == Rect::MakeSize(texture_->GetSize()) &&
       (opacity >= 1 - kEhCloseEnough || defer_applying_opacity_)) {
@@ -104,37 +104,30 @@ std::optional<Snapshot> TextureContents::RenderToSnapshot(
       label);                                            // label
 }
 
-static TextureFillVertexShader::PerVertexData ComputeVertexData(
-    const Point& position,
-    const Rect& coverage_rect,
-    const ISize& texture_size,
-    const Rect& source_rect) {
-  TextureFillVertexShader::PerVertexData data;
-  data.position = position;
-  auto coverage_coords = (position - coverage_rect.origin) / coverage_rect.size;
-  data.texture_coords =
-      (source_rect.origin + source_rect.size * coverage_coords) / texture_size;
-  return data;
-}
-
 bool TextureContents::Render(const ContentContext& renderer,
                              const Entity& entity,
                              RenderPass& pass) const {
   using VS = TextureFillVertexShader;
   using FS = TextureFillFragmentShader;
 
-  const auto coverage_rect = rect_;
-
-  if (coverage_rect.size.IsEmpty() || source_rect_.IsEmpty() ||
+  if (destination_rect_.size.IsEmpty() || source_rect_.IsEmpty() ||
       texture_ == nullptr || texture_->GetSize().IsEmpty()) {
-    return true;
+    return true;  // Nothing to render.
   }
 
+  // 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 texture_coords =
+      Rect::MakeSize(texture_->GetSize()).Project(source_rect_.Expand(0.5));
+
   VertexBufferBuilder<VS::PerVertexData> vertex_builder;
-  for (const auto position : rect_.GetPoints()) {
-    vertex_builder.AppendVertex(ComputeVertexData(
-        position, coverage_rect, texture_->GetSize(), source_rect_));
-  }
+
+  vertex_builder.AddVertices({
+      {destination_rect_.GetLeftTop(), texture_coords.GetLeftTop()},
+      {destination_rect_.GetRightTop(), texture_coords.GetRightTop()},
+      {destination_rect_.GetLeftBottom(), texture_coords.GetLeftBottom()},
+      {destination_rect_.GetRightBottom(), texture_coords.GetRightBottom()},
+  });
 
   auto& host_buffer = pass.GetTransientsBuffer();
 

impeller/entity/contents/texture_contents.h

@@ -30,7 +30,7 @@ class TextureContents final : public Contents {
 
   void SetLabel(std::string label);
 
-  void SetRect(Rect rect);
+  void SetDestinationRect(Rect rect);
 
   void SetTexture(std::shared_ptr<Texture> texture);
 
@@ -78,7 +78,7 @@ class TextureContents final : public Contents {
  private:
   std::string label_;
 
-  Rect rect_;
+  Rect destination_rect_;
   bool stencil_enabled_ = true;
 
   std::shared_ptr<Texture> texture_;

impeller/entity/entity_unittests.cc

@@ -1069,7 +1069,7 @@ TEST_P(EntityTest, GaussianBlurFilter) {
     if (selected_input_type == 0) {
       auto texture = std::make_shared<TextureContents>();
       texture->SetSourceRect(Rect::MakeSize(boston->GetSize()));
-      texture->SetRect(input_rect);
+      texture->SetDestinationRect(input_rect);
       texture->SetTexture(boston);
       texture->SetOpacity(input_color.alpha);
 
@@ -1192,7 +1192,7 @@ TEST_P(EntityTest, MorphologyFilter) {
         Rect::MakeXYWH(path_rect[0], path_rect[1], path_rect[2], path_rect[3]);
     auto texture = std::make_shared<TextureContents>();
     texture->SetSourceRect(Rect::MakeSize(boston->GetSize()));
-    texture->SetRect(input_rect);
+    texture->SetDestinationRect(input_rect);
     texture->SetTexture(boston);
     texture->SetOpacity(input_color.alpha);
 

impeller/geometry/geometry_unittests.cc

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "gtest/gtest.h"
 #include "impeller/geometry/geometry_asserts.h"
 
 #include <limits>
@@ -2142,6 +2143,20 @@ TEST(GeometryTest, RectScale) {
   }
 }
 
+TEST(GeometryTest, RectDirections) {
+  auto r = Rect::MakeLTRB(1, 2, 3, 4);
+
+  ASSERT_EQ(r.GetLeft(), 1);
+  ASSERT_EQ(r.GetTop(), 2);
+  ASSERT_EQ(r.GetRight(), 3);
+  ASSERT_EQ(r.GetBottom(), 4);
+
+  ASSERT_POINT_NEAR(r.GetLeftTop(), Point(1, 2));
+  ASSERT_POINT_NEAR(r.GetRightTop(), Point(3, 2));
+  ASSERT_POINT_NEAR(r.GetLeftBottom(), Point(1, 4));
+  ASSERT_POINT_NEAR(r.GetRightBottom(), Point(3, 4));
+}
+
 TEST(GeometryTest, RectProject) {
   {
     auto r = Rect::MakeLTRB(-100, -100, 100, 100);

impeller/geometry/rect.h

@@ -162,6 +162,16 @@ struct TRect {
     return std::max(origin.y, origin.y + size.height);
   }
 
+  constexpr TPoint<T> GetLeftTop() const { return {GetLeft(), GetTop()}; }
+
+  constexpr TPoint<T> GetRightTop() const { return {GetRight(), GetTop()}; }
+
+  constexpr TPoint<T> GetLeftBottom() const { return {GetLeft(), GetBottom()}; }
+
+  constexpr TPoint<T> GetRightBottom() const {
+    return {GetRight(), GetBottom()};
+  }
+
   constexpr std::array<T, 4> GetLTRB() const {
     return {GetLeft(), GetTop(), GetRight(), GetBottom()};
   }