From 69301f160c21dcde772f7b6e9ac8dd211ccbf3a8 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Wed, 18 Jan 2023 16:02:20 -0800 Subject: [PATCH 01/11] add support to run dl_canvas_unittests on GPU surfaces (off by default) --- ci/licenses_golden/licenses_flutter | 42 +- display_list/BUILD.gn | 77 +- display_list/display_list_benchmarks.cc | 259 +++--- display_list/display_list_benchmarks.h | 17 +- .../display_list_benchmarks_canvas_provider.h | 52 -- display_list/display_list_benchmarks_gl.cc | 52 -- display_list/display_list_benchmarks_gl.h | 35 - display_list/display_list_benchmarks_metal.cc | 40 - display_list/display_list_benchmarks_metal.h | 35 - .../display_list_benchmarks_software.cc | 30 - .../display_list_benchmarks_software.h | 31 - .../display_list_builder_benchmarks.cc | 2 +- display_list/display_list_canvas_unittests.cc | 847 ++++++++++++------ .../display_list_color_filter_unittests.cc | 2 +- .../display_list_color_source_unittests.cc | 2 +- display_list/display_list_color_unittests.cc | 2 +- .../display_list_complexity_unittests.cc | 2 +- .../display_list_image_filter_unittests.cc | 2 +- .../display_list_mask_filter_unittests.cc | 2 +- .../display_list_path_effect_unittests.cc | 2 +- display_list/display_list_unittests.cc | 2 +- .../display_list_vertices_unittests.cc | 2 +- display_list/testing/BUILD.gn | 97 ++ .../dl_test_equality.h} | 6 +- .../dl_test_snippets.cc} | 2 +- .../dl_test_snippets.h} | 6 +- display_list/testing/dl_test_surface_gl.cc | 47 + display_list/testing/dl_test_surface_gl.h | 42 + display_list/testing/dl_test_surface_metal.cc | 58 ++ display_list/testing/dl_test_surface_metal.h | 42 + .../testing/dl_test_surface_provider.cc | 63 ++ .../testing/dl_test_surface_provider.h | 82 ++ .../testing/dl_test_surface_software.cc | 32 + .../testing/dl_test_surface_software.h | 39 + flow/BUILD.gn | 2 +- flow/raster_cache_unittests.cc | 2 +- 36 files changed, 1263 insertions(+), 794 deletions(-) delete mode 100644 display_list/display_list_benchmarks_canvas_provider.h delete mode 100644 display_list/display_list_benchmarks_gl.cc delete mode 100644 display_list/display_list_benchmarks_gl.h delete mode 100644 display_list/display_list_benchmarks_metal.cc delete mode 100644 display_list/display_list_benchmarks_metal.h delete mode 100644 display_list/display_list_benchmarks_software.cc delete mode 100644 display_list/display_list_benchmarks_software.h create mode 100644 display_list/testing/BUILD.gn rename display_list/{display_list_attributes_testing.h => testing/dl_test_equality.h} (86%) rename display_list/{display_list_test_utils.cc => testing/dl_test_snippets.cc} (99%) rename display_list/{display_list_test_utils.h => testing/dl_test_snippets.h} (98%) create mode 100644 display_list/testing/dl_test_surface_gl.cc create mode 100644 display_list/testing/dl_test_surface_gl.h create mode 100644 display_list/testing/dl_test_surface_metal.cc create mode 100644 display_list/testing/dl_test_surface_metal.h create mode 100644 display_list/testing/dl_test_surface_provider.cc create mode 100644 display_list/testing/dl_test_surface_provider.h create mode 100644 display_list/testing/dl_test_surface_software.cc create mode 100644 display_list/testing/dl_test_surface_software.h diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 605e0b8c614e7..9c89a41e2a2f2 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -822,16 +822,8 @@ ORIGIN: ../../../flutter/common/task_runners.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/display_list.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/display_list.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/display_list_attributes.h + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/display_list/display_list_attributes_testing.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/display_list_benchmarks.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/display_list_benchmarks.h + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/display_list/display_list_benchmarks_canvas_provider.h + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/display_list/display_list_benchmarks_gl.cc + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/display_list/display_list_benchmarks_gl.h + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/display_list/display_list_benchmarks_metal.cc + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/display_list/display_list_benchmarks_metal.h + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/display_list/display_list_benchmarks_software.cc + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/display_list/display_list_benchmarks_software.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/display_list_blend_mode.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/display_list_blend_mode.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/display_list_builder.cc + ../../../flutter/LICENSE @@ -881,13 +873,22 @@ ORIGIN: ../../../flutter/display_list/display_list_rtree.h + ../../../flutter/LI ORIGIN: ../../../flutter/display_list/display_list_runtime_effect.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/display_list_runtime_effect.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/display_list_sampling_options.h + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/display_list/display_list_test_utils.cc + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/display_list/display_list_test_utils.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/display_list_tile_mode.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/display_list_utils.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/display_list_utils.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/display_list_vertices.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/display_list_vertices.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/display_list/testing/dl_test_equality.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/display_list/testing/dl_test_snippets.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/display_list/testing/dl_test_snippets.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/display_list/testing/dl_test_surface_gl.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/display_list/testing/dl_test_surface_gl.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/display_list/testing/dl_test_surface_metal.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/display_list/testing/dl_test_surface_metal.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/display_list/testing/dl_test_surface_provider.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/display_list/testing/dl_test_surface_provider.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/display_list/testing/dl_test_surface_software.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/display_list/testing/dl_test_surface_software.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/types.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/flow/compositor_context.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/flow/compositor_context.h + ../../../flutter/LICENSE @@ -3309,16 +3310,8 @@ FILE: ../../../flutter/common/task_runners.h FILE: ../../../flutter/display_list/display_list.cc FILE: ../../../flutter/display_list/display_list.h FILE: ../../../flutter/display_list/display_list_attributes.h -FILE: ../../../flutter/display_list/display_list_attributes_testing.h FILE: ../../../flutter/display_list/display_list_benchmarks.cc FILE: ../../../flutter/display_list/display_list_benchmarks.h -FILE: ../../../flutter/display_list/display_list_benchmarks_canvas_provider.h -FILE: ../../../flutter/display_list/display_list_benchmarks_gl.cc -FILE: ../../../flutter/display_list/display_list_benchmarks_gl.h -FILE: ../../../flutter/display_list/display_list_benchmarks_metal.cc -FILE: ../../../flutter/display_list/display_list_benchmarks_metal.h -FILE: ../../../flutter/display_list/display_list_benchmarks_software.cc -FILE: ../../../flutter/display_list/display_list_benchmarks_software.h FILE: ../../../flutter/display_list/display_list_blend_mode.cc FILE: ../../../flutter/display_list/display_list_blend_mode.h FILE: ../../../flutter/display_list/display_list_builder.cc @@ -3368,13 +3361,22 @@ FILE: ../../../flutter/display_list/display_list_rtree.h FILE: ../../../flutter/display_list/display_list_runtime_effect.cc FILE: ../../../flutter/display_list/display_list_runtime_effect.h FILE: ../../../flutter/display_list/display_list_sampling_options.h -FILE: ../../../flutter/display_list/display_list_test_utils.cc -FILE: ../../../flutter/display_list/display_list_test_utils.h FILE: ../../../flutter/display_list/display_list_tile_mode.h FILE: ../../../flutter/display_list/display_list_utils.cc FILE: ../../../flutter/display_list/display_list_utils.h FILE: ../../../flutter/display_list/display_list_vertices.cc FILE: ../../../flutter/display_list/display_list_vertices.h +FILE: ../../../flutter/display_list/testing/dl_equality.h +FILE: ../../../flutter/display_list/testing/dl_test_snippets.cc +FILE: ../../../flutter/display_list/testing/dl_test_snippets.h +FILE: ../../../flutter/display_list/testing/dl_test_surface_gl.cc +FILE: ../../../flutter/display_list/testing/dl_test_surface_gl.h +FILE: ../../../flutter/display_list/testing/dl_test_surface_metal.cc +FILE: ../../../flutter/display_list/testing/dl_test_surface_metal.h +FILE: ../../../flutter/display_list/testing/dl_test_surface_provider.cc +FILE: ../../../flutter/display_list/testing/dl_test_surface_provider.h +FILE: ../../../flutter/display_list/testing/dl_test_surface_software.cc +FILE: ../../../flutter/display_list/testing/dl_test_surface_software.h FILE: ../../../flutter/display_list/types.h FILE: ../../../flutter/flow/compositor_context.cc FILE: ../../../flutter/flow/compositor_context.h diff --git a/display_list/BUILD.gn b/display_list/BUILD.gn index 32756ec2597df..160647a8612ba 100644 --- a/display_list/BUILD.gn +++ b/display_list/BUILD.gn @@ -86,23 +86,6 @@ if (enable_unittests) { fixtures = [] } - source_set("display_list_testing") { - testonly = true - - sources = [ - "display_list_attributes_testing.h", - "display_list_test_utils.cc", - "display_list_test_utils.h", - ] - - deps = [ - "//flutter/testing:testing_lib", - "//third_party/skia", - ] - - public_deps = [ ":display_list" ] - } - executable("display_list_unittests") { testonly = true @@ -126,7 +109,7 @@ if (enable_unittests) { deps = [ ":display_list", ":display_list_fixtures", - ":display_list_testing", + "//flutter/display_list/testing:display_list_testing", "//flutter/testing", ] @@ -153,7 +136,8 @@ if (enable_unittests) { deps = [ ":display_list", ":display_list_fixtures", - ":display_list_testing", + "//flutter/display_list/testing:display_list_surface_provider", + "//flutter/display_list/testing:display_list_testing", "//flutter/testing", ] @@ -180,8 +164,8 @@ if (enable_unittests) { deps = [ ":display_list", ":display_list_fixtures", - ":display_list_testing", "//flutter/benchmarking", + "//flutter/display_list/testing:display_list_testing", "//flutter/testing:testing_lib", ] } @@ -204,64 +188,13 @@ source_set("display_list_benchmarks_source") { ":display_list_benchmarks_fixtures", "//flutter/benchmarking", "//flutter/common/graphics", + "//flutter/display_list/testing:display_list_surface_provider", "//flutter/fml", "//flutter/testing:skia", "//flutter/testing:testing_lib", "//third_party/dart/runtime:libdart_jit", # for tracing "//third_party/skia", ] - - defines = [] - - if (is_android) { - libs = [ - "android", - "EGL", - "GLESv2", - ] - } - - # We only do software benchmarks on non-mobile platforms - if (!is_android && !is_ios) { - sources += [ - "display_list_benchmarks_software.cc", - "display_list_benchmarks_software.h", - ] - defines += [ "ENABLE_SOFTWARE_BENCHMARKS" ] - } - - # iOS and Fuchsia don't support OpenGL - _enable_gl_benchmarks = !is_fuchsia && !is_ios - - # TODO (https://github.com/flutter/flutter/issues/107357): - # impeller_enable_vulkan currently requires skia to not use VMA, which inturn - # causes linkage problems with swiftshader. - if (impeller_enable_vulkan) { - _enable_gl_benchmarks = false - } - - if (_enable_gl_benchmarks) { - defines += [ "ENABLE_OPENGL_BENCHMARKS" ] - sources += [ - "display_list_benchmarks_gl.cc", - "display_list_benchmarks_gl.h", - ] - deps += [ "//flutter/testing:opengl" ] - } - - if (is_mac || is_ios) { - defines += [ "ENABLE_METAL_BENCHMARKS" ] - sources += [ - "display_list_benchmarks_metal.cc", - "display_list_benchmarks_metal.h", - ] - deps += [ "//flutter/testing:metal" ] - } - - # Don't snapshot test results on mobile platforms - if (is_android || is_ios) { - defines += [ "BENCHMARKS_NO_SNAPSHOT" ] - } } executable("display_list_benchmarks") { diff --git a/display_list/display_list_benchmarks.cc b/display_list/display_list_benchmarks.cc index 03dd0a49172c0..356b3c7a7343d 100644 --- a/display_list/display_list_benchmarks.cc +++ b/display_list/display_list_benchmarks.cc @@ -5,6 +5,7 @@ #include "flutter/display_list/display_list_benchmarks.h" #include "flutter/display_list/display_list_builder.h" #include "flutter/display_list/display_list_flags.h" +#include "flutter/testing/assertions_skia.h" #include "third_party/skia/include/core/SkPoint.h" #include "third_party/skia/include/core/SkSurface.h" @@ -13,27 +14,6 @@ namespace flutter { namespace testing { -std::unique_ptr CreateCanvasProvider(BackendType backend_type) { - switch (backend_type) { -#ifdef ENABLE_SOFTWARE_BENCHMARKS - case kSoftware_Backend: - return std::make_unique(); -#endif -#ifdef ENABLE_OPENGL_BENCHMARKS - case kOpenGL_Backend: - return std::make_unique(); -#endif -#ifdef ENABLE_METAL_BENCHMARKS - case kMetal_Backend: - return std::make_unique(); -#endif - default: - return nullptr; - } - - return nullptr; -} - SkPaint GetPaintForRun(unsigned attributes) { SkPaint paint; @@ -95,7 +75,7 @@ constexpr size_t kFixedCanvasSize = 1024; void BM_DrawLine(benchmark::State& state, BackendType backend_type, unsigned attributes) { - auto canvas_provider = CreateCanvasProvider(backend_type); + auto surface_provider = DlSurfaceProvider::Create(backend_type); DisplayListBuilder builder; builder.setAttributesFromPaint(GetPaintForRun(attributes), DisplayListOpFlags::kDrawLineFlags); @@ -103,8 +83,9 @@ void BM_DrawLine(benchmark::State& state, size_t length = state.range(0); - canvas_provider->InitializeSurface(length, length); - auto canvas = canvas_provider->GetSurface()->getCanvas(); + surface_provider->InitializeSurface(length, length); + auto surface = surface_provider->GetPrimarySurface()->sk_surface(); + auto canvas = surface->getCanvas(); state.counters["DrawCallCount"] = kLinesToDraw; for (size_t i = 0; i < kLinesToDraw; i++) { @@ -117,12 +98,12 @@ void BM_DrawLine(benchmark::State& state, // We only want to time the actual rasterization. for ([[maybe_unused]] auto _ : state) { display_list->RenderTo(canvas); - canvas_provider->GetSurface()->flushAndSubmit(true); + surface->flushAndSubmit(true); } - auto filename = canvas_provider->BackendName() + "-DrawLine-" + + auto filename = surface_provider->backend_name() + "-DrawLine-" + std::to_string(state.range(0)) + ".png"; - canvas_provider->Snapshot(filename); + surface_provider->Snapshot(filename); } // Draws a series of square rects of the requested width across @@ -132,7 +113,7 @@ void BM_DrawLine(benchmark::State& state, void BM_DrawRect(benchmark::State& state, BackendType backend_type, unsigned attributes) { - auto canvas_provider = CreateCanvasProvider(backend_type); + auto surface_provider = DlSurfaceProvider::Create(backend_type); DisplayListBuilder builder; builder.setAttributesFromPaint(GetPaintForRun(attributes), DisplayListOpFlags::kDrawRectFlags); @@ -140,8 +121,9 @@ void BM_DrawRect(benchmark::State& state, size_t length = state.range(0); size_t canvas_size = length * 2; - canvas_provider->InitializeSurface(canvas_size, canvas_size); - auto canvas = canvas_provider->GetSurface()->getCanvas(); + surface_provider->InitializeSurface(canvas_size, canvas_size); + auto surface = surface_provider->GetPrimarySurface()->sk_surface(); + auto canvas = surface->getCanvas(); // As rects have SkScalar dimensions, we want to ensure that we also // draw rects with non-integer position and size @@ -165,12 +147,12 @@ void BM_DrawRect(benchmark::State& state, // We only want to time the actual rasterization. for ([[maybe_unused]] auto _ : state) { display_list->RenderTo(canvas); - canvas_provider->GetSurface()->flushAndSubmit(true); + surface->flushAndSubmit(true); } - auto filename = canvas_provider->BackendName() + "-DrawRect-" + + auto filename = surface_provider->backend_name() + "-DrawRect-" + std::to_string(state.range(0)) + ".png"; - canvas_provider->Snapshot(filename); + surface_provider->Snapshot(filename); } // Draws a series of ovals of the requested height with aspect ratio 3:2 across @@ -180,7 +162,7 @@ void BM_DrawRect(benchmark::State& state, void BM_DrawOval(benchmark::State& state, BackendType backend_type, unsigned attributes) { - auto canvas_provider = CreateCanvasProvider(backend_type); + auto surface_provider = DlSurfaceProvider::Create(backend_type); DisplayListBuilder builder; builder.setAttributesFromPaint(GetPaintForRun(attributes), DisplayListOpFlags::kDrawOvalFlags); @@ -188,8 +170,9 @@ void BM_DrawOval(benchmark::State& state, size_t length = state.range(0); size_t canvas_size = length * 2; - canvas_provider->InitializeSurface(canvas_size, canvas_size); - auto canvas = canvas_provider->GetSurface()->getCanvas(); + surface_provider->InitializeSurface(canvas_size, canvas_size); + auto surface = surface_provider->GetPrimarySurface()->sk_surface(); + auto canvas = surface->getCanvas(); SkRect rect = SkRect::MakeXYWH(0, 0, length * 1.5f, length); const SkScalar offset = 0.5f; @@ -210,12 +193,12 @@ void BM_DrawOval(benchmark::State& state, // We only want to time the actual rasterization. for ([[maybe_unused]] auto _ : state) { display_list->RenderTo(canvas); - canvas_provider->GetSurface()->flushAndSubmit(true); + surface->flushAndSubmit(true); } - auto filename = canvas_provider->BackendName() + "-DrawOval-" + + auto filename = surface_provider->backend_name() + "-DrawOval-" + std::to_string(state.range(0)) + ".png"; - canvas_provider->Snapshot(filename); + surface_provider->Snapshot(filename); } // Draws a series of circles of the requested radius across @@ -225,7 +208,7 @@ void BM_DrawOval(benchmark::State& state, void BM_DrawCircle(benchmark::State& state, BackendType backend_type, unsigned attributes) { - auto canvas_provider = CreateCanvasProvider(backend_type); + auto surface_provider = DlSurfaceProvider::Create(backend_type); DisplayListBuilder builder; builder.setAttributesFromPaint(GetPaintForRun(attributes), DisplayListOpFlags::kDrawCircleFlags); @@ -233,8 +216,9 @@ void BM_DrawCircle(benchmark::State& state, size_t length = state.range(0); size_t canvas_size = length * 2; - canvas_provider->InitializeSurface(canvas_size, canvas_size); - auto canvas = canvas_provider->GetSurface()->getCanvas(); + surface_provider->InitializeSurface(canvas_size, canvas_size); + auto surface = surface_provider->GetPrimarySurface()->sk_surface(); + auto canvas = surface->getCanvas(); SkScalar radius = length / 2.0f; const SkScalar offset = 0.5f; @@ -257,12 +241,12 @@ void BM_DrawCircle(benchmark::State& state, // We only want to time the actual rasterization. for ([[maybe_unused]] auto _ : state) { display_list->RenderTo(canvas); - canvas_provider->GetSurface()->flushAndSubmit(true); + surface->flushAndSubmit(true); } - auto filename = canvas_provider->BackendName() + "-DrawCircle-" + + auto filename = surface_provider->backend_name() + "-DrawCircle-" + std::to_string(state.range(0)) + ".png"; - canvas_provider->Snapshot(filename); + surface_provider->Snapshot(filename); } // Draws a series of rounded rects of the requested width across @@ -273,7 +257,7 @@ void BM_DrawRRect(benchmark::State& state, BackendType backend_type, unsigned attributes, SkRRect::Type type) { - auto canvas_provider = CreateCanvasProvider(backend_type); + auto surface_provider = DlSurfaceProvider::Create(backend_type); DisplayListBuilder builder; builder.setAttributesFromPaint(GetPaintForRun(attributes), DisplayListOpFlags::kDrawRRectFlags); @@ -281,8 +265,9 @@ void BM_DrawRRect(benchmark::State& state, size_t length = state.range(0); size_t canvas_size = length * 2; - canvas_provider->InitializeSurface(canvas_size, canvas_size); - auto canvas = canvas_provider->GetSurface()->getCanvas(); + surface_provider->InitializeSurface(canvas_size, canvas_size); + auto surface = surface_provider->GetPrimarySurface()->sk_surface(); + auto canvas = surface->getCanvas(); SkVector radii[4] = {}; switch (type) { @@ -334,12 +319,12 @@ void BM_DrawRRect(benchmark::State& state, // We only want to time the actual rasterization. for ([[maybe_unused]] auto _ : state) { display_list->RenderTo(canvas); - canvas_provider->GetSurface()->flushAndSubmit(true); + surface->flushAndSubmit(true); } - auto filename = canvas_provider->BackendName() + "-DrawRRect-" + + auto filename = surface_provider->backend_name() + "-DrawRRect-" + std::to_string(state.range(0)) + ".png"; - canvas_provider->Snapshot(filename); + surface_provider->Snapshot(filename); } // Draws a series of "DR" rects of the requested width across @@ -353,7 +338,7 @@ void BM_DrawDRRect(benchmark::State& state, BackendType backend_type, unsigned attributes, SkRRect::Type type) { - auto canvas_provider = CreateCanvasProvider(backend_type); + auto surface_provider = DlSurfaceProvider::Create(backend_type); DisplayListBuilder builder; builder.setAttributesFromPaint(GetPaintForRun(attributes), DisplayListOpFlags::kDrawDRRectFlags); @@ -361,8 +346,9 @@ void BM_DrawDRRect(benchmark::State& state, size_t length = state.range(0); size_t canvas_size = length * 2; - canvas_provider->InitializeSurface(canvas_size, canvas_size); - auto canvas = canvas_provider->GetSurface()->getCanvas(); + surface_provider->InitializeSurface(canvas_size, canvas_size); + auto surface = surface_provider->GetPrimarySurface()->sk_surface(); + auto canvas = surface->getCanvas(); SkVector radii[4] = {}; switch (type) { @@ -415,18 +401,18 @@ void BM_DrawDRRect(benchmark::State& state, // We only want to time the actual rasterization. for ([[maybe_unused]] auto _ : state) { display_list->RenderTo(canvas); - canvas_provider->GetSurface()->flushAndSubmit(true); + surface->flushAndSubmit(true); } - auto filename = canvas_provider->BackendName() + "-DrawDRRect-" + + auto filename = surface_provider->backend_name() + "-DrawDRRect-" + std::to_string(state.range(0)) + ".png"; - canvas_provider->Snapshot(filename); + surface_provider->Snapshot(filename); } void BM_DrawArc(benchmark::State& state, BackendType backend_type, unsigned attributes) { - auto canvas_provider = CreateCanvasProvider(backend_type); + auto surface_provider = DlSurfaceProvider::Create(backend_type); DisplayListBuilder builder; builder.setAttributesFromPaint(GetPaintForRun(attributes), DisplayListOpFlags::kDrawArcNoCenterFlags); @@ -435,8 +421,9 @@ void BM_DrawArc(benchmark::State& state, size_t length = state.range(0); size_t canvas_size = length * 2; - canvas_provider->InitializeSurface(canvas_size, canvas_size); - auto canvas = canvas_provider->GetSurface()->getCanvas(); + surface_provider->InitializeSurface(canvas_size, canvas_size); + auto surface = surface_provider->GetPrimarySurface()->sk_surface(); + auto canvas = surface->getCanvas(); SkScalar starting_angle = 0.0f; SkScalar offset = 0.5f; @@ -467,12 +454,12 @@ void BM_DrawArc(benchmark::State& state, // We only want to time the actual rasterization. for ([[maybe_unused]] auto _ : state) { display_list->RenderTo(canvas); - canvas_provider->GetSurface()->flushAndSubmit(true); + surface->flushAndSubmit(true); } - auto filename = canvas_provider->BackendName() + "-DrawArc-" + + auto filename = surface_provider->backend_name() + "-DrawArc-" + std::to_string(state.range(0)) + ".png"; - canvas_provider->Snapshot(filename); + surface_provider->Snapshot(filename); } // Returns a list of SkPoints that represent `n` points equally spaced out @@ -641,15 +628,16 @@ void BM_DrawPath(benchmark::State& state, BackendType backend_type, unsigned attributes, SkPath::Verb type) { - auto canvas_provider = CreateCanvasProvider(backend_type); + auto surface_provider = DlSurfaceProvider::Create(backend_type); DisplayListBuilder builder; builder.setAttributesFromPaint(GetPaintForRun(attributes), DisplayListOpFlags::kDrawPathFlags); AnnotateAttributes(attributes, state, DisplayListOpFlags::kDrawPathFlags); size_t length = kFixedCanvasSize; - canvas_provider->InitializeSurface(length, length); - auto canvas = canvas_provider->GetSurface()->getCanvas(); + surface_provider->InitializeSurface(length, length); + auto surface = surface_provider->GetPrimarySurface()->sk_surface(); + auto canvas = surface->getCanvas(); SkPath path; @@ -669,12 +657,12 @@ void BM_DrawPath(benchmark::State& state, // We only want to time the actual rasterization. for ([[maybe_unused]] auto _ : state) { display_list->RenderTo(canvas); - canvas_provider->GetSurface()->flushAndSubmit(true); + surface->flushAndSubmit(true); } - auto filename = canvas_provider->BackendName() + "-DrawPath-" + label + "-" + - std::to_string(state.range(0)) + ".png"; - canvas_provider->Snapshot(filename); + auto filename = surface_provider->backend_name() + "-DrawPath-" + label + + "-" + std::to_string(state.range(0)) + ".png"; + surface_provider->Snapshot(filename); } // Returns a set of vertices that describe a circle that has a @@ -778,15 +766,16 @@ void BM_DrawVertices(benchmark::State& state, BackendType backend_type, unsigned attributes, DlVertexMode mode) { - auto canvas_provider = CreateCanvasProvider(backend_type); + auto surface_provider = DlSurfaceProvider::Create(backend_type); DisplayListBuilder builder; builder.setAttributesFromPaint(GetPaintForRun(attributes), DisplayListOpFlags::kDrawVerticesFlags); AnnotateAttributes(attributes, state, DisplayListOpFlags::kDrawVerticesFlags); size_t length = kFixedCanvasSize; - canvas_provider->InitializeSurface(length, length); - auto canvas = canvas_provider->GetSurface()->getCanvas(); + surface_provider->InitializeSurface(length, length); + auto surface = surface_provider->GetPrimarySurface()->sk_surface(); + auto canvas = surface->getCanvas(); SkPoint center = SkPoint::Make(length / 2.0f, length / 2.0f); @@ -814,13 +803,13 @@ void BM_DrawVertices(benchmark::State& state, // We only want to time the actual rasterization. for ([[maybe_unused]] auto _ : state) { display_list->RenderTo(canvas); - canvas_provider->GetSurface()->flushAndSubmit(true); + surface->flushAndSubmit(true); } - auto filename = canvas_provider->BackendName() + "-DrawVertices-" + + auto filename = surface_provider->backend_name() + "-DrawVertices-" + std::to_string(disc_count) + "-" + VertexModeToString(mode) + ".png"; - canvas_provider->Snapshot(filename); + surface_provider->Snapshot(filename); } // Generate `count` test points. @@ -873,7 +862,7 @@ void BM_DrawPoints(benchmark::State& state, BackendType backend_type, unsigned attributes, SkCanvas::PointMode mode) { - auto canvas_provider = CreateCanvasProvider(backend_type); + auto surface_provider = DlSurfaceProvider::Create(backend_type); DisplayListBuilder builder; SkPaint paint; switch (mode) { @@ -901,8 +890,9 @@ void BM_DrawPoints(benchmark::State& state, } size_t length = kFixedCanvasSize; - canvas_provider->InitializeSurface(length, length); - auto canvas = canvas_provider->GetSurface()->getCanvas(); + surface_provider->InitializeSurface(length, length); + auto surface = surface_provider->GetPrimarySurface()->sk_surface(); + auto canvas = surface->getCanvas(); size_t point_count = state.range(0); state.SetComplexityN(point_count); @@ -917,13 +907,13 @@ void BM_DrawPoints(benchmark::State& state, for ([[maybe_unused]] auto _ : state) { display_list->RenderTo(canvas); - canvas_provider->GetSurface()->flushAndSubmit(true); + surface->flushAndSubmit(true); } - auto filename = canvas_provider->BackendName() + "-DrawPoints-" + + auto filename = surface_provider->backend_name() + "-DrawPoints-" + PointModeToString(mode) + "-" + std::to_string(point_count) + ".png"; - canvas_provider->Snapshot(filename); + surface_provider->Snapshot(filename); } sk_sp ImageFromBitmapWithNewID(const SkBitmap& bitmap) { @@ -942,7 +932,7 @@ void BM_DrawImage(benchmark::State& state, unsigned attributes, DlImageSampling options, bool upload_bitmap) { - auto canvas_provider = CreateCanvasProvider(backend_type); + auto surface_provider = DlSurfaceProvider::Create(backend_type); DisplayListBuilder builder; builder.setAttributesFromPaint(GetPaintForRun(attributes), DisplayListOpFlags::kDrawImageWithPaintFlags); @@ -951,10 +941,12 @@ void BM_DrawImage(benchmark::State& state, size_t bitmap_size = state.range(0); size_t canvas_size = 2 * bitmap_size; - canvas_provider->InitializeSurface(canvas_size, canvas_size); - auto canvas = canvas_provider->GetSurface()->getCanvas(); + surface_provider->InitializeSurface(canvas_size, canvas_size); + auto surface = surface_provider->GetPrimarySurface()->sk_surface(); + auto canvas = surface->getCanvas(); sk_sp image; + std::shared_ptr offscreen_instance; sk_sp offscreen; SkBitmap bitmap; @@ -965,7 +957,9 @@ void BM_DrawImage(benchmark::State& state, bitmap.allocPixels(info, 0); bitmap.eraseColor(SK_ColorBLUE); } else { - offscreen = canvas_provider->MakeOffscreenSurface(bitmap_size, bitmap_size); + offscreen_instance = + surface_provider->MakeOffscreenSurface(bitmap_size, bitmap_size); + offscreen = offscreen_instance->sk_surface(); offscreen->getCanvas()->clear(SK_ColorRED); } @@ -991,13 +985,13 @@ void BM_DrawImage(benchmark::State& state, for ([[maybe_unused]] auto _ : state) { display_list->RenderTo(canvas); - canvas_provider->GetSurface()->flushAndSubmit(true); + surface->flushAndSubmit(true); } - auto filename = canvas_provider->BackendName() + "-DrawImage-" + + auto filename = surface_provider->backend_name() + "-DrawImage-" + (upload_bitmap ? "Upload-" : "Texture-") + std::to_string(bitmap_size) + ".png"; - canvas_provider->Snapshot(filename); + surface_provider->Snapshot(filename); } std::string ConstraintToString(SkCanvas::SrcRectConstraint constraint) { @@ -1021,7 +1015,7 @@ void BM_DrawImageRect(benchmark::State& state, DlImageSampling options, SkCanvas::SrcRectConstraint constraint, bool upload_bitmap) { - auto canvas_provider = CreateCanvasProvider(backend_type); + auto surface_provider = DlSurfaceProvider::Create(backend_type); DisplayListBuilder builder; builder.setAttributesFromPaint( GetPaintForRun(attributes), @@ -1031,10 +1025,12 @@ void BM_DrawImageRect(benchmark::State& state, size_t bitmap_size = state.range(0); size_t canvas_size = 2 * bitmap_size; - canvas_provider->InitializeSurface(canvas_size, canvas_size); - auto canvas = canvas_provider->GetSurface()->getCanvas(); + surface_provider->InitializeSurface(canvas_size, canvas_size); + auto surface = surface_provider->GetPrimarySurface()->sk_surface(); + auto canvas = surface->getCanvas(); sk_sp image; + std::shared_ptr offscreen_instance; sk_sp offscreen; SkBitmap bitmap; @@ -1045,7 +1041,9 @@ void BM_DrawImageRect(benchmark::State& state, bitmap.allocPixels(info, 0); bitmap.eraseColor(SK_ColorBLUE); } else { - offscreen = canvas_provider->MakeOffscreenSurface(bitmap_size, bitmap_size); + offscreen_instance = + surface_provider->MakeOffscreenSurface(bitmap_size, bitmap_size); + offscreen = offscreen_instance->sk_surface(); offscreen->getCanvas()->clear(SK_ColorRED); } @@ -1074,14 +1072,14 @@ void BM_DrawImageRect(benchmark::State& state, for ([[maybe_unused]] auto _ : state) { display_list->RenderTo(canvas); - canvas_provider->GetSurface()->flushAndSubmit(true); + surface->flushAndSubmit(true); } - auto filename = canvas_provider->BackendName() + "-DrawImageRect-" + + auto filename = surface_provider->backend_name() + "-DrawImageRect-" + (upload_bitmap ? "Upload-" : "Texture-") + ConstraintToString(constraint) + "-" + std::to_string(bitmap_size) + ".png"; - canvas_provider->Snapshot(filename); + surface_provider->Snapshot(filename); } std::string FilterModeToString(const DlFilterMode mode) { @@ -1105,7 +1103,7 @@ void BM_DrawImageNine(benchmark::State& state, unsigned attributes, const DlFilterMode filter, bool upload_bitmap) { - auto canvas_provider = CreateCanvasProvider(backend_type); + auto surface_provider = DlSurfaceProvider::Create(backend_type); DisplayListBuilder builder; builder.setAttributesFromPaint( GetPaintForRun(attributes), @@ -1115,13 +1113,15 @@ void BM_DrawImageNine(benchmark::State& state, size_t bitmap_size = state.range(0); size_t canvas_size = 2 * bitmap_size; - canvas_provider->InitializeSurface(canvas_size, canvas_size); - auto canvas = canvas_provider->GetSurface()->getCanvas(); + surface_provider->InitializeSurface(canvas_size, canvas_size); + auto surface = surface_provider->GetPrimarySurface()->sk_surface(); + auto canvas = surface->getCanvas(); SkIRect center = SkIRect::MakeXYWH(bitmap_size / 4, bitmap_size / 4, bitmap_size / 2, bitmap_size / 2); sk_sp image; + std::shared_ptr offscreen_instance; sk_sp offscreen; SkBitmap bitmap; @@ -1132,7 +1132,9 @@ void BM_DrawImageNine(benchmark::State& state, bitmap.allocPixels(info, 0); bitmap.eraseColor(SK_ColorBLUE); } else { - offscreen = canvas_provider->MakeOffscreenSurface(bitmap_size, bitmap_size); + offscreen_instance = + surface_provider->MakeOffscreenSurface(bitmap_size, bitmap_size); + offscreen = offscreen_instance->sk_surface(); offscreen->getCanvas()->clear(SK_ColorRED); } @@ -1158,14 +1160,14 @@ void BM_DrawImageNine(benchmark::State& state, for ([[maybe_unused]] auto _ : state) { display_list->RenderTo(canvas); - canvas_provider->GetSurface()->flushAndSubmit(true); + surface->flushAndSubmit(true); } - auto filename = canvas_provider->BackendName() + "-DrawImageNine-" + + auto filename = surface_provider->backend_name() + "-DrawImageNine-" + (upload_bitmap ? "Upload-" : "Texture-") + FilterModeToString(filter) + "-" + std::to_string(bitmap_size) + ".png"; - canvas_provider->Snapshot(filename); + surface_provider->Snapshot(filename); } // Draws a series of glyph runs with 32 glyphs in each run. The number of runs @@ -1178,7 +1180,7 @@ void BM_DrawImageNine(benchmark::State& state, void BM_DrawTextBlob(benchmark::State& state, BackendType backend_type, unsigned attributes) { - auto canvas_provider = CreateCanvasProvider(backend_type); + auto surface_provider = DlSurfaceProvider::Create(backend_type); DisplayListBuilder builder; builder.setAttributesFromPaint(GetPaintForRun(attributes), DisplayListOpFlags::kDrawTextBlobFlags); @@ -1186,8 +1188,9 @@ void BM_DrawTextBlob(benchmark::State& state, size_t draw_calls = state.range(0); size_t canvas_size = kFixedCanvasSize; - canvas_provider->InitializeSurface(canvas_size, canvas_size); - auto canvas = canvas_provider->GetSurface()->getCanvas(); + surface_provider->InitializeSurface(canvas_size, canvas_size); + auto surface = surface_provider->GetPrimarySurface()->sk_surface(); + auto canvas = surface->getCanvas(); state.counters["DrawCallCount_Varies"] = draw_calls; state.counters["GlyphCount"] = draw_calls; @@ -1203,12 +1206,12 @@ void BM_DrawTextBlob(benchmark::State& state, for ([[maybe_unused]] auto _ : state) { display_list->RenderTo(canvas); - canvas_provider->GetSurface()->flushAndSubmit(true); + surface->flushAndSubmit(true); } - auto filename = canvas_provider->BackendName() + "-DrawTextBlob-" + + auto filename = surface_provider->backend_name() + "-DrawTextBlob-" + std::to_string(draw_calls) + ".png"; - canvas_provider->Snapshot(filename); + surface_provider->Snapshot(filename); } // Draw the shadow for a 10-sided regular polygon where the polygon's @@ -1224,15 +1227,16 @@ void BM_DrawShadow(benchmark::State& state, unsigned attributes, bool transparent_occluder, SkPath::Verb type) { - auto canvas_provider = CreateCanvasProvider(backend_type); + auto surface_provider = DlSurfaceProvider::Create(backend_type); DisplayListBuilder builder; builder.setAttributesFromPaint(GetPaintForRun(attributes), DisplayListOpFlags::kDrawShadowFlags); AnnotateAttributes(attributes, state, DisplayListOpFlags::kDrawShadowFlags); size_t length = kFixedCanvasSize; - canvas_provider->InitializeSurface(length, length); - auto canvas = canvas_provider->GetSurface()->getCanvas(); + surface_provider->InitializeSurface(length, length); + auto surface = surface_provider->GetPrimarySurface()->sk_surface(); + auto canvas = surface->getCanvas(); SkPath path; @@ -1267,14 +1271,14 @@ void BM_DrawShadow(benchmark::State& state, // We only want to time the actual rasterization. for ([[maybe_unused]] auto _ : state) { display_list->RenderTo(canvas); - canvas_provider->GetSurface()->flushAndSubmit(true); + surface->flushAndSubmit(true); } - auto filename = canvas_provider->BackendName() + "-DrawShadow-" + + auto filename = surface_provider->backend_name() + "-DrawShadow-" + VerbToString(type) + "-" + (transparent_occluder ? "Transparent-" : "Opaque-") + std::to_string(elevation) + "-" + ".png"; - canvas_provider->Snapshot(filename); + surface_provider->Snapshot(filename); } // Calls saveLayer N times from the root canvas layer, and optionally calls @@ -1287,15 +1291,16 @@ void BM_SaveLayer(benchmark::State& state, BackendType backend_type, unsigned attributes, size_t save_depth) { - auto canvas_provider = CreateCanvasProvider(backend_type); + auto surface_provider = DlSurfaceProvider::Create(backend_type); DisplayListBuilder builder; builder.setAttributesFromPaint(GetPaintForRun(attributes), DisplayListOpFlags::kSaveLayerFlags); AnnotateAttributes(attributes, state, DisplayListOpFlags::kSaveLayerFlags); size_t length = kFixedCanvasSize; - canvas_provider->InitializeSurface(length, length); - auto canvas = canvas_provider->GetSurface()->getCanvas(); + surface_provider->InitializeSurface(length, length); + auto surface = surface_provider->GetPrimarySurface()->sk_surface(); + auto canvas = surface->getCanvas(); size_t save_layer_calls = state.range(0); @@ -1320,14 +1325,26 @@ void BM_SaveLayer(benchmark::State& state, // We only want to time the actual rasterization. for ([[maybe_unused]] auto _ : state) { display_list->RenderTo(canvas); - canvas_provider->GetSurface()->flushAndSubmit(true); + surface->flushAndSubmit(true); } - auto filename = canvas_provider->BackendName() + "-SaveLayer-" + + auto filename = surface_provider->backend_name() + "-SaveLayer-" + std::to_string(save_depth) + "-" + std::to_string(save_layer_calls) + ".png"; - canvas_provider->Snapshot(filename); + surface_provider->Snapshot(filename); } +#ifdef ENABLE_SOFTWARE_BENCHMARKS +RUN_DISPLAYLIST_BENCHMARKS(Software) +#endif + +#ifdef ENABLE_OPENGL_BENCHMARKS +RUN_DISPLAYLIST_BENCHMARKS(OpenGL) +#endif + +#ifdef ENABLE_METAL_BENCHMARKS +RUN_DISPLAYLIST_BENCHMARKS(Metal) +#endif + } // namespace testing } // namespace flutter diff --git a/display_list/display_list_benchmarks.h b/display_list/display_list_benchmarks.h index 38c27edc8c8c4..ad64fdc0cff04 100644 --- a/display_list/display_list_benchmarks.h +++ b/display_list/display_list_benchmarks.h @@ -5,9 +5,9 @@ #ifndef FLUTTER_FLOW_DISPLAY_LIST_BENCHMARKS_H_ #define FLUTTER_FLOW_DISPLAY_LIST_BENCHMARKS_H_ -#include "flutter/display_list/display_list_benchmarks_canvas_provider.h" #include "flutter/display_list/display_list_sampling_options.h" #include "flutter/display_list/display_list_vertices.h" +#include "flutter/display_list/testing/dl_test_surface_provider.h" #include "third_party/benchmark/include/benchmark/benchmark.h" #include "third_party/skia/include/core/SkCanvas.h" @@ -16,21 +16,9 @@ #include "third_party/skia/include/core/SkSurface.h" #include "third_party/skia/include/core/SkVertices.h" -#ifdef ENABLE_SOFTWARE_BENCHMARKS -#include "flutter/display_list/display_list_benchmarks_software.h" -#endif -#ifdef ENABLE_OPENGL_BENCHMARKS -#include "flutter/display_list/display_list_benchmarks_gl.h" -#endif -#ifdef ENABLE_METAL_BENCHMARKS -#include "flutter/display_list/display_list_benchmarks_metal.h" -#endif - namespace flutter { namespace testing { -typedef enum { kSoftware_Backend, kOpenGL_Backend, kMetal_Backend } BackendType; - enum BenchmarkAttributes { kEmpty_Flag = 0, kStrokedStyle_Flag = 1 << 0, @@ -39,9 +27,10 @@ enum BenchmarkAttributes { kAntiAliasing_Flag = 1 << 3 }; -std::unique_ptr CreateCanvasProvider(BackendType backend_type); SkPaint GetPaintForRun(unsigned attributes); +using BackendType = DlSurfaceProvider::BackendType; + // Benchmarks void BM_DrawLine(benchmark::State& state, diff --git a/display_list/display_list_benchmarks_canvas_provider.h b/display_list/display_list_benchmarks_canvas_provider.h deleted file mode 100644 index 3b12ef7b3b33d..0000000000000 --- a/display_list/display_list_benchmarks_canvas_provider.h +++ /dev/null @@ -1,52 +0,0 @@ -// 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_FLOW_DISPLAY_LIST_BENCHMARKS_CANVAS_PROVIDER_H_ -#define FLUTTER_FLOW_DISPLAY_LIST_BENCHMARKS_CANVAS_PROVIDER_H_ - -#include "flutter/fml/mapping.h" -#include "flutter/testing/testing.h" - -#include "third_party/skia/include/core/SkData.h" -#include "third_party/skia/include/core/SkSurface.h" - -namespace flutter { -namespace testing { - -class CanvasProvider { - public: - virtual ~CanvasProvider() = default; - virtual const std::string BackendName() = 0; - virtual void InitializeSurface(const size_t width, const size_t height) = 0; - virtual sk_sp GetSurface() = 0; - virtual sk_sp MakeOffscreenSurface(const size_t width, - const size_t height) = 0; - - virtual bool Snapshot(std::string filename) { -#ifdef BENCHMARKS_NO_SNAPSHOT - return false; -#else - auto image = GetSurface()->makeImageSnapshot(); - if (!image) { - return false; - } - auto raster = image->makeRasterImage(); - if (!raster) { - return false; - } - auto data = raster->encodeToData(); - if (!data) { - return false; - } - fml::NonOwnedMapping mapping(static_cast(data->data()), - data->size()); - return WriteAtomically(OpenFixturesDirectory(), filename.c_str(), mapping); -#endif - } -}; - -} // namespace testing -} // namespace flutter - -#endif // FLUTTER_FLOW_DISPLAY_LIST_BENCHMARKS_CANVAS_PROVIDER_H_ diff --git a/display_list/display_list_benchmarks_gl.cc b/display_list/display_list_benchmarks_gl.cc deleted file mode 100644 index fcf5f967d4884..0000000000000 --- a/display_list/display_list_benchmarks_gl.cc +++ /dev/null @@ -1,52 +0,0 @@ -// 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/display_list/display_list_benchmarks_gl.h" -#include "flutter/display_list/display_list_benchmarks.h" - -#include "third_party/skia/include/core/SkCanvas.h" -#include "third_party/skia/include/core/SkSurface.h" - -namespace flutter { -namespace testing { - -void OpenGLCanvasProvider::InitializeSurface(const size_t width, - const size_t height) { - surface_size_ = SkISize::Make(width, height); - - gl_surface_ = std::make_unique(surface_size_); - gl_surface_->MakeCurrent(); - - const auto image_info = SkImageInfo::MakeN32Premul(surface_size_); - surface_ = SkSurface::MakeRenderTarget( - gl_surface_->GetGrContext().get(), skgpu::Budgeted::kNo, image_info, 1, - kTopLeft_GrSurfaceOrigin, nullptr, false); - surface_->getCanvas()->clear(SK_ColorTRANSPARENT); -} - -sk_sp OpenGLCanvasProvider::GetSurface() { - if (!gl_surface_->MakeCurrent()) { - return nullptr; - } - return surface_; -} - -sk_sp OpenGLCanvasProvider::MakeOffscreenSurface( - const size_t width, - const size_t height) { - surface_size_ = SkISize::Make(width, height); - const auto image_info = SkImageInfo::MakeN32Premul(surface_size_); - - auto offscreen_surface = SkSurface::MakeRenderTarget( - gl_surface_->GetGrContext().get(), skgpu::Budgeted::kNo, image_info, 1, - kTopLeft_GrSurfaceOrigin, nullptr, false); - - offscreen_surface->getCanvas()->clear(SK_ColorTRANSPARENT); - return offscreen_surface; -} - -RUN_DISPLAYLIST_BENCHMARKS(OpenGL) - -} // namespace testing -} // namespace flutter diff --git a/display_list/display_list_benchmarks_gl.h b/display_list/display_list_benchmarks_gl.h deleted file mode 100644 index e00eaeb118b80..0000000000000 --- a/display_list/display_list_benchmarks_gl.h +++ /dev/null @@ -1,35 +0,0 @@ -// 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_FLOW_DISPLAY_LIST_BENCHMARKS_GL_H_ -#define FLUTTER_FLOW_DISPLAY_LIST_BENCHMARKS_GL_H_ - -#include "flutter/display_list/display_list_benchmarks_canvas_provider.h" - -#include "flutter/testing/test_gl_surface.h" - -#include "third_party/skia/include/core/SkSurface.h" - -namespace flutter { -namespace testing { - -class OpenGLCanvasProvider : public CanvasProvider { - public: - virtual ~OpenGLCanvasProvider() = default; - void InitializeSurface(const size_t width, const size_t height) override; - sk_sp GetSurface() override; - sk_sp MakeOffscreenSurface(const size_t width, - const size_t height) override; - const std::string BackendName() override { return "OpenGL"; } - - private: - SkISize surface_size_; - sk_sp surface_; - std::unique_ptr gl_surface_; -}; - -} // namespace testing -} // namespace flutter - -#endif // FLUTTER_FLOW_DISPLAY_LIST_BENCHMARKS_GL_H_ diff --git a/display_list/display_list_benchmarks_metal.cc b/display_list/display_list_benchmarks_metal.cc deleted file mode 100644 index 2437663d62024..0000000000000 --- a/display_list/display_list_benchmarks_metal.cc +++ /dev/null @@ -1,40 +0,0 @@ -// 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/display_list/display_list_benchmarks_metal.h" -#include "flutter/display_list/display_list_benchmarks.h" - -#include "third_party/skia/include/core/SkCanvas.h" -#include "third_party/skia/include/core/SkSurface.h" - -namespace flutter { -namespace testing { - -void MetalCanvasProvider::InitializeSurface(const size_t width, - const size_t height) { - metal_context_ = std::make_unique(); - metal_surface_ = - TestMetalSurface::Create(*metal_context_, SkISize::Make(width, height)); - metal_surface_->GetSurface()->getCanvas()->clear(SK_ColorTRANSPARENT); -} - -sk_sp MetalCanvasProvider::GetSurface() { - if (!metal_surface_) { - return nullptr; - } - return metal_surface_->GetSurface(); -} - -sk_sp MetalCanvasProvider::MakeOffscreenSurface( - const size_t width, - const size_t height) { - metal_offscreen_surface_ = - TestMetalSurface::Create(*metal_context_, SkISize::Make(width, height)); - return metal_offscreen_surface_->GetSurface(); -} - -RUN_DISPLAYLIST_BENCHMARKS(Metal) - -} // namespace testing -} // namespace flutter diff --git a/display_list/display_list_benchmarks_metal.h b/display_list/display_list_benchmarks_metal.h deleted file mode 100644 index bb8f635cdadbb..0000000000000 --- a/display_list/display_list_benchmarks_metal.h +++ /dev/null @@ -1,35 +0,0 @@ -// 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_FLOW_DISPLAY_LIST_BENCHMARKS_METAL_H_ -#define FLUTTER_FLOW_DISPLAY_LIST_BENCHMARKS_METAL_H_ - -#include "flutter/display_list/display_list_benchmarks_canvas_provider.h" -#include "flutter/testing/test_metal_surface.h" - -#include "third_party/skia/include/core/SkSurface.h" - -namespace flutter { -namespace testing { - -class MetalCanvasProvider : public CanvasProvider { - public: - virtual ~MetalCanvasProvider() = default; - - void InitializeSurface(const size_t width, const size_t height) override; - sk_sp GetSurface() override; - sk_sp MakeOffscreenSurface(const size_t width, - const size_t height) override; - const std::string BackendName() override { return "Metal"; } - - private: - std::unique_ptr metal_context_; - std::unique_ptr metal_surface_; - std::unique_ptr metal_offscreen_surface_; -}; - -} // namespace testing -} // namespace flutter - -#endif // FLUTTER_FLOW_DISPLAY_LIST_BENCHMARKS_METAL_H_ diff --git a/display_list/display_list_benchmarks_software.cc b/display_list/display_list_benchmarks_software.cc deleted file mode 100644 index 7e2e7f40a53af..0000000000000 --- a/display_list/display_list_benchmarks_software.cc +++ /dev/null @@ -1,30 +0,0 @@ -// 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/display_list/display_list_benchmarks_software.h" -#include "flutter/display_list/display_list_benchmarks.h" - -#include "third_party/skia/include/core/SkSurface.h" - -namespace flutter { -namespace testing { - -void SoftwareCanvasProvider::InitializeSurface(const size_t width, - const size_t height) { - surface_ = SkSurface::MakeRasterN32Premul(width, height); - surface_->getCanvas()->clear(SK_ColorTRANSPARENT); -} - -sk_sp SoftwareCanvasProvider::MakeOffscreenSurface( - const size_t width, - const size_t height) { - auto surface = SkSurface::MakeRasterN32Premul(width, height); - surface->getCanvas()->clear(SK_ColorTRANSPARENT); - return surface; -} - -RUN_DISPLAYLIST_BENCHMARKS(Software) - -} // namespace testing -} // namespace flutter diff --git a/display_list/display_list_benchmarks_software.h b/display_list/display_list_benchmarks_software.h deleted file mode 100644 index 6451c0cbed472..0000000000000 --- a/display_list/display_list_benchmarks_software.h +++ /dev/null @@ -1,31 +0,0 @@ -// 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_FLOW_DISPLAY_LIST_BENCHMARKS_SOFTWARE_H_ -#define FLUTTER_FLOW_DISPLAY_LIST_BENCHMARKS_SOFTWARE_H_ - -#include "flutter/display_list/display_list_benchmarks_canvas_provider.h" - -#include "third_party/skia/include/core/SkSurface.h" - -namespace flutter { -namespace testing { - -class SoftwareCanvasProvider : public CanvasProvider { - public: - virtual ~SoftwareCanvasProvider() = default; - void InitializeSurface(const size_t width, const size_t height) override; - sk_sp GetSurface() override { return surface_; } - sk_sp MakeOffscreenSurface(const size_t width, - const size_t height) override; - const std::string BackendName() override { return "Software"; } - - private: - sk_sp surface_; -}; - -} // namespace testing -} // namespace flutter - -#endif // FLUTTER_FLOW_DISPLAY_LIST_BENCHMARKS_SOFTWARE_H_ diff --git a/display_list/display_list_builder_benchmarks.cc b/display_list/display_list_builder_benchmarks.cc index 57270578ca90f..5228c4b0f986f 100644 --- a/display_list/display_list_builder_benchmarks.cc +++ b/display_list/display_list_builder_benchmarks.cc @@ -3,7 +3,7 @@ // found in the LICENSE file. #include "flutter/benchmarking/benchmarking.h" -#include "flutter/display_list/display_list_test_utils.h" +#include "flutter/display_list/testing/dl_test_snippets.h" namespace flutter { namespace { diff --git a/display_list/display_list_canvas_unittests.cc b/display_list/display_list_canvas_unittests.cc index aebb618d424a1..eaeff1076ffbe 100644 --- a/display_list/display_list_canvas_unittests.cc +++ b/display_list/display_list_canvas_unittests.cc @@ -10,6 +10,7 @@ #include "flutter/display_list/display_list_comparable.h" #include "flutter/display_list/display_list_flags.h" #include "flutter/display_list/display_list_sampling_options.h" +#include "flutter/display_list/testing/dl_test_surface_provider.h" #include "flutter/fml/math.h" #include "flutter/testing/testing.h" #include "third_party/skia/include/core/SkPictureRecorder.h" @@ -176,6 +177,13 @@ class BoundsTolerance { return copy; } + BoundsTolerance addPostClipPadding(SkScalar absolute_pad_x, + SkScalar absolute_pad_y) const { + BoundsTolerance copy = BoundsTolerance(*this); + copy.clip_pad_.offset(absolute_pad_x, absolute_pad_y); + return copy; + } + BoundsTolerance addDiscreteOffset(SkScalar discrete_offset) const { BoundsTolerance copy = BoundsTolerance(*this); copy.discrete_offset_ += discrete_offset; @@ -206,6 +214,7 @@ class BoundsTolerance { if (!allowed.intersect(clip_)) { allowed.setEmpty(); } + allowed.outset(clip_pad_.fX, clip_pad_.fY); SkIRect rounded = allowed.roundOut(); int pad_left = std::max(0, pix_bounds.fLeft - rounded.fLeft); int pad_top = std::max(0, pix_bounds.fTop - rounded.fTop); @@ -215,7 +224,7 @@ class BoundsTolerance { int allowed_pad_y = std::max(pad_top, pad_bottom); if (worst_bounds_pad_x > allowed_pad_x || worst_bounds_pad_y > allowed_pad_y) { - FML_LOG(ERROR) << "allowed pad: " // + FML_LOG(ERROR) << "acceptable bounds padding: " // << allowed_pad_x << ", " << allowed_pad_y; } return (worst_bounds_pad_x > allowed_pad_x || @@ -229,6 +238,7 @@ class BoundsTolerance { SkPoint scale_ = {1, 1}; SkPoint absolute_pad_ = {0, 0}; SkRect clip_ = {-1E9, -1E9, 1E9, 1E9}; + SkPoint clip_pad_ = {0, 0}; SkScalar discrete_offset_ = 0; }; @@ -236,99 +246,201 @@ class BoundsTolerance { typedef const std::function CvSetup; typedef const std::function CvRenderer; typedef const std::function DlRenderer; -static void EmptyCvRenderer(SkCanvas*, const SkPaint&) {} -static void EmptyDlRenderer(DisplayListBuilder&) {} +static const CvSetup kEmptyCvSetup = [](SkCanvas*, SkPaint&) {}; +static const CvRenderer kEmptyCvRenderer = [](SkCanvas*, const SkPaint&) {}; +static const DlRenderer kEmptyDlRenderer = [](DisplayListBuilder&) {}; + +using PixelFormat = DlSurfaceProvider::PixelFormat; +using BackendType = DlSurfaceProvider::BackendType; -class RenderSurface { +class RenderResult { public: - explicit RenderSurface(sk_sp surface) - : surface_(std::move(surface)) { - EXPECT_EQ(canvas()->save(), 1); - } - ~RenderSurface() { sk_free(addr_); } - - SkCanvas* canvas() { return surface_->getCanvas(); } - - const SkPixmap* pixmap() { - if (!pixmap_.addr()) { - canvas()->restoreToCount(1); - SkImageInfo info = surface_->imageInfo(); - if (info.colorType() != kN32_SkColorType || - !surface_->peekPixels(&pixmap_)) { - info = SkImageInfo::MakeN32Premul(info.dimensions()); - addr_ = malloc(info.computeMinByteSize() * info.height()); - pixmap_.reset(info, addr_, info.minRowBytes()); - EXPECT_TRUE(surface_->readPixels(pixmap_, 0, 0)); - } - } - return &pixmap_; + RenderResult(const sk_sp& surface, + const SkMatrix& matrix, + const SkIRect& clip_bounds) + : matrix_(matrix), clip_bounds_(clip_bounds) { + SkImageInfo info = surface->imageInfo(); + info = SkImageInfo::MakeN32Premul(info.dimensions()); + addr_ = malloc(info.computeMinByteSize() * info.height()); + pixmap_.reset(info, addr_, info.minRowBytes()); + EXPECT_TRUE(surface->readPixels(pixmap_, 0, 0)); } + ~RenderResult() { free(addr_); } + + int width() const { return pixmap_.width(); } + int height() const { return pixmap_.height(); } + const SkMatrix& matrix() const { return matrix_; } + const SkIRect& clip_bounds() const { return clip_bounds_; } + const uint32_t* addr32(int x, int y) const { return pixmap_.addr32(x, y); } private: - sk_sp surface_; SkPixmap pixmap_; void* addr_ = nullptr; + SkMatrix matrix_; + SkIRect clip_bounds_; +}; + +#define RENDER_JOB_BASE \ + int width = kTestWidth; \ + int height = kTestHeight; \ + DlColor bg = DlColor::kTransparent(); \ + SkScalar scale = SK_Scalar1; \ + SkMatrix setup_matrix; \ + SkIRect setup_clip_bounds + +struct CvRenderJob { + RENDER_JOB_BASE; + CvSetup& cv_setup = kEmptyCvSetup; + CvRenderer& cv_render = kEmptyCvRenderer; + CvRenderer& cv_restore = kEmptyCvRenderer; + sk_sp picture; + SkPaint setup_paint; + + void render(SkCanvas* canvas) { + if (picture) { + picture->playback(canvas); + } else { + SkPaint paint; + cv_setup(canvas, paint); + setup_paint = paint; + setup_matrix = canvas->getTotalMatrix(); + setup_clip_bounds = canvas->getDeviceClipBounds(); + cv_render(canvas, paint); + cv_restore(canvas, paint); + } + } + + sk_sp get_picture() { + SkPictureRecorder recorder; + SkRTreeFactory rtree_factory; + SkCanvas* cv = recorder.beginRecording(kTestBounds, &rtree_factory); + render(cv); + return recorder.finishRecordingAsPicture(); + } +}; + +struct DlRenderJob { + RENDER_JOB_BASE; + DlRenderer& dl_setup = kEmptyDlRenderer; + DlRenderer& dl_render = kEmptyDlRenderer; + DlRenderer& dl_restore = kEmptyDlRenderer; + SkScalar opacity = SK_Scalar1; + sk_sp display_list; + + void render(SkCanvas* canvas) { + get_display_list()->RenderTo(canvas, opacity); + } + + sk_sp get_display_list() { + if (!display_list) { + DisplayListBuilder builder(SkRect::MakeWH(width, height)); + dl_setup(builder); + setup_matrix = builder.getTransform(); + setup_clip_bounds = builder.getDestinationClipBounds().roundOut(); + dl_render(builder); + dl_restore(builder); + display_list = builder.Build(); + } + return display_list; + } }; class RenderEnvironment { public: - static RenderEnvironment Make565() { - return RenderEnvironment( - SkImageInfo::Make({1, 1}, kRGB_565_SkColorType, kOpaque_SkAlphaType)); + RenderEnvironment(const DlSurfaceProvider* provider, PixelFormat format) + : provider_(provider), format_(format) { + if (provider->supports(format)) { + main_surface_ = + provider->MakeOffscreenSurface(kTestWidth, kTestHeight, format); + alt_surface_ = provider->MakeOffscreenSurface(kTestWidth * 2, + kTestHeight * 2, format); + } } - static RenderEnvironment MakeN32() { - return RenderEnvironment(SkImageInfo::MakeN32Premul(1, 1)); + static RenderEnvironment Make565(const DlSurfaceProvider* provider) { + return RenderEnvironment(provider, PixelFormat::k565_PixelFormat); } - std::unique_ptr MakeSurface( - const DlColor bg = DlColor::kTransparent(), - int width = kTestWidth, - int height = kTestHeight) const { - sk_sp surface = - SkSurface::MakeRaster(info_.makeWH(width, height)); - surface->getCanvas()->clear(bg); - return std::make_unique(surface); + static RenderEnvironment MakeN32(const DlSurfaceProvider* provider) { + return RenderEnvironment(provider, PixelFormat::kN32Premul_PixelFormat); } void init_ref(CvRenderer& cv_renderer, DlColor bg = DlColor::kTransparent()) { - init_ref([=](SkCanvas*, SkPaint&) {}, cv_renderer, - [=](DisplayListBuilder&) {}, bg); + init_ref(kEmptyCvSetup, cv_renderer, kEmptyDlRenderer, bg); } void init_ref(CvSetup& cv_setup, CvRenderer& cv_renderer, DlRenderer& dl_setup, DlColor bg = DlColor::kTransparent()) { - ref_canvas()->clear(bg); + CvRenderJob job = { + .bg = bg, + .cv_setup = cv_setup, + .cv_render = cv_renderer, + }; + ref_result_ = getResult(job); dl_setup(ref_attr_); - SkPaint paint; - cv_setup(ref_canvas(), paint); - ref_matrix_ = ref_canvas()->getTotalMatrix(); - ref_clip_ = ref_canvas()->getDeviceClipBounds(); - cv_renderer(ref_canvas(), paint); - ref_pixmap_ = ref_surface_->pixmap(); } - const SkImageInfo& info() const { return info_; } - SkCanvas* ref_canvas() { return ref_surface_->canvas(); } + template + std::unique_ptr getResult(J& job) const { + auto surface = getSurface(job.width, job.height); + FML_DCHECK(surface != nullptr); + auto canvas = surface->getCanvas(); + canvas->clear(job.bg); + + int restore_count = canvas->save(); + canvas->scale(job.scale, job.scale); + job.render(canvas); + canvas->restoreToCount(restore_count); + + canvas->flush(); + surface->flushAndSubmit(true); + return std::make_unique(surface, job.setup_matrix, + job.setup_clip_bounds); + } + + std::unique_ptr getResult(sk_sp dl) const { + DlRenderJob job = { + .display_list = std::move(dl), + }; + return getResult(job); + } + + const DlSurfaceProvider* provider() const { return provider_; } + bool valid() const { return provider_->supports(format_); } + const std::string backend_name() const { return provider_->backend_name(); } + + PixelFormat format() const { return format_; } const DisplayListBuilder& ref_attr() const { return ref_attr_; } - const SkMatrix& ref_matrix() const { return ref_matrix_; } - const SkIRect& ref_clip_bounds() const { return ref_clip_; } - const SkPixmap* ref_pixmap() const { return ref_pixmap_; } + const RenderResult* ref_result() const { return ref_result_.get(); } private: - explicit RenderEnvironment(const SkImageInfo& info) : info_(info) { - ref_surface_ = MakeSurface(); + static bool matches(const DlSurfaceInstance* surface, int w, int h) { + return surface && surface->width() == w && surface->height() == h; + } + + sk_sp getSurface(int width, int height) const { + FML_DCHECK(valid()); + FML_DCHECK(main_surface_ != nullptr); + FML_DCHECK(alt_surface_ != nullptr); + if (matches(main_surface_.get(), width, height)) { + return main_surface_->sk_surface(); + } + if (matches(alt_surface_.get(), width, height)) { + return alt_surface_->sk_surface(); + } + FML_DCHECK(false); + return nullptr; } - const SkImageInfo info_; + const DlSurfaceProvider* provider_; + const PixelFormat format_; + std::shared_ptr main_surface_; + std::shared_ptr alt_surface_; DisplayListBuilder ref_attr_; - SkMatrix ref_matrix_; - SkIRect ref_clip_; - std::unique_ptr ref_surface_; - const SkPixmap* ref_pixmap_ = nullptr; + std::unique_ptr ref_result_; }; class TestParameters { @@ -349,10 +461,11 @@ class TestParameters { if (has_mutating_save_layer) { return false; } - if (env.ref_clip_bounds() != device_clip || has_diff_clip) { + const RenderResult* ref_result = env.ref_result(); + if (ref_result->clip_bounds() != device_clip || has_diff_clip) { return false; } - if (env.ref_matrix() != matrix && !flags_.is_flood()) { + if (ref_result->matrix() != matrix && !flags_.is_flood()) { return false; } if (flags_.ignores_paint()) { @@ -545,6 +658,7 @@ class TestParameters { bool is_draw_display_list() const { return is_draw_display_list_; } bool is_draw_line() const { return is_draw_line_; } bool is_draw_arc_center() const { return is_draw_arc_center_; } + bool is_draw_path() const { return is_draw_path_; } bool is_horizontal_line() const { return is_horizontal_line_; } bool is_vertical_line() const { return is_vertical_line_; } bool ignores_dashes() const { return ignores_dashes_; } @@ -569,6 +683,10 @@ class TestParameters { is_draw_arc_center_ = true; return *this; } + TestParameters& set_draw_path() { + is_draw_path_ = true; + return *this; + } TestParameters& set_ignores_dashes() { ignores_dashes_ = true; return *this; @@ -592,6 +710,7 @@ class TestParameters { bool is_draw_display_list_ = false; bool is_draw_line_ = false; bool is_draw_arc_center_ = false; + bool is_draw_path_ = false; bool ignores_dashes_ = false; bool is_horizontal_line_ = false; bool is_vertical_line_ = false; @@ -600,14 +719,14 @@ class TestParameters { class CaseParameters { public: explicit CaseParameters(std::string info) - : CaseParameters(std::move(info), EmptyCvRenderer, EmptyDlRenderer) {} + : CaseParameters(std::move(info), kEmptyCvRenderer, kEmptyDlRenderer) {} CaseParameters(std::string info, CvSetup& cv_setup, DlRenderer& dl_setup) : CaseParameters(std::move(info), cv_setup, dl_setup, - EmptyCvRenderer, - EmptyDlRenderer, + kEmptyCvRenderer, + kEmptyDlRenderer, SK_ColorTRANSPARENT, false, false, @@ -694,18 +813,22 @@ class CaseParameters { class CanvasCompareTester { public: + static std::vector> kTestProviders; + static BoundsTolerance DefaultTolerance; static void RenderAll(const TestParameters& params, const BoundsTolerance& tolerance = DefaultTolerance) { - RenderEnvironment env = RenderEnvironment::MakeN32(); - env.init_ref(params.cv_renderer()); - RenderWithTransforms(params, env, tolerance); - RenderWithClips(params, env, tolerance); - RenderWithSaveRestore(params, env, tolerance); - // Only test attributes if the canvas version uses the paint object - if (params.uses_paint()) { - RenderWithAttributes(params, env, tolerance); + for (auto& provider : kTestProviders) { + RenderEnvironment env = RenderEnvironment::MakeN32(provider.get()); + env.init_ref(params.cv_renderer()); + RenderWithTransforms(params, env, tolerance); + RenderWithClips(params, env, tolerance); + RenderWithSaveRestore(params, env, tolerance); + // Only test attributes if the canvas version uses the paint object + if (params.uses_paint()) { + RenderWithAttributes(params, env, tolerance); + } } } @@ -827,7 +950,8 @@ class CanvasCompareTester { // so we set an alpha value which works for all primitives except for // drawColor which can override the alpha with its color, but it now uses // a non-opaque color to avoid that problem. - RenderEnvironment backdrop_env = RenderEnvironment::MakeN32(); + RenderEnvironment backdrop_env = + RenderEnvironment::MakeN32(env.provider()); CvSetup cv_backdrop_setup = [=](SkCanvas* cv, SkPaint& p) { SkPaint setup_p; setup_p.setShader(kTestImageColorSource.skia_object()); @@ -991,7 +1115,7 @@ class CanvasCompareTester { // CPU renderer with default line width of 0 does not show antialiasing // for stroked primitives, so we make a new reference with a non-trivial // stroke width to demonstrate the differences - RenderEnvironment aa_env = RenderEnvironment::MakeN32(); + RenderEnvironment aa_env = RenderEnvironment::MakeN32(env.provider()); // Tweak the bounds tolerance for the displacement of 1/10 of a pixel const BoundsTolerance aa_tolerance = tolerance.addBoundsPadding(1, 1); CvSetup cv_aa_setup = [=](SkCanvas* cv, SkPaint& p) { @@ -1033,44 +1157,54 @@ class CanvasCompareTester { // surface, so we use a shader instead of a color. Also, thin stroked // primitives (mainly drawLine and drawPoints) do not show much // dithering so we use a non-trivial stroke width as well. - RenderEnvironment dither_env = RenderEnvironment::Make565(); - DlColor dither_bg = DlColor::kBlack(); - CvSetup cv_dither_setup = [=](SkCanvas*, SkPaint& p) { - p.setShader(kTestImageColorSource.skia_object()); - p.setAlpha(0xf0); - p.setStrokeWidth(5.0); - }; - DlRenderer dl_dither_setup = [=](DisplayListBuilder& b) { - b.setColorSource(&kTestImageColorSource); - b.setColor(DlColor(0xf0000000)); - b.setStrokeWidth(5.0); - }; - dither_env.init_ref(cv_dither_setup, testP.cv_renderer(), // - dl_dither_setup, dither_bg); - RenderWith(testP, dither_env, tolerance, - CaseParameters( - "Dither == True", - [=](SkCanvas* cv, SkPaint& p) { - cv_dither_setup(cv, p); - p.setDither(true); - }, - [=](DisplayListBuilder& b) { - dl_dither_setup(b); - b.setDither(true); - }) - .with_bg(dither_bg)); - RenderWith(testP, dither_env, tolerance, - CaseParameters( - "Dither = False", - [=](SkCanvas* cv, SkPaint& p) { - cv_dither_setup(cv, p); - p.setDither(false); - }, - [=](DisplayListBuilder& b) { - dl_dither_setup(b); - b.setDither(false); - }) - .with_bg(dither_bg)); + RenderEnvironment dither_env = RenderEnvironment::Make565(env.provider()); + if (!dither_env.valid()) { + // Currently only happens on Metal backend + static std::set warnings_sent; + std::string name = dither_env.backend_name(); + if (warnings_sent.find(name) == warnings_sent.end()) { + warnings_sent.insert(name); + FML_LOG(INFO) << "Skipping Dithering tests on " << name; + } + } else { + DlColor dither_bg = DlColor::kBlack(); + CvSetup cv_dither_setup = [=](SkCanvas*, SkPaint& p) { + p.setShader(kTestImageColorSource.skia_object()); + p.setAlpha(0xf0); + p.setStrokeWidth(5.0); + }; + DlRenderer dl_dither_setup = [=](DisplayListBuilder& b) { + b.setColorSource(&kTestImageColorSource); + b.setColor(DlColor(0xf0000000)); + b.setStrokeWidth(5.0); + }; + dither_env.init_ref(cv_dither_setup, testP.cv_renderer(), // + dl_dither_setup, dither_bg); + RenderWith(testP, dither_env, tolerance, + CaseParameters( + "Dither == True", + [=](SkCanvas* cv, SkPaint& p) { + cv_dither_setup(cv, p); + p.setDither(true); + }, + [=](DisplayListBuilder& b) { + dl_dither_setup(b); + b.setDither(true); + }) + .with_bg(dither_bg)); + RenderWith(testP, dither_env, tolerance, + CaseParameters( + "Dither = False", + [=](SkCanvas* cv, SkPaint& p) { + cv_dither_setup(cv, p); + p.setDither(false); + }, + [=](DisplayListBuilder& b) { + dl_dither_setup(b); + b.setDither(false); + }) + .with_bg(dither_bg)); + } } RenderWith(testP, env, tolerance, @@ -1143,7 +1277,7 @@ class CanvasCompareTester { // Being able to see a blur requires some non-default attributes, // like a non-trivial stroke width and a shader rather than a color // (for drawPaint) so we create a new environment for these tests. - RenderEnvironment blur_env = RenderEnvironment::MakeN32(); + RenderEnvironment blur_env = RenderEnvironment::MakeN32(env.provider()); CvSetup cv_blur_setup = [=](SkCanvas*, SkPaint& p) { p.setShader(kTestImageColorSource.skia_object()); p.setStrokeWidth(5.0); @@ -1188,7 +1322,7 @@ class CanvasCompareTester { // Being able to see a dilate requires some non-default attributes, // like a non-trivial stroke width and a shader rather than a color // (for drawPaint) so we create a new environment for these tests. - RenderEnvironment dilate_env = RenderEnvironment::MakeN32(); + RenderEnvironment dilate_env = RenderEnvironment::MakeN32(env.provider()); CvSetup cv_dilate_setup = [=](SkCanvas*, SkPaint& p) { p.setShader(kTestImageColorSource.skia_object()); p.setStrokeWidth(5.0); @@ -1217,7 +1351,7 @@ class CanvasCompareTester { // Being able to see an erode requires some non-default attributes, // like a non-trivial stroke width and a shader rather than a color // (for drawPaint) so we create a new environment for these tests. - RenderEnvironment erode_env = RenderEnvironment::MakeN32(); + RenderEnvironment erode_env = RenderEnvironment::MakeN32(env.provider()); CvSetup cv_erode_setup = [=](SkCanvas*, SkPaint& p) { p.setShader(kTestImageColorSource.skia_object()); p.setStrokeWidth(6.0); @@ -1420,13 +1554,28 @@ class CanvasCompareTester { [=](DisplayListBuilder& b) { // b.setStyle(DlDrawStyle::kFill); })); + // Skia on HW produces a strong miter consistent with width=1.0 + // for any width less than a pixel, but the bounds computations of + // both DL and SkPicture do not account for this. We will get + // OOB pixel errors for the highly mitered drawPath geometry if + // we don't set stroke width to 1.0 for that test on HW. + // See https://bugs.chromium.org/p/skia/issues/detail?id=14046 + bool no_hairlines = + testP.is_draw_path() && + env.provider()->backend_type() != BackendType::kSoftware_Backend; RenderWith(testP, env, tolerance, CaseParameters( "Stroke + defaults", [=](SkCanvas*, SkPaint& p) { // + if (no_hairlines) { + p.setStrokeWidth(1.0); + } p.setStyle(SkPaint::kStroke_Style); }, [=](DisplayListBuilder& b) { // + if (no_hairlines) { + b.setStrokeWidth(1.0); + } b.setStyle(DlDrawStyle::kStroke); })); @@ -1442,7 +1591,8 @@ class CanvasCompareTester { b.setStrokeWidth(10.0); })); - RenderEnvironment stroke_base_env = RenderEnvironment::MakeN32(); + RenderEnvironment stroke_base_env = + RenderEnvironment::MakeN32(env.provider()); CvSetup cv_stroke_setup = [=](SkCanvas*, SkPaint& p) { p.setStyle(SkPaint::kStroke_Style); p.setStrokeWidth(5.0); @@ -1685,11 +1835,20 @@ class CanvasCompareTester { static void RenderWithClips(const TestParameters& testP, const RenderEnvironment& env, const BoundsTolerance& diff_tolerance) { - SkRect r_clip = kRenderBounds.makeInset(15.5, 15.5); + // We used to use an inset of 15.5 pixels here, but since Skia's rounding + // behavior at the center of pixels does not match between HW and SW, we + // ended up with some clips including different pixels between the two + // destinations and this interacted poorly with the carefully chosen + // geometry in some of the tests which was designed to have just the + // right features fully filling the clips based on the SW rounding. By + // moving to a 15.4 inset, the edge of the clip is never on the "rounding + // edge" of a pixel. + SkRect r_clip = kRenderBounds.makeInset(15.4, 15.4); BoundsTolerance intersect_tolerance = diff_tolerance.clip(r_clip); + intersect_tolerance = intersect_tolerance.addPostClipPadding(1, 1); RenderWith(testP, env, intersect_tolerance, CaseParameters( - "Hard ClipRect inset by 15.5", + "Hard ClipRect inset by 15.4", [=](SkCanvas* c, SkPaint&) { c->clipRect(r_clip, SkClipOp::kIntersect, false); }, @@ -1698,7 +1857,7 @@ class CanvasCompareTester { })); RenderWith(testP, env, intersect_tolerance, CaseParameters( - "AntiAlias ClipRect inset by 15.5", + "AntiAlias ClipRect inset by 15.4", [=](SkCanvas* c, SkPaint&) { c->clipRect(r_clip, SkClipOp::kIntersect, true); }, @@ -1707,7 +1866,7 @@ class CanvasCompareTester { })); RenderWith(testP, env, diff_tolerance, CaseParameters( - "Hard ClipRect Diff, inset by 15.5", + "Hard ClipRect Diff, inset by 15.4", [=](SkCanvas* c, SkPaint&) { c->clipRect(r_clip, SkClipOp::kDifference, false); }, @@ -1715,10 +1874,15 @@ class CanvasCompareTester { b.clipRect(r_clip, SkClipOp::kDifference, false); }) .with_diff_clip()); - SkRRect rr_clip = SkRRect::MakeRectXY(r_clip, 1.8, 2.7); + // This test RR clip used to use very small radii, but due to + // optimizations in the HW rrect rasterization, this caused small + // bulges in the corners of the RRect which were interpreted as + // "clip overruns" by the clip OOB pixel testing code. Using less + // abusively small radii fixes the problem. + SkRRect rr_clip = SkRRect::MakeRectXY(r_clip, 9, 9); RenderWith(testP, env, intersect_tolerance, CaseParameters( - "Hard ClipRRect inset by 15.5", + "Hard ClipRRect with radius of 15.4", [=](SkCanvas* c, SkPaint&) { c->clipRRect(rr_clip, SkClipOp::kIntersect, false); }, @@ -1727,7 +1891,7 @@ class CanvasCompareTester { })); RenderWith(testP, env, intersect_tolerance, CaseParameters( - "AntiAlias ClipRRect inset by 15.5", + "AntiAlias ClipRRect with radius of 15.4", [=](SkCanvas* c, SkPaint&) { c->clipRRect(rr_clip, SkClipOp::kIntersect, true); }, @@ -1736,7 +1900,7 @@ class CanvasCompareTester { })); RenderWith(testP, env, diff_tolerance, CaseParameters( - "Hard ClipRRect Diff, inset by 15.5", + "Hard ClipRRect Diff, with radius of 15.4", [=](SkCanvas* c, SkPaint&) { c->clipRRect(rr_clip, SkClipOp::kDifference, false); }, @@ -1750,7 +1914,7 @@ class CanvasCompareTester { path_clip.addCircle(kRenderCenterX, kRenderCenterY, 1.0); RenderWith(testP, env, intersect_tolerance, CaseParameters( - "Hard ClipPath inset by 15.5", + "Hard ClipPath inset by 15.4", [=](SkCanvas* c, SkPaint&) { c->clipPath(path_clip, SkClipOp::kIntersect, false); }, @@ -1759,7 +1923,7 @@ class CanvasCompareTester { })); RenderWith(testP, env, intersect_tolerance, CaseParameters( - "AntiAlias ClipPath inset by 15.5", + "AntiAlias ClipPath inset by 15.4", [=](SkCanvas* c, SkPaint&) { c->clipPath(path_clip, SkClipOp::kIntersect, true); }, @@ -1768,7 +1932,7 @@ class CanvasCompareTester { })); RenderWith(testP, env, diff_tolerance, CaseParameters( - "Hard ClipPath Diff, inset by 15.5", + "Hard ClipPath Diff, inset by 15.4", [=](SkCanvas* c, SkPaint&) { c->clipPath(path_clip, SkClipOp::kDifference, false); }, @@ -1791,47 +1955,52 @@ class CanvasCompareTester { const RenderEnvironment& env, const BoundsTolerance& tolerance_in, const CaseParameters& caseP) { - // sk_surface is a direct rendering via SkCanvas to SkSurface - // DisplayList mechanisms are not involved in this operation - const std::string info = caseP.info(); + const std::string info = env.backend_name() + ": " + caseP.info(); const DlColor bg = caseP.bg(); - std::unique_ptr sk_surface = env.MakeSurface(bg); - SkCanvas* sk_canvas = sk_surface->canvas(); - SkPaint sk_paint; - caseP.cv_setup()(sk_canvas, sk_paint); - SkMatrix sk_matrix = sk_canvas->getTotalMatrix(); - SkIRect sk_clip = sk_canvas->getDeviceClipBounds(); + + // sk_result is a direct rendering via SkCanvas to SkSurface + // DisplayList mechanisms are not involved in this operation + // SkPaint sk_paint; + CvRenderJob sk_job = { + .bg = caseP.bg(), + .cv_setup = caseP.cv_setup(), + .cv_render = testP.cv_renderer(), + .cv_restore = caseP.cv_restore(), + }; + auto sk_result = env.getResult(sk_job); const BoundsTolerance tolerance = - testP.adjust(tolerance_in, sk_paint, sk_canvas->getTotalMatrix()); - testP.render_to(sk_canvas, sk_paint); - caseP.cv_restore()(sk_canvas, sk_paint); - const sk_sp sk_picture = getSkPicture(testP, caseP); + testP.adjust(tolerance_in, sk_job.setup_paint, sk_job.setup_matrix); + const sk_sp sk_picture = sk_job.get_picture(); SkRect sk_bounds = sk_picture->cullRect(); - const SkPixmap* sk_pixels = sk_surface->pixmap(); - ASSERT_EQ(sk_pixels->width(), kTestWidth) << info; - ASSERT_EQ(sk_pixels->height(), kTestHeight) << info; - ASSERT_EQ(sk_pixels->info().bytesPerPixel(), 4) << info; - checkPixels(sk_pixels, sk_bounds, info + " (Skia reference)", bg); + ASSERT_EQ(sk_result->width(), kTestWidth) << info; + ASSERT_EQ(sk_result->height(), kTestHeight) << info; + checkPixels(sk_result.get(), sk_bounds, info + " (Skia reference)", bg); DisplayListBuilder dl_attr; caseP.dl_setup()(dl_attr); - if (testP.should_match(env, dl_attr, sk_matrix, sk_clip, - caseP.has_diff_clip(), + if (testP.should_match(env, dl_attr, sk_result->matrix(), + sk_result->clip_bounds(), caseP.has_diff_clip(), caseP.has_mutating_save_layer())) { - quickCompareToReference(env.ref_pixmap(), sk_pixels, true, + quickCompareToReference(env.ref_result(), sk_result.get(), true, info + " (attribute has no effect)"); } else { - quickCompareToReference(env.ref_pixmap(), sk_pixels, false, + quickCompareToReference(env.ref_result(), sk_result.get(), false, info + " (attribute affects rendering)"); } + sk_sp display_list; { // This sequence plays the provided equivalently constructed // DisplayList onto the SkCanvas of the surface // DisplayList => direct rendering - DisplayListBuilder builder(kTestBounds); - caseP.render_to(builder, testP); - sk_sp display_list = builder.Build(); + DlRenderJob dl_job = { + .bg = caseP.bg(), + .dl_setup = caseP.dl_setup(), + .dl_render = testP.dl_renderer(), + .dl_restore = caseP.dl_restore(), + }; + auto dl_result = env.getResult(dl_job); + display_list = dl_job.display_list; SkRect dl_bounds = display_list->bounds(); if (!sk_bounds.roundOut().contains(dl_bounds)) { FML_LOG(ERROR) << "For " << info; @@ -1866,15 +2035,13 @@ class CanvasCompareTester { << info; } - std::unique_ptr dl_surface = env.MakeSurface(bg); - display_list->RenderTo(dl_surface->canvas()); - compareToReference(dl_surface->pixmap(), sk_pixels, + compareToReference(dl_result.get(), sk_result.get(), info + " (DisplayList built directly -> surface)", &dl_bounds, &tolerance, bg, caseP.fuzzy_compare_components()); if (display_list->can_apply_group_opacity()) { - checkGroupOpacity(env, display_list, dl_surface->pixmap(), + checkGroupOpacity(env, display_list, dl_result.get(), info + " with Group Opacity", bg); } } @@ -1885,11 +2052,14 @@ class CanvasCompareTester { // This sequence renders SkCanvas calls to a DisplayList and then // plays them back on SkCanvas to SkSurface // SkCanvas calls => DisplayList => rendering - std::unique_ptr cv_dl_surface = env.MakeSurface(bg); DisplayListCanvasRecorder dl_recorder(kTestBounds); - caseP.render_to(&dl_recorder, testP); - dl_recorder.builder()->Build()->RenderTo(cv_dl_surface->canvas()); - compareToReference(cv_dl_surface->pixmap(), sk_pixels, + sk_job.render(&dl_recorder); + DlRenderJob cv_dl_job = { + .bg = bg, + .display_list = dl_recorder.Build(), + }; + auto cv_dl_result = env.getResult(cv_dl_job); + compareToReference(cv_dl_result.get(), sk_result.get(), info + " (Skia calls -> DisplayList -> surface)", nullptr, nullptr, bg, caseP.fuzzy_compare_components()); @@ -1905,31 +2075,26 @@ class CanvasCompareTester { const int test_height_2 = kTestHeight * 2; const SkScalar test_scale = 2.0; - SkPictureRecorder sk_x2_recorder; - SkCanvas* ref_canvas = sk_x2_recorder.beginRecording(kTestBounds); - SkPaint ref_paint; - caseP.render_to(ref_canvas, testP); - sk_sp ref_x2_picture = - sk_x2_recorder.finishRecordingAsPicture(); - std::unique_ptr ref_x2_surface = - env.MakeSurface(bg, test_width_2, test_height_2); - SkCanvas* ref_x2_canvas = ref_x2_surface->canvas(); - ref_x2_canvas->scale(test_scale, test_scale); - ref_x2_picture->playback(ref_x2_canvas); - const SkPixmap* ref_x2_pixels = ref_x2_surface->pixmap(); - ASSERT_EQ(ref_x2_pixels->width(), test_width_2) << info; - ASSERT_EQ(ref_x2_pixels->height(), test_height_2) << info; - ASSERT_EQ(ref_x2_pixels->info().bytesPerPixel(), 4) << info; - - DisplayListBuilder builder_x2(kTestBounds); - caseP.render_to(builder_x2, testP); - sk_sp display_list_x2 = builder_x2.Build(); - std::unique_ptr test_x2_surface = - env.MakeSurface(bg, test_width_2, test_height_2); - SkCanvas* test_x2_canvas = test_x2_surface->canvas(); - test_x2_canvas->scale(test_scale, test_scale); - display_list_x2->RenderTo(test_x2_canvas); - compareToReference(test_x2_surface->pixmap(), ref_x2_pixels, + CvRenderJob sk_job_x2 = { + .width = test_width_2, + .height = test_height_2, + .bg = bg, + .scale = test_scale, + .picture = sk_picture, + }; + auto ref_x2_result = env.getResult(sk_job_x2); + ASSERT_EQ(ref_x2_result->width(), test_width_2) << info; + ASSERT_EQ(ref_x2_result->height(), test_height_2) << info; + + DlRenderJob dl_job_x2 = { + .width = test_width_2, + .height = test_height_2, + .bg = bg, + .scale = test_scale, + .display_list = display_list, + }; + auto test_x2_result = env.getResult(dl_job_x2); + compareToReference(test_x2_result.get(), ref_x2_result.get(), info + " (Both rendered scaled 2x)", nullptr, nullptr, bg, caseP.fuzzy_compare_components(), // test_width_2, test_height_2, false); @@ -1947,28 +2112,40 @@ class CanvasCompareTester { return true; } + static int groupOpacityFudgeFactor(const RenderEnvironment& env) { + if (env.format() == PixelFormat::k565_PixelFormat) { + return 9; + } + if (env.provider()->backend_type() == BackendType::kOpenGL_Backend) { + // OpenGL gets a little fuzzy at times. Still, "within 5" (aka +/-4) + // for byte samples is not bad, though the other backends give +/-1 + return 5; + } + return 2; + } static void checkGroupOpacity(const RenderEnvironment& env, const sk_sp& display_list, - const SkPixmap* ref_pixmap, + const RenderResult* ref_result, const std::string& info, DlColor bg) { SkScalar opacity = 128.0 / 255.0; - std::unique_ptr group_opacity_surface = env.MakeSurface(bg); - SkCanvas* group_opacity_canvas = group_opacity_surface->canvas(); - display_list->RenderTo(group_opacity_canvas, opacity); - const SkPixmap* group_opacity_pixmap = group_opacity_surface->pixmap(); + DlRenderJob opacity_job = { + .bg = bg, + .opacity = opacity, + .display_list = display_list, + }; + auto group_opacity_result = env.getResult(opacity_job); - ASSERT_EQ(group_opacity_pixmap->width(), kTestWidth) << info; - ASSERT_EQ(group_opacity_pixmap->height(), kTestHeight) << info; - ASSERT_EQ(group_opacity_pixmap->info().bytesPerPixel(), 4) << info; + ASSERT_EQ(group_opacity_result->width(), kTestWidth) << info; + ASSERT_EQ(group_opacity_result->height(), kTestHeight) << info; - ASSERT_EQ(ref_pixmap->width(), kTestWidth) << info; - ASSERT_EQ(ref_pixmap->height(), kTestHeight) << info; - ASSERT_EQ(ref_pixmap->info().bytesPerPixel(), 4) << info; + ASSERT_EQ(ref_result->width(), kTestWidth) << info; + ASSERT_EQ(ref_result->height(), kTestHeight) << info; int pixels_touched = 0; int pixels_different = 0; + int max_diff = 0; // We need to allow some slight differences per component due to the // fact that rearranging discrete calculations can compound round off // errors. Off-by-2 is enough for 8 bit components, but for the 565 @@ -1977,10 +2154,10 @@ class CanvasCompareTester { // max step of 8 converting 5 bits to 8 bits, but it is really // converting 31 steps to 255 steps with an average step size of // 8.23 - 24 of the steps are by 8, but 7 of them are by 9.) - int fudge = env.info().bytesPerPixel() < 4 ? 9 : 2; + int fudge = groupOpacityFudgeFactor(env); for (int y = 0; y < kTestHeight; y++) { - const uint32_t* ref_row = ref_pixmap->addr32(0, y); - const uint32_t* test_row = group_opacity_pixmap->addr32(0, y); + const uint32_t* ref_row = ref_result->addr32(0, y); + const uint32_t* test_row = group_opacity_result->addr32(0, y); for (int x = 0; x < kTestWidth; x++) { uint32_t ref_pixel = ref_row[x]; uint32_t test_pixel = test_row[x]; @@ -1992,6 +2169,10 @@ class CanvasCompareTester { SkScalar faded_comp = bg_comp + (ref_comp - bg_comp) * opacity; int test_comp = (test_pixel >> i) & 0xff; if (std::abs(faded_comp - test_comp) > fudge) { + int diff = std::abs(faded_comp - test_comp); + if (max_diff < diff) { + max_diff = diff; + } pixels_different++; break; } @@ -2000,10 +2181,13 @@ class CanvasCompareTester { } } ASSERT_GT(pixels_touched, 20) << info; + if (pixels_different > 1) { + FML_LOG(ERROR) << "max diff == " << max_diff << " for " << info; + } ASSERT_LE(pixels_different, 1) << info; } - static void checkPixels(const SkPixmap* ref_pixels, + static void checkPixels(const RenderResult* ref_result, const SkRect ref_bounds, const std::string& info, const DlColor bg) { @@ -2012,7 +2196,7 @@ class CanvasCompareTester { int pixels_oob = 0; SkIRect i_bounds = ref_bounds.roundOut(); for (int y = 0; y < kTestHeight; y++) { - const uint32_t* ref_row = ref_pixels->addr32(0, y); + const uint32_t* ref_row = ref_result->addr32(0, y); for (int x = 0; x < kTestWidth; x++) { if (ref_row[x] != untouched) { pixels_touched++; @@ -2026,19 +2210,19 @@ class CanvasCompareTester { ASSERT_GT(pixels_touched, 0) << info; } - static void quickCompareToReference(const SkPixmap* ref_pixels, - const SkPixmap* test_pixels, + static void quickCompareToReference(const RenderResult* ref_result, + const RenderResult* test_result, bool should_match, const std::string& info) { - ASSERT_EQ(test_pixels->width(), ref_pixels->width()) << info; - ASSERT_EQ(test_pixels->height(), ref_pixels->height()) << info; - ASSERT_EQ(test_pixels->info().bytesPerPixel(), 4) << info; - ASSERT_EQ(ref_pixels->info().bytesPerPixel(), 4) << info; + int w = test_result->width(); + int h = test_result->height(); + ASSERT_EQ(w, ref_result->width()) << info; + ASSERT_EQ(h, ref_result->height()) << info; int pixels_different = 0; - for (int y = 0; y < test_pixels->height(); y++) { - const uint32_t* ref_row = ref_pixels->addr32(0, y); - const uint32_t* test_row = test_pixels->addr32(0, y); - for (int x = 0; x < test_pixels->width(); x++) { + for (int y = 0; y < h; y++) { + const uint32_t* ref_row = ref_result->addr32(0, y); + const uint32_t* test_row = test_result->addr32(0, y); + for (int x = 0; x < w; x++) { if (ref_row[x] != test_row[x]) { if (should_match) { FML_LOG(ERROR) << std::hex << ref_row[x] << " != " << test_row[x]; @@ -2054,8 +2238,8 @@ class CanvasCompareTester { } } - static void compareToReference(const SkPixmap* test_pixels, - const SkPixmap* ref_pixels, + static void compareToReference(const RenderResult* test_result, + const RenderResult* ref_result, const std::string& info, SkRect* bounds, const BoundsTolerance* tolerance, @@ -2065,10 +2249,8 @@ class CanvasCompareTester { int height = kTestHeight, bool printMismatches = false) { uint32_t untouched = bg.premultipliedArgb(); - ASSERT_EQ(test_pixels->width(), width) << info; - ASSERT_EQ(test_pixels->height(), height) << info; - ASSERT_EQ(test_pixels->info().bytesPerPixel(), 4) << info; - ASSERT_EQ(ref_pixels->info().bytesPerPixel(), 4) << info; + ASSERT_EQ(test_result->width(), width) << info; + ASSERT_EQ(test_result->height(), height) << info; SkIRect i_bounds = bounds ? bounds->roundOut() : SkIRect::MakeWH(width, height); @@ -2079,8 +2261,8 @@ class CanvasCompareTester { int max_x = 0; int max_y = 0; for (int y = 0; y < height; y++) { - const uint32_t* ref_row = ref_pixels->addr32(0, y); - const uint32_t* test_row = test_pixels->addr32(0, y); + const uint32_t* ref_row = ref_result->addr32(0, y); + const uint32_t* test_row = test_result->addr32(0, y); for (int x = 0; x < width; x++) { if (bounds && test_row[x] != untouched) { if (min_x > x) { @@ -2146,7 +2328,7 @@ class CanvasCompareTester { int worst_pad_x = std::max(pad_left, pad_right); int worst_pad_y = std::max(pad_top, pad_bottom); if (tolerance->overflows(pix_bounds, worst_pad_x, worst_pad_y)) { - FML_LOG(ERROR) << "Overflow for " << info; + FML_LOG(ERROR) << "Computed bounds for " << info; FML_LOG(ERROR) << "pix bounds[" // << pixLeft << ", " << pixTop << " => " // << pixRight << ", " << pixBottom // @@ -2156,14 +2338,15 @@ class CanvasCompareTester { << " => " // << bounds.fRight << ", " << bounds.fBottom // << "]"; - FML_LOG(ERROR) << "Bounds overflowed by up to " // + FML_LOG(ERROR) << "Bounds overly conservative by up to " // << worst_pad_x << ", " << worst_pad_y // << " (" << (worst_pad_x * 100.0 / pix_width) // << "%, " << (worst_pad_y * 100.0 / pix_height) << "%)"; int pix_area = pix_size.area(); int dl_area = bounds.width() * bounds.height(); FML_LOG(ERROR) << "Total overflow area: " << (dl_area - pix_area) // - << " (+" << (dl_area * 100.0 / pix_area - 100.0) << "%)"; + << " (+" << (dl_area * 100.0 / pix_area - 100.0) // + << "% larger)"; FML_LOG(ERROR); } } @@ -2201,6 +2384,9 @@ class CanvasCompareTester { } }; +std::vector> + CanvasCompareTester::kTestProviders; + BoundsTolerance CanvasCompareTester::DefaultTolerance = BoundsTolerance().addAbsolutePadding(1, 1); @@ -2219,6 +2405,73 @@ class DisplayListCanvasTestBase : public BaseT, protected DisplayListOpFlags { public: DisplayListCanvasTestBase() = default; + static bool StartsWith(std::string str, std::string prefix) { + if (prefix.length() > str.length()) { + return false; + } + for (size_t i = 0; i < prefix.length(); i++) { + if (str[i] != prefix[i]) { + return false; + } + } + return true; + } + + static void AddProvider(BackendType type, const std::string& name) { + auto provider = DlSurfaceProvider::Create(type); + if (provider == nullptr) { + FML_LOG(ERROR) << "provider " << name << " not supported (ignoring)"; + return; + } + provider->InitializeSurface(kTestWidth, kTestHeight, + PixelFormat::kN32Premul_PixelFormat); + CanvasCompareTester::kTestProviders.push_back(std::move(provider)); + } + + static void SetUpTestSuite() { + bool do_software = true; + bool do_opengl = false; + bool do_metal = false; + std::vector args = ::testing::internal::GetArgvs(); + for (auto p_arg = std::next(args.begin()); p_arg != args.end(); p_arg++) { + std::string arg = *p_arg; + bool enable = true; + if (StartsWith(arg, "--no")) { + enable = false; + arg = "-" + arg.substr(4); + } + if (arg == "--enable-software") { + do_software = enable; + } else if (arg == "--enable-opengl") { + do_opengl = enable; + } else if (arg == "--enable-metal") { + do_metal = enable; + } + } + if (do_software) { + AddProvider(BackendType::kSoftware_Backend, "Software"); + } + if (do_opengl) { + AddProvider(BackendType::kOpenGL_Backend, "OpenGL"); + } + if (do_metal) { + AddProvider(BackendType::kMetal_Backend, "Metal"); + } + std::string providers = ""; + auto begin = CanvasCompareTester::kTestProviders.cbegin(); + auto end = CanvasCompareTester::kTestProviders.cend(); + while (begin != end) { + providers += " " + (*begin++)->backend_name(); + } + FML_LOG(INFO) << "Running tests on [" << providers << " ]"; + } + + static void TearDownTestSuite() { + // Deleting these provider objects allows Metal to clean up its + // resources before the exit handler reports them as leaks. + CanvasCompareTester::kTestProviders.clear(); + } + private: FML_DISALLOW_COPY_AND_ASSIGN(DisplayListCanvasTestBase); }; @@ -2236,7 +2489,28 @@ TEST_F(DisplayListCanvas, DrawPaint) { kDrawPaintFlags)); } -TEST_F(DisplayListCanvas, DrawColor) { +TEST_F(DisplayListCanvas, DrawOpaqueColor) { + // We use a non-opaque color to avoid obliterating any backdrop filter output + CanvasCompareTester::RenderAll( // + TestParameters( + [=](SkCanvas* canvas, const SkPaint& paint) { + // DrawColor is not tested against attributes because it is supposed + // to ignore them. So, if the paint has an alpha, it is because we + // are doing a saveLayer+backdrop test and we need to not flood over + // the backdrop output with a solid color. So, we perform an alpha + // drawColor for that case only. + SkColor color = SkColorSetA(0xFFFF00FF, paint.getAlpha()); + canvas->drawColor(color); + }, + [=](DisplayListBuilder& builder) { + DlColor color = 0xFFFF00FF; + color = color.withAlpha(builder.getColor().getAlpha()); + builder.drawColor(color, DlBlendMode::kSrcOver); + }, + kDrawColorFlags)); +} + +TEST_F(DisplayListCanvas, DrawAlphaColor) { // We use a non-opaque color to avoid obliterating any backdrop filter output CanvasCompareTester::RenderAll( // TestParameters( @@ -2254,6 +2528,12 @@ TEST_F(DisplayListCanvas, DrawDiagonalLines) { SkPoint p2 = SkPoint::Make(kRenderRight, kRenderBottom); SkPoint p3 = SkPoint::Make(kRenderLeft, kRenderBottom); SkPoint p4 = SkPoint::Make(kRenderRight, kRenderTop); + // Adding some edge center to edge center diagonals to better fill + // out the RRect Clip so bounds checking sees less empty bounds space. + SkPoint p5 = SkPoint::Make(kRenderCenterX, kRenderTop); + SkPoint p6 = SkPoint::Make(kRenderRight, kRenderCenterY); + SkPoint p7 = SkPoint::Make(kRenderLeft, kRenderCenterY); + SkPoint p8 = SkPoint::Make(kRenderCenterX, kRenderBottom); CanvasCompareTester::RenderAll( // TestParameters( @@ -2265,10 +2545,14 @@ TEST_F(DisplayListCanvas, DrawDiagonalLines) { p.setStyle(SkPaint::kStroke_Style); canvas->drawLine(p1, p2, p); canvas->drawLine(p3, p4, p); + canvas->drawLine(p5, p6, p); + canvas->drawLine(p7, p8, p); }, [=](DisplayListBuilder& builder) { // builder.drawLine(p1, p2); builder.drawLine(p3, p4); + builder.drawLine(p5, p6); + builder.drawLine(p7, p8); }, kDrawLineFlags) .set_draw_line()); @@ -2419,7 +2703,8 @@ TEST_F(DisplayListCanvas, DrawPath) { [=](DisplayListBuilder& builder) { // builder.drawPath(path); }, - kDrawPathFlags)); + kDrawPathFlags) + .set_draw_path()); } TEST_F(DisplayListCanvas, DrawArc) { @@ -2558,13 +2843,16 @@ TEST_F(DisplayListCanvas, DrawPointsAsLines) { TEST_F(DisplayListCanvas, DrawPointsAsPolygon) { const SkPoint points1[] = { - // RenderBounds box with a diagonal + // RenderBounds box with a diamond SkPoint::Make(kRenderLeft, kRenderTop), SkPoint::Make(kRenderRight, kRenderTop), SkPoint::Make(kRenderRight, kRenderBottom), SkPoint::Make(kRenderLeft, kRenderBottom), SkPoint::Make(kRenderLeft, kRenderTop), - SkPoint::Make(kRenderRight, kRenderBottom), + SkPoint::Make(kRenderCenterX, kRenderTop), + SkPoint::Make(kRenderRight, kRenderCenterY), + SkPoint::Make(kRenderCenterX, kRenderBottom), + SkPoint::Make(kRenderLeft, kRenderCenterY), }; const int count1 = sizeof(points1) / sizeof(points1[0]); @@ -3042,7 +3330,8 @@ TEST_F(DisplayListCanvas, DrawAtlasLinear) { sk_sp makeTestPicture() { SkPictureRecorder recorder; - SkCanvas* cv = recorder.beginRecording(kRenderBounds); + SkRTreeFactory rtree_factory; + SkCanvas* cv = recorder.beginRecording(kRenderBounds, &rtree_factory); SkPaint p; p.setStyle(SkPaint::kFill_Style); p.setColor(SK_ColorRED); @@ -3281,7 +3570,12 @@ TEST_F(DisplayListCanvas, SaveLayerConsolidation) { std::make_shared(contract_matrix, DlImageSampling::kLinear), }; - RenderEnvironment env = RenderEnvironment::MakeN32(); + std::vector> environments; + for (auto& provider : CanvasCompareTester::kTestProviders) { + auto env = std::make_unique( + provider.get(), PixelFormat::kN32Premul_PixelFormat); + environments.push_back(std::move(env)); + } auto render_content = [](DisplayListBuilder& builder) { builder.drawRect( @@ -3298,50 +3592,57 @@ TEST_F(DisplayListCanvas, SaveLayerConsolidation) { DlPaint().setColor(DlColor::kRed().modulateOpacity(0.5f))); }; - // clang-format off - auto test_attributes = [&env, render_content] - (DlPaint& paint1, DlPaint& paint2, const DlPaint& paint_both, - bool same, bool rev_same, const std::string& desc1, - const std::string& desc2) { - // clang-format on - DisplayListBuilder nested_builder; - nested_builder.saveLayer(&kTestBounds, &paint1); - nested_builder.saveLayer(&kTestBounds, &paint2); - render_content(nested_builder); - auto nested_surface = env.MakeSurface(); - nested_builder.Build()->RenderTo(nested_surface->canvas()); - - DisplayListBuilder reverse_builder; - reverse_builder.saveLayer(&kTestBounds, &paint2); - reverse_builder.saveLayer(&kTestBounds, &paint1); - render_content(reverse_builder); - auto reverse_surface = env.MakeSurface(); - reverse_builder.Build()->RenderTo(reverse_surface->canvas()); - - DisplayListBuilder combined_builder; - combined_builder.saveLayer(&kTestBounds, &paint_both); - render_content(combined_builder); - auto combined_surface = env.MakeSurface(); - combined_builder.Build()->RenderTo(combined_surface->canvas()); - - // Set this boolean to true to test if combinations that are marked - // as incompatible actually are compatible despite our predictions. - // Some of the combinations that we treat as incompatible actually - // are compatible with swapping the order of the operations, but - // it would take a bit of new infrastructure to really identify - // those combinations. The only hard constraint to test here is - // when we claim that they are compatible and they aren't. - const bool always = false; - - if (always || same) { - CanvasCompareTester::quickCompareToReference( - nested_surface->pixmap(), combined_surface->pixmap(), same, - "nested " + desc1 + " then " + desc2); - } - if (always || rev_same) { - CanvasCompareTester::quickCompareToReference( - reverse_surface->pixmap(), combined_surface->pixmap(), rev_same, - "nested " + desc2 + " then " + desc1); + auto test_attributes_env = + [render_content](DlPaint& paint1, DlPaint& paint2, + const DlPaint& paint_both, bool same, bool rev_same, + const std::string& desc1, const std::string& desc2, + const RenderEnvironment* env) { + DisplayListBuilder nested_builder; + nested_builder.saveLayer(&kTestBounds, &paint1); + nested_builder.saveLayer(&kTestBounds, &paint2); + render_content(nested_builder); + auto nested_results = env->getResult(nested_builder.Build()); + + DisplayListBuilder reverse_builder; + reverse_builder.saveLayer(&kTestBounds, &paint2); + reverse_builder.saveLayer(&kTestBounds, &paint1); + render_content(reverse_builder); + auto reverse_results = env->getResult(reverse_builder.Build()); + + DisplayListBuilder combined_builder; + combined_builder.saveLayer(&kTestBounds, &paint_both); + render_content(combined_builder); + auto combined_results = env->getResult(combined_builder.Build()); + + // Set this boolean to true to test if combinations that are marked + // as incompatible actually are compatible despite our predictions. + // Some of the combinations that we treat as incompatible actually + // are compatible with swapping the order of the operations, but + // it would take a bit of new infrastructure to really identify + // those combinations. The only hard constraint to test here is + // when we claim that they are compatible and they aren't. + const bool always = false; + + if (always || same) { + CanvasCompareTester::quickCompareToReference( + nested_results.get(), combined_results.get(), same, + "nested " + desc1 + " then " + desc2); + } + if (always || rev_same) { + CanvasCompareTester::quickCompareToReference( + reverse_results.get(), combined_results.get(), rev_same, + "nested " + desc2 + " then " + desc1); + } + }; + + auto test_attributes = [test_attributes_env, &environments]( + DlPaint& paint1, DlPaint& paint2, + const DlPaint& paint_both, bool same, + bool rev_same, const std::string& desc1, + const std::string& desc2) { + for (auto& env : environments) { + test_attributes_env(paint1, paint2, paint_both, // + same, rev_same, desc1, desc2, env.get()); } }; diff --git a/display_list/display_list_color_filter_unittests.cc b/display_list/display_list_color_filter_unittests.cc index 3642ec8a917d0..6884c6068d052 100644 --- a/display_list/display_list_color_filter_unittests.cc +++ b/display_list/display_list_color_filter_unittests.cc @@ -2,9 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "flutter/display_list/display_list_attributes_testing.h" #include "flutter/display_list/display_list_builder.h" #include "flutter/display_list/display_list_color_filter.h" +#include "flutter/display_list/testing/dl_test_equality.h" #include "flutter/display_list/types.h" namespace flutter { diff --git a/display_list/display_list_color_source_unittests.cc b/display_list/display_list_color_source_unittests.cc index 5c9a62abd8697..8968ff503e1c0 100644 --- a/display_list/display_list_color_source_unittests.cc +++ b/display_list/display_list_color_source_unittests.cc @@ -5,12 +5,12 @@ #include #include -#include "flutter/display_list/display_list_attributes_testing.h" #include "flutter/display_list/display_list_builder.h" #include "flutter/display_list/display_list_color_source.h" #include "flutter/display_list/display_list_image.h" #include "flutter/display_list/display_list_runtime_effect.h" #include "flutter/display_list/display_list_sampling_options.h" +#include "flutter/display_list/testing/dl_test_equality.h" #include "flutter/display_list/types.h" #include "third_party/skia/include/core/SkString.h" #include "third_party/skia/include/core/SkSurface.h" diff --git a/display_list/display_list_color_unittests.cc b/display_list/display_list_color_unittests.cc index f1506051ca2d2..0d44688ac19a0 100644 --- a/display_list/display_list_color_unittests.cc +++ b/display_list/display_list_color_unittests.cc @@ -2,9 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "flutter/display_list/display_list_attributes_testing.h" #include "flutter/display_list/display_list_builder.h" #include "flutter/display_list/display_list_color_source.h" +#include "flutter/display_list/testing/dl_test_equality.h" #include "flutter/display_list/types.h" #include "third_party/skia/include/core/SkSurface.h" diff --git a/display_list/display_list_complexity_unittests.cc b/display_list/display_list_complexity_unittests.cc index d2dfbcd55161d..403ca3094ef3d 100644 --- a/display_list/display_list_complexity_unittests.cc +++ b/display_list/display_list_complexity_unittests.cc @@ -8,7 +8,7 @@ #include "flutter/display_list/display_list_complexity_gl.h" #include "flutter/display_list/display_list_complexity_metal.h" #include "flutter/display_list/display_list_sampling_options.h" -#include "flutter/display_list/display_list_test_utils.h" +#include "flutter/display_list/testing/dl_test_snippets.h" #include "flutter/testing/testing.h" #include "third_party/skia/include/core/SkColor.h" diff --git a/display_list/display_list_image_filter_unittests.cc b/display_list/display_list_image_filter_unittests.cc index 95807c061666b..6170ab2bb6d0d 100644 --- a/display_list/display_list_image_filter_unittests.cc +++ b/display_list/display_list_image_filter_unittests.cc @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "flutter/display_list/display_list_attributes_testing.h" #include "flutter/display_list/display_list_blend_mode.h" #include "flutter/display_list/display_list_builder.h" #include "flutter/display_list/display_list_color.h" @@ -11,6 +10,7 @@ #include "flutter/display_list/display_list_image_filter.h" #include "flutter/display_list/display_list_sampling_options.h" #include "flutter/display_list/display_list_tile_mode.h" +#include "flutter/display_list/testing/dl_test_equality.h" #include "flutter/display_list/types.h" #include "gtest/gtest.h" diff --git a/display_list/display_list_mask_filter_unittests.cc b/display_list/display_list_mask_filter_unittests.cc index 5a46856e22cf7..a508dd25b7873 100644 --- a/display_list/display_list_mask_filter_unittests.cc +++ b/display_list/display_list_mask_filter_unittests.cc @@ -2,10 +2,10 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "flutter/display_list/display_list_attributes_testing.h" #include "flutter/display_list/display_list_builder.h" #include "flutter/display_list/display_list_comparable.h" #include "flutter/display_list/display_list_mask_filter.h" +#include "flutter/display_list/testing/dl_test_equality.h" #include "flutter/display_list/types.h" #include "gtest/gtest.h" diff --git a/display_list/display_list_path_effect_unittests.cc b/display_list/display_list_path_effect_unittests.cc index 9a89e2229a823..f9bdc0dc8c641 100644 --- a/display_list/display_list_path_effect_unittests.cc +++ b/display_list/display_list_path_effect_unittests.cc @@ -2,9 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "flutter/display_list/display_list_attributes_testing.h" #include "flutter/display_list/display_list_builder.h" #include "flutter/display_list/display_list_comparable.h" +#include "flutter/display_list/testing/dl_test_equality.h" #include "flutter/display_list/types.h" #include "gtest/gtest.h" #include "include/core/SkPath.h" diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index 91ec814c53f7f..63e34f235db16 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -14,8 +14,8 @@ #include "flutter/display_list/display_list_canvas_recorder.h" #include "flutter/display_list/display_list_paint.h" #include "flutter/display_list/display_list_rtree.h" -#include "flutter/display_list/display_list_test_utils.h" #include "flutter/display_list/display_list_utils.h" +#include "flutter/display_list/testing/dl_test_snippets.h" #include "flutter/fml/logging.h" #include "flutter/fml/math.h" #include "flutter/testing/display_list_testing.h" diff --git a/display_list/display_list_vertices_unittests.cc b/display_list/display_list_vertices_unittests.cc index 04d9549b0aaae..0323b621a53ac 100644 --- a/display_list/display_list_vertices_unittests.cc +++ b/display_list/display_list_vertices_unittests.cc @@ -2,10 +2,10 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "flutter/display_list/display_list_attributes_testing.h" #include "flutter/display_list/display_list_builder.h" #include "flutter/display_list/display_list_comparable.h" #include "flutter/display_list/display_list_vertices.h" +#include "flutter/display_list/testing/dl_test_equality.h" #include "flutter/display_list/types.h" #include "gtest/gtest.h" diff --git a/display_list/testing/BUILD.gn b/display_list/testing/BUILD.gn new file mode 100644 index 0000000000000..5cbadb96c50e9 --- /dev/null +++ b/display_list/testing/BUILD.gn @@ -0,0 +1,97 @@ +# 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. +# We only do software benchmarks on non-mobile platforms + +import("//flutter/impeller/tools/impeller.gni") + +source_set("display_list_testing") { + testonly = true + + sources = [ + "dl_test_equality.h", + "dl_test_snippets.cc", + "dl_test_snippets.h", + ] + + deps = [ + "//flutter/testing:testing_lib", + "//third_party/skia", + ] + + public_deps = [ "//flutter/display_list:display_list" ] +} + +surface_provider_include_software = !is_android && !is_ios + +# iOS and Fuchsia don't support OpenGL +surface_provider_include_gl = !is_fuchsia && !is_ios && !impeller_enable_vulkan + +# TODO (https://github.com/flutter/flutter/issues/107357): +# impeller_enable_vulkan currently requires skia to not use VMA, which inturn +# causes linkage problems with swiftshader. + +surface_provider_include_metal = is_mac || is_ios + +config("surface_provider_config") { + defines = [] + + if (surface_provider_include_software) { + defines += [ "ENABLE_SOFTWARE_BENCHMARKS" ] + } + if (surface_provider_include_gl) { + defines += [ "ENABLE_OPENGL_BENCHMARKS" ] + } + if (surface_provider_include_metal) { + defines += [ "ENABLE_METAL_BENCHMARKS" ] + } + + # Don't snapshot test results on mobile platforms + if (is_android || is_ios) { + defines += [ "BENCHMARKS_NO_SNAPSHOT" ] + } +} + +source_set("display_list_surface_provider") { + testonly = true + + sources = [ "dl_test_surface_provider.cc" ] + + deps = [ + "//flutter/common/graphics", + "//flutter/testing:testing_lib", + ] + + public_configs = [ ":surface_provider_config" ] + + if (is_android) { + libs = [ + "android", + "EGL", + "GLESv2", + ] + } + + if (surface_provider_include_software) { + sources += [ + "dl_test_surface_software.cc", + "dl_test_surface_software.h", + ] + } + + if (surface_provider_include_gl) { + sources += [ + "dl_test_surface_gl.cc", + "dl_test_surface_gl.h", + ] + deps += [ "//flutter/testing:opengl" ] + } + + if (surface_provider_include_metal) { + sources += [ + "dl_test_surface_metal.cc", + "dl_test_surface_metal.h", + ] + deps += [ "//flutter/testing:metal" ] + } +} diff --git a/display_list/display_list_attributes_testing.h b/display_list/testing/dl_test_equality.h similarity index 86% rename from display_list/display_list_attributes_testing.h rename to display_list/testing/dl_test_equality.h index 37ef0154746c3..8e511e5523142 100644 --- a/display_list/display_list_attributes_testing.h +++ b/display_list/testing/dl_test_equality.h @@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef FLUTTER_DISPLAY_LIST_DISPLAY_LIST_ATTRIBUTES_TESTING_H_ -#define FLUTTER_DISPLAY_LIST_DISPLAY_LIST_ATTRIBUTES_TESTING_H_ +#ifndef FLUTTER_DISPLAY_LIST_TESTING_DL_TEST_EQUALITY_H_ +#define FLUTTER_DISPLAY_LIST_TESTING_DL_TEST_EQUALITY_H_ #include "flutter/display_list/display_list_attributes.h" #include "flutter/display_list/display_list_comparable.h" @@ -39,4 +39,4 @@ static void TestNotEquals(T& source1, T& source2, std::string label) { } // namespace testing } // namespace flutter -#endif // FLUTTER_DISPLAY_LIST_DISPLAY_LIST_ATTRIBUTES_TESTING_H_ +#endif // FLUTTER_DISPLAY_LIST_TESTING_DL_TEST_EQUALITY_H_ diff --git a/display_list/display_list_test_utils.cc b/display_list/testing/dl_test_snippets.cc similarity index 99% rename from display_list/display_list_test_utils.cc rename to display_list/testing/dl_test_snippets.cc index 9d8ee65bb9c03..1ba810f6d5880 100644 --- a/display_list/display_list_test_utils.cc +++ b/display_list/testing/dl_test_snippets.cc @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "flutter/display_list/display_list_test_utils.h" +#include "flutter/display_list/testing/dl_test_snippets.h" #include "flutter/display_list/display_list_builder.h" namespace flutter { diff --git a/display_list/display_list_test_utils.h b/display_list/testing/dl_test_snippets.h similarity index 98% rename from display_list/display_list_test_utils.h rename to display_list/testing/dl_test_snippets.h index 711b7182a5815..07396f5a3f069 100644 --- a/display_list/display_list_test_utils.h +++ b/display_list/testing/dl_test_snippets.h @@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef FLUTTER_FLOW_DISPLAY_LIST_COMPLEXITY_UNITTESTS_H_ -#define FLUTTER_FLOW_DISPLAY_LIST_COMPLEXITY_UNITTESTS_H_ +#ifndef FLUTTER_DISPLAY_LIST_TESTING_DL_TEST_SNIPPETS_H_ +#define FLUTTER_DISPLAY_LIST_TESTING_DL_TEST_SNIPPETS_H_ #include "flutter/display_list/display_list.h" #include "flutter/display_list/display_list_builder.h" @@ -333,4 +333,4 @@ std::vector CreateAllGroups(); } // namespace testing } // namespace flutter -#endif // FLUTTER_FLOW_DISPLAY_LIST_COMPLEXITY_UNITTESTS_H_ +#endif // FLUTTER_DISPLAY_LIST_TESTING_DL_TEST_SNIPPETS_H_ diff --git a/display_list/testing/dl_test_surface_gl.cc b/display_list/testing/dl_test_surface_gl.cc new file mode 100644 index 0000000000000..ce9d627b36f2a --- /dev/null +++ b/display_list/testing/dl_test_surface_gl.cc @@ -0,0 +1,47 @@ +// 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/display_list/testing/dl_test_surface_gl.h" + +#include "third_party/skia/include/core/SkCanvas.h" +#include "third_party/skia/include/core/SkSurface.h" + +namespace flutter { +namespace testing { + +using PixelFormat = DlSurfaceProvider::PixelFormat; + +bool DlOpenGLSurfaceProvider::InitializeSurface(size_t width, + size_t height, + PixelFormat format) { + gl_surface_ = std::make_unique(SkISize::Make(width, height)); + gl_surface_->MakeCurrent(); + + primary_ = MakeOffscreenSurface(width, height, format); + return true; +} + +std::shared_ptr DlOpenGLSurfaceProvider::GetPrimarySurface() + const { + if (!gl_surface_->MakeCurrent()) { + return nullptr; + } + return primary_; +} + +std::shared_ptr +DlOpenGLSurfaceProvider::MakeOffscreenSurface(size_t width, + size_t height, + PixelFormat format) const { + auto offscreen_surface = SkSurface::MakeRenderTarget( + (GrRecordingContext*)gl_surface_->GetGrContext().get(), + skgpu::Budgeted::kNo, MakeInfo(format, width, height), 1, + kTopLeft_GrSurfaceOrigin, nullptr, false); + + offscreen_surface->getCanvas()->clear(SK_ColorTRANSPARENT); + return std::make_shared(offscreen_surface); +} + +} // namespace testing +} // namespace flutter diff --git a/display_list/testing/dl_test_surface_gl.h b/display_list/testing/dl_test_surface_gl.h new file mode 100644 index 0000000000000..882e0fd6d243b --- /dev/null +++ b/display_list/testing/dl_test_surface_gl.h @@ -0,0 +1,42 @@ +// 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_DISPLAY_LIST_TESTING_DL_TEST_SURFACE_GL_H_ +#define FLUTTER_DISPLAY_LIST_TESTING_DL_TEST_SURFACE_GL_H_ + +#include "flutter/display_list/testing/dl_test_surface_provider.h" + +#include "flutter/testing/test_gl_surface.h" + +namespace flutter { +namespace testing { + +class DlOpenGLSurfaceProvider : public DlSurfaceProvider { + public: + DlOpenGLSurfaceProvider() : DlSurfaceProvider() {} + virtual ~DlOpenGLSurfaceProvider() = default; + + bool InitializeSurface(size_t width, + size_t height, + PixelFormat format) override; + std::shared_ptr GetPrimarySurface() const override; + std::shared_ptr MakeOffscreenSurface( + size_t width, + size_t height, + PixelFormat format) const override; + const std::string backend_name() const override { return "OpenGL"; } + BackendType backend_type() const override { return kOpenGL_Backend; } + bool supports(PixelFormat format) const override { + return format == kN32Premul_PixelFormat; + } + + private: + std::shared_ptr primary_; + std::unique_ptr gl_surface_; +}; + +} // namespace testing +} // namespace flutter + +#endif // FLUTTER_DISPLAY_LIST_TESTING_DL_TEST_SURFACE_GL_H_ diff --git a/display_list/testing/dl_test_surface_metal.cc b/display_list/testing/dl_test_surface_metal.cc new file mode 100644 index 0000000000000..7c3617e8d63ca --- /dev/null +++ b/display_list/testing/dl_test_surface_metal.cc @@ -0,0 +1,58 @@ +// 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/display_list/testing/dl_test_surface_metal.h" + +#include "third_party/skia/include/core/SkCanvas.h" +#include "third_party/skia/include/core/SkSurface.h" + +namespace flutter { +namespace testing { + +class DlMetalSurfaceInstance : public DlSurfaceInstance { + public: + explicit DlMetalSurfaceInstance( + std::unique_ptr metal_surface) + : metal_surface_(std::move(metal_surface)) {} + ~DlMetalSurfaceInstance() = default; + + sk_sp sk_surface() const override { + return metal_surface_->GetSurface(); + } + + private: + std::unique_ptr metal_surface_; +}; + +bool DlMetalSurfaceProvider::InitializeSurface(size_t width, + size_t height, + PixelFormat format) { + if (format != kN32Premul_PixelFormat) { + return false; + } + metal_context_ = std::make_unique(); + metal_surface_ = MakeOffscreenSurface(width, height, format); + return true; +} + +std::shared_ptr DlMetalSurfaceProvider::GetPrimarySurface() + const { + if (!metal_surface_) { + return nullptr; + } + return metal_surface_; +} + +std::shared_ptr DlMetalSurfaceProvider::MakeOffscreenSurface( + size_t width, + size_t height, + PixelFormat format) const { + auto surface = + TestMetalSurface::Create(*metal_context_, SkISize::Make(width, height)); + surface->GetSurface()->getCanvas()->clear(SK_ColorTRANSPARENT); + return std::make_shared(std::move(surface)); +} + +} // namespace testing +} // namespace flutter diff --git a/display_list/testing/dl_test_surface_metal.h b/display_list/testing/dl_test_surface_metal.h new file mode 100644 index 0000000000000..c660c6dc95454 --- /dev/null +++ b/display_list/testing/dl_test_surface_metal.h @@ -0,0 +1,42 @@ +// 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_DISPLAY_LIST_TESTING_DL_TEST_SURFACE_METAL_H_ +#define FLUTTER_DISPLAY_LIST_TESTING_DL_TEST_SURFACE_METAL_H_ + +#include "flutter/display_list/testing/dl_test_surface_provider.h" + +#include "flutter/testing/test_metal_surface.h" + +namespace flutter { +namespace testing { + +class DlMetalSurfaceProvider : public DlSurfaceProvider { + public: + explicit DlMetalSurfaceProvider() : DlSurfaceProvider() {} + virtual ~DlMetalSurfaceProvider() = default; + + bool InitializeSurface(size_t width, + size_t height, + PixelFormat format) override; + std::shared_ptr GetPrimarySurface() const override; + std::shared_ptr MakeOffscreenSurface( + size_t width, + size_t height, + PixelFormat format) const override; + const std::string backend_name() const override { return "Metal"; } + BackendType backend_type() const override { return kMetal_Backend; } + bool supports(PixelFormat format) const override { + return format == kN32Premul_PixelFormat; + } + + private: + std::unique_ptr metal_context_; + std::shared_ptr metal_surface_; +}; + +} // namespace testing +} // namespace flutter + +#endif // FLUTTER_DISPLAY_LIST_TESTING_DL_TEST_SURFACE_METAL_H_ diff --git a/display_list/testing/dl_test_surface_provider.cc b/display_list/testing/dl_test_surface_provider.cc new file mode 100644 index 0000000000000..f8bb06a9635b2 --- /dev/null +++ b/display_list/testing/dl_test_surface_provider.cc @@ -0,0 +1,63 @@ +// 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/display_list/testing/dl_test_surface_provider.h" + +#include "third_party/skia/include/core/SkCanvas.h" +#include "third_party/skia/include/core/SkData.h" +#include "third_party/skia/include/core/SkSurface.h" + +#include "flutter/display_list/testing/dl_test_surface_gl.h" +#include "flutter/display_list/testing/dl_test_surface_metal.h" +#include "flutter/display_list/testing/dl_test_surface_software.h" + +namespace flutter { +namespace testing { + +std::unique_ptr DlSurfaceProvider::Create( + BackendType backend_type) { + switch (backend_type) { +#ifdef ENABLE_SOFTWARE_BENCHMARKS + case kSoftware_Backend: + return std::make_unique(); +#endif +#ifdef ENABLE_OPENGL_BENCHMARKS + case kOpenGL_Backend: + return std::make_unique(); +#endif +#ifdef ENABLE_METAL_BENCHMARKS + case kMetal_Backend: + return std::make_unique(); +#endif + default: + return nullptr; + } + + return nullptr; +} + +bool DlSurfaceProvider::Snapshot(std::string& filename) const { +#ifdef BENCHMARKS_NO_SNAPSHOT + return false; +#else + auto image = GetPrimarySurface()->sk_surface()->makeImageSnapshot(); + if (!image) { + return false; + } + auto raster = image->makeRasterImage(); + if (!raster) { + return false; + } + auto data = raster->encodeToData(); + if (!data) { + return false; + } + fml::NonOwnedMapping mapping(static_cast(data->data()), + data->size()); + return WriteAtomically(OpenFixturesDirectory(), filename.c_str(), mapping); +#endif +} + +} // namespace testing +} // namespace flutter diff --git a/display_list/testing/dl_test_surface_provider.h b/display_list/testing/dl_test_surface_provider.h new file mode 100644 index 0000000000000..d3944aba21ae7 --- /dev/null +++ b/display_list/testing/dl_test_surface_provider.h @@ -0,0 +1,82 @@ +// 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_DISPLAY_LIST_TESTING_DL_TEST_SURFACE_PROVIDER_H_ +#define FLUTTER_DISPLAY_LIST_TESTING_DL_TEST_SURFACE_PROVIDER_H_ + +#include "flutter/fml/mapping.h" +#include "flutter/testing/testing.h" + +#include "third_party/skia/include/core/SkSurface.h" + +namespace flutter { +namespace testing { + +class DlSurfaceInstance { + public: + virtual ~DlSurfaceInstance() = default; + + virtual sk_sp sk_surface() const = 0; + + int width() const { return sk_surface()->width(); } + int height() const { return sk_surface()->height(); } +}; + +class DlSurfaceInstanceBase : public DlSurfaceInstance { + public: + DlSurfaceInstanceBase(sk_sp surface) : surface_(surface) {} + ~DlSurfaceInstanceBase() = default; + + sk_sp sk_surface() const override { return surface_; } + + private: + sk_sp surface_; +}; + +class DlSurfaceProvider { + public: + typedef enum { kN32Premul_PixelFormat, k565_PixelFormat } PixelFormat; + typedef enum { + kSoftware_Backend, + kOpenGL_Backend, + kMetal_Backend + } BackendType; + + static SkImageInfo MakeInfo(PixelFormat format, int w, int h) { + switch (format) { + case kN32Premul_PixelFormat: + return SkImageInfo::MakeN32Premul(w, h); + case k565_PixelFormat: + return SkImageInfo::Make(SkISize::Make(w, h), kRGB_565_SkColorType, + kOpaque_SkAlphaType); + } + FML_DCHECK(false); + } + + static std::unique_ptr Create(BackendType backend_type); + + virtual ~DlSurfaceProvider() = default; + virtual const std::string backend_name() const = 0; + virtual BackendType backend_type() const = 0; + virtual bool supports(PixelFormat format) const = 0; + virtual bool InitializeSurface( + size_t width, + size_t height, + PixelFormat format = kN32Premul_PixelFormat) = 0; + virtual std::shared_ptr GetPrimarySurface() const = 0; + virtual std::shared_ptr MakeOffscreenSurface( + size_t width, + size_t height, + PixelFormat format = kN32Premul_PixelFormat) const = 0; + + virtual bool Snapshot(std::string& filename) const; + + protected: + DlSurfaceProvider() = default; +}; + +} // namespace testing +} // namespace flutter + +#endif // FLUTTER_DISPLAY_LIST_TESTING_DL_TEST_SURFACE_PROVIDER_H_ diff --git a/display_list/testing/dl_test_surface_software.cc b/display_list/testing/dl_test_surface_software.cc new file mode 100644 index 0000000000000..ca551c18b50f9 --- /dev/null +++ b/display_list/testing/dl_test_surface_software.cc @@ -0,0 +1,32 @@ +// 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/display_list/testing/dl_test_surface_software.h" + +#include "third_party/skia/include/core/SkCanvas.h" +#include "third_party/skia/include/core/SkSurface.h" + +namespace flutter { +namespace testing { + +using PixelFormat = DlSurfaceProvider::PixelFormat; + +bool DlSoftwareSurfaceProvider::InitializeSurface(size_t width, + size_t height, + PixelFormat format) { + primary_ = MakeOffscreenSurface(width, height, format); + return true; +} + +std::shared_ptr +DlSoftwareSurfaceProvider::MakeOffscreenSurface(size_t width, + size_t height, + PixelFormat format) const { + auto surface = SkSurface::MakeRaster(MakeInfo(format, width, height)); + surface->getCanvas()->clear(SK_ColorTRANSPARENT); + return std::make_shared(surface); +} + +} // namespace testing +} // namespace flutter diff --git a/display_list/testing/dl_test_surface_software.h b/display_list/testing/dl_test_surface_software.h new file mode 100644 index 0000000000000..68eae12f74e74 --- /dev/null +++ b/display_list/testing/dl_test_surface_software.h @@ -0,0 +1,39 @@ +// 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_DISPLAY_LIST_TESTING_DL_TEST_SURFACE_SOFTWARE_H_ +#define FLUTTER_DISPLAY_LIST_TESTING_DL_TEST_SURFACE_SOFTWARE_H_ + +#include "flutter/display_list/testing/dl_test_surface_provider.h" + +namespace flutter { +namespace testing { + +class DlSoftwareSurfaceProvider : public DlSurfaceProvider { + public: + DlSoftwareSurfaceProvider() : DlSurfaceProvider() {} + virtual ~DlSoftwareSurfaceProvider() = default; + + bool InitializeSurface(size_t width, + size_t height, + PixelFormat format) override; + std::shared_ptr GetPrimarySurface() const override { + return primary_; + } + std::shared_ptr MakeOffscreenSurface( + size_t width, + size_t height, + PixelFormat format) const override; + const std::string backend_name() const override { return "Software"; } + BackendType backend_type() const override { return kSoftware_Backend; } + bool supports(PixelFormat format) const override { return true; } + + private: + std::shared_ptr primary_; +}; + +} // namespace testing +} // namespace flutter + +#endif // FLUTTER_DISPLAY_LIST_TESTING_DL_TEST_SURFACE_SOFTWARE_H_ diff --git a/flow/BUILD.gn b/flow/BUILD.gn index 09ed594157cab..c3261cbe9fb42 100644 --- a/flow/BUILD.gn +++ b/flow/BUILD.gn @@ -183,7 +183,7 @@ if (enable_unittests) { ":flow_fixtures", ":flow_testing", "//flutter/common/graphics", - "//flutter/display_list:display_list_testing", + "//flutter/display_list/testing:display_list_testing", "//flutter/fml", "//flutter/testing:skia", "//flutter/testing:testing_lib", diff --git a/flow/raster_cache_unittests.cc b/flow/raster_cache_unittests.cc index 90d31ef08fd15..d3bb3cbb93c07 100644 --- a/flow/raster_cache_unittests.cc +++ b/flow/raster_cache_unittests.cc @@ -4,7 +4,7 @@ #include "flutter/display_list/display_list.h" #include "flutter/display_list/display_list_builder.h" -#include "flutter/display_list/display_list_test_utils.h" +#include "flutter/display_list/testing/dl_test_snippets.h" #include "flutter/flow/layers/container_layer.h" #include "flutter/flow/layers/display_list_layer.h" #include "flutter/flow/layers/image_filter_layer.h" From fb4074cdf4309265fd9edad0a21f57a755b18100 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Wed, 18 Jan 2023 22:49:09 -0800 Subject: [PATCH 02/11] empty commit to trigger build From 0ed3cf82399235ccfe9225272e942ccc4ced201a Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Wed, 18 Jan 2023 23:41:43 -0800 Subject: [PATCH 03/11] conditionally include platform-specific surface provider headers --- display_list/testing/dl_test_surface_provider.cc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/display_list/testing/dl_test_surface_provider.cc b/display_list/testing/dl_test_surface_provider.cc index f8bb06a9635b2..99ac2f7720933 100644 --- a/display_list/testing/dl_test_surface_provider.cc +++ b/display_list/testing/dl_test_surface_provider.cc @@ -8,9 +8,15 @@ #include "third_party/skia/include/core/SkData.h" #include "third_party/skia/include/core/SkSurface.h" +#ifdef ENABLE_SOFTWARE_BENCHMARKS +#include "flutter/display_list/testing/dl_test_surface_software.h" +#endif +#ifdef ENABLE_OPENGL_BENCHMARKS #include "flutter/display_list/testing/dl_test_surface_gl.h" +#endif +#ifdef ENABLE_METAL_BENCHMARKS #include "flutter/display_list/testing/dl_test_surface_metal.h" -#include "flutter/display_list/testing/dl_test_surface_software.h" +#endif namespace flutter { namespace testing { From 8f69582a230fb929cf22dfb63eac3582d06bd107 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Thu, 19 Jan 2023 00:09:51 -0800 Subject: [PATCH 04/11] ignore DL test files during license checks --- ci/licenses_golden/excluded_files | 1 + ci/licenses_golden/licenses_flutter | 22 ---------------------- 2 files changed, 1 insertion(+), 22 deletions(-) diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files index d800da415d156..7d6275ae3408a 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -43,6 +43,7 @@ ../../../flutter/display_list/display_list_unittests.cc ../../../flutter/display_list/display_list_utils_unittests.cc ../../../flutter/display_list/display_list_vertices_unittests.cc +../../../flutter/display_list/testing ../../../flutter/docs ../../../flutter/examples ../../../flutter/flow/README.md diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 9c89a41e2a2f2..d2468dfc5c775 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -878,17 +878,6 @@ ORIGIN: ../../../flutter/display_list/display_list_utils.cc + ../../../flutter/L ORIGIN: ../../../flutter/display_list/display_list_utils.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/display_list_vertices.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/display_list_vertices.h + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/display_list/testing/dl_test_equality.h + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/display_list/testing/dl_test_snippets.cc + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/display_list/testing/dl_test_snippets.h + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/display_list/testing/dl_test_surface_gl.cc + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/display_list/testing/dl_test_surface_gl.h + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/display_list/testing/dl_test_surface_metal.cc + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/display_list/testing/dl_test_surface_metal.h + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/display_list/testing/dl_test_surface_provider.cc + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/display_list/testing/dl_test_surface_provider.h + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/display_list/testing/dl_test_surface_software.cc + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/display_list/testing/dl_test_surface_software.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/types.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/flow/compositor_context.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/flow/compositor_context.h + ../../../flutter/LICENSE @@ -3366,17 +3355,6 @@ FILE: ../../../flutter/display_list/display_list_utils.cc FILE: ../../../flutter/display_list/display_list_utils.h FILE: ../../../flutter/display_list/display_list_vertices.cc FILE: ../../../flutter/display_list/display_list_vertices.h -FILE: ../../../flutter/display_list/testing/dl_equality.h -FILE: ../../../flutter/display_list/testing/dl_test_snippets.cc -FILE: ../../../flutter/display_list/testing/dl_test_snippets.h -FILE: ../../../flutter/display_list/testing/dl_test_surface_gl.cc -FILE: ../../../flutter/display_list/testing/dl_test_surface_gl.h -FILE: ../../../flutter/display_list/testing/dl_test_surface_metal.cc -FILE: ../../../flutter/display_list/testing/dl_test_surface_metal.h -FILE: ../../../flutter/display_list/testing/dl_test_surface_provider.cc -FILE: ../../../flutter/display_list/testing/dl_test_surface_provider.h -FILE: ../../../flutter/display_list/testing/dl_test_surface_software.cc -FILE: ../../../flutter/display_list/testing/dl_test_surface_software.h FILE: ../../../flutter/display_list/types.h FILE: ../../../flutter/flow/compositor_context.cc FILE: ../../../flutter/flow/compositor_context.h From 3fbd906a5532f97c2426a0082f96d8aec1661d8a Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Thu, 19 Jan 2023 14:50:02 -0800 Subject: [PATCH 05/11] typo --- display_list/testing/BUILD.gn | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/display_list/testing/BUILD.gn b/display_list/testing/BUILD.gn index 5cbadb96c50e9..a1c17ce9f9d09 100644 --- a/display_list/testing/BUILD.gn +++ b/display_list/testing/BUILD.gn @@ -28,7 +28,7 @@ surface_provider_include_software = !is_android && !is_ios surface_provider_include_gl = !is_fuchsia && !is_ios && !impeller_enable_vulkan # TODO (https://github.com/flutter/flutter/issues/107357): -# impeller_enable_vulkan currently requires skia to not use VMA, which inturn +# impeller_enable_vulkan currently requires skia to not use VMA, which in turn # causes linkage problems with swiftshader. surface_provider_include_metal = is_mac || is_ios From fc540ccac05c2b7fa823e4b7311033fd7df1d34e Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Fri, 20 Jan 2023 15:36:52 -0800 Subject: [PATCH 06/11] add dependencies to rendertests to hopefully build on Windows --- display_list/BUILD.gn | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/display_list/BUILD.gn b/display_list/BUILD.gn index 160647a8612ba..af77d257d14e9 100644 --- a/display_list/BUILD.gn +++ b/display_list/BUILD.gn @@ -136,9 +136,14 @@ if (enable_unittests) { deps = [ ":display_list", ":display_list_fixtures", + "//flutter/benchmarking", + "//flutter/common/graphics", "//flutter/display_list/testing:display_list_surface_provider", "//flutter/display_list/testing:display_list_testing", - "//flutter/testing", + "//flutter/fml", + "//flutter/testing:skia", + "//flutter/testing:testing_lib", + "//third_party/skia", ] if (!defined(defines)) { From fbc7d1a03fc11bf5b43870ff311935c56ba065ef Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Fri, 20 Jan 2023 16:19:12 -0800 Subject: [PATCH 07/11] remove benchmarking deps from dl_rendertests --- display_list/BUILD.gn | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/display_list/BUILD.gn b/display_list/BUILD.gn index af77d257d14e9..5e7a0289ef85e 100644 --- a/display_list/BUILD.gn +++ b/display_list/BUILD.gn @@ -136,13 +136,12 @@ if (enable_unittests) { deps = [ ":display_list", ":display_list_fixtures", - "//flutter/benchmarking", "//flutter/common/graphics", "//flutter/display_list/testing:display_list_surface_provider", "//flutter/display_list/testing:display_list_testing", "//flutter/fml", + "//flutter/testing", "//flutter/testing:skia", - "//flutter/testing:testing_lib", "//third_party/skia", ] From 645df47d610acc481c577cf862b085dcbdcfecb0 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Tue, 24 Jan 2023 15:55:56 -0800 Subject: [PATCH 08/11] more changes to get Windows rendertests to link --- display_list/BUILD.gn | 1 + display_list/display_list_benchmarks.cc | 1 - display_list/testing/BUILD.gn | 5 ++++- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/display_list/BUILD.gn b/display_list/BUILD.gn index 5e7a0289ef85e..39a3fb11cadc6 100644 --- a/display_list/BUILD.gn +++ b/display_list/BUILD.gn @@ -142,6 +142,7 @@ if (enable_unittests) { "//flutter/fml", "//flutter/testing", "//flutter/testing:skia", + "//third_party/benchmark", "//third_party/skia", ] diff --git a/display_list/display_list_benchmarks.cc b/display_list/display_list_benchmarks.cc index 356b3c7a7343d..0ade02a2dfa87 100644 --- a/display_list/display_list_benchmarks.cc +++ b/display_list/display_list_benchmarks.cc @@ -5,7 +5,6 @@ #include "flutter/display_list/display_list_benchmarks.h" #include "flutter/display_list/display_list_builder.h" #include "flutter/display_list/display_list_flags.h" -#include "flutter/testing/assertions_skia.h" #include "third_party/skia/include/core/SkPoint.h" #include "third_party/skia/include/core/SkSurface.h" diff --git a/display_list/testing/BUILD.gn b/display_list/testing/BUILD.gn index a1c17ce9f9d09..327631f1cf677 100644 --- a/display_list/testing/BUILD.gn +++ b/display_list/testing/BUILD.gn @@ -25,11 +25,14 @@ source_set("display_list_testing") { surface_provider_include_software = !is_android && !is_ios # iOS and Fuchsia don't support OpenGL -surface_provider_include_gl = !is_fuchsia && !is_ios && !impeller_enable_vulkan +surface_provider_include_gl = !is_fuchsia && !is_ios # TODO (https://github.com/flutter/flutter/issues/107357): # impeller_enable_vulkan currently requires skia to not use VMA, which in turn # causes linkage problems with swiftshader. +if (impeller_enable_vulkan) { + surface_provider_include_gl = false +} surface_provider_include_metal = is_mac || is_ios From 1adf0ab47c79c20858d97ab38c9400f13ab73eeb Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Thu, 26 Jan 2023 12:43:28 -0800 Subject: [PATCH 09/11] add --enable-gl synonym and prevent non-SW surface provider dest on Windows --- display_list/BUILD.gn | 5 ----- display_list/display_list_canvas_unittests.cc | 2 +- display_list/testing/BUILD.gn | 6 +++++- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/display_list/BUILD.gn b/display_list/BUILD.gn index 39a3fb11cadc6..160647a8612ba 100644 --- a/display_list/BUILD.gn +++ b/display_list/BUILD.gn @@ -136,14 +136,9 @@ if (enable_unittests) { deps = [ ":display_list", ":display_list_fixtures", - "//flutter/common/graphics", "//flutter/display_list/testing:display_list_surface_provider", "//flutter/display_list/testing:display_list_testing", - "//flutter/fml", "//flutter/testing", - "//flutter/testing:skia", - "//third_party/benchmark", - "//third_party/skia", ] if (!defined(defines)) { diff --git a/display_list/display_list_canvas_unittests.cc b/display_list/display_list_canvas_unittests.cc index eaeff1076ffbe..555b58314361e 100644 --- a/display_list/display_list_canvas_unittests.cc +++ b/display_list/display_list_canvas_unittests.cc @@ -2442,7 +2442,7 @@ class DisplayListCanvasTestBase : public BaseT, protected DisplayListOpFlags { } if (arg == "--enable-software") { do_software = enable; - } else if (arg == "--enable-opengl") { + } else if (arg == "--enable-opengl" || arg == "--enable-gl") { do_opengl = enable; } else if (arg == "--enable-metal") { do_metal = enable; diff --git a/display_list/testing/BUILD.gn b/display_list/testing/BUILD.gn index 327631f1cf677..fd6c852319199 100644 --- a/display_list/testing/BUILD.gn +++ b/display_list/testing/BUILD.gn @@ -25,7 +25,11 @@ source_set("display_list_testing") { surface_provider_include_software = !is_android && !is_ios # iOS and Fuchsia don't support OpenGL -surface_provider_include_gl = !is_fuchsia && !is_ios +surface_provider_include_gl = !is_fuchsia && !is_ios && !is_win +# Note: Windows would need some work to support OpenGL +# benchmarks do not run on Windows and rendertests only runs +# on SW by default so this restriction currently only limits +# the ability to cross-check OpenGL on Windows for rendertests # TODO (https://github.com/flutter/flutter/issues/107357): # impeller_enable_vulkan currently requires skia to not use VMA, which in turn From 3e08124179ada34076168ed54c39bb082900052a Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Thu, 26 Jan 2023 12:48:22 -0800 Subject: [PATCH 10/11] fix gn formatting --- display_list/testing/BUILD.gn | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/display_list/testing/BUILD.gn b/display_list/testing/BUILD.gn index fd6c852319199..fabb2900683a6 100644 --- a/display_list/testing/BUILD.gn +++ b/display_list/testing/BUILD.gn @@ -24,12 +24,12 @@ source_set("display_list_testing") { surface_provider_include_software = !is_android && !is_ios -# iOS and Fuchsia don't support OpenGL -surface_provider_include_gl = !is_fuchsia && !is_ios && !is_win +# iOS and Fuchsia and Windows don't support OpenGL # Note: Windows would need some work to support OpenGL -# benchmarks do not run on Windows and rendertests only runs -# on SW by default so this restriction currently only limits -# the ability to cross-check OpenGL on Windows for rendertests +# But, since benchmarks do not run on Windows and rendertests only +# runs on SW by default, this restriction currently only limits the +# ability to manually cross-check OpenGL on Windows for rendertests +surface_provider_include_gl = !is_fuchsia && !is_ios && !is_win # TODO (https://github.com/flutter/flutter/issues/107357): # impeller_enable_vulkan currently requires skia to not use VMA, which in turn From f342e3d77a7c70ea924b979674e01c5f2095a2ee Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Fri, 10 Feb 2023 00:10:37 -0800 Subject: [PATCH 11/11] review feedback --- display_list/display_list_canvas_unittests.cc | 283 +++++++++++------- .../testing/dl_test_surface_software.cc | 2 +- .../testing/dl_test_surface_software.h | 2 +- 3 files changed, 171 insertions(+), 116 deletions(-) diff --git a/display_list/display_list_canvas_unittests.cc b/display_list/display_list_canvas_unittests.cc index 555b58314361e..29401b98896d9 100644 --- a/display_list/display_list_canvas_unittests.cc +++ b/display_list/display_list_canvas_unittests.cc @@ -280,69 +280,138 @@ class RenderResult { SkIRect clip_bounds_; }; -#define RENDER_JOB_BASE \ - int width = kTestWidth; \ - int height = kTestHeight; \ - DlColor bg = DlColor::kTransparent(); \ - SkScalar scale = SK_Scalar1; \ - SkMatrix setup_matrix; \ - SkIRect setup_clip_bounds - -struct CvRenderJob { - RENDER_JOB_BASE; - CvSetup& cv_setup = kEmptyCvSetup; - CvRenderer& cv_render = kEmptyCvRenderer; - CvRenderer& cv_restore = kEmptyCvRenderer; - sk_sp picture; - SkPaint setup_paint; - - void render(SkCanvas* canvas) { - if (picture) { - picture->playback(canvas); - } else { - SkPaint paint; - cv_setup(canvas, paint); - setup_paint = paint; - setup_matrix = canvas->getTotalMatrix(); - setup_clip_bounds = canvas->getDeviceClipBounds(); - cv_render(canvas, paint); - cv_restore(canvas, paint); +struct RenderJobInfo { + int width = kTestWidth; + int height = kTestHeight; + DlColor bg = DlColor::kTransparent(); + SkScalar scale = SK_Scalar1; + SkScalar opacity = SK_Scalar1; +}; + +struct JobRenderer { + virtual void Render(SkCanvas* canvas, + const RenderJobInfo& info, + SkMatrix* setup_matrix = nullptr, + SkIRect* setup_clip_bounds = nullptr) = 0; +}; + +struct CvJobRenderer : public JobRenderer { + explicit CvJobRenderer(CvSetup& cv_setup = kEmptyCvSetup, + CvRenderer& cv_render = kEmptyCvRenderer, + CvRenderer& cv_restore = kEmptyCvRenderer) + : cv_setup_(cv_setup), cv_render_(cv_render), cv_restore_(cv_restore) {} + + void Render(SkCanvas* canvas, + const RenderJobInfo& info, + SkMatrix* setup_matrix = nullptr, + SkIRect* setup_clip_bounds = nullptr) override { + FML_DCHECK(info.opacity == SK_Scalar1); + SkPaint paint; + cv_setup_(canvas, paint); + setup_paint_ = paint; + paint_is_setup_ = true; + if (setup_matrix) { + *setup_matrix = canvas->getTotalMatrix(); + } + if (setup_clip_bounds) { + *setup_clip_bounds = canvas->getDeviceClipBounds(); } + cv_render_(canvas, paint); + cv_restore_(canvas, paint); } - sk_sp get_picture() { + sk_sp MakePicture() { SkPictureRecorder recorder; SkRTreeFactory rtree_factory; SkCanvas* cv = recorder.beginRecording(kTestBounds, &rtree_factory); - render(cv); + Render(cv, {}, nullptr, nullptr); return recorder.finishRecordingAsPicture(); } + + SkPaint& setup_paint() { + FML_CHECK(paint_is_setup_); + return setup_paint_; + } + + private: + CvSetup cv_setup_; + CvRenderer cv_render_; + CvRenderer cv_restore_; + bool paint_is_setup_ = false; + SkPaint setup_paint_; }; -struct DlRenderJob { - RENDER_JOB_BASE; - DlRenderer& dl_setup = kEmptyDlRenderer; - DlRenderer& dl_render = kEmptyDlRenderer; - DlRenderer& dl_restore = kEmptyDlRenderer; - SkScalar opacity = SK_Scalar1; - sk_sp display_list; +struct CvPictureRenderJob : public JobRenderer { + explicit CvPictureRenderJob(sk_sp picture) + : picture_(std::move(picture)) {} + + void Render(SkCanvas* canvas, + const RenderJobInfo& info, + SkMatrix* setup_matrix = nullptr, + SkIRect* setup_clip_bounds = nullptr) { + FML_DCHECK(info.opacity == SK_Scalar1); + picture_->playback(canvas); + } + + private: + sk_sp picture_; +}; + +struct DlRenderJob : public JobRenderer { + explicit DlRenderJob(DlRenderer& dl_setup = kEmptyDlRenderer, + DlRenderer& dl_render = kEmptyDlRenderer, + DlRenderer& dl_restore = kEmptyDlRenderer) + : dl_setup_(dl_setup), + dl_render_(dl_render), + dl_restore_(dl_restore), + is_built_(false) {} + + explicit DlRenderJob(sk_sp display_list) + : dl_setup_(kEmptyDlRenderer), + dl_render_(kEmptyDlRenderer), + dl_restore_(kEmptyDlRenderer), + is_built_(true), + display_list_(std::move(display_list)) {} + + void Render(SkCanvas* canvas, + const RenderJobInfo& info, + SkMatrix* setup_matrix = nullptr, + SkIRect* setup_clip_bounds = nullptr) { + get_display_list(info, setup_matrix, setup_clip_bounds) + ->RenderTo(canvas, info.opacity); + } - void render(SkCanvas* canvas) { - get_display_list()->RenderTo(canvas, opacity); + sk_sp display_list() { + FML_CHECK(is_built_); + return display_list_; } - sk_sp get_display_list() { - if (!display_list) { - DisplayListBuilder builder(SkRect::MakeWH(width, height)); - dl_setup(builder); - setup_matrix = builder.getTransform(); - setup_clip_bounds = builder.getDestinationClipBounds().roundOut(); - dl_render(builder); - dl_restore(builder); - display_list = builder.Build(); + private: + sk_sp get_display_list(const RenderJobInfo& info, + SkMatrix* setup_matrix, + SkIRect* setup_clip_bounds) { + if (!display_list_) { + DisplayListBuilder builder(SkRect::MakeWH(info.width, info.height)); + dl_setup_(builder); + if (setup_matrix) { + *setup_matrix = builder.getTransform(); + } + if (setup_clip_bounds) { + *setup_clip_bounds = builder.getDestinationClipBounds().roundOut(); + } + dl_render_(builder); + dl_restore_(builder); + display_list_ = builder.Build(); + is_built_ = true; } - return display_list; + return display_list_; } + + const DlRenderer dl_setup_; + const DlRenderer dl_render_; + const DlRenderer dl_restore_; + bool is_built_; + sk_sp display_list_; }; class RenderEnvironment { @@ -350,10 +419,10 @@ class RenderEnvironment { RenderEnvironment(const DlSurfaceProvider* provider, PixelFormat format) : provider_(provider), format_(format) { if (provider->supports(format)) { - main_surface_ = + surface_1x_ = provider->MakeOffscreenSurface(kTestWidth, kTestHeight, format); - alt_surface_ = provider->MakeOffscreenSurface(kTestWidth * 2, - kTestHeight * 2, format); + surface_2x_ = provider->MakeOffscreenSurface(kTestWidth * 2, + kTestHeight * 2, format); } } @@ -373,38 +442,38 @@ class RenderEnvironment { CvRenderer& cv_renderer, DlRenderer& dl_setup, DlColor bg = DlColor::kTransparent()) { - CvRenderJob job = { + CvJobRenderer job(cv_setup, cv_renderer); + RenderJobInfo info = { .bg = bg, - .cv_setup = cv_setup, - .cv_render = cv_renderer, }; - ref_result_ = getResult(job); + ref_result_ = getResult(info, job); dl_setup(ref_attr_); } - template - std::unique_ptr getResult(J& job) const { - auto surface = getSurface(job.width, job.height); + std::unique_ptr getResult(const RenderJobInfo& info, + JobRenderer& renderer) const { + auto surface = getSurface(info.width, info.height); FML_DCHECK(surface != nullptr); auto canvas = surface->getCanvas(); - canvas->clear(job.bg); + canvas->clear(info.bg); int restore_count = canvas->save(); - canvas->scale(job.scale, job.scale); - job.render(canvas); + canvas->scale(info.scale, info.scale); + SkMatrix setup_matrix; + SkIRect setup_clip_bounds; + renderer.Render(canvas, info, &setup_matrix, &setup_clip_bounds); canvas->restoreToCount(restore_count); canvas->flush(); surface->flushAndSubmit(true); - return std::make_unique(surface, job.setup_matrix, - job.setup_clip_bounds); + return std::make_unique(surface, setup_matrix, + setup_clip_bounds); } std::unique_ptr getResult(sk_sp dl) const { - DlRenderJob job = { - .display_list = std::move(dl), - }; - return getResult(job); + DlRenderJob job(std::move(dl)); + RenderJobInfo info = {}; + return getResult(info, job); } const DlSurfaceProvider* provider() const { return provider_; } @@ -416,28 +485,26 @@ class RenderEnvironment { const RenderResult* ref_result() const { return ref_result_.get(); } private: - static bool matches(const DlSurfaceInstance* surface, int w, int h) { - return surface && surface->width() == w && surface->height() == h; - } - sk_sp getSurface(int width, int height) const { FML_DCHECK(valid()); - FML_DCHECK(main_surface_ != nullptr); - FML_DCHECK(alt_surface_ != nullptr); - if (matches(main_surface_.get(), width, height)) { - return main_surface_->sk_surface(); + FML_DCHECK(surface_1x_ != nullptr); + FML_DCHECK(surface_2x_ != nullptr); + if (width == kTestWidth && height == kTestHeight) { + return surface_1x_->sk_surface(); } - if (matches(alt_surface_.get(), width, height)) { - return alt_surface_->sk_surface(); + if (width == kTestWidth * 2 && height == kTestHeight * 2) { + return surface_2x_->sk_surface(); } + FML_LOG(ERROR) << "Test surface size (" << width << " x " << height + << ") not supported."; FML_DCHECK(false); return nullptr; } const DlSurfaceProvider* provider_; const PixelFormat format_; - std::shared_ptr main_surface_; - std::shared_ptr alt_surface_; + std::shared_ptr surface_1x_; + std::shared_ptr surface_2x_; DisplayListBuilder ref_attr_; std::unique_ptr ref_result_; @@ -1961,16 +2028,16 @@ class CanvasCompareTester { // sk_result is a direct rendering via SkCanvas to SkSurface // DisplayList mechanisms are not involved in this operation // SkPaint sk_paint; - CvRenderJob sk_job = { + CvJobRenderer sk_job(caseP.cv_setup(), // + testP.cv_renderer(), // + caseP.cv_restore()); + RenderJobInfo base_info = { .bg = caseP.bg(), - .cv_setup = caseP.cv_setup(), - .cv_render = testP.cv_renderer(), - .cv_restore = caseP.cv_restore(), }; - auto sk_result = env.getResult(sk_job); + auto sk_result = env.getResult(base_info, sk_job); const BoundsTolerance tolerance = - testP.adjust(tolerance_in, sk_job.setup_paint, sk_job.setup_matrix); - const sk_sp sk_picture = sk_job.get_picture(); + testP.adjust(tolerance_in, sk_job.setup_paint(), sk_result->matrix()); + const sk_sp sk_picture = sk_job.MakePicture(); SkRect sk_bounds = sk_picture->cullRect(); ASSERT_EQ(sk_result->width(), kTestWidth) << info; ASSERT_EQ(sk_result->height(), kTestHeight) << info; @@ -1993,14 +2060,11 @@ class CanvasCompareTester { // This sequence plays the provided equivalently constructed // DisplayList onto the SkCanvas of the surface // DisplayList => direct rendering - DlRenderJob dl_job = { - .bg = caseP.bg(), - .dl_setup = caseP.dl_setup(), - .dl_render = testP.dl_renderer(), - .dl_restore = caseP.dl_restore(), - }; - auto dl_result = env.getResult(dl_job); - display_list = dl_job.display_list; + DlRenderJob dl_job(caseP.dl_setup(), // + testP.dl_renderer(), // + caseP.dl_restore()); + auto dl_result = env.getResult(base_info, dl_job); + display_list = dl_job.display_list(); SkRect dl_bounds = display_list->bounds(); if (!sk_bounds.roundOut().contains(dl_bounds)) { FML_LOG(ERROR) << "For " << info; @@ -2053,12 +2117,9 @@ class CanvasCompareTester { // plays them back on SkCanvas to SkSurface // SkCanvas calls => DisplayList => rendering DisplayListCanvasRecorder dl_recorder(kTestBounds); - sk_job.render(&dl_recorder); - DlRenderJob cv_dl_job = { - .bg = bg, - .display_list = dl_recorder.Build(), - }; - auto cv_dl_result = env.getResult(cv_dl_job); + sk_job.Render(&dl_recorder, base_info); + DlRenderJob cv_dl_job(dl_recorder.Build()); + auto cv_dl_result = env.getResult(base_info, cv_dl_job); compareToReference(cv_dl_result.get(), sk_result.get(), info + " (Skia calls -> DisplayList -> surface)", nullptr, nullptr, bg, @@ -2075,25 +2136,19 @@ class CanvasCompareTester { const int test_height_2 = kTestHeight * 2; const SkScalar test_scale = 2.0; - CvRenderJob sk_job_x2 = { + CvPictureRenderJob sk_job_x2(sk_picture); + RenderJobInfo info_2x = { .width = test_width_2, .height = test_height_2, .bg = bg, .scale = test_scale, - .picture = sk_picture, }; - auto ref_x2_result = env.getResult(sk_job_x2); + auto ref_x2_result = env.getResult(info_2x, sk_job_x2); ASSERT_EQ(ref_x2_result->width(), test_width_2) << info; ASSERT_EQ(ref_x2_result->height(), test_height_2) << info; - DlRenderJob dl_job_x2 = { - .width = test_width_2, - .height = test_height_2, - .bg = bg, - .scale = test_scale, - .display_list = display_list, - }; - auto test_x2_result = env.getResult(dl_job_x2); + DlRenderJob dl_job_x2(display_list); + auto test_x2_result = env.getResult(info_2x, dl_job_x2); compareToReference(test_x2_result.get(), ref_x2_result.get(), info + " (Both rendered scaled 2x)", nullptr, nullptr, bg, caseP.fuzzy_compare_components(), // @@ -2130,12 +2185,12 @@ class CanvasCompareTester { DlColor bg) { SkScalar opacity = 128.0 / 255.0; - DlRenderJob opacity_job = { + DlRenderJob opacity_job(display_list); + RenderJobInfo opacity_info = { .bg = bg, .opacity = opacity, - .display_list = display_list, }; - auto group_opacity_result = env.getResult(opacity_job); + auto group_opacity_result = env.getResult(opacity_info, opacity_job); ASSERT_EQ(group_opacity_result->width(), kTestWidth) << info; ASSERT_EQ(group_opacity_result->height(), kTestHeight) << info; diff --git a/display_list/testing/dl_test_surface_software.cc b/display_list/testing/dl_test_surface_software.cc index ca551c18b50f9..8a887a66f36c4 100644 --- a/display_list/testing/dl_test_surface_software.cc +++ b/display_list/testing/dl_test_surface_software.cc @@ -16,7 +16,7 @@ bool DlSoftwareSurfaceProvider::InitializeSurface(size_t width, size_t height, PixelFormat format) { primary_ = MakeOffscreenSurface(width, height, format); - return true; + return primary_ != nullptr; } std::shared_ptr diff --git a/display_list/testing/dl_test_surface_software.h b/display_list/testing/dl_test_surface_software.h index 68eae12f74e74..5fbbe19457d9f 100644 --- a/display_list/testing/dl_test_surface_software.h +++ b/display_list/testing/dl_test_surface_software.h @@ -12,7 +12,7 @@ namespace testing { class DlSoftwareSurfaceProvider : public DlSurfaceProvider { public: - DlSoftwareSurfaceProvider() : DlSurfaceProvider() {} + DlSoftwareSurfaceProvider() = default; virtual ~DlSoftwareSurfaceProvider() = default; bool InitializeSurface(size_t width,