Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 14242a0

Browse files
dnfielddnfield
authored
[Impeller] DlAiksCanvas as a DlCanvas wrapper for impeller::Canvas (#45131)
This is a reland of #44248 Fixes flutter/flutter#132416 Changes from last time: - The `drawPoints` benchmark was failing to render anything meaningful because of an error in `AiksLayer` that resulted in an infinitely sized bounding rectangle poisoning the bounds with `NaN` (this happens, for example, with a `drawPaint` call, which that benchmark happens to use). Added a test covering this and filed flutter/flutter#132770 to explore ways to avoid this in the future. - There was a bug in `DlAiksCanvas::SaveLayer` where a `nullptr` `paint` but non-`nullptr` `backdrop` was failing to actually save the layer. This resulted in incorrect rendering. - There was a bug in `impeller::Canvas::DrawPicture` that resulted in incorrect stencil depth counting. That was fixed separately by @bdero, but was the cause of incorrect rendering in some Wonderous screens. - I've added a simple implementation for `AiksLayer::IsReplacing`. It does not currently compare as deeply as the `DisplayListLayer` version potentially does, but it is good enough to avoid the regression noted in flutter/flutter#132071. That regression was on a benchmark that greatly benefits from partial repaint. With the new implementation, it still gains partial repaint where it previously did not. There is more work that can be done here, filed flutter/flutter#133361 to track that work. I merged but did not fully integrate the `DisplayListBuilder`/`CanvasToReceiver` work that @flar has done. I have a local experiment with that, but would prefer to see this land and run through the device lab so we get some better comparison numbers for which one performs better.
1 parent c9b584d commit 14242a0

File tree

85 files changed

+2611
-420
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

85 files changed

+2611
-420
lines changed

‎ci/licenses_golden/excluded_files‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
../../../flutter/display_list/benchmarking/dl_complexity_unittests.cc
3333
../../../flutter/display_list/display_list_unittests.cc
3434
../../../flutter/display_list/dl_color_unittests.cc
35+
../../../flutter/display_list/dl_op_spy_unittests.cc
3536
../../../flutter/display_list/dl_paint_unittests.cc
3637
../../../flutter/display_list/dl_vertices_unittests.cc
3738
../../../flutter/display_list/effects/dl_color_filter_unittests.cc
@@ -54,6 +55,7 @@
5455
../../../flutter/flow/frame_timings_recorder_unittests.cc
5556
../../../flutter/flow/gl_context_switch_unittests.cc
5657
../../../flutter/flow/instrumentation_unittests.cc
58+
../../../flutter/flow/layers/aiks_layer_unittests.cc
5759
../../../flutter/flow/layers/backdrop_filter_layer_unittests.cc
5860
../../../flutter/flow/layers/checkerboard_layertree_unittests.cc
5961
../../../flutter/flow/layers/clip_path_layer_unittests.cc
@@ -219,7 +221,6 @@
219221
../../../flutter/runtime/type_conversions_unittests.cc
220222
../../../flutter/shell/common/animator_unittests.cc
221223
../../../flutter/shell/common/context_options_unittests.cc
222-
../../../flutter/shell/common/dl_op_spy_unittests.cc
223224
../../../flutter/shell/common/engine_unittests.cc
224225
../../../flutter/shell/common/fixtures
225226
../../../flutter/shell/common/input_events_unittests.cc

‎ci/licenses_golden/licenses_flutter‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -735,6 +735,8 @@ ORIGIN: ../../../flutter/display_list/dl_op_recorder.cc + ../../../flutter/LICEN
735735
ORIGIN: ../../../flutter/display_list/dl_op_recorder.h + ../../../flutter/LICENSE
736736
ORIGIN: ../../../flutter/display_list/dl_op_records.cc + ../../../flutter/LICENSE
737737
ORIGIN: ../../../flutter/display_list/dl_op_records.h + ../../../flutter/LICENSE
738+
ORIGIN: ../../../flutter/display_list/dl_op_spy.cc + ../../../flutter/LICENSE
739+
ORIGIN: ../../../flutter/display_list/dl_op_spy.h + ../../../flutter/LICENSE
738740
ORIGIN: ../../../flutter/display_list/dl_paint.cc + ../../../flutter/LICENSE
739741
ORIGIN: ../../../flutter/display_list/dl_paint.h + ../../../flutter/LICENSE
740742
ORIGIN: ../../../flutter/display_list/dl_sampling_options.h + ../../../flutter/LICENSE
@@ -791,6 +793,8 @@ ORIGIN: ../../../flutter/flow/instrumentation.cc + ../../../flutter/LICENSE
791793
ORIGIN: ../../../flutter/flow/instrumentation.h + ../../../flutter/LICENSE
792794
ORIGIN: ../../../flutter/flow/layer_snapshot_store.cc + ../../../flutter/LICENSE
793795
ORIGIN: ../../../flutter/flow/layer_snapshot_store.h + ../../../flutter/LICENSE
796+
ORIGIN: ../../../flutter/flow/layers/aiks_layer.cc + ../../../flutter/LICENSE
797+
ORIGIN: ../../../flutter/flow/layers/aiks_layer.h + ../../../flutter/LICENSE
794798
ORIGIN: ../../../flutter/flow/layers/backdrop_filter_layer.cc + ../../../flutter/LICENSE
795799
ORIGIN: ../../../flutter/flow/layers/backdrop_filter_layer.h + ../../../flutter/LICENSE
796800
ORIGIN: ../../../flutter/flow/layers/cacheable_layer.cc + ../../../flutter/LICENSE
@@ -1163,6 +1167,8 @@ ORIGIN: ../../../flutter/impeller/core/texture_descriptor.cc + ../../../flutter/
11631167
ORIGIN: ../../../flutter/impeller/core/texture_descriptor.h + ../../../flutter/LICENSE
11641168
ORIGIN: ../../../flutter/impeller/core/vertex_buffer.cc + ../../../flutter/LICENSE
11651169
ORIGIN: ../../../flutter/impeller/core/vertex_buffer.h + ../../../flutter/LICENSE
1170+
ORIGIN: ../../../flutter/impeller/display_list/dl_aiks_canvas.cc + ../../../flutter/LICENSE
1171+
ORIGIN: ../../../flutter/impeller/display_list/dl_aiks_canvas.h + ../../../flutter/LICENSE
11661172
ORIGIN: ../../../flutter/impeller/display_list/dl_dispatcher.cc + ../../../flutter/LICENSE
11671173
ORIGIN: ../../../flutter/impeller/display_list/dl_dispatcher.h + ../../../flutter/LICENSE
11681174
ORIGIN: ../../../flutter/impeller/display_list/dl_image_impeller.cc + ../../../flutter/LICENSE
@@ -2243,8 +2249,6 @@ ORIGIN: ../../../flutter/shell/common/display.cc + ../../../flutter/LICENSE
22432249
ORIGIN: ../../../flutter/shell/common/display.h + ../../../flutter/LICENSE
22442250
ORIGIN: ../../../flutter/shell/common/display_manager.cc + ../../../flutter/LICENSE
22452251
ORIGIN: ../../../flutter/shell/common/display_manager.h + ../../../flutter/LICENSE
2246-
ORIGIN: ../../../flutter/shell/common/dl_op_spy.cc + ../../../flutter/LICENSE
2247-
ORIGIN: ../../../flutter/shell/common/dl_op_spy.h + ../../../flutter/LICENSE
22482252
ORIGIN: ../../../flutter/shell/common/engine.cc + ../../../flutter/LICENSE
22492253
ORIGIN: ../../../flutter/shell/common/engine.h + ../../../flutter/LICENSE
22502254
ORIGIN: ../../../flutter/shell/common/pipeline.cc + ../../../flutter/LICENSE
@@ -3481,6 +3485,8 @@ FILE: ../../../flutter/display_list/dl_op_recorder.cc
34813485
FILE: ../../../flutter/display_list/dl_op_recorder.h
34823486
FILE: ../../../flutter/display_list/dl_op_records.cc
34833487
FILE: ../../../flutter/display_list/dl_op_records.h
3488+
FILE: ../../../flutter/display_list/dl_op_spy.cc
3489+
FILE: ../../../flutter/display_list/dl_op_spy.h
34843490
FILE: ../../../flutter/display_list/dl_paint.cc
34853491
FILE: ../../../flutter/display_list/dl_paint.h
34863492
FILE: ../../../flutter/display_list/dl_sampling_options.h
@@ -3537,6 +3543,8 @@ FILE: ../../../flutter/flow/instrumentation.cc
35373543
FILE: ../../../flutter/flow/instrumentation.h
35383544
FILE: ../../../flutter/flow/layer_snapshot_store.cc
35393545
FILE: ../../../flutter/flow/layer_snapshot_store.h
3546+
FILE: ../../../flutter/flow/layers/aiks_layer.cc
3547+
FILE: ../../../flutter/flow/layers/aiks_layer.h
35403548
FILE: ../../../flutter/flow/layers/backdrop_filter_layer.cc
35413549
FILE: ../../../flutter/flow/layers/backdrop_filter_layer.h
35423550
FILE: ../../../flutter/flow/layers/cacheable_layer.cc
@@ -3909,6 +3917,8 @@ FILE: ../../../flutter/impeller/core/texture_descriptor.cc
39093917
FILE: ../../../flutter/impeller/core/texture_descriptor.h
39103918
FILE: ../../../flutter/impeller/core/vertex_buffer.cc
39113919
FILE: ../../../flutter/impeller/core/vertex_buffer.h
3920+
FILE: ../../../flutter/impeller/display_list/dl_aiks_canvas.cc
3921+
FILE: ../../../flutter/impeller/display_list/dl_aiks_canvas.h
39123922
FILE: ../../../flutter/impeller/display_list/dl_dispatcher.cc
39133923
FILE: ../../../flutter/impeller/display_list/dl_dispatcher.h
39143924
FILE: ../../../flutter/impeller/display_list/dl_image_impeller.cc
@@ -4994,8 +5004,6 @@ FILE: ../../../flutter/shell/common/display.cc
49945004
FILE: ../../../flutter/shell/common/display.h
49955005
FILE: ../../../flutter/shell/common/display_manager.cc
49965006
FILE: ../../../flutter/shell/common/display_manager.h
4997-
FILE: ../../../flutter/shell/common/dl_op_spy.cc
4998-
FILE: ../../../flutter/shell/common/dl_op_spy.h
49995007
FILE: ../../../flutter/shell/common/engine.cc
50005008
FILE: ../../../flutter/shell/common/engine.h
50015009
FILE: ../../../flutter/shell/common/pipeline.cc

‎display_list/BUILD.gn‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ source_set("display_list") {
4343
"dl_op_recorder.h",
4444
"dl_op_records.cc",
4545
"dl_op_records.h",
46+
"dl_op_spy.cc",
47+
"dl_op_spy.h",
4648
"dl_paint.cc",
4749
"dl_paint.h",
4850
"dl_sampling_options.h",
@@ -111,6 +113,7 @@ if (enable_unittests) {
111113
"benchmarking/dl_complexity_unittests.cc",
112114
"display_list_unittests.cc",
113115
"dl_color_unittests.cc",
116+
"dl_op_spy_unittests.cc",
114117
"dl_paint_unittests.cc",
115118
"dl_vertices_unittests.cc",
116119
"effects/dl_color_filter_unittests.cc",

‎display_list/display_list.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ DisplayList::DisplayList(DlStorage&& storage,
3333
bool can_apply_group_opacity,
3434
bool is_ui_thread_safe,
3535
bool modifies_transparent_black,
36-
sk_sp<const DlRTree> rtree)
36+
std::shared_ptr<const DlRTree> rtree)
3737
: storage_(std::move(storage)),
3838
op_count_(op_count),
3939
nested_byte_count_(nested_byte_count),

‎display_list/display_list.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ class DisplayList : public SkRefCnt {
234234
const SkRect& bounds() const { return bounds_; }
235235

236236
bool has_rtree() const { return rtree_ != nullptr; }
237-
sk_sp<const DlRTree> rtree() const { return rtree_; }
237+
std::shared_ptr<const DlRTree> rtree() const { return rtree_; }
238238

239239
bool Equals(const DisplayList* other) const;
240240
bool Equals(const DisplayList& other) const { return Equals(&other); }
@@ -300,7 +300,7 @@ class DisplayList : public SkRefCnt {
300300
bool can_apply_group_opacity,
301301
bool is_ui_thread_safe,
302302
bool modifies_transparent_black,
303-
sk_sp<const DlRTree> rtree);
303+
std::shared_ptr<const DlRTree> rtree);
304304

305305
static uint32_t next_unique_id();
306306

@@ -317,7 +317,7 @@ class DisplayList : public SkRefCnt {
317317
const bool is_ui_thread_safe_;
318318
const bool modifies_transparent_black_;
319319

320-
const sk_sp<const DlRTree> rtree_;
320+
const std::shared_ptr<const DlRTree> rtree_;
321321

322322
void Dispatch(DlOpReceiver& ctx,
323323
uint8_t* ptr,

‎display_list/display_list_unittests.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1932,7 +1932,7 @@ TEST_F(DisplayListTest, FlatDrawPointsProducesBounds) {
19321932
}
19331933
}
19341934

1935-
static void test_rtree(const sk_sp<const DlRTree>& rtree,
1935+
static void test_rtree(const std::shared_ptr<const DlRTree>& rtree,
19361936
const SkRect& query,
19371937
std::vector<SkRect> expected_rects,
19381938
const std::vector<int>& expected_indices) {

‎display_list/dl_canvas.h‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@
1818
#include "third_party/skia/include/core/SkRect.h"
1919
#include "third_party/skia/include/core/SkTextBlob.h"
2020

21+
namespace impeller {
22+
struct Picture;
23+
} // namespace impeller
24+
2125
namespace flutter {
2226

2327
// The primary class used to express rendering operations in the
@@ -59,7 +63,12 @@ class DlCanvas {
5963
const DlImageFilter* backdrop = nullptr) = 0;
6064
virtual void Restore() = 0;
6165
virtual int GetSaveCount() const = 0;
62-
virtual void RestoreToCount(int restore_count) = 0;
66+
virtual void RestoreToCount(int restore_count) {
67+
FML_DCHECK(restore_count <= GetSaveCount());
68+
while (restore_count < GetSaveCount() && GetSaveCount() > 1) {
69+
Restore();
70+
}
71+
}
6372

6473
virtual void Translate(SkScalar tx, SkScalar ty) = 0;
6574
virtual void Scale(SkScalar sx, SkScalar sy) = 0;
@@ -199,6 +208,9 @@ class DlCanvas {
199208
const DlPaint* paint = nullptr) = 0;
200209
virtual void DrawDisplayList(const sk_sp<DisplayList> display_list,
201210
SkScalar opacity = SK_Scalar1) = 0;
211+
virtual void DrawImpellerPicture(
212+
const std::shared_ptr<const impeller::Picture>& picture,
213+
SkScalar opacity = SK_Scalar1) = 0;
202214
virtual void DrawTextBlob(const sk_sp<SkTextBlob>& blob,
203215
SkScalar x,
204216
SkScalar y,

‎display_list/dl_canvas_to_receiver.cc‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -811,6 +811,12 @@ void DlCanvasToReceiver::DrawDisplayList(const sk_sp<DisplayList> display_list,
811811
: OpResult::kPreservesTransparency,
812812
display_list->can_apply_group_opacity());
813813
}
814+
void DlCanvasToReceiver::DrawImpellerPicture(
815+
const std::shared_ptr<const impeller::Picture>& picture,
816+
SkScalar opacity) {
817+
FML_LOG(ERROR) << "Cannot draw Impeller Picture in to a a display list.";
818+
FML_DCHECK(false);
819+
}
814820
void DlCanvasToReceiver::DrawTextBlob(const sk_sp<SkTextBlob>& blob,
815821
SkScalar x,
816822
SkScalar y,

‎display_list/dl_canvas_to_receiver.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@
1010
#include "flutter/display_list/dl_op_receiver.h"
1111
#include "flutter/fml/macros.h"
1212

13+
namespace impeller {
14+
struct Picture;
15+
}
16+
1317
namespace flutter {
1418

1519
class DlCanvasReceiver : public DlOpReceiver {
@@ -337,6 +341,10 @@ class DlCanvasToReceiver : public virtual DlCanvas, //
337341
void DrawDisplayList(const sk_sp<DisplayList> display_list,
338342
SkScalar opacity = SK_Scalar1) override;
339343
// |DlCanvas|
344+
void DrawImpellerPicture(
345+
const std::shared_ptr<const impeller::Picture>& picture,
346+
SkScalar opacity = SK_Scalar1) override;
347+
// |DlCanvas|
340348
void DrawTextBlob(const sk_sp<SkTextBlob>& blob,
341349
SkScalar x,
342350
SkScalar y,

‎shell/common/dl_op_spy.cc‎ renamed to ‎display_list/dl_op_spy.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
#include "flutter/shell/common/dl_op_spy.h"
5+
#include "flutter/display_list/dl_op_spy.h"
66

77
namespace flutter {
88

0 commit comments

Comments
 (0)