[Impeller] fix drawPoints scaling factors. (#54368)

Fixes flutter/flutter#152780
Fixes flutter/flutter#152794

Problems:

* If point scale is < 1, we discarded it too early by computing circle divisions with the rounded scale. Since we multiply the scale by the transform _after_ rounding, we may end up generating too many points in some circumstances.
* Tessellator used max basis XYZ instead of max basis XY. The latter will never allow scaling factors less than 1 as Flutter canvas scale does not impact Z, only transform layers do. 
* Computation of max basis required squaring the scaling factor, which would cause us to hit float::inf too early. For translate scale just take the max of m[0] and m[5].
* Draw points minimum circumference is 1, so the min radius should be 0.5

Author: Jonah Williams

impeller/display_list/aiks_dl_unittests.cc

@@ -800,5 +800,46 @@ TEST_P(AiksTest, MatrixSaveLayerFilter) {
   ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
 }
 
+// Regression test for flutter/flutter#152780
+TEST_P(AiksTest, CanDrawScaledPointsSmallScaleLargeRadius) {
+  std::vector<SkPoint> point = {
+      {0, 0},  //
+  };
+
+  DlPaint paint;
+  paint.setStrokeCap(DlStrokeCap::kRound);
+  paint.setColor(DlColor::kRed());
+  paint.setStrokeWidth(100 * 1000000);
+
+  DisplayListBuilder builder(GetCullRect(GetWindowSize()));
+  builder.Translate(200, 200);
+  builder.Scale(0.000001, 0.000001);
+
+  builder.DrawPoints(DlCanvas::PointMode::kPoints, point.size(), point.data(),
+                     paint);
+
+  ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
+}
+
+// Regression test for flutter/flutter#152780
+TEST_P(AiksTest, CanDrawScaledPointsLargeScaleSmallRadius) {
+  std::vector<SkPoint> point = {
+      {0, 0},  //
+  };
+
+  DlPaint paint;
+  paint.setStrokeCap(DlStrokeCap::kRound);
+  paint.setColor(DlColor::kRed());
+  paint.setStrokeWidth(100 * 0.000001);
+
+  DisplayListBuilder builder(GetCullRect(GetWindowSize()));
+  builder.Translate(200, 200);
+  builder.Scale(1000000, 1000000);
+
+  builder.DrawPoints(DlCanvas::PointMode::kPoints, point.size(), point.data(),
+                     paint);
+  ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
+}
+
 }  // namespace testing
 }  // namespace impeller

impeller/entity/geometry/point_field_geometry.cc

@@ -4,7 +4,7 @@
 
 #include "impeller/entity/geometry/point_field_geometry.h"
 
-#include "impeller/core/vertex_buffer.h"
+#include "impeller/core/formats.h"
 #include "impeller/entity/geometry/geometry.h"
 #include "impeller/renderer/command_buffer.h"
 
@@ -22,18 +22,17 @@ GeometryResult PointFieldGeometry::GetPositionBuffer(
   if (radius_ < 0.0) {
     return {};
   }
-  Matrix transform = entity.GetTransform();
-  Scalar determinant = transform.GetDeterminant();
-  if (determinant == 0) {
+  const Matrix& transform = entity.GetTransform();
+
+  Scalar max_basis = transform.GetMaxBasisLengthXY();
+  if (max_basis == 0) {
     return {};
   }
-
-  Scalar min_size = 1.0f / sqrt(std::abs(determinant));
+  Scalar min_size = 0.5f / max_basis;
   Scalar radius = std::max(radius_, min_size);
 
   HostBuffer& host_buffer = renderer.GetTransientsBuffer();
   VertexBufferBuilder<SolidFillVertexShader::PerVertexData> vtx_builder;
-
   if (round_) {
     // Get triangulation relative to {0, 0} so we can translate it to each
     // point in turn.

impeller/geometry/matrix.h

@@ -298,6 +298,13 @@ struct Matrix {
   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
+    // precision for small and large scales. Instead, check for the common cases
+    // and directly return the max scaling factor.
+    if (e[0][1] == 0 && e[1][0] == 0) {
+      return std::max(e[0][0], e[1][1]);
+    }
     return std::sqrt(std::max(e[0][0] * e[0][0] + e[0][1] * e[0][1],
                               e[1][0] * e[1][0] + e[1][1] * e[1][1]));
   }

impeller/geometry/matrix_unittests.cc

@@ -7,6 +7,7 @@
 #include "flutter/impeller/geometry/matrix.h"
 
 #include "flutter/impeller/geometry/geometry_asserts.h"
+#include "impeller/geometry/constants.h"
 
 namespace impeller {
 namespace testing {
@@ -160,5 +161,21 @@ TEST(MatrixTest, TransformHomogenous) {
             Vector3(32.0f, 33.0f, 41.0f));
 }
 
+// Verifies a translate scale matrix doesn't need to compute sqrt(pow(scale, 2))
+TEST(MatrixTest, GetMaxBasisXYWithLargeAndSmallScalingFactor) {
+  Matrix m = Matrix::MakeScale({2.625e+20, 2.625e+20, 1});
+  EXPECT_NEAR(m.GetMaxBasisLengthXY(), 2.625e+20, 1e+20);
+
+  m = Matrix::MakeScale({2.625e-20, 2.625e-20, 1});
+  EXPECT_NEAR(m.GetMaxBasisLengthXY(), 2.625e-20, 1e-20);
+}
+
+TEST(MatrixTest, GetMaxBasisXYWithLargeAndSmallScalingFactorNonScaleTranslate) {
+  Matrix m = Matrix::MakeScale({2.625e+20, 2.625e+20, 1});
+  m.e[0][1] = 2;
+
+  EXPECT_TRUE(std::isinf(m.GetMaxBasisLengthXY()));
+}
+
 }  // namespace testing
 }  // namespace impeller

impeller/tessellator/tessellator.cc

@@ -238,8 +238,8 @@ EllipticalVertexGenerator Tessellator::FilledCircle(
     const Matrix& view_transform,
     const Point& center,
     Scalar radius) {
-  auto divisions =
-      ComputeQuadrantDivisions(view_transform.GetMaxBasisLength() * radius);
+  size_t divisions =
+      ComputeQuadrantDivisions(view_transform.GetMaxBasisLengthXY() * radius);
   return EllipticalVertexGenerator(Tessellator::GenerateFilledCircle,
                                    GetTrigsForDivisions(divisions),
                                    PrimitiveType::kTriangleStrip, 4,
@@ -257,7 +257,7 @@ EllipticalVertexGenerator Tessellator::StrokedCircle(
     Scalar half_width) {
   if (half_width > 0) {
     auto divisions = ComputeQuadrantDivisions(
-        view_transform.GetMaxBasisLength() * radius + half_width);
+        view_transform.GetMaxBasisLengthXY() * radius + half_width);
     return EllipticalVertexGenerator(Tessellator::GenerateStrokedCircle,
                                      GetTrigsForDivisions(divisions),
                                      PrimitiveType::kTriangleStrip, 8,
@@ -280,7 +280,7 @@ EllipticalVertexGenerator Tessellator::RoundCapLine(
   auto length = along.GetLength();
   if (length > kEhCloseEnough) {
     auto divisions =
-        ComputeQuadrantDivisions(view_transform.GetMaxBasisLength() * radius);
+        ComputeQuadrantDivisions(view_transform.GetMaxBasisLengthXY() * radius);
     return EllipticalVertexGenerator(Tessellator::GenerateRoundCapLine,
                                      GetTrigsForDivisions(divisions),
                                      PrimitiveType::kTriangleStrip, 4,
@@ -302,8 +302,8 @@ EllipticalVertexGenerator Tessellator::FilledEllipse(
                         bounds.GetWidth() * 0.5f);
   }
   auto max_radius = bounds.GetSize().MaxDimension();
-  auto divisions =
-      ComputeQuadrantDivisions(view_transform.GetMaxBasisLength() * max_radius);
+  auto divisions = ComputeQuadrantDivisions(
+      view_transform.GetMaxBasisLengthXY() * max_radius);
   auto center = bounds.GetCenter();
   return EllipticalVertexGenerator(Tessellator::GenerateFilledEllipse,
                                    GetTrigsForDivisions(divisions),
@@ -323,7 +323,7 @@ EllipticalVertexGenerator Tessellator::FilledRoundRect(
       radii.height * 2 < bounds.GetHeight()) {
     auto max_radius = radii.MaxDimension();
     auto divisions = ComputeQuadrantDivisions(
-        view_transform.GetMaxBasisLength() * max_radius);
+        view_transform.GetMaxBasisLengthXY() * max_radius);
     auto upper_left = bounds.GetLeftTop() + radii;
     auto lower_right = bounds.GetRightBottom() - radii;
     return EllipticalVertexGenerator(Tessellator::GenerateFilledRoundRect,

testing/impeller_golden_tests_output.txt

@@ -218,6 +218,12 @@ impeller_Play_AiksTest_CanDrawPointsWithTextureMap_Vulkan.png
 impeller_Play_AiksTest_CanDrawPoints_Metal.png
 impeller_Play_AiksTest_CanDrawPoints_OpenGLES.png
 impeller_Play_AiksTest_CanDrawPoints_Vulkan.png
+impeller_Play_AiksTest_CanDrawScaledPointsLargeScaleSmallRadius_Metal.png
+impeller_Play_AiksTest_CanDrawScaledPointsLargeScaleSmallRadius_OpenGLES.png
+impeller_Play_AiksTest_CanDrawScaledPointsLargeScaleSmallRadius_Vulkan.png
+impeller_Play_AiksTest_CanDrawScaledPointsSmallScaleLargeRadius_Metal.png
+impeller_Play_AiksTest_CanDrawScaledPointsSmallScaleLargeRadius_OpenGLES.png
+impeller_Play_AiksTest_CanDrawScaledPointsSmallScaleLargeRadius_Vulkan.png
 impeller_Play_AiksTest_CanEmptyPictureConvertToImage_Metal.png
 impeller_Play_AiksTest_CanEmptyPictureConvertToImage_OpenGLES.png
 impeller_Play_AiksTest_CanEmptyPictureConvertToImage_Vulkan.png