Set path for linear gradient contents; don't fail renders for failed tessellations (#162)

bdero · dnfield · commit 4c75fe6e3bd6 · 2022-04-27T15:57:33.000-07:00
diff --git a/impeller/BUILD.gn b/impeller/BUILD.gn
@@ -20,6 +20,7 @@ config("impeller_public_config") {
   if (is_win) {
     defines += [
       "_USE_MATH_DEFINES",
+
       # TODO(dnfield): https://github.com/flutter/flutter/issues/50053
       "_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING",
     ]
@@ -55,6 +56,7 @@ executable("impeller_unittests") {
     "blobcat:blobcat_unittests",
     "compiler:compiler_unittests",
     "geometry:geometry_unittests",
+    "tessellator:tessellator_unittests",
   ]
 
   if (impeller_supports_rendering) {
diff --git a/impeller/aiks/paint.cc b/impeller/aiks/paint.cc
@@ -11,6 +11,7 @@ namespace impeller {
 std::shared_ptr<Contents> Paint::CreateContentsForEntity(Path path,
                                                          bool cover) const {
   if (contents) {
+    contents->SetPath(std::move(path));
     return contents;
   }
 
diff --git a/impeller/aiks/paint.h b/impeller/aiks/paint.h
@@ -9,6 +9,7 @@
 #include "flutter/fml/macros.h"
 #include "impeller/entity/contents/contents.h"
 #include "impeller/entity/contents/filters/filter_contents.h"
+#include "impeller/entity/contents/linear_gradient_contents.h"
 #include "impeller/entity/contents/solid_stroke_contents.h"
 #include "impeller/entity/entity.h"
 #include "impeller/geometry/color.h"
@@ -27,14 +28,15 @@ struct Paint {
   };
 
   Color color = Color::Black();
+  std::shared_ptr<LinearGradientContents> contents;
+
   Scalar stroke_width = 0.0;
   SolidStrokeContents::Cap stroke_cap = SolidStrokeContents::Cap::kButt;
   SolidStrokeContents::Join stroke_join = SolidStrokeContents::Join::kMiter;
   Scalar stroke_miter = 4.0;
   Style style = Style::kFill;
   Entity::BlendMode blend_mode = Entity::BlendMode::kSourceOver;
   std::optional<MaskBlur> mask_blur;
-  std::shared_ptr<Contents> contents;
 
   /// @brief      Wrap this paint's configured filters to the given contents.
   /// @param[in]  input           The contents to wrap with paint's filters.
diff --git a/impeller/entity/contents/linear_gradient_contents.cc b/impeller/entity/contents/linear_gradient_contents.cc
@@ -4,6 +4,7 @@
 
 #include "linear_gradient_contents.h"
 
+#include "flutter/fml/logging.h"
 #include "impeller/entity/contents/content_context.h"
 #include "impeller/entity/entity.h"
 #include "impeller/renderer/render_pass.h"
@@ -58,7 +59,11 @@ bool LinearGradientContents::Render(const ContentContext& renderer,
                                    vtx.vertices = point;
                                    vertices_builder.AppendVertex(vtx);
                                  });
-    if (!result) {
+
+    if (result == Tessellator::Result::kInputError) {
+      return true;
+    }
+    if (result == Tessellator::Result::kTessellationError) {
       return false;
     }
   }
diff --git a/impeller/entity/contents/solid_color_contents.cc b/impeller/entity/contents/solid_color_contents.cc
@@ -50,7 +50,7 @@ VertexBuffer SolidColorContents::CreateSolidFillVertices(const Path& path,
         vtx.vertices = point;
         vtx_builder.AppendVertex(vtx);
       });
-  if (!tesselation_result) {
+  if (tesselation_result != Tessellator::Result::kSuccess) {
     return {};
   }
 
diff --git a/impeller/entity/contents/solid_stroke_contents.cc b/impeller/entity/contents/solid_stroke_contents.cc
@@ -201,9 +201,9 @@ bool SolidStrokeContents::Render(const ContentContext& renderer,
   auto smoothing = SmoothingApproximation(
       5.0 / (stroke_size_ * entity.GetTransformation().GetMaxBasisLength()),
       0.0, 0.0);
-  cmd.BindVertices(CreateSolidStrokeVertices(
-      path_, pass.GetTransientsBuffer(), cap_proc_, join_proc_,
-      miter_limit_, smoothing));
+  cmd.BindVertices(CreateSolidStrokeVertices(path_, pass.GetTransientsBuffer(),
+                                             cap_proc_, join_proc_,
+                                             miter_limit_, smoothing));
   VS::BindFrameInfo(cmd, pass.GetTransientsBuffer().EmplaceUniform(frame_info));
   VS::BindStrokeInfo(cmd,
                      pass.GetTransientsBuffer().EmplaceUniform(stroke_info));
diff --git a/impeller/entity/contents/texture_contents.cc b/impeller/entity/contents/texture_contents.cc
@@ -81,7 +81,11 @@ bool TextureContents::Render(const ContentContext& renderer,
               texture_size;
           vertex_builder.AppendVertex(data);
         });
-    if (!tess_result) {
+
+    if (tess_result == Tessellator::Result::kInputError) {
+      return true;
+    }
+    if (tess_result == Tessellator::Result::kTessellationError) {
       return false;
     }
   }
diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc
@@ -18,6 +18,7 @@
 #include "impeller/playground/widgets.h"
 #include "impeller/renderer/render_pass.h"
 #include "impeller/renderer/vertex_buffer_builder.h"
+#include "impeller/tessellator/tessellator.h"
 #include "third_party/imgui/imgui.h"
 
 namespace impeller {
diff --git a/impeller/geometry/geometry_unittests.cc b/impeller/geometry/geometry_unittests.cc
@@ -264,6 +264,19 @@ TEST(GeometryTest, QuaternionLerp) {
   ASSERT_QUATERNION_NEAR(q3, expected);
 }
 
+TEST(GeometryTest, EmptyPath) {
+  auto path = PathBuilder{}.TakePath();
+  ASSERT_EQ(path.GetComponentCount(), 1u);
+
+  ContourComponent c;
+  path.GetContourComponentAtIndex(0, c);
+  ASSERT_POINT_NEAR(c.destination, Point());
+
+  Path::Polyline polyline = path.CreatePolyline();
+  ASSERT_TRUE(polyline.points.empty());
+  ASSERT_TRUE(polyline.contours.empty());
+}
+
 TEST(GeometryTest, SimplePath) {
   Path path;
 
diff --git a/impeller/renderer/renderer_unittests.cc b/impeller/renderer/renderer_unittests.cc
@@ -279,7 +279,8 @@ TEST_P(RendererTest, CanRenderInstanced) {
 
   VertexBufferBuilder<VS::PerVertexData> builder;
 
-  ASSERT_TRUE(
+  ASSERT_EQ(
+      Tessellator::Result::kSuccess,
       Tessellator{}.Tessellate(FillType::kPositive,
                                PathBuilder{}
                                    .AddRect(Rect::MakeXYWH(10, 10, 100, 100))
diff --git a/impeller/tessellator/BUILD.gn b/impeller/tessellator/BUILD.gn
@@ -23,10 +23,9 @@ impeller_component("tessellator_shared") {
     output_name = "tessellator"
   }
 
-
   sources = [
-    "c/tessellator.h",
     "c/tessellator.cc",
+    "c/tessellator.h",
     "tessellator.cc",
     "tessellator.h",
   ]
@@ -36,3 +35,12 @@ impeller_component("tessellator_shared") {
     "//third_party/libtess2",
   ]
 }
+
+impeller_component("tessellator_unittests") {
+  testonly = true
+  sources = [ "tessellator_unittests.cc" ]
+  deps = [
+    ":tessellator",
+    "//flutter/testing",
+  ]
+}
diff --git a/impeller/tessellator/c/tessellator.cc b/impeller/tessellator/c/tessellator.cc
@@ -47,11 +47,11 @@ struct Vertices* Tessellate(PathBuilder* builder,
   auto polyline = path.CreatePolyline(smoothing);
 
   std::vector<float> points;
-  if (!Tessellator{}.Tessellate(path.GetFillType(), polyline,
-                                [&points](Point vertex) {
-                                  points.push_back(vertex.x);
-                                  points.push_back(vertex.y);
-                                })) {
+  if (Tessellator{}.Tessellate(path.GetFillType(), polyline,
+                               [&points](Point vertex) {
+                                 points.push_back(vertex.x);
+                                 points.push_back(vertex.y);
+                               }) != Tessellator::Result::kSuccess) {
     return nullptr;
   }
 
diff --git a/impeller/tessellator/tessellator.cc b/impeller/tessellator/tessellator.cc
@@ -34,11 +34,15 @@ static void DestroyTessellator(TESStesselator* tessellator) {
   }
 }
 
-bool Tessellator::Tessellate(FillType fill_type,
-                             const Path::Polyline& polyline,
-                             VertexCallback callback) const {
+Tessellator::Result Tessellator::Tessellate(FillType fill_type,
+                                            const Path::Polyline& polyline,
+                                            VertexCallback callback) const {
   if (!callback) {
-    return false;
+    return Result::kInputError;
+  }
+
+  if (polyline.points.empty()) {
+    return Result::kInputError;
   }
 
   using CTessellator =
@@ -49,7 +53,7 @@ bool Tessellator::Tessellate(FillType fill_type,
       DestroyTessellator);
 
   if (!tessellator) {
-    return false;
+    return Result::kTessellationError;
   }
 
   constexpr int kVertexSize = 2;
@@ -85,7 +89,7 @@ bool Tessellator::Tessellate(FillType fill_type,
   );
 
   if (result != 1) {
-    return false;
+    return Result::kTessellationError;
   }
 
   // TODO(csg): This copy can be elided entirely for the current use case.
@@ -109,7 +113,7 @@ bool Tessellator::Tessellate(FillType fill_type,
     callback(vtx);
   }
 
-  return true;
+  return Result::kSuccess;
 }
 
 }  // namespace impeller
diff --git a/impeller/tessellator/tessellator.h b/impeller/tessellator/tessellator.h
@@ -26,6 +26,12 @@ enum class WindingOrder {
 ///
 class Tessellator {
  public:
+  enum class Result {
+    kSuccess,
+    kInputError,
+    kTessellationError,
+  };
+
   Tessellator();
 
   ~Tessellator();
@@ -39,11 +45,11 @@ class Tessellator {
   /// @param[in]  polyline  The polyline
   /// @param[in]  callback  The callback
   ///
-  /// @return If tessellation was successful.
+  /// @return The result status of the tessellation.
   ///
-  bool Tessellate(FillType fill_type,
-                  const Path::Polyline& polyline,
-                  VertexCallback callback) const;
+  Tessellator::Result Tessellate(FillType fill_type,
+                                 const Path::Polyline& polyline,
+                                 VertexCallback callback) const;
 
  private:
   FML_DISALLOW_COPY_AND_ASSIGN(Tessellator);
diff --git a/impeller/tessellator/tessellator_unittests.cc b/impeller/tessellator/tessellator_unittests.cc
@@ -0,0 +1,50 @@
+// 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.
+
+#include "flutter/testing/testing.h"
+#include "gtest/gtest.h"
+#include "impeller/geometry/path_builder.h"
+#include "impeller/tessellator/tessellator.h"
+
+namespace impeller {
+namespace testing {
+
+TEST(TessellatorTest, TessellatorReturnsCorrectResultStatus) {
+  // Zero points.
+  {
+    Tessellator t;
+    auto polyline = PathBuilder{}.TakePath().CreatePolyline();
+    Tessellator::Result result =
+        t.Tessellate(FillType::kPositive, polyline, [](Point point) {});
+
+    ASSERT_EQ(polyline.points.size(), 0u);
+    ASSERT_EQ(result, Tessellator::Result::kInputError);
+  }
+
+  // One point.
+  {
+    Tessellator t;
+    auto polyline = PathBuilder{}.LineTo({0, 0}).TakePath().CreatePolyline();
+    Tessellator::Result result =
+        t.Tessellate(FillType::kPositive, polyline, [](Point point) {});
+
+    ASSERT_EQ(polyline.points.size(), 1u);
+    ASSERT_EQ(result, Tessellator::Result::kSuccess);
+  }
+
+  // Two points.
+  {
+    Tessellator t;
+    auto polyline =
+        PathBuilder{}.AddLine({0, 0}, {0, 1}).TakePath().CreatePolyline();
+    Tessellator::Result result =
+        t.Tessellate(FillType::kPositive, polyline, [](Point point) {});
+
+    ASSERT_EQ(polyline.points.size(), 2u);
+    ASSERT_EQ(result, Tessellator::Result::kSuccess);
+  }
+}
+
+}  // namespace testing
+}  // namespace impeller

Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,7 @@ config("impeller_public_config") {`
`20`	`20`	`if (is_win) {`
`21`	`21`	`defines += [`
`22`	`22`	`"_USE_MATH_DEFINES",`
	`23`	`+`
`23`	`24`	`# TODO(dnfield): https://github.com/flutter/flutter/issues/50053`
`24`	`25`	`"_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING",`
`25`	`26`	`]`
`@@ -55,6 +56,7 @@ executable("impeller_unittests") {`
`55`	`56`	`"blobcat:blobcat_unittests",`
`56`	`57`	`"compiler:compiler_unittests",`
`57`	`58`	`"geometry:geometry_unittests",`
	`59`	`+ "tessellator:tessellator_unittests",`
`58`	`60`	`]`
`59`	`61`
`60`	`62`	`if (impeller_supports_rendering) {`
Original file line number	Diff line number	Diff line change
`@@ -11,6 +11,7 @@ namespace impeller {`
`11`	`11`	`std::shared_ptr<Contents> Paint::CreateContentsForEntity(Path path,`
`12`	`12`	`bool cover) const {`
`13`	`13`	`if (contents) {`
	`14`	`+ contents->SetPath(std::move(path));`
`14`	`15`	`return contents;`
`15`	`16`	`}`
`16`	`17`
Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,7 @@ VertexBuffer SolidColorContents::CreateSolidFillVertices(const Path& path,`
`50`	`50`	`vtx.vertices = point;`
`51`	`51`	`vtx_builder.AppendVertex(vtx);`
`52`	`52`	`});`
`53`		`- if (!tesselation_result) {`
	`53`	`+ if (tesselation_result != Tessellator::Result::kSuccess) {`
`54`	`54`	`return {};`
`55`	`55`	`}`
`56`	`56`
Original file line number	Diff line number	Diff line change
`@@ -81,7 +81,11 @@ bool TextureContents::Render(const ContentContext& renderer,`
`81`	`81`	`texture_size;`
`82`	`82`	`vertex_builder.AppendVertex(data);`
`83`	`83`	`});`
`84`		`- if (!tess_result) {`
	`84`	`+`
	`85`	`+ if (tess_result == Tessellator::Result::kInputError) {`
	`86`	`+ return true;`
	`87`	`+ }`
	`88`	`+ if (tess_result == Tessellator::Result::kTessellationError) {`
`85`	`89`	`return false;`
`86`	`90`	`}`
`87`	`91`	`}`