[Impeller] Linear sample atlas glyphs when the CTM isn't translation/…

…scale only (#39112)
flutter · Jan 25, 2023 · 55bb8de · 55bb8de
1 parent f1464b4
commit 55bb8de
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 2 deletions.
diff --git a/impeller/entity/contents/text_contents.cc b/impeller/entity/contents/text_contents.cc
@@ -78,8 +78,18 @@ static bool CommonRender(
   VS::BindFrameInfo(cmd, pass.GetTransientsBuffer().EmplaceUniform(frame_info));
 
   SamplerDescriptor sampler_desc;
-  sampler_desc.min_filter = MinMagFilter::kNearest;
-  sampler_desc.mag_filter = MinMagFilter::kNearest;
+  if (entity.GetTransformation().IsTranslationScaleOnly()) {
+    sampler_desc.min_filter = MinMagFilter::kNearest;
+    sampler_desc.mag_filter = MinMagFilter::kNearest;
+  } else {
+    // Currently, we only propagate the scale of the transform to the atlas
+    // renderer, so if the transform has more than just a translation, we turn
+    // on linear sampling to prevent crunchiness caused by the pixel grid not
+    // being perfectly aligned.
+    // The downside is that this slightly over-blurs rotated/skewed text.
+    sampler_desc.min_filter = MinMagFilter::kLinear;
+    sampler_desc.mag_filter = MinMagFilter::kLinear;
+  }
   sampler_desc.mip_filter = MipFilter::kNone;
 
   typename FS::FragInfo frag_info;

diff --git a/impeller/geometry/geometry_unittests.cc b/impeller/geometry/geometry_unittests.cc
@@ -444,6 +444,32 @@ TEST(GeometryTest, MatrixIsAligned) {
   }
 }
 
+TEST(GeometryTest, MatrixTranslationScaleOnly) {
+  {
+    auto m = Matrix();
+    bool result = m.IsTranslationScaleOnly();
+    ASSERT_TRUE(result);
+  }
+
+  {
+    auto m = Matrix::MakeScale(Vector3(2, 3, 4));
+    bool result = m.IsTranslationScaleOnly();
+    ASSERT_TRUE(result);
+  }
+
+  {
+    auto m = Matrix::MakeTranslation(Vector3(2, 3, 4));
+    bool result = m.IsTranslationScaleOnly();
+    ASSERT_TRUE(result);
+  }
+
+  {
+    auto m = Matrix::MakeRotationZ(Degrees(10));
+    bool result = m.IsTranslationScaleOnly();
+    ASSERT_FALSE(result);
+  }
+}
+
 TEST(GeometryTest, MatrixLookAt) {
   {
     auto m = Matrix::MakeLookAt(Vector3(0, 0, -1), Vector3(0, 0, 1),

diff --git a/impeller/geometry/matrix.h b/impeller/geometry/matrix.h
@@ -347,6 +347,19 @@ struct Matrix {
     );
   }
 
+  /// @brief  Returns true if the matrix has a scale-only basis and is
+  ///         non-projective. Note that an identity matrix meets this criteria.
+  constexpr bool IsTranslationScaleOnly() const {
+    return (
+        // clang-format off
+        m[0] != 0.0 && m[1]  == 0.0 && m[2]  == 0.0 && m[3]  == 0.0 &&
+        m[4] == 0.0 && m[5]  != 0.0 && m[6]  == 0.0 && m[7]  == 0.0 &&
+        m[8] == 0.0 && m[9]  == 0.0 && m[10] != 0.0 && m[11] == 0.0 &&
+                                                       m[15] == 1.0
+        // clang-format on
+    );
+  }
+
   std::optional<MatrixDecomposition> Decompose() const;
 
   constexpr bool operator==(const Matrix& m) const {