From 4c75fe6e3bd6f09c64e4f3bdc0469f62ac9b0d69 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Tue, 26 Apr 2022 01:05:08 -0700 Subject: [PATCH] Set path for linear gradient contents; don't fail renders for failed tessellations (#162) --- impeller/BUILD.gn | 2 + impeller/aiks/paint.cc | 1 + impeller/aiks/paint.h | 4 +- .../contents/linear_gradient_contents.cc | 7 ++- .../entity/contents/solid_color_contents.cc | 2 +- .../entity/contents/solid_stroke_contents.cc | 6 +-- impeller/entity/contents/texture_contents.cc | 6 ++- impeller/entity/entity_unittests.cc | 1 + impeller/geometry/geometry_unittests.cc | 13 +++++ impeller/renderer/renderer_unittests.cc | 3 +- impeller/tessellator/BUILD.gn | 12 ++++- impeller/tessellator/c/tessellator.cc | 10 ++-- impeller/tessellator/tessellator.cc | 18 ++++--- impeller/tessellator/tessellator.h | 14 ++++-- impeller/tessellator/tessellator_unittests.cc | 50 +++++++++++++++++++ 15 files changed, 123 insertions(+), 26 deletions(-) create mode 100644 impeller/tessellator/tessellator_unittests.cc diff --git a/impeller/BUILD.gn b/impeller/BUILD.gn index 46bd75f3f4e2e..4bb5bc1d040ea 100644 --- 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 index b34b6787e56f6..f06129536366d 100644 --- a/impeller/aiks/paint.cc +++ b/impeller/aiks/paint.cc @@ -11,6 +11,7 @@ namespace impeller { std::shared_ptr 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 index a6650de47a342..3817bab0da623 100644 --- 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,6 +28,8 @@ struct Paint { }; Color color = Color::Black(); + std::shared_ptr contents; + Scalar stroke_width = 0.0; SolidStrokeContents::Cap stroke_cap = SolidStrokeContents::Cap::kButt; SolidStrokeContents::Join stroke_join = SolidStrokeContents::Join::kMiter; @@ -34,7 +37,6 @@ struct Paint { Style style = Style::kFill; Entity::BlendMode blend_mode = Entity::BlendMode::kSourceOver; std::optional mask_blur; - std::shared_ptr 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 index 91e7912117842..d4188efbecad4 100644 --- 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 index fce8d7ab7f868..0fe07b756c73a 100644 --- 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 index 1cfbfb9f8bdc8..a7b8507b977b0 100644 --- 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 index f66936c6b3704..ffc926b6b7edc 100644 --- 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 index 549e5b4fb4876..50998e6dcc26e 100644 --- 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 index 82eb5633b50b1..b840572799bac 100644 --- 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 index 0bd6640223221..721d1f478502c 100644 --- a/impeller/renderer/renderer_unittests.cc +++ b/impeller/renderer/renderer_unittests.cc @@ -279,7 +279,8 @@ TEST_P(RendererTest, CanRenderInstanced) { VertexBufferBuilder 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 index 7a615464ac5da..a3d1b3ef1c42e 100644 --- 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 index e6c4661ebae7e..bb11fb145c84d 100644 --- 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 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 index 542cefbbb2737..fe1569365e3ca 100644 --- 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 index d7c00132eee94..e42dfcbe0766c 100644 --- 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 new file mode 100644 index 0000000000000..6f0b6bab804a5 --- /dev/null +++ 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