Skip to content

Commit

Permalink
[Impeller] Reland DlAiksCanvas (flutter#45232)
Browse files Browse the repository at this point in the history
Relands flutter#45131

Fixes flutter/flutter#132416

Differences from last time:

- Some minor merge conflict fixes
- Use the RTree to get the bounds instead of recalculating the bounds
- Make the iOS platform view controller implementation use the impeller-aware slices instead of the display list ones. This has been fixed for Android and the desktop embedding, but I missed iOS. The unit tests weren't actually running before I branched for my PR, @zanderso fixed them up separately and this resulted in catching the failures on post submit last time.
  • Loading branch information
dnfield authored Aug 31, 2023
1 parent 64ae66a commit 3b75e3d
Show file tree
Hide file tree
Showing 92 changed files with 2,697 additions and 466 deletions.
3 changes: 2 additions & 1 deletion ci/licenses_golden/excluded_files
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
../../../flutter/display_list/benchmarking/dl_complexity_unittests.cc
../../../flutter/display_list/display_list_unittests.cc
../../../flutter/display_list/dl_color_unittests.cc
../../../flutter/display_list/dl_op_spy_unittests.cc
../../../flutter/display_list/dl_paint_unittests.cc
../../../flutter/display_list/dl_vertices_unittests.cc
../../../flutter/display_list/effects/dl_color_filter_unittests.cc
Expand All @@ -53,6 +54,7 @@
../../../flutter/flow/flow_run_all_unittests.cc
../../../flutter/flow/frame_timings_recorder_unittests.cc
../../../flutter/flow/gl_context_switch_unittests.cc
../../../flutter/flow/layers/aiks_layer_unittests.cc
../../../flutter/flow/layers/backdrop_filter_layer_unittests.cc
../../../flutter/flow/layers/checkerboard_layertree_unittests.cc
../../../flutter/flow/layers/clip_path_layer_unittests.cc
Expand Down Expand Up @@ -222,7 +224,6 @@
../../../flutter/runtime/type_conversions_unittests.cc
../../../flutter/shell/common/animator_unittests.cc
../../../flutter/shell/common/context_options_unittests.cc
../../../flutter/shell/common/dl_op_spy_unittests.cc
../../../flutter/shell/common/engine_unittests.cc
../../../flutter/shell/common/fixtures
../../../flutter/shell/common/input_events_unittests.cc
Expand Down
16 changes: 12 additions & 4 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,8 @@ ORIGIN: ../../../flutter/display_list/dl_op_recorder.cc + ../../../flutter/LICEN
ORIGIN: ../../../flutter/display_list/dl_op_recorder.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/dl_op_records.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/dl_op_records.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/dl_op_spy.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/dl_op_spy.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/dl_paint.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/dl_paint.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/dl_sampling_options.h + ../../../flutter/LICENSE
Expand Down Expand Up @@ -789,6 +791,8 @@ ORIGIN: ../../../flutter/flow/frame_timings.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/flow/frame_timings.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/flow/layer_snapshot_store.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/flow/layer_snapshot_store.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/flow/layers/aiks_layer.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/flow/layers/aiks_layer.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/flow/layers/backdrop_filter_layer.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/flow/layers/backdrop_filter_layer.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/flow/layers/cacheable_layer.cc + ../../../flutter/LICENSE
Expand Down Expand Up @@ -1167,6 +1171,8 @@ ORIGIN: ../../../flutter/impeller/core/texture_descriptor.cc + ../../../flutter/
ORIGIN: ../../../flutter/impeller/core/texture_descriptor.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/core/vertex_buffer.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/core/vertex_buffer.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/display_list/dl_aiks_canvas.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/display_list/dl_aiks_canvas.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/display_list/dl_dispatcher.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/display_list/dl_dispatcher.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/display_list/dl_image_impeller.cc + ../../../flutter/LICENSE
Expand Down Expand Up @@ -2247,8 +2253,6 @@ ORIGIN: ../../../flutter/shell/common/display.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/common/display.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/common/display_manager.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/common/display_manager.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/common/dl_op_spy.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/common/dl_op_spy.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/common/engine.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/common/engine.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/common/pipeline.cc + ../../../flutter/LICENSE
Expand Down Expand Up @@ -3485,6 +3489,8 @@ FILE: ../../../flutter/display_list/dl_op_recorder.cc
FILE: ../../../flutter/display_list/dl_op_recorder.h
FILE: ../../../flutter/display_list/dl_op_records.cc
FILE: ../../../flutter/display_list/dl_op_records.h
FILE: ../../../flutter/display_list/dl_op_spy.cc
FILE: ../../../flutter/display_list/dl_op_spy.h
FILE: ../../../flutter/display_list/dl_paint.cc
FILE: ../../../flutter/display_list/dl_paint.h
FILE: ../../../flutter/display_list/dl_sampling_options.h
Expand Down Expand Up @@ -3539,6 +3545,8 @@ FILE: ../../../flutter/flow/frame_timings.cc
FILE: ../../../flutter/flow/frame_timings.h
FILE: ../../../flutter/flow/layer_snapshot_store.cc
FILE: ../../../flutter/flow/layer_snapshot_store.h
FILE: ../../../flutter/flow/layers/aiks_layer.cc
FILE: ../../../flutter/flow/layers/aiks_layer.h
FILE: ../../../flutter/flow/layers/backdrop_filter_layer.cc
FILE: ../../../flutter/flow/layers/backdrop_filter_layer.h
FILE: ../../../flutter/flow/layers/cacheable_layer.cc
Expand Down Expand Up @@ -3917,6 +3925,8 @@ FILE: ../../../flutter/impeller/core/texture_descriptor.cc
FILE: ../../../flutter/impeller/core/texture_descriptor.h
FILE: ../../../flutter/impeller/core/vertex_buffer.cc
FILE: ../../../flutter/impeller/core/vertex_buffer.h
FILE: ../../../flutter/impeller/display_list/dl_aiks_canvas.cc
FILE: ../../../flutter/impeller/display_list/dl_aiks_canvas.h
FILE: ../../../flutter/impeller/display_list/dl_dispatcher.cc
FILE: ../../../flutter/impeller/display_list/dl_dispatcher.h
FILE: ../../../flutter/impeller/display_list/dl_image_impeller.cc
Expand Down Expand Up @@ -5002,8 +5012,6 @@ FILE: ../../../flutter/shell/common/display.cc
FILE: ../../../flutter/shell/common/display.h
FILE: ../../../flutter/shell/common/display_manager.cc
FILE: ../../../flutter/shell/common/display_manager.h
FILE: ../../../flutter/shell/common/dl_op_spy.cc
FILE: ../../../flutter/shell/common/dl_op_spy.h
FILE: ../../../flutter/shell/common/engine.cc
FILE: ../../../flutter/shell/common/engine.h
FILE: ../../../flutter/shell/common/pipeline.cc
Expand Down
3 changes: 3 additions & 0 deletions display_list/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ source_set("display_list") {
"dl_op_recorder.h",
"dl_op_records.cc",
"dl_op_records.h",
"dl_op_spy.cc",
"dl_op_spy.h",
"dl_paint.cc",
"dl_paint.h",
"dl_sampling_options.h",
Expand Down Expand Up @@ -111,6 +113,7 @@ if (enable_unittests) {
"benchmarking/dl_complexity_unittests.cc",
"display_list_unittests.cc",
"dl_color_unittests.cc",
"dl_op_spy_unittests.cc",
"dl_paint_unittests.cc",
"dl_vertices_unittests.cc",
"effects/dl_color_filter_unittests.cc",
Expand Down
2 changes: 1 addition & 1 deletion display_list/display_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ DisplayList::DisplayList(DlStorage&& storage,
bool can_apply_group_opacity,
bool is_ui_thread_safe,
bool modifies_transparent_black,
sk_sp<const DlRTree> rtree)
std::shared_ptr<const DlRTree> rtree)
: storage_(std::move(storage)),
op_count_(op_count),
nested_byte_count_(nested_byte_count),
Expand Down
6 changes: 3 additions & 3 deletions display_list/display_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ class DisplayList : public SkRefCnt {
const SkRect& bounds() const { return bounds_; }

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

bool Equals(const DisplayList* other) const;
bool Equals(const DisplayList& other) const { return Equals(&other); }
Expand Down Expand Up @@ -300,7 +300,7 @@ class DisplayList : public SkRefCnt {
bool can_apply_group_opacity,
bool is_ui_thread_safe,
bool modifies_transparent_black,
sk_sp<const DlRTree> rtree);
std::shared_ptr<const DlRTree> rtree);

static uint32_t next_unique_id();

Expand All @@ -317,7 +317,7 @@ class DisplayList : public SkRefCnt {
const bool is_ui_thread_safe_;
const bool modifies_transparent_black_;

const sk_sp<const DlRTree> rtree_;
const std::shared_ptr<const DlRTree> rtree_;

void Dispatch(DlOpReceiver& ctx,
uint8_t* ptr,
Expand Down
2 changes: 1 addition & 1 deletion display_list/display_list_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1932,7 +1932,7 @@ TEST_F(DisplayListTest, FlatDrawPointsProducesBounds) {
}
}

static void test_rtree(const sk_sp<const DlRTree>& rtree,
static void test_rtree(const std::shared_ptr<const DlRTree>& rtree,
const SkRect& query,
std::vector<SkRect> expected_rects,
const std::vector<int>& expected_indices) {
Expand Down
14 changes: 13 additions & 1 deletion display_list/dl_canvas.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
#include "third_party/skia/include/core/SkRect.h"
#include "third_party/skia/include/core/SkTextBlob.h"

namespace impeller {
struct Picture;
} // namespace impeller

namespace flutter {

//------------------------------------------------------------------------------
Expand Down Expand Up @@ -61,7 +65,12 @@ class DlCanvas {
const DlImageFilter* backdrop = nullptr) = 0;
virtual void Restore() = 0;
virtual int GetSaveCount() const = 0;
virtual void RestoreToCount(int restore_count) = 0;
virtual void RestoreToCount(int restore_count) {
FML_DCHECK(restore_count <= GetSaveCount());
while (restore_count < GetSaveCount() && GetSaveCount() > 1) {
Restore();
}
}

virtual void Translate(SkScalar tx, SkScalar ty) = 0;
virtual void Scale(SkScalar sx, SkScalar sy) = 0;
Expand Down Expand Up @@ -201,6 +210,9 @@ class DlCanvas {
const DlPaint* paint = nullptr) = 0;
virtual void DrawDisplayList(const sk_sp<DisplayList> display_list,
SkScalar opacity = SK_Scalar1) = 0;
virtual void DrawImpellerPicture(
const std::shared_ptr<const impeller::Picture>& picture,
SkScalar opacity = SK_Scalar1) = 0;
virtual void DrawTextBlob(const sk_sp<SkTextBlob>& blob,
SkScalar x,
SkScalar y,
Expand Down
6 changes: 6 additions & 0 deletions display_list/dl_canvas_to_receiver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,12 @@ void DlCanvasToReceiver::DrawDisplayList(const sk_sp<DisplayList> display_list,
: OpResult::kPreservesTransparency,
display_list->can_apply_group_opacity());
}
void DlCanvasToReceiver::DrawImpellerPicture(
const std::shared_ptr<const impeller::Picture>& picture,
SkScalar opacity) {
FML_LOG(ERROR) << "Cannot draw Impeller Picture in to a a display list.";
FML_DCHECK(false);
}
void DlCanvasToReceiver::DrawTextBlob(const sk_sp<SkTextBlob>& blob,
SkScalar x,
SkScalar y,
Expand Down
8 changes: 8 additions & 0 deletions display_list/dl_canvas_to_receiver.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
#include "flutter/display_list/dl_op_receiver.h"
#include "flutter/fml/macros.h"

namespace impeller {
struct Picture;
}

namespace flutter {

class DlCanvasReceiver : public DlOpReceiver {
Expand Down Expand Up @@ -337,6 +341,10 @@ class DlCanvasToReceiver : public virtual DlCanvas, //
void DrawDisplayList(const sk_sp<DisplayList> display_list,
SkScalar opacity = SK_Scalar1) override;
// |DlCanvas|
void DrawImpellerPicture(
const std::shared_ptr<const impeller::Picture>& picture,
SkScalar opacity = SK_Scalar1) override;
// |DlCanvas|
void DrawTextBlob(const sk_sp<SkTextBlob>& blob,
SkScalar x,
SkScalar y,
Expand Down
2 changes: 1 addition & 1 deletion shell/common/dl_op_spy.cc → display_list/dl_op_spy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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/shell/common/dl_op_spy.h"
#include "flutter/display_list/dl_op_spy.h"

namespace flutter {

Expand Down
5 changes: 1 addition & 4 deletions shell/common/dl_op_spy.h → display_list/dl_op_spy.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef FLUTTER_DISPLAY_LIST_DL_OP_SPY_H_
#define FLUTTER_DISPLAY_LIST_DL_OP_SPY_H_
#pragma once

#include "flutter/display_list/dl_op_receiver.h"
#include "flutter/display_list/utils/dl_receiver_utils.h"
Expand Down Expand Up @@ -105,5 +104,3 @@ class DlOpSpy final : public virtual DlOpReceiver,
};

} // namespace flutter

#endif // FLUTTER_DISPLAY_LIST_DL_OP_SPY_H_
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

#include "flutter/display_list/display_list.h"
#include "flutter/display_list/display_list_builder.h"
#include "flutter/shell/common/dl_op_spy.h"
#include "flutter/display_list/dl_op_spy.h"
#include "flutter/testing/testing.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "third_party/skia/include/core/SkRSXform.h"
Expand Down
2 changes: 1 addition & 1 deletion display_list/geometry/dl_rtree.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace flutter {
/// - Query for a set of non-overlapping rectangles that are joined
/// from the original rectangles that intersect a query rect
/// @see |searchAndConsolidateRects|
class DlRTree : public SkRefCnt {
class DlRTree {
private:
static constexpr int kMaxChildren = 11;

Expand Down
7 changes: 7 additions & 0 deletions display_list/skia/dl_sk_canvas.cc
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,13 @@ void DlSkCanvasAdapter::DrawAtlas(const sk_sp<DlImage>& atlas,
ToSk(sampling), cullRect, sk_paint());
}

void DlSkCanvasAdapter::DrawImpellerPicture(
const std::shared_ptr<const impeller::Picture>& picture,
SkScalar opacity) {
FML_LOG(ERROR) << "Cannot draw Impeller Picture in to a Skia canvas.";
FML_DCHECK(false);
}

void DlSkCanvasAdapter::DrawDisplayList(const sk_sp<DisplayList> display_list,
SkScalar opacity) {
const int restore_count = delegate_->getSaveCount();
Expand Down
3 changes: 3 additions & 0 deletions display_list/skia/dl_sk_canvas.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ class DlSkCanvasAdapter final : public virtual DlCanvas {
DlImageSampling sampling,
const SkRect* cullRect,
const DlPaint* paint = nullptr) override;
void DrawImpellerPicture(
const std::shared_ptr<const impeller::Picture>& picture,
SkScalar opacity = SK_Scalar1) override;
void DrawDisplayList(const sk_sp<DisplayList> display_list,
SkScalar opacity = SK_Scalar1) override;
void DrawTextBlob(const sk_sp<SkTextBlob>& blob,
Expand Down
7 changes: 4 additions & 3 deletions display_list/utils/dl_bounds_accumulator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,11 @@ SkRect RTreeBoundsAccumulator::bounds() const {
return accumulator.bounds();
}

sk_sp<DlRTree> RTreeBoundsAccumulator::rtree() const {
std::shared_ptr<DlRTree> RTreeBoundsAccumulator::rtree() const {
FML_DCHECK(saved_offsets_.empty());
return sk_make_sp<DlRTree>(rects_.data(), rects_.size(), rect_indices_.data(),
[](int id) { return id >= 0; });
return std::make_shared<DlRTree>(rects_.data(), rects_.size(),
rect_indices_.data(),
[](int id) { return id >= 0; });
}

} // namespace flutter
6 changes: 3 additions & 3 deletions display_list/utils/dl_bounds_accumulator.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class BoundsAccumulator {

virtual SkRect bounds() const = 0;

virtual sk_sp<DlRTree> rtree() const = 0;
virtual std::shared_ptr<DlRTree> rtree() const = 0;
};

class RectBoundsAccumulator final : public virtual BoundsAccumulator {
Expand All @@ -111,7 +111,7 @@ class RectBoundsAccumulator final : public virtual BoundsAccumulator {

Type type() const override { return Type::kRect; }

sk_sp<DlRTree> rtree() const override { return nullptr; }
std::shared_ptr<DlRTree> rtree() const override { return nullptr; }

private:
class AccumulationRect {
Expand Down Expand Up @@ -150,7 +150,7 @@ class RTreeBoundsAccumulator final : public virtual BoundsAccumulator {

SkRect bounds() const override;

sk_sp<DlRTree> rtree() const override;
std::shared_ptr<DlRTree> rtree() const override;

Type type() const override { return Type::kRTree; }

Expand Down
7 changes: 7 additions & 0 deletions flow/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ source_set("flow") {
"frame_timings.h",
"layer_snapshot_store.cc",
"layer_snapshot_store.h",
"layers/aiks_layer.cc",
"layers/aiks_layer.h",
"layers/backdrop_filter_layer.cc",
"layers/backdrop_filter_layer.h",
"layers/cacheable_layer.cc",
Expand Down Expand Up @@ -192,6 +194,11 @@ if (enable_unittests) {
"//third_party/skia",
]

if (impeller_supports_rendering) {
sources += [ "layers/aiks_layer_unittests.cc" ]
deps += [ "//flutter/impeller" ]
}

if (!defined(defines)) {
defines = []
}
Expand Down
Loading

0 comments on commit 3b75e3d

Please sign in to comment.