From 8dfe6fff4081f599cb2943b51935c05554534f07 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 12 Apr 2024 10:17:08 -0700 Subject: [PATCH 1/4] [Impeller] use wangs formula for quad/cubic division. --- impeller/geometry/BUILD.gn | 2 + impeller/geometry/path_component.cc | 75 +++---------------------- impeller/geometry/path_component.h | 25 --------- impeller/geometry/wangs_formula.cc | 59 +++++++++++++++++++ impeller/geometry/wangs_formula.h | 57 +++++++++++++++++++ impeller/renderer/compute_tessellator.h | 2 +- 6 files changed, 128 insertions(+), 92 deletions(-) create mode 100644 impeller/geometry/wangs_formula.cc create mode 100644 impeller/geometry/wangs_formula.h diff --git a/impeller/geometry/BUILD.gn b/impeller/geometry/BUILD.gn index 0db775b86090a..a2ea1defebea0 100644 --- a/impeller/geometry/BUILD.gn +++ b/impeller/geometry/BUILD.gn @@ -43,6 +43,8 @@ impeller_component("geometry") { "type_traits.h", "vector.cc", "vector.h", + "wangs_formula.cc", + "wangs_formula.h", ] deps = [ diff --git a/impeller/geometry/path_component.cc b/impeller/geometry/path_component.cc index 55a4565708417..c5c3eded5c3fe 100644 --- a/impeller/geometry/path_component.cc +++ b/impeller/geometry/path_component.cc @@ -6,6 +6,8 @@ #include +#include "impeller/geometry/wangs_formula.h" + namespace impeller { /* @@ -98,11 +100,6 @@ Point QuadraticPathComponent::SolveDerivative(Scalar time) const { }; } -static Scalar ApproximateParabolaIntegral(Scalar x) { - constexpr Scalar d = 0.67; - return x / (1.0 - d + sqrt(sqrt(pow(d, 4) + 0.25 * x * x))); -} - void QuadraticPathComponent::AppendPolylinePoints( Scalar scale_factor, std::vector& points) const { @@ -114,42 +111,10 @@ void QuadraticPathComponent::AppendPolylinePoints( void QuadraticPathComponent::ToLinearPathComponents( Scalar scale_factor, const PointProc& proc) const { - auto tolerance = kDefaultCurveTolerance / scale_factor; - auto sqrt_tolerance = sqrt(tolerance); - - auto d01 = cp - p1; - auto d12 = p2 - cp; - auto dd = d01 - d12; - auto cross = (p2 - p1).Cross(dd); - auto x0 = d01.Dot(dd) * 1 / cross; - auto x2 = d12.Dot(dd) * 1 / cross; - auto scale = std::abs(cross / (hypot(dd.x, dd.y) * (x2 - x0))); - - auto a0 = ApproximateParabolaIntegral(x0); - auto a2 = ApproximateParabolaIntegral(x2); - Scalar val = 0.f; - if (std::isfinite(scale)) { - auto da = std::abs(a2 - a0); - auto sqrt_scale = sqrt(scale); - if ((x0 < 0 && x2 < 0) || (x0 >= 0 && x2 >= 0)) { - val = da * sqrt_scale; - } else { - // cusp case - auto xmin = sqrt_tolerance / sqrt_scale; - val = sqrt_tolerance * da / ApproximateParabolaIntegral(xmin); - } - } - auto u0 = ApproximateParabolaIntegral(a0); - auto u2 = ApproximateParabolaIntegral(a2); - auto uscale = 1 / (u2 - u0); - - auto line_count = std::max(1., ceil(0.5 * val / sqrt_tolerance)); - auto step = 1 / line_count; + Scalar line_count = + std::ceilf(ComputeQuadradicSubdivisions(scale_factor, *this)); for (size_t i = 1; i < line_count; i += 1) { - auto u = i * step; - auto a = a0 + (a2 - a0) * u; - auto t = (ApproximateParabolaIntegral(a) - u0) * uscale; - proc(Solve(t)); + proc(Solve(i / line_count)); } proc(p2); } @@ -217,33 +182,11 @@ CubicPathComponent CubicPathComponent::Subsegment(Scalar t0, Scalar t1) const { void CubicPathComponent::ToLinearPathComponents(Scalar scale, const PointProc& proc) const { - constexpr Scalar accuracy = 0.1; - // The maximum error, as a vector from the cubic to the best approximating - // quadratic, is proportional to the third derivative, which is constant - // across the segment. Thus, the error scales down as the third power of - // the number of subdivisions. Our strategy then is to subdivide `t` evenly. - // - // This is an overestimate of the error because only the component - // perpendicular to the first derivative is important. But the simplicity is - // appealing. - - // This magic number is the square of 36 / sqrt(3). - // See: http://caffeineowl.com/graphics/2d/vectorial/cubic2quad01.html - auto max_hypot2 = 432.0 * accuracy * accuracy; - auto p1x2 = 3.0 * cp1 - p1; - auto p2x2 = 3.0 * cp2 - p2; - auto p = p2x2 - p1x2; - auto err = p.Dot(p); - auto quad_count = std::max(1., ceil(pow(err / max_hypot2, 1. / 6.0))); - for (size_t i = 0; i < quad_count; i++) { - auto t0 = i / quad_count; - auto t1 = (i + 1) / quad_count; - auto seg = Subsegment(t0, t1); - auto p1x2 = 3.0 * seg.cp1 - seg.p1; - auto p2x2 = 3.0 * seg.cp2 - seg.p2; - QuadraticPathComponent(seg.p1, ((p1x2 + p2x2) / 4.0), seg.p2) - .ToLinearPathComponents(scale, proc); + Scalar line_count = std::ceilf(ComputeCubicSubdivisions(scale, *this)); + for (size_t i = 0; i < line_count; i++) { + proc(Solve(i / line_count)); } + proc(p2); } static inline bool NearEqual(Scalar a, Scalar b, Scalar epsilon) { diff --git a/impeller/geometry/path_component.h b/impeller/geometry/path_component.h index 58aa1f9f83530..c310052a893dc 100644 --- a/impeller/geometry/path_component.h +++ b/impeller/geometry/path_component.h @@ -16,16 +16,6 @@ namespace impeller { -// The default tolerance value for QuadraticCurveComponent::AppendPolylinePoints -// and CubicCurveComponent::AppendPolylinePoints. It also impacts the number of -// quadratics created when flattening a cubic curve to a polyline. -// -// Smaller numbers mean more points. This number seems suitable for particularly -// curvy curves at scales close to 1.0. As the scale increases, this number -// should be divided by Matrix::GetMaxBasisLength to avoid generating too few -// points for the given scale. -static constexpr Scalar kDefaultCurveTolerance = .1f; - struct LinearPathComponent { Point p1; Point p2; @@ -67,16 +57,6 @@ struct QuadraticPathComponent { Point SolveDerivative(Scalar time) const; - // Uses the algorithm described by Raph Levien in - // https://raphlinus.github.io/graphics/curves/2019/12/23/flatten-quadbez.html. - // - // The algorithm has several benefits: - // - It does not require elevation to cubics for processing. - // - It generates fewer and more accurate points than recursive subdivision. - // - Each turn of the core iteration loop has no dependencies on other turns, - // making it trivially parallelizable. - // - // See also the implementation in kurbo: https://github.com/linebender/kurbo. void AppendPolylinePoints(Scalar scale_factor, std::vector& points) const; @@ -121,11 +101,6 @@ struct CubicPathComponent { Point SolveDerivative(Scalar time) const; - // This method approximates the cubic component with quadratics, and then - // generates a polyline from those quadratics. - // - // See the note on QuadraticPathComponent::AppendPolylinePoints for - // references. void AppendPolylinePoints(Scalar scale, std::vector& points) const; std::vector Extrema() const; diff --git a/impeller/geometry/wangs_formula.cc b/impeller/geometry/wangs_formula.cc new file mode 100644 index 0000000000000..0f701c019e671 --- /dev/null +++ b/impeller/geometry/wangs_formula.cc @@ -0,0 +1,59 @@ +// 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 "impeller/geometry/wangs_formula.h" + +namespace impeller { + +namespace { + +// Don't allow linearized segments to be off by more than 1/4th of a pixel from +// the true curve. +constexpr static Scalar kPrecision = 4; + +static inline Scalar length(Point n) { + Point nn = n * n; + return std::sqrt(nn.x + nn.y); +} + +static inline Point Max(Point a, Point b) { + return Point{ + a.x > b.x ? a.x : b.x, // + a.y > b.y ? a.y : b.y // + }; +} + +} // namespace + +Scalar ComputeCubicSubdivisions(Scalar intolerance, + Point p0, + Point p1, + Point p2, + Point p3) { + Scalar k = intolerance * .75f * kPrecision; + Point a = (p0 - p1 * 2 + p2).Abs(); + Point b = (p1 - p2 * 2 + p3).Abs(); + return std::sqrt(k * length(Max(a, b))); +} + +Scalar ComputeQuadradicSubdivisions(Scalar intolerance, + Point p0, + Point p1, + Point p2) { + Scalar k = intolerance * .25f * kPrecision; + return std::sqrt(k * length(p0 - p1 * 2 + p2)); +} + +Scalar ComputeQuadradicSubdivisions(Scalar intolerance, + const QuadraticPathComponent& quad) { + return ComputeQuadradicSubdivisions(intolerance, quad.p1, quad.cp, quad.p2); +} + +Scalar ComputeCubicSubdivisions(float intolerance, + const CubicPathComponent& cub) { + return ComputeCubicSubdivisions(intolerance, cub.p1, cub.cp1, cub.cp2, + cub.p2); +} + +} // namespace impeller diff --git a/impeller/geometry/wangs_formula.h b/impeller/geometry/wangs_formula.h new file mode 100644 index 0000000000000..ddd2488a93f86 --- /dev/null +++ b/impeller/geometry/wangs_formula.h @@ -0,0 +1,57 @@ +// 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. + +#ifndef FLUTTER_IMPELLER_GEOMETRY_WANGS_FORMULA_H_ +#define FLUTTER_IMPELLER_GEOMETRY_WANGS_FORMULA_H_ + +#include "impeller/geometry/path_component.h" +#include "impeller/geometry/point.h" +#include "impeller/geometry/scalar.h" + +// Skia GPU Ports + +// Wang's formula gives the minimum number of evenly spaced (in the parametric +// sense) line segments that a bezier curve must be chopped into in order to +// guarantee all lines stay within a distance of "1/precision" pixels from the +// true curve. Its definition for a bezier curve of degree "n" is as follows: +// +// maxLength = max([length(p[i+2] - 2p[i+1] + p[i]) for (0 <= i <= n-2)]) +// numParametricSegments = sqrt(maxLength * precision * n*(n - 1)/8) +// +// (Goldman, Ron. (2003). 5.6.3 Wang's Formula. "Pyramid Algorithms: A Dynamic +// Programming Approach to Curves and Surfaces for Geometric Modeling". Morgan +// Kaufmann Publishers.) +namespace impeller { + +/// Returns the minimum number of evenly spaced (in the parametric sense) line +/// segments that the cubic must be chopped into in order to guarantee all lines +/// stay within a distance of "1/intolerance" pixels from the true curve. +Scalar ComputeCubicSubdivisions(Scalar intolerance, + Point p0, + Point p1, + Point p2, + Point p3); + +/// Returns the minimum number of evenly spaced (in the parametric sense) line +/// segments that the quadratic must be chopped into in order to guarantee all +/// lines stay within a distance of "1/intolerance" pixels from the true curve. +Scalar ComputeQuadradicSubdivisions(Scalar intolerance, + Point p0, + Point p1, + Point p2); + +/// Returns the minimum number of evenly spaced (in the parametric sense) line +/// segments that the quadratic must be chopped into in order to guarantee all +/// lines stay within a distance of "1/intolerance" pixels from the true curve. +Scalar ComputeQuadradicSubdivisions(Scalar intolerance, + const QuadraticPathComponent& quad); + +/// Returns the minimum number of evenly spaced (in the parametric sense) line +/// segments that the cubic must be chopped into in order to guarantee all lines +/// stay within a distance of "1/intolerance" pixels from the true curve. +Scalar ComputeCubicSubdivisions(float intolerance, + const CubicPathComponent& cub); +} // namespace impeller + +#endif // FLUTTER_IMPELLER_GEOMETRY_WANGS_FORMULA_H_ diff --git a/impeller/renderer/compute_tessellator.h b/impeller/renderer/compute_tessellator.h index 0dc6b56bc1e76..82ec336f72680 100644 --- a/impeller/renderer/compute_tessellator.h +++ b/impeller/renderer/compute_tessellator.h @@ -77,7 +77,7 @@ class ComputeTessellator { Cap stroke_cap_ = Cap::kButt; Join stroke_join_ = Join::kMiter; Scalar miter_limit_ = 4.0f; - Scalar cubic_accuracy_ = kDefaultCurveTolerance; + Scalar cubic_accuracy_ = .1f; Scalar quad_tolerance_ = .1f; ComputeTessellator(const ComputeTessellator&) = delete; From 6382b8659d0a16a856f55a550a5ec16a6671046f Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 12 Apr 2024 18:16:48 -0700 Subject: [PATCH 2/4] dont include first point. --- ci/licenses_golden/licenses_flutter | 4 ++++ impeller/geometry/path_component.cc | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 5c4a81d485eca..46154f16680a2 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -40113,6 +40113,8 @@ ORIGIN: ../../../flutter/impeller/geometry/type_traits.cc + ../../../flutter/LIC ORIGIN: ../../../flutter/impeller/geometry/type_traits.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/geometry/vector.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/geometry/vector.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/geometry/wangs_formula.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/geometry/wangs_formula.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/golden_tests/golden_digest.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/golden_tests/golden_digest.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/golden_tests/golden_playground_test.h + ../../../flutter/LICENSE @@ -42994,6 +42996,8 @@ FILE: ../../../flutter/impeller/geometry/type_traits.cc FILE: ../../../flutter/impeller/geometry/type_traits.h FILE: ../../../flutter/impeller/geometry/vector.cc FILE: ../../../flutter/impeller/geometry/vector.h +FILE: ../../../flutter/impeller/geometry/wangs_formula.cc +FILE: ../../../flutter/impeller/geometry/wangs_formula.h FILE: ../../../flutter/impeller/golden_tests/golden_digest.cc FILE: ../../../flutter/impeller/golden_tests/golden_digest.h FILE: ../../../flutter/impeller/golden_tests/golden_playground_test.h diff --git a/impeller/geometry/path_component.cc b/impeller/geometry/path_component.cc index c5c3eded5c3fe..3a00414dfa804 100644 --- a/impeller/geometry/path_component.cc +++ b/impeller/geometry/path_component.cc @@ -183,7 +183,7 @@ CubicPathComponent CubicPathComponent::Subsegment(Scalar t0, Scalar t1) const { void CubicPathComponent::ToLinearPathComponents(Scalar scale, const PointProc& proc) const { Scalar line_count = std::ceilf(ComputeCubicSubdivisions(scale, *this)); - for (size_t i = 0; i < line_count; i++) { + for (size_t i = 1; i < line_count; i++) { proc(Solve(i / line_count)); } proc(p2); From a7288ec6fbe4b7c9df5522ec6107558fc746db72 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 12 Apr 2024 18:47:47 -0700 Subject: [PATCH 3/4] ++ --- impeller/tessellator/tessellator_unittests.cc | 24 ------------------- 1 file changed, 24 deletions(-) diff --git a/impeller/tessellator/tessellator_unittests.cc b/impeller/tessellator/tessellator_unittests.cc index 723f42f629639..2c54d4282db01 100644 --- a/impeller/tessellator/tessellator_unittests.cc +++ b/impeller/tessellator/tessellator_unittests.cc @@ -78,30 +78,6 @@ TEST(TessellatorTest, TessellatorBuilderReturnsCorrectResultStatus) { ASSERT_EQ(result, Tessellator::Result::kInputError); } - - // More than uint16 points, odd fill mode. - { - Tessellator t; - PathBuilder builder = {}; - for (auto i = 0; i < 1000; i++) { - builder.AddCircle(Point(i, i), 4); - } - auto path = builder.TakePath(FillType::kOdd); - bool no_indices = false; - size_t count = 0u; - Tessellator::Result result = t.Tessellate( - path, 1.0f, - [&no_indices, &count](const float* vertices, size_t vertices_count, - const uint16_t* indices, size_t indices_count) { - no_indices = indices == nullptr; - count = vertices_count; - return true; - }); - - ASSERT_TRUE(no_indices); - ASSERT_TRUE(count >= USHRT_MAX); - ASSERT_EQ(result, Tessellator::Result::kSuccess); - } } TEST(TessellatorTest, TessellateConvex) { From ff75dfdc19137c9b240322be2c00103707b0408d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 14 Apr 2024 09:36:35 -0700 Subject: [PATCH 4/4] chinmay review. --- impeller/geometry/wangs_formula.cc | 30 ++++++++++++------------------ impeller/geometry/wangs_formula.h | 16 ++++++++++++---- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/impeller/geometry/wangs_formula.cc b/impeller/geometry/wangs_formula.cc index 0f701c019e671..302785b61983d 100644 --- a/impeller/geometry/wangs_formula.cc +++ b/impeller/geometry/wangs_formula.cc @@ -9,50 +9,44 @@ namespace impeller { namespace { // Don't allow linearized segments to be off by more than 1/4th of a pixel from -// the true curve. +// the true curve. This value should be scaled by the max basis of the +// X and Y directions. constexpr static Scalar kPrecision = 4; -static inline Scalar length(Point n) { +constexpr Scalar length(Point n) { Point nn = n * n; return std::sqrt(nn.x + nn.y); } -static inline Point Max(Point a, Point b) { - return Point{ - a.x > b.x ? a.x : b.x, // - a.y > b.y ? a.y : b.y // - }; -} - } // namespace -Scalar ComputeCubicSubdivisions(Scalar intolerance, +Scalar ComputeCubicSubdivisions(Scalar scale_factor, Point p0, Point p1, Point p2, Point p3) { - Scalar k = intolerance * .75f * kPrecision; + Scalar k = scale_factor * .75f * kPrecision; Point a = (p0 - p1 * 2 + p2).Abs(); Point b = (p1 - p2 * 2 + p3).Abs(); - return std::sqrt(k * length(Max(a, b))); + return std::sqrt(k * length(a.Max(b))); } -Scalar ComputeQuadradicSubdivisions(Scalar intolerance, +Scalar ComputeQuadradicSubdivisions(Scalar scale_factor, Point p0, Point p1, Point p2) { - Scalar k = intolerance * .25f * kPrecision; + Scalar k = scale_factor * .25f * kPrecision; return std::sqrt(k * length(p0 - p1 * 2 + p2)); } -Scalar ComputeQuadradicSubdivisions(Scalar intolerance, +Scalar ComputeQuadradicSubdivisions(Scalar scale_factor, const QuadraticPathComponent& quad) { - return ComputeQuadradicSubdivisions(intolerance, quad.p1, quad.cp, quad.p2); + return ComputeQuadradicSubdivisions(scale_factor, quad.p1, quad.cp, quad.p2); } -Scalar ComputeCubicSubdivisions(float intolerance, +Scalar ComputeCubicSubdivisions(float scale_factor, const CubicPathComponent& cub) { - return ComputeCubicSubdivisions(intolerance, cub.p1, cub.cp1, cub.cp2, + return ComputeCubicSubdivisions(scale_factor, cub.p1, cub.cp1, cub.cp2, cub.p2); } diff --git a/impeller/geometry/wangs_formula.h b/impeller/geometry/wangs_formula.h index ddd2488a93f86..df26365db3a10 100644 --- a/impeller/geometry/wangs_formula.h +++ b/impeller/geometry/wangs_formula.h @@ -27,7 +27,9 @@ namespace impeller { /// Returns the minimum number of evenly spaced (in the parametric sense) line /// segments that the cubic must be chopped into in order to guarantee all lines /// stay within a distance of "1/intolerance" pixels from the true curve. -Scalar ComputeCubicSubdivisions(Scalar intolerance, +/// +/// The scale_factor should be the max basis XY of the current transform. +Scalar ComputeCubicSubdivisions(Scalar scale_factor, Point p0, Point p1, Point p2, @@ -36,7 +38,9 @@ Scalar ComputeCubicSubdivisions(Scalar intolerance, /// Returns the minimum number of evenly spaced (in the parametric sense) line /// segments that the quadratic must be chopped into in order to guarantee all /// lines stay within a distance of "1/intolerance" pixels from the true curve. -Scalar ComputeQuadradicSubdivisions(Scalar intolerance, +/// +/// The scale_factor should be the max basis XY of the current transform. +Scalar ComputeQuadradicSubdivisions(Scalar scale_factor, Point p0, Point p1, Point p2); @@ -44,13 +48,17 @@ Scalar ComputeQuadradicSubdivisions(Scalar intolerance, /// Returns the minimum number of evenly spaced (in the parametric sense) line /// segments that the quadratic must be chopped into in order to guarantee all /// lines stay within a distance of "1/intolerance" pixels from the true curve. -Scalar ComputeQuadradicSubdivisions(Scalar intolerance, +/// +/// The scale_factor should be the max basis XY of the current transform. +Scalar ComputeQuadradicSubdivisions(Scalar scale_factor, const QuadraticPathComponent& quad); /// Returns the minimum number of evenly spaced (in the parametric sense) line /// segments that the cubic must be chopped into in order to guarantee all lines /// stay within a distance of "1/intolerance" pixels from the true curve. -Scalar ComputeCubicSubdivisions(float intolerance, +/// +/// The scale_factor should be the max basis XY of the current transform. +Scalar ComputeCubicSubdivisions(float scale_factor, const CubicPathComponent& cub); } // namespace impeller