From 767efe1ad7c1ed3dbc8ed0cd6fe0def290d7b4a1 Mon Sep 17 00:00:00 2001
From: Jonah Williams <jonahwilliams@google.com>
Date: Fri, 4 Oct 2024 19:16:37 -0500
Subject: [PATCH] [Impeller] remove usage of MaxBasisLength in favor of XY
 variant. (flutter/engine#55670)

MaxBasisXYZ prevents usage of scaling factors less than one since Z is almost always 1.

Fixes https://github.com/flutter/flutter/issues/153451
---
 .../entity/geometry/fill_path_geometry.cc         |  2 +-
 .../entity/geometry/stroke_path_geometry.cc       |  2 +-
 .../impeller/geometry/geometry_unittests.cc       | 15 ---------------
 engine/src/flutter/impeller/geometry/matrix.cc    |  9 ---------
 engine/src/flutter/impeller/geometry/matrix.h     |  2 --
 5 files changed, 2 insertions(+), 28 deletions(-)

diff --git a/engine/src/flutter/impeller/entity/geometry/fill_path_geometry.cc b/engine/src/flutter/impeller/entity/geometry/fill_path_geometry.cc
index 209a9d647a..7316182749 100644
--- a/engine/src/flutter/impeller/entity/geometry/fill_path_geometry.cc
+++ b/engine/src/flutter/impeller/entity/geometry/fill_path_geometry.cc
@@ -37,7 +37,7 @@ GeometryResult FillPathGeometry::GetPositionBuffer(
   }
 
   VertexBuffer vertex_buffer = renderer.GetTessellator()->TessellateConvex(
-      path_, host_buffer, entity.GetTransform().GetMaxBasisLength());
+      path_, host_buffer, entity.GetTransform().GetMaxBasisLengthXY());
 
   return GeometryResult{
       .type = PrimitiveType::kTriangleStrip,
diff --git a/engine/src/flutter/impeller/entity/geometry/stroke_path_geometry.cc b/engine/src/flutter/impeller/entity/geometry/stroke_path_geometry.cc
index cd188c6044..aa06c3ad98 100644
--- a/engine/src/flutter/impeller/entity/geometry/stroke_path_geometry.cc
+++ b/engine/src/flutter/impeller/entity/geometry/stroke_path_geometry.cc
@@ -581,7 +581,7 @@ GeometryResult StrokePathGeometry::GetPositionBuffer(
   Scalar stroke_width = std::max(stroke_width_, min_size);
 
   auto& host_buffer = renderer.GetTransientsBuffer();
-  auto scale = entity.GetTransform().GetMaxBasisLength();
+  auto scale = entity.GetTransform().GetMaxBasisLengthXY();
 
   PositionWriter position_writer;
   auto polyline = renderer.GetTessellator()->CreateTempPolyline(path_, scale);
diff --git a/engine/src/flutter/impeller/geometry/geometry_unittests.cc b/engine/src/flutter/impeller/geometry/geometry_unittests.cc
index 697471e286..cdc4078c49 100644
--- a/engine/src/flutter/impeller/geometry/geometry_unittests.cc
+++ b/engine/src/flutter/impeller/geometry/geometry_unittests.cc
@@ -330,21 +330,6 @@ TEST(GeometryTest, MatrixTransformDirection) {
   }
 }
 
-TEST(GeometryTest, MatrixGetMaxBasisLength) {
-  {
-    auto m = Matrix::MakeScale({3, 1, 1});
-    ASSERT_EQ(m.GetMaxBasisLength(), 3);
-
-    m = m * Matrix::MakeSkew(0, 4);
-    ASSERT_EQ(m.GetMaxBasisLength(), 5);
-  }
-
-  {
-    auto m = Matrix::MakeScale({-3, 4, 2});
-    ASSERT_EQ(m.GetMaxBasisLength(), 4);
-  }
-}
-
 TEST(GeometryTest, MatrixGetMaxBasisLengthXY) {
   {
     auto m = Matrix::MakeScale({3, 1, 1});
diff --git a/engine/src/flutter/impeller/geometry/matrix.cc b/engine/src/flutter/impeller/geometry/matrix.cc
index 5f1c367c7b..e47dd867bd 100644
--- a/engine/src/flutter/impeller/geometry/matrix.cc
+++ b/engine/src/flutter/impeller/geometry/matrix.cc
@@ -193,15 +193,6 @@ Scalar Matrix::GetDeterminant() const {
   return b00 * b11 - b01 * b10 + b02 * b09 + b03 * b08 - b04 * b07 + b05 * b06;
 }
 
-Scalar Matrix::GetMaxBasisLength() const {
-  Scalar max = 0;
-  for (int i = 0; i < 3; i++) {
-    max = std::max(max,
-                   e[i][0] * e[i][0] + e[i][1] * e[i][1] + e[i][2] * e[i][2]);
-  }
-  return std::sqrt(max);
-}
-
 /*
  *  Adapted for Impeller from Graphics Gems:
  *  http://www.realtimerendering.com/resources/GraphicsGems/gemsii/unmatrix.c
diff --git a/engine/src/flutter/impeller/geometry/matrix.h b/engine/src/flutter/impeller/geometry/matrix.h
index 3f9960737c..1726c30151 100644
--- a/engine/src/flutter/impeller/geometry/matrix.h
+++ b/engine/src/flutter/impeller/geometry/matrix.h
@@ -295,8 +295,6 @@ struct Matrix {
 
   Scalar GetDeterminant() const;
 
-  Scalar GetMaxBasisLength() const;
-
   constexpr Scalar GetMaxBasisLengthXY() const {
     // The full basis computation requires computing the squared scaling factor
     // for translate/scale only matrices. This substantially limits the range of