Skip to content

Commit a98f406

Browse files
flarromanejaquez
flar
authored and
romanejaquez
committed
[Impeller] Render conics without conversion from Flutter apps (flutter#166305)
Now that Impeller performs high fidelity tessellation of Conic curves we will no longer convert Flutter app's conic curves into approximated quadratic curves.
1 parent a0c37bc commit a98f406

File tree

8 files changed

+281
-64
lines changed

8 files changed

+281
-64
lines changed

‎engine/src/flutter/display_list/geometry/dl_path.cc‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
#include "flutter/display_list/geometry/dl_path.h"
66

77
#include "flutter/display_list/geometry/dl_geometry_types.h"
8+
#include "flutter/impeller/geometry/path.h"
89
#include "flutter/impeller/geometry/path_builder.h"
9-
#include "impeller/geometry/path.h"
1010

1111
namespace {
1212
inline constexpr flutter::DlPathFillType ToDlFillType(SkPathFillType sk_type) {
@@ -452,9 +452,10 @@ class ImpellerPathReceiver final : public DlPathReceiver {
452452
void QuadTo(const DlPoint& cp, const DlPoint& p2) override {
453453
builder_.QuadraticCurveTo(cp, p2);
454454
}
455-
// For legacy compatibility we do not override ConicTo to let the dispatcher
456-
// convert conics to quads until we update Impeller for full support of
457-
// rational quadratics
455+
bool ConicTo(const DlPoint& cp, const DlPoint& p2, DlScalar weight) override {
456+
builder_.ConicCurveTo(cp, p2, weight);
457+
return true;
458+
}
458459
void CubicTo(const DlPoint& cp1,
459460
const DlPoint& cp2,
460461
const DlPoint& p2) override {

‎engine/src/flutter/display_list/geometry/dl_path_unittests.cc‎

Lines changed: 99 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44

55
#include "flutter/display_list/geometry/dl_path.h"
66

7-
#include "gmock/gmock.h"
87
#include "gtest/gtest.h"
98

9+
#include "flutter/display_list/testing/dl_test_mock_path_receiver.h"
1010
#include "flutter/third_party/skia/include/core/SkRRect.h"
1111

1212
namespace flutter {
@@ -595,31 +595,6 @@ TEST(DisplayListPath, IsLineFromImpellerPath) {
595595
}
596596

597597
namespace {
598-
class DlPathReceiverMock : public DlPathReceiver {
599-
public:
600-
MOCK_METHOD(void,
601-
RecommendSizes,
602-
(size_t verb_count, size_t point_count),
603-
(override));
604-
MOCK_METHOD(void, RecommendBounds, (const DlRect& bounds), (override));
605-
MOCK_METHOD(void,
606-
SetPathInfo,
607-
(DlPathFillType fill_type, bool is_convex),
608-
(override));
609-
MOCK_METHOD(void, MoveTo, (const DlPoint& p2), (override));
610-
MOCK_METHOD(void, LineTo, (const DlPoint& p2), (override));
611-
MOCK_METHOD(void, QuadTo, (const DlPoint& cp, const DlPoint& p2), (override));
612-
MOCK_METHOD(bool,
613-
ConicTo,
614-
(const DlPoint& cp, const DlPoint& p2, DlScalar weight),
615-
(override));
616-
MOCK_METHOD(void,
617-
CubicTo,
618-
(const DlPoint& cp1, const DlPoint& cp2, const DlPoint& p2),
619-
(override));
620-
MOCK_METHOD(void, Close, (), (override));
621-
};
622-
623598
using ::testing::AtMost;
624599
using ::testing::Return;
625600
} // namespace
@@ -925,6 +900,104 @@ TEST(DisplayListPath, DispatchImpellerPathConvexSpecified) {
925900
path.Dispatch(mock_receiver);
926901
}
927902

903+
TEST(DisplayListPath, DispatchSkiaPathConicToQuads) {
904+
// If we execute conicTo with a weight of exactly 1.0, SkPath will turn
905+
// it into a quadTo, so we avoid that by using 0.999
906+
SkScalar weights[4] = {
907+
0.02f,
908+
0.5f,
909+
SK_ScalarSqrt2 * 0.5f,
910+
1.0f - kEhCloseEnough,
911+
};
912+
913+
for (SkScalar weight : weights) {
914+
SkPath sk_path;
915+
sk_path.moveTo(10, 10);
916+
sk_path.conicTo(20, 10, 20, 20, weight);
917+
918+
std::array<DlPoint, 5> i_points;
919+
impeller::ConicPathComponent i_conic(DlPoint(10, 10), DlPoint(20, 10),
920+
DlPoint(20, 20), weight);
921+
i_conic.SubdivideToQuadraticPoints(i_points);
922+
923+
::testing::StrictMock<DlPathReceiverMock> mock_receiver;
924+
925+
// Recommendations must happen before any of the path segments is dispatched
926+
::testing::ExpectationSet all_recommendations;
927+
all_recommendations += //
928+
EXPECT_CALL(mock_receiver, RecommendSizes(2u, 3u)) //
929+
.Times(AtMost(1));
930+
all_recommendations +=
931+
EXPECT_CALL(mock_receiver,
932+
RecommendBounds(DlRect::MakeLTRB(10, 10, 20, 20)))
933+
.Times(AtMost(1));
934+
EXPECT_CALL(mock_receiver, SetPathInfo(DlPathFillType::kNonZero, true));
935+
936+
{
937+
::testing::InSequence sequence;
938+
939+
EXPECT_CALL(mock_receiver, MoveTo(DlPoint(10, 10)))
940+
.After(all_recommendations);
941+
EXPECT_CALL(mock_receiver,
942+
ConicTo(DlPoint(20, 10), DlPoint(20, 20), weight))
943+
.WillOnce(Return(false));
944+
EXPECT_CALL(mock_receiver, QuadTo(i_points[1], i_points[2]));
945+
EXPECT_CALL(mock_receiver, QuadTo(i_points[3], i_points[4]));
946+
}
947+
948+
DlPath(sk_path).Dispatch(mock_receiver);
949+
}
950+
}
951+
952+
TEST(DisplayListPath, DispatchImpellerPathConicToQuads) {
953+
// If we execute conicTo with a weight of exactly 1.0, SkPath will turn
954+
// it into a quadTo, so we avoid that by using 0.999
955+
DlScalar weights[4] = {
956+
0.02f,
957+
0.5f,
958+
SK_ScalarSqrt2 * 0.5f,
959+
1.0f - kEhCloseEnough,
960+
};
961+
962+
for (DlScalar weight : weights) {
963+
DlPathBuilder path_builder;
964+
path_builder.MoveTo(DlPoint(10, 10));
965+
path_builder.ConicCurveTo(DlPoint(20, 10), DlPoint(20, 20), weight);
966+
967+
std::array<DlPoint, 5> i_points;
968+
impeller::ConicPathComponent i_conic(DlPoint(10, 10), DlPoint(20, 10),
969+
DlPoint(20, 20), weight);
970+
i_conic.SubdivideToQuadraticPoints(i_points);
971+
972+
::testing::StrictMock<DlPathReceiverMock> mock_receiver;
973+
974+
// Recommendations must happen before any of the path segments is dispatched
975+
::testing::ExpectationSet all_recommendations;
976+
all_recommendations += //
977+
EXPECT_CALL(mock_receiver, RecommendSizes(2u, 6u)) //
978+
.Times(AtMost(1));
979+
all_recommendations +=
980+
EXPECT_CALL(mock_receiver,
981+
RecommendBounds(DlRect::MakeLTRB(10, 10, 20, 20)))
982+
.Times(AtMost(1));
983+
EXPECT_CALL(mock_receiver, SetPathInfo(DlPathFillType::kNonZero, false));
984+
985+
{
986+
::testing::InSequence sequence;
987+
988+
EXPECT_CALL(mock_receiver, MoveTo(DlPoint(10, 10)))
989+
.After(all_recommendations);
990+
EXPECT_CALL(mock_receiver,
991+
ConicTo(DlPoint(20, 10), DlPoint(20, 20), weight))
992+
.WillOnce(Return(false));
993+
EXPECT_CALL(mock_receiver, QuadTo(i_points[1], i_points[2]));
994+
EXPECT_CALL(mock_receiver, QuadTo(i_points[3], i_points[4]));
995+
}
996+
997+
DlPath(path_builder).Dispatch(mock_receiver);
998+
}
999+
}
1000+
9281001
#ifndef NDEBUG
9291002
// Tests that verify we don't try to use inverse path modes as they aren't
9301003
// supported by either Flutter public APIs or Impeller

‎engine/src/flutter/display_list/skia/dl_sk_conversions_unittests.cc‎

Lines changed: 13 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,11 @@
1212
#include "flutter/display_list/effects/dl_image_filters.h"
1313
#include "flutter/display_list/skia/dl_sk_conversions.h"
1414
#include "flutter/impeller/geometry/path_component.h"
15+
#include "flutter/third_party/skia/include/core/SkColorSpace.h"
16+
#include "flutter/third_party/skia/include/core/SkSamplingOptions.h"
17+
#include "flutter/third_party/skia/include/core/SkTileMode.h"
18+
1519
#include "gtest/gtest.h"
16-
#include "third_party/skia/include/core/SkColorSpace.h"
17-
#include "third_party/skia/include/core/SkSamplingOptions.h"
18-
#include "third_party/skia/include/core/SkTileMode.h"
1920

2021
namespace flutter {
2122
namespace testing {
@@ -402,9 +403,7 @@ TEST(DisplayListSkConversions, ConicToQuads) {
402403
}
403404
}
404405

405-
// This tests the new conic subdivision code in the Impeller conic path
406-
// component object vs the code we used to rely on inside Skia
407-
TEST(DisplayListSkConversions, ConicPathToQuads) {
406+
TEST(DisplayListSkConversions, ConicPathToImpeller) {
408407
// If we execute conicTo with a weight of exactly 1.0, SkPath will turn
409408
// it into a quadTo, so we avoid that by using 0.999
410409
SkScalar weights[4] = {
@@ -426,35 +425,16 @@ TEST(DisplayListSkConversions, ConicPathToQuads) {
426425
ASSERT_EQ(it.type(), impeller::Path::ComponentType::kContour);
427426
++it;
428427

429-
ASSERT_EQ(it.type(), impeller::Path::ComponentType::kQuadratic);
430-
auto quad1 = it.quadratic();
431-
ASSERT_NE(quad1, nullptr);
432-
++it;
433-
434-
ASSERT_EQ(it.type(), impeller::Path::ComponentType::kQuadratic);
435-
auto quad2 = it.quadratic();
436-
ASSERT_NE(quad2, nullptr);
428+
ASSERT_EQ(it.type(), impeller::Path::ComponentType::kConic);
429+
auto conic = it.conic();
430+
ASSERT_NE(conic, nullptr);
437431
++it;
438432

439-
SkPoint sk_points[5];
440-
int ncurves = SkPath::ConvertConicToQuads(
441-
SkPoint::Make(10, 10), SkPoint::Make(20, 10), SkPoint::Make(20, 20),
442-
weight, sk_points, 1);
443-
ASSERT_EQ(ncurves, 2);
444-
445-
EXPECT_FLOAT_EQ(sk_points[0].fX, quad1->p1.x) << "weight: " << weight;
446-
EXPECT_FLOAT_EQ(sk_points[0].fY, quad1->p1.y) << "weight: " << weight;
447-
EXPECT_FLOAT_EQ(sk_points[1].fX, quad1->cp.x) << "weight: " << weight;
448-
EXPECT_FLOAT_EQ(sk_points[1].fY, quad1->cp.y) << "weight: " << weight;
449-
EXPECT_FLOAT_EQ(sk_points[2].fX, quad1->p2.x) << "weight: " << weight;
450-
EXPECT_FLOAT_EQ(sk_points[2].fY, quad1->p2.y) << "weight: " << weight;
451-
452-
EXPECT_FLOAT_EQ(sk_points[2].fX, quad2->p1.x) << "weight: " << weight;
453-
EXPECT_FLOAT_EQ(sk_points[2].fY, quad2->p1.y) << "weight: " << weight;
454-
EXPECT_FLOAT_EQ(sk_points[3].fX, quad2->cp.x) << "weight: " << weight;
455-
EXPECT_FLOAT_EQ(sk_points[3].fY, quad2->cp.y) << "weight: " << weight;
456-
EXPECT_FLOAT_EQ(sk_points[4].fX, quad2->p2.x) << "weight: " << weight;
457-
EXPECT_FLOAT_EQ(sk_points[4].fY, quad2->p2.y) << "weight: " << weight;
433+
EXPECT_EQ(conic->p1, DlPoint(10, 10)) << "weight: " << weight;
434+
EXPECT_EQ(conic->cp, DlPoint(20, 10)) << "weight: " << weight;
435+
EXPECT_EQ(conic->p2, DlPoint(20, 20)) << "weight: " << weight;
436+
EXPECT_EQ(conic->weight.x, weight) << "weight: " << weight;
437+
EXPECT_EQ(conic->weight.y, weight) << "weight: " << weight;
458438
}
459439
}
460440

‎engine/src/flutter/display_list/testing/BUILD.gn‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ source_set("display_list_testing") {
1010

1111
sources = [
1212
"dl_test_equality.h",
13+
"dl_test_mock_path_receiver.h",
1314
"dl_test_snippets.cc",
1415
"dl_test_snippets.h",
1516
]

‎engine/src/flutter/display_list/testing/dl_test_mock_path_receiver.h‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#ifndef FLUTTER_DISPLAY_LIST_TESTING_DL_TEST_MOCK_PATH_RECEIVER_H_
6+
#define FLUTTER_DISPLAY_LIST_TESTING_DL_TEST_MOCK_PATH_RECEIVER_H_
7+
8+
#include "gmock/gmock.h"
9+
10+
#include "flutter/display_list/geometry/dl_path.h"
11+
12+
namespace flutter {
13+
namespace testing {
14+
class DlPathReceiverMock : public DlPathReceiver {
15+
public:
16+
MOCK_METHOD(void,
17+
RecommendSizes,
18+
(size_t verb_count, size_t point_count),
19+
(override));
20+
MOCK_METHOD(void, RecommendBounds, (const DlRect& bounds), (override));
21+
MOCK_METHOD(void,
22+
SetPathInfo,
23+
(DlPathFillType fill_type, bool is_convex),
24+
(override));
25+
MOCK_METHOD(void, MoveTo, (const DlPoint& p2), (override));
26+
MOCK_METHOD(void, LineTo, (const DlPoint& p2), (override));
27+
MOCK_METHOD(void, QuadTo, (const DlPoint& cp, const DlPoint& p2), (override));
28+
MOCK_METHOD(bool,
29+
ConicTo,
30+
(const DlPoint& cp, const DlPoint& p2, DlScalar weight),
31+
(override));
32+
MOCK_METHOD(void,
33+
CubicTo,
34+
(const DlPoint& cp1, const DlPoint& cp2, const DlPoint& p2),
35+
(override));
36+
MOCK_METHOD(void, Close, (), (override));
37+
};
38+
39+
} // namespace testing
40+
} // namespace flutter
41+
42+
#endif // FLUTTER_DISPLAY_LIST_TESTING_DL_TEST_MOCK_PATH_RECEIVER_H_

‎engine/src/flutter/impeller/geometry/path_component.cc‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,9 @@ void ConicPathComponent::AppendPolylinePoints(
344344
Scalar scale_factor,
345345
std::vector<Point>& points) const {
346346
ToLinearPathComponents(scale_factor, [&points](const Point& point) {
347-
points.emplace_back(point);
347+
if (point != points.back()) {
348+
points.emplace_back(point);
349+
}
348350
});
349351
}
350352

0 commit comments

Comments
 (0)