Skip to content

Commit d80be47

Browse files
authored
Migrate DisplayList unit tests to DL/Impeller geometry classes (#161453)
Address the first item in flutter/flutter#161456 (Unit tests in the display_list/ directory) Some new `DlPath::Make<Shape>` factories were added to make test writing simpler. `DlPath` is now bi-directional! You can construct one from either an `SkPath` or an `impeller::Path` and it will auto-convert to the other as needed. This allows unit tests with custom paths to rely on `impeller::Path` for path construction instead of `SkPath` (as long as only simple move/line/quad/curve verbs are needed). `RoundRect` now normalizes the argument rect in all constructors to match Flutter expectations and `SkRRect` legacy behavior. This behavior was already being enforced in `ui.rrect` but the unit tests we have to verify the behavior are written against the `RoundRect` object itself so this was the simplest way to make the unit tests work right, while ensuring that we maintain correct behavior for `ui` objects. Ideally these issues would be tested at the `ui` native interface instead of as unit tests on our internal objects and we should be allowed to decide how we want our internal APIs to behave with regard to this concept. Skia inverted path types are no longer allowed in `DlPath` and all use of them should be eliminated in the engine (except to test if they crash when used in a debug unit test) A couple of unit tests for `DlOpSpy` and Impeller's interop package were migrated here along for the ride even though this PR was focused primarily on `display_list/` unit tests.
1 parent 8856ccb commit d80be47

25 files changed

+1926
-1882
lines changed

engine/src/flutter/display_list/benchmarking/dl_benchmarks.cc

Lines changed: 100 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,8 @@ void BM_DrawLine(benchmark::State& state,
103103

104104
state.counters["DrawCallCount"] = kLinesToDraw;
105105
for (size_t i = 0; i < kLinesToDraw; i++) {
106-
builder.DrawLine(SkPoint::Make(i % length, 0),
107-
SkPoint::Make(length - i % length, length), paint);
106+
builder.DrawLine(DlPoint(i % length, 0),
107+
DlPoint(length - i % length, length), paint);
108108
}
109109

110110
auto display_list = builder.Build();
@@ -139,20 +139,20 @@ void BM_DrawRect(benchmark::State& state,
139139
auto surface = surface_provider->GetPrimarySurface()->sk_surface();
140140
auto canvas = DlSkCanvasAdapter(surface->getCanvas());
141141

142-
// As rects have SkScalar dimensions, we want to ensure that we also
142+
// As rects have DlScalar dimensions, we want to ensure that we also
143143
// draw rects with non-integer position and size
144-
const SkScalar offset = 0.5f;
145-
SkRect rect = SkRect::MakeLTRB(0, 0, length, length);
144+
const DlScalar offset = 0.5f;
145+
DlRect rect = DlRect::MakeLTRB(0, 0, length, length);
146146

147147
state.counters["DrawCallCount"] = kRectsToDraw;
148148
for (size_t i = 0; i < kRectsToDraw; i++) {
149149
builder.DrawRect(rect, paint);
150-
rect.offset(offset, offset);
151-
if (rect.right() > canvas_size) {
152-
rect.offset(-canvas_size, 0);
150+
rect = rect.Shift(offset, offset);
151+
if (rect.GetRight() > canvas_size) {
152+
rect = rect.Shift(-canvas_size, 0);
153153
}
154-
if (rect.bottom() > canvas_size) {
155-
rect.offset(0, -canvas_size);
154+
if (rect.GetBottom() > canvas_size) {
155+
rect = rect.Shift(0, -canvas_size);
156156
}
157157
}
158158

@@ -188,18 +188,18 @@ void BM_DrawOval(benchmark::State& state,
188188
auto surface = surface_provider->GetPrimarySurface()->sk_surface();
189189
auto canvas = DlSkCanvasAdapter(surface->getCanvas());
190190

191-
SkRect rect = SkRect::MakeXYWH(0, 0, length * 1.5f, length);
192-
const SkScalar offset = 0.5f;
191+
DlRect rect = DlRect::MakeXYWH(0, 0, length * 1.5f, length);
192+
const DlScalar offset = 0.5f;
193193

194194
state.counters["DrawCallCount"] = kOvalsToDraw;
195195
for (size_t i = 0; i < kOvalsToDraw; i++) {
196196
builder.DrawOval(rect, paint);
197-
rect.offset(offset, offset);
198-
if (rect.right() > canvas_size) {
199-
rect.offset(-canvas_size, 0);
197+
rect = rect.Shift(offset, offset);
198+
if (rect.GetRight() > canvas_size) {
199+
rect = rect.Shift(-canvas_size, 0);
200200
}
201-
if (rect.bottom() > canvas_size) {
202-
rect.offset(0, -canvas_size);
201+
if (rect.GetBottom() > canvas_size) {
202+
rect = rect.Shift(0, -canvas_size);
203203
}
204204
}
205205
auto display_list = builder.Build();
@@ -234,20 +234,21 @@ void BM_DrawCircle(benchmark::State& state,
234234
auto surface = surface_provider->GetPrimarySurface()->sk_surface();
235235
auto canvas = DlSkCanvasAdapter(surface->getCanvas());
236236

237-
SkScalar radius = length / 2.0f;
238-
const SkScalar offset = 0.5f;
237+
DlScalar radius = length / 2.0f;
238+
const DlScalar offset = 0.5f;
239239

240-
SkPoint center = SkPoint::Make(radius, radius);
240+
DlPoint center = DlPoint(radius, radius);
241+
DlPoint shift = DlPoint(offset, offset);
241242

242243
state.counters["DrawCallCount"] = kCirclesToDraw;
243244
for (size_t i = 0; i < kCirclesToDraw; i++) {
244245
builder.DrawCircle(center, radius, paint);
245-
center.offset(offset, offset);
246-
if (center.x() + radius > canvas_size) {
247-
center.set(radius, center.y());
246+
center += shift;
247+
if (center.x + radius > canvas_size) {
248+
center.x = radius;
248249
}
249-
if (center.y() + radius > canvas_size) {
250-
center.set(center.x(), radius);
250+
if (center.y + radius > canvas_size) {
251+
center.y = radius;
251252
}
252253
}
253254
auto display_list = builder.Build();
@@ -270,7 +271,7 @@ void BM_DrawCircle(benchmark::State& state,
270271
void BM_DrawRRect(benchmark::State& state,
271272
BackendType backend_type,
272273
unsigned attributes,
273-
SkRRect::Type type) {
274+
RRectType type) {
274275
auto surface_provider = DlSurfaceProvider::Create(backend_type);
275276
DisplayListBuilder builder;
276277
DlPaint paint = GetPaintForRun(attributes);
@@ -283,49 +284,45 @@ void BM_DrawRRect(benchmark::State& state,
283284
auto surface = surface_provider->GetPrimarySurface()->sk_surface();
284285
auto canvas = DlSkCanvasAdapter(surface->getCanvas());
285286

286-
SkVector radii[4] = {};
287+
DlRoundingRadii radii;
287288
switch (type) {
288-
case SkRRect::Type::kSimple_Type:
289-
radii[0] = SkVector::Make(5.0f, 5.0f);
290-
radii[1] = SkVector::Make(5.0f, 5.0f);
291-
radii[2] = SkVector::Make(5.0f, 5.0f);
292-
radii[3] = SkVector::Make(5.0f, 5.0f);
289+
case RRectType::kSimple:
290+
radii.top_left = DlSize(5.0f, 5.0f);
291+
radii.top_right = DlSize(5.0f, 5.0f);
292+
radii.bottom_right = DlSize(5.0f, 5.0f);
293+
radii.bottom_left = DlSize(5.0f, 5.0f);
293294
break;
294-
case SkRRect::Type::kNinePatch_Type:
295-
radii[0] = SkVector::Make(5.0f, 2.0f);
296-
radii[1] = SkVector::Make(3.0f, 2.0f);
297-
radii[2] = SkVector::Make(3.0f, 4.0f);
298-
radii[3] = SkVector::Make(5.0f, 4.0f);
295+
case RRectType::kNinePatch:
296+
radii.top_left = DlSize(5.0f, 2.0f);
297+
radii.top_right = DlSize(3.0f, 2.0f);
298+
radii.bottom_right = DlSize(3.0f, 4.0f);
299+
radii.bottom_left = DlSize(5.0f, 4.0f);
299300
break;
300-
case SkRRect::Type::kComplex_Type:
301-
radii[0] = SkVector::Make(5.0f, 4.0f);
302-
radii[1] = SkVector::Make(4.0f, 5.0f);
303-
radii[2] = SkVector::Make(3.0f, 6.0f);
304-
radii[3] = SkVector::Make(2.0f, 7.0f);
301+
case RRectType::kComplex:
302+
radii.top_left = DlSize(5.0f, 4.0f);
303+
radii.top_right = DlSize(4.0f, 5.0f);
304+
radii.bottom_right = DlSize(3.0f, 6.0f);
305+
radii.bottom_left = DlSize(2.0f, 7.0f);
305306
break;
306307
default:
307-
break;
308+
FML_UNREACHABLE();
308309
}
309310

310-
const SkScalar offset = 0.5f;
311-
const SkScalar multiplier = length / 16.0f;
312-
SkRRect rrect;
311+
const DlScalar offset = 0.5f;
312+
const DlScalar multiplier = length / 16.0f;
313313

314-
SkVector set_radii[4];
315-
for (size_t i = 0; i < 4; i++) {
316-
set_radii[i] = radii[i] * multiplier;
317-
}
318-
rrect.setRectRadii(SkRect::MakeLTRB(0, 0, length, length), set_radii);
314+
DlRoundRect rrect = DlRoundRect::MakeRectRadii(
315+
DlRect::MakeLTRB(0, 0, length, length), radii * multiplier);
319316

320317
state.counters["DrawCallCount"] = kRRectsToDraw;
321318
for (size_t i = 0; i < kRRectsToDraw; i++) {
322-
builder.DrawRRect(rrect, paint);
323-
rrect.offset(offset, offset);
324-
if (rrect.rect().right() > canvas_size) {
325-
rrect.offset(-canvas_size, 0);
319+
builder.DrawRoundRect(rrect, paint);
320+
rrect = rrect.Shift(offset, offset);
321+
if (rrect.GetBounds().GetRight() > canvas_size) {
322+
rrect = rrect.Shift(-canvas_size, 0);
326323
}
327-
if (rrect.rect().bottom() > canvas_size) {
328-
rrect.offset(0, -canvas_size);
324+
if (rrect.GetBounds().GetBottom() > canvas_size) {
325+
rrect = rrect.Shift(0, -canvas_size);
329326
}
330327
}
331328
auto display_list = builder.Build();
@@ -351,7 +348,7 @@ void BM_DrawRRect(benchmark::State& state,
351348
void BM_DrawDRRect(benchmark::State& state,
352349
BackendType backend_type,
353350
unsigned attributes,
354-
SkRRect::Type type) {
351+
RRectType type) {
355352
auto surface_provider = DlSurfaceProvider::Create(backend_type);
356353
DisplayListBuilder builder;
357354
DlPaint paint = GetPaintForRun(attributes);
@@ -364,50 +361,51 @@ void BM_DrawDRRect(benchmark::State& state,
364361
auto surface = surface_provider->GetPrimarySurface()->sk_surface();
365362
auto canvas = DlSkCanvasAdapter(surface->getCanvas());
366363

367-
SkVector radii[4] = {};
364+
DlRoundingRadii radii;
368365
switch (type) {
369-
case SkRRect::Type::kSimple_Type:
370-
radii[0] = SkVector::Make(5.0f, 5.0f);
371-
radii[1] = SkVector::Make(5.0f, 5.0f);
372-
radii[2] = SkVector::Make(5.0f, 5.0f);
373-
radii[3] = SkVector::Make(5.0f, 5.0f);
366+
case RRectType::kSimple:
367+
radii.top_left = DlSize(5.0f, 5.0f);
368+
radii.top_right = DlSize(5.0f, 5.0f);
369+
radii.bottom_right = DlSize(5.0f, 5.0f);
370+
radii.bottom_left = DlSize(5.0f, 5.0f);
374371
break;
375-
case SkRRect::Type::kNinePatch_Type:
376-
radii[0] = SkVector::Make(5.0f, 7.0f);
377-
radii[1] = SkVector::Make(3.0f, 7.0f);
378-
radii[2] = SkVector::Make(3.0f, 4.0f);
379-
radii[3] = SkVector::Make(5.0f, 4.0f);
372+
case RRectType::kNinePatch:
373+
radii.top_left = DlSize(5.0f, 7.0f);
374+
radii.top_right = DlSize(3.0f, 7.0f);
375+
radii.bottom_right = DlSize(3.0f, 4.0f);
376+
radii.bottom_left = DlSize(5.0f, 4.0f);
380377
break;
381-
case SkRRect::Type::kComplex_Type:
382-
radii[0] = SkVector::Make(5.0f, 4.0f);
383-
radii[1] = SkVector::Make(4.0f, 5.0f);
384-
radii[2] = SkVector::Make(3.0f, 6.0f);
385-
radii[3] = SkVector::Make(8.0f, 7.0f);
378+
case RRectType::kComplex:
379+
radii.top_left = DlSize(5.0f, 4.0f);
380+
radii.top_right = DlSize(4.0f, 5.0f);
381+
radii.bottom_right = DlSize(3.0f, 6.0f);
382+
radii.bottom_left = DlSize(8.0f, 7.0f);
386383
break;
387384
default:
388-
break;
385+
FML_UNREACHABLE();
389386
}
390387

391-
const SkScalar offset = 0.5f;
392-
const SkScalar multiplier = length / 16.0f;
393-
SkRRect rrect, rrect_2;
388+
const DlScalar offset = 0.5f;
389+
const DlScalar multiplier = length / 16.0f;
394390

395-
SkVector set_radii[4];
396-
for (size_t i = 0; i < 4; i++) {
397-
set_radii[i] = radii[i] * multiplier;
398-
}
399-
rrect.setRectRadii(SkRect::MakeLTRB(0, 0, length, length), set_radii);
391+
DlRoundRect rrect = DlRoundRect::MakeRectRadii(
392+
DlRect::MakeLTRB(0, 0, length, length), radii * multiplier);
393+
DlRoundRect rrect_2 = DlRoundRect::MakeRectRadii(
394+
DlRect::MakeLTRB(0, 0, length, length).Expand(-0.1f * length),
395+
radii * multiplier);
400396

401397
state.counters["DrawCallCount"] = kDRRectsToDraw;
402398
for (size_t i = 0; i < kDRRectsToDraw; i++) {
403-
rrect.inset(0.1f * length, 0.1f * length, &rrect_2);
404-
builder.DrawDRRect(rrect, rrect_2, paint);
405-
rrect.offset(offset, offset);
406-
if (rrect.rect().right() > canvas_size) {
407-
rrect.offset(-canvas_size, 0);
399+
builder.DrawDiffRoundRect(rrect, rrect_2, paint);
400+
rrect = rrect.Shift(offset, offset);
401+
rrect_2 = rrect_2.Shift(offset, offset);
402+
if (rrect.GetBounds().GetRight() > canvas_size) {
403+
rrect = rrect.Shift(-canvas_size, 0);
404+
rrect_2 = rrect_2.Shift(-canvas_size, 0);
408405
}
409-
if (rrect.rect().bottom() > canvas_size) {
410-
rrect.offset(0, -canvas_size);
406+
if (rrect.GetBounds().GetBottom() > canvas_size) {
407+
rrect = rrect.Shift(0, -canvas_size);
408+
rrect_2 = rrect_2.Shift(0, -canvas_size);
411409
}
412410
}
413411
auto display_list = builder.Build();
@@ -439,27 +437,27 @@ void BM_DrawArc(benchmark::State& state,
439437
auto surface = surface_provider->GetPrimarySurface()->sk_surface();
440438
auto canvas = DlSkCanvasAdapter(surface->getCanvas());
441439

442-
SkScalar starting_angle = 0.0f;
443-
SkScalar offset = 0.5f;
440+
DlScalar starting_angle = 0.0f;
441+
DlScalar offset = 0.5f;
444442

445443
// Just some random sweeps that will mostly circumnavigate the circle
446-
std::vector<SkScalar> segment_sweeps = {5.5f, -10.0f, 42.0f, 71.7f, 90.0f,
444+
std::vector<DlScalar> segment_sweeps = {5.5f, -10.0f, 42.0f, 71.7f, 90.0f,
447445
37.5f, 17.9f, 32.0f, 379.4f};
448446

449-
SkRect bounds = SkRect::MakeLTRB(0, 0, length, length);
447+
DlRect bounds = DlRect::MakeLTRB(0, 0, length, length);
450448

451449
state.counters["DrawCallCount"] = kArcSweepSetsToDraw * segment_sweeps.size();
452450
for (size_t i = 0; i < kArcSweepSetsToDraw; i++) {
453-
for (SkScalar sweep : segment_sweeps) {
451+
for (DlScalar sweep : segment_sweeps) {
454452
builder.DrawArc(bounds, starting_angle, sweep, false, paint);
455453
starting_angle += sweep + 5.0f;
456454
}
457-
bounds.offset(offset, offset);
458-
if (bounds.right() > canvas_size) {
459-
bounds.offset(-canvas_size, 0);
455+
bounds = bounds.Shift(offset, offset);
456+
if (bounds.GetRight() > canvas_size) {
457+
bounds = bounds.Shift(-canvas_size, 0);
460458
}
461-
if (bounds.bottom() > canvas_size) {
462-
bounds.offset(0, -canvas_size);
459+
if (bounds.GetBottom() > canvas_size) {
460+
bounds = bounds.Shift(0, -canvas_size);
463461
}
464462
}
465463

@@ -478,9 +476,9 @@ void BM_DrawArc(benchmark::State& state,
478476

479477
// Returns a list of SkPoints that represent `n` points equally spaced out
480478
// along the circumference of a circle with radius `r` and centered on `center`.
481-
std::vector<SkPoint> GetPolygonPoints(size_t n, SkPoint center, SkScalar r) {
479+
std::vector<SkPoint> GetPolygonPoints(size_t n, SkPoint center, DlScalar r) {
482480
std::vector<SkPoint> points;
483-
SkScalar x, y;
481+
DlScalar x, y;
484482
float angle;
485483
float full_circle = 2.0f * M_PI;
486484
for (size_t i = 0; i < n; i++) {

0 commit comments

Comments
 (0)