From ff02cd0f002dfc3a0cea14cdadb8641cae99f77e Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Wed, 23 Oct 2024 18:52:03 -0700 Subject: [PATCH 1/3] Remove legacy size fields in DLOp records --- ci/licenses_golden/excluded_files | 1 + ci/licenses_golden/licenses_flutter | 4 + display_list/BUILD.gn | 3 + display_list/display_list.cc | 123 +++++----- display_list/display_list.h | 38 +-- display_list/display_list_unittests.cc | 38 ++- display_list/dl_builder.cc | 64 +++-- display_list/dl_builder.h | 3 +- display_list/dl_op_records.h | 308 ++++++++++++++----------- display_list/dl_storage.cc | 47 ++++ display_list/dl_storage.h | 54 +++++ display_list/dl_storage_unittests.cc | 47 ++++ 12 files changed, 459 insertions(+), 271 deletions(-) create mode 100644 display_list/dl_storage.cc create mode 100644 display_list/dl_storage.h create mode 100644 display_list/dl_storage_unittests.cc diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files index 7be947a7cdd3e..21eb9a1a37e5a 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -34,6 +34,7 @@ ../../../flutter/display_list/display_list_unittests.cc ../../../flutter/display_list/dl_color_unittests.cc ../../../flutter/display_list/dl_paint_unittests.cc +../../../flutter/display_list/dl_storage_unittests.cc ../../../flutter/display_list/dl_vertices_unittests.cc ../../../flutter/display_list/effects/dl_color_filter_unittests.cc ../../../flutter/display_list/effects/dl_color_source_unittests.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 45e22b2e4a3e8..8eef52296b3ca 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -42525,6 +42525,8 @@ ORIGIN: ../../../flutter/display_list/dl_op_records.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 +ORIGIN: ../../../flutter/display_list/dl_storage.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/display_list/dl_storage.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/dl_tile_mode.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/dl_vertices.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/dl_vertices.h + ../../../flutter/LICENSE @@ -45386,6 +45388,8 @@ FILE: ../../../flutter/display_list/dl_op_records.h FILE: ../../../flutter/display_list/dl_paint.cc FILE: ../../../flutter/display_list/dl_paint.h FILE: ../../../flutter/display_list/dl_sampling_options.h +FILE: ../../../flutter/display_list/dl_storage.cc +FILE: ../../../flutter/display_list/dl_storage.h FILE: ../../../flutter/display_list/dl_tile_mode.h FILE: ../../../flutter/display_list/dl_vertices.cc FILE: ../../../flutter/display_list/dl_vertices.h diff --git a/display_list/BUILD.gn b/display_list/BUILD.gn index 1948cca0887fb..48f1fd2341014 100644 --- a/display_list/BUILD.gn +++ b/display_list/BUILD.gn @@ -42,6 +42,8 @@ source_set("display_list") { "dl_paint.cc", "dl_paint.h", "dl_sampling_options.h", + "dl_storage.cc", + "dl_storage.h", "dl_tile_mode.h", "dl_vertices.cc", "dl_vertices.h", @@ -111,6 +113,7 @@ if (enable_unittests) { "display_list_unittests.cc", "dl_color_unittests.cc", "dl_paint_unittests.cc", + "dl_storage_unittests.cc", "dl_vertices_unittests.cc", "effects/dl_color_filter_unittests.cc", "effects/dl_color_source_unittests.cc", diff --git a/display_list/display_list.cc b/display_list/display_list.cc index 1d30ebd2813a3..557c3802d7828 100644 --- a/display_list/display_list.cc +++ b/display_list/display_list.cc @@ -15,8 +15,7 @@ const SaveLayerOptions SaveLayerOptions::kWithAttributes = kNoAttributes.with_renders_with_attributes(); DisplayList::DisplayList() - : byte_count_(0), - op_count_(0), + : op_count_(0), nested_byte_count_(0), nested_op_count_(0), total_depth_(0), @@ -27,25 +26,13 @@ DisplayList::DisplayList() modifies_transparent_black_(false), root_has_backdrop_filter_(false), root_is_unbounded_(false), - max_root_blend_mode_(DlBlendMode::kClear) {} - -// Eventually we should rework DisplayListBuilder to compute these and -// deliver the vector alongside the storage. -static std::vector MakeOffsets(const DisplayListStorage& storage, - size_t byte_count) { - std::vector offsets; - const uint8_t* start = storage.get(); - const uint8_t* end = start + byte_count; - const uint8_t* ptr = start; - while (ptr < end) { - offsets.push_back(ptr - start); - ptr += reinterpret_cast(ptr)->size; - } - return offsets; + max_root_blend_mode_(DlBlendMode::kClear) { + FML_DCHECK(offsets_.size() == 0u); + FML_DCHECK(storage_.size() == 0u); } DisplayList::DisplayList(DisplayListStorage&& storage, - size_t byte_count, + std::vector&& offsets, uint32_t op_count, size_t nested_byte_count, uint32_t nested_op_count, @@ -59,8 +46,7 @@ DisplayList::DisplayList(DisplayListStorage&& storage, bool root_is_unbounded, sk_sp rtree) : storage_(std::move(storage)), - offsets_(MakeOffsets(storage_, byte_count)), - byte_count_(byte_count), + offsets_(std::move(offsets)), op_count_(op_count), nested_byte_count_(nested_byte_count), nested_op_count_(nested_op_count), @@ -73,11 +59,12 @@ DisplayList::DisplayList(DisplayListStorage&& storage, root_has_backdrop_filter_(root_has_backdrop_filter), root_is_unbounded_(root_is_unbounded), max_root_blend_mode_(max_root_blend_mode), - rtree_(std::move(rtree)) {} + rtree_(std::move(rtree)) { + FML_DCHECK(offsets_.capacity() == offsets_.size()); +} DisplayList::~DisplayList() { - const uint8_t* ptr = storage_.get(); - DisposeOps(ptr, ptr + byte_count_); + DisposeOps(storage_, offsets_); } uint32_t DisplayList::next_unique_id() { @@ -132,7 +119,7 @@ void DisplayList::RTreeResultsToIndexVector( return; } } - const uint8_t* ptr = storage_.get() + offsets_[index]; + const uint8_t* ptr = storage_.base() + offsets_[index]; const DLOp* op = reinterpret_cast(ptr); switch (GetOpCategory(op->type)) { case DisplayListOpCategory::kAttribute: @@ -193,7 +180,7 @@ void DisplayList::RTreeResultsToIndexVector( } void DisplayList::Dispatch(DlOpReceiver& receiver) const { - const uint8_t* base = storage_.get(); + const uint8_t* base = storage_.base(); for (size_t offset : offsets_) { DispatchOneOp(receiver, base + offset); } @@ -213,7 +200,7 @@ void DisplayList::Dispatch(DlOpReceiver& receiver, Dispatch(receiver); } else { auto op_indices = GetCulledIndices(cull_rect); - const uint8_t* base = storage_.get(); + const uint8_t* base = storage_.base(); for (DlIndex index : op_indices) { DispatchOneOp(receiver, base + offsets_[index]); } @@ -240,11 +227,14 @@ void DisplayList::DispatchOneOp(DlOpReceiver& receiver, } } -void DisplayList::DisposeOps(const uint8_t* ptr, const uint8_t* end) { - while (ptr < end) { - auto op = reinterpret_cast(ptr); - ptr += op->size; - FML_DCHECK(ptr <= end); +void DisplayList::DisposeOps(const DisplayListStorage& storage, + const std::vector& offsets) { + const uint8_t* base = storage.base(); + if (!base) { + return; + } + for (size_t offset : offsets) { + auto op = reinterpret_cast(base + offset); switch (op->type) { #define DL_OP_DISPOSE(name) \ case DisplayListOpType::k##name: \ @@ -362,10 +352,9 @@ DisplayListOpType DisplayList::GetOpType(DlIndex index) const { } size_t offset = offsets_[index]; - FML_DCHECK(offset < byte_count_); - auto ptr = storage_.get() + offset; + FML_DCHECK(offset < storage_.size()); + auto ptr = storage_.base() + offset; auto op = reinterpret_cast(ptr); - FML_DCHECK(ptr + op->size <= storage_.get() + byte_count_); return op->type; } @@ -399,34 +388,32 @@ bool DisplayList::Dispatch(DlOpReceiver& receiver, DlIndex index) const { } size_t offset = offsets_[index]; - FML_DCHECK(offset < byte_count_); - auto ptr = storage_.get() + offset; - FML_DCHECK(offset + reinterpret_cast(ptr)->size <= byte_count_); + FML_DCHECK(offset < storage_.size()); + auto ptr = storage_.base() + offset; DispatchOneOp(receiver, ptr); return true; } -static bool CompareOps(const uint8_t* ptrA, - const uint8_t* endA, - const uint8_t* ptrB, - const uint8_t* endB) { +static bool CompareOps(const DisplayListStorage& storageA, + const std::vector& offsetsA, + const DisplayListStorage& storageB, + const std::vector& offsetsB) { + const uint8_t* base_a = storageA.base(); + const uint8_t* base_b = storageB.base(); // These conditions are checked by the caller... - FML_DCHECK((endA - ptrA) == (endB - ptrB)); - FML_DCHECK(ptrA != ptrB); - const uint8_t* bulk_start_a = ptrA; - const uint8_t* bulk_start_b = ptrB; - while (ptrA < endA && ptrB < endB) { - auto opA = reinterpret_cast(ptrA); - auto opB = reinterpret_cast(ptrB); - if (opA->type != opB->type || opA->size != opB->size) { + FML_DCHECK(offsetsA.size() == offsetsB.size()); + FML_DCHECK(base_a != base_b); + size_t bulk_start = 0u; + for (size_t i = 0; i < offsetsA.size(); i++) { + size_t offset = offsetsA[i]; + FML_DCHECK(offsetsB[i] == offset); + auto opA = reinterpret_cast(base_a + offset); + auto opB = reinterpret_cast(base_b + offset); + if (opA->type != opB->type) { return false; } - ptrA += opA->size; - ptrB += opB->size; - FML_DCHECK(ptrA <= endA); - FML_DCHECK(ptrB <= endB); DisplayListCompare result; switch (opA->type) { #define DL_OP_EQUALS(name) \ @@ -451,23 +438,23 @@ static bool CompareOps(const uint8_t* ptrA, case DisplayListCompare::kEqual: // Check if we have a backlog of bytes to bulk compare and then // reset the bulk compare pointers to the address following this op - auto bulk_bytes = reinterpret_cast(opA) - bulk_start_a; - if (bulk_bytes > 0) { - if (memcmp(bulk_start_a, bulk_start_b, bulk_bytes) != 0) { + if (bulk_start < offset) { + const uint8_t* bulk_start_a = base_a + bulk_start; + const uint8_t* bulk_start_b = base_b + bulk_start; + if (memcmp(bulk_start_a, bulk_start_b, offset - bulk_start) != 0) { return false; } } - bulk_start_a = ptrA; - bulk_start_b = ptrB; + bulk_start = + i + 1 < offsetsA.size() ? offsetsA[i + 1] : storageA.size(); break; } } - if (ptrA != endA || ptrB != endB) { - return false; - } - if (bulk_start_a < ptrA) { + if (bulk_start < storageA.size()) { // Perform a final bulk compare if we have remaining bytes waiting - if (memcmp(bulk_start_a, bulk_start_b, ptrA - bulk_start_a) != 0) { + const uint8_t* bulk_start_a = base_a + bulk_start; + const uint8_t* bulk_start_b = base_b + bulk_start; + if (memcmp(bulk_start_a, bulk_start_b, storageA.size() - bulk_start) != 0) { return false; } } @@ -478,15 +465,15 @@ bool DisplayList::Equals(const DisplayList* other) const { if (this == other) { return true; } - if (byte_count_ != other->byte_count_ || op_count_ != other->op_count_) { + if (offsets_.size() != other->offsets_.size() || + storage_.size() != other->storage_.size() || + op_count_ != other->op_count_) { return false; } - const uint8_t* ptr = storage_.get(); - const uint8_t* o_ptr = other->storage_.get(); - if (ptr == o_ptr) { + if (storage_.base() == other->storage_.base()) { return true; } - return CompareOps(ptr, ptr + byte_count_, o_ptr, o_ptr + other->byte_count_); + return CompareOps(storage_, offsets_, other->storage_, other->offsets_); } } // namespace flutter diff --git a/display_list/display_list.h b/display_list/display_list.h index 339c291c2f684..0c13a437df60e 100644 --- a/display_list/display_list.h +++ b/display_list/display_list.h @@ -5,14 +5,10 @@ #ifndef FLUTTER_DISPLAY_LIST_DISPLAY_LIST_H_ #define FLUTTER_DISPLAY_LIST_DISPLAY_LIST_H_ -#include -#include - #include "flutter/display_list/dl_blend_mode.h" -#include "flutter/display_list/dl_sampling_options.h" +#include "flutter/display_list/dl_storage.h" #include "flutter/display_list/geometry/dl_geometry_types.h" #include "flutter/display_list/geometry/dl_rtree.h" -#include "flutter/fml/logging.h" // The Flutter DisplayList mechanism encapsulates a persistent sequence of // rendering operations. @@ -263,28 +259,6 @@ class SaveLayerOptions { }; }; -// Manages a buffer allocated with malloc. -class DisplayListStorage { - public: - DisplayListStorage() = default; - DisplayListStorage(DisplayListStorage&&) = default; - - uint8_t* get() { return ptr_.get(); } - - const uint8_t* get() const { return ptr_.get(); } - - void realloc(size_t count) { - ptr_.reset(static_cast(std::realloc(ptr_.release(), count))); - FML_CHECK(ptr_); - } - - private: - struct FreeDeleter { - void operator()(uint8_t* p) { std::free(p); } - }; - std::unique_ptr ptr_; -}; - using DlIndex = uint32_t; // The base class that contains a sequence of rendering operations @@ -304,7 +278,7 @@ class DisplayList : public SkRefCnt { // but nested ops are only included if requested. The defaults used // here for these accessors follow that pattern. size_t bytes(bool nested = true) const { - return sizeof(DisplayList) + byte_count_ + + return sizeof(DisplayList) + storage_.size() + (nested ? nested_byte_count_ : 0); } @@ -530,7 +504,7 @@ class DisplayList : public SkRefCnt { private: DisplayList(DisplayListStorage&& ptr, - size_t byte_count, + std::vector&& offsets, uint32_t op_count, size_t nested_byte_count, uint32_t nested_op_count, @@ -546,13 +520,13 @@ class DisplayList : public SkRefCnt { static uint32_t next_unique_id(); - static void DisposeOps(const uint8_t* ptr, const uint8_t* end); + static void DisposeOps(const DisplayListStorage& storage, + const std::vector& offsets); const DisplayListStorage storage_; const std::vector offsets_; - const size_t byte_count_; - const uint32_t op_count_; + const uint32_t op_count_; const size_t nested_byte_count_; const uint32_t nested_op_count_; diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index d43a2334dc928..8264e79e0e888 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -459,15 +459,15 @@ TEST_F(DisplayListTest, BuildRestoresAttributes) { DlOpReceiver& receiver = ToReceiver(builder); receiver.setAntiAlias(true); - builder.Build(); + FML_LOG(ERROR) << *builder.Build(); check_defaults(builder, cull_rect); receiver.setInvertColors(true); - builder.Build(); + FML_LOG(ERROR) << *builder.Build(); check_defaults(builder, cull_rect); receiver.setColor(DlColor::kRed()); - builder.Build(); + FML_LOG(ERROR) << *builder.Build(); check_defaults(builder, cull_rect); receiver.setBlendMode(DlBlendMode::kColorBurn); @@ -5894,5 +5894,37 @@ TEST_F(DisplayListTest, BackdropFilterCulledAlongsideClipAndTransform) { } } +TEST_F(DisplayListTest, RecordManyLargeDisplayListOperations) { + DisplayListBuilder builder; + + // 2050 points is sizeof(DlPoint) * 2050 = 16400 bytes, this is more + // than the page size of 16384 bytes. + std::vector points(2050); + builder.DrawPoints(PointMode::kPoints, points.size(), points.data(), + DlPaint{}); + builder.DrawPoints(PointMode::kPoints, points.size(), points.data(), + DlPaint{}); + builder.DrawPoints(PointMode::kPoints, points.size(), points.data(), + DlPaint{}); + builder.DrawPoints(PointMode::kPoints, points.size(), points.data(), + DlPaint{}); + builder.DrawPoints(PointMode::kPoints, points.size(), points.data(), + DlPaint{}); + builder.DrawPoints(PointMode::kPoints, points.size(), points.data(), + DlPaint{}); + + EXPECT_TRUE(!!builder.Build()); +} + +TEST_F(DisplayListTest, RecordSingleLargeDisplayListOperation) { + DisplayListBuilder builder; + + std::vector points(40000); + builder.DrawPoints(PointMode::kPoints, points.size(), points.data(), + DlPaint{}); + + EXPECT_TRUE(!!builder.Build()); +} + } // namespace testing } // namespace flutter diff --git a/display_list/dl_builder.cc b/display_list/dl_builder.cc index 2d10ad33e44b5..ba70463fc1fc8 100644 --- a/display_list/dl_builder.cc +++ b/display_list/dl_builder.cc @@ -15,8 +15,6 @@ namespace flutter { -#define DL_BUILDER_PAGE 4096 - // CopyV(dst, src,n, src,n, ...) copies any number of typed srcs into dst. static void CopyV(void* dst) {} @@ -35,32 +33,29 @@ static void CopyV(void* dst, const S* src, int n, Rest&&... rest) { CopyV(dst, std::forward(rest)...); } -static constexpr inline bool is_power_of_two(int value) { - return (value & (value - 1)) == 0; -} - template void* DisplayListBuilder::Push(size_t pod, Args&&... args) { + // Plan out where and how large a space we need size_t size = SkAlignPtr(sizeof(T) + pod); - FML_CHECK(size < (1 << 24)); - if (used_ + size > allocated_) { - static_assert(is_power_of_two(DL_BUILDER_PAGE), - "This math needs updating for non-pow2."); - // Next greater multiple of DL_BUILDER_PAGE. - allocated_ = (used_ + size + DL_BUILDER_PAGE) & ~(DL_BUILDER_PAGE - 1); - storage_.realloc(allocated_); - FML_CHECK(storage_.get()); - memset(storage_.get() + used_, 0, allocated_ - used_); - } - FML_CHECK(used_ + size <= allocated_); - auto op = reinterpret_cast(storage_.get() + used_); - used_ += size; + size_t offset = storage_.size(); + + // Allocate the space + auto ptr = storage_.allocate(size); + FML_CHECK(ptr); + + // Initialize the space via the constructor + auto op = reinterpret_cast(ptr); new (op) T{std::forward(args)...}; - op->type = T::kType; - op->size = size; + FML_DCHECK(op->type == T::kType); + + // Adjust the counters and offsets (the memory is mostly initialized + // at this point except that the caller might do some pod-based copying + // past the end of the DlOp structure itself when we return) + offsets_.push_back(offset); render_op_count_ += T::kRenderOpInc; depth_ += T::kDepthInc * render_op_depth_cost_; op_index_++; + return op + 1; } @@ -69,7 +64,6 @@ sk_sp DisplayListBuilder::Build() { restore(); } - size_t bytes = used_; int count = render_op_count_; size_t nested_bytes = nested_bytes_; int nested_count = nested_op_count_; @@ -97,7 +91,7 @@ sk_sp DisplayListBuilder::Build() { bounds = current_layer().global_space_accumulator.bounds(); } - used_ = allocated_ = render_op_count_ = op_index_ = 0; + render_op_count_ = op_index_ = 0; nested_bytes_ = nested_op_count_ = 0; depth_ = 0; is_ui_thread_safe_ = true; @@ -108,12 +102,13 @@ sk_sp DisplayListBuilder::Build() { save_stack_.pop_back(); Init(rtree != nullptr); - storage_.realloc(bytes); + storage_.trim(); + offsets_.shrink_to_fit(); return sk_sp(new DisplayList( - std::move(storage_), bytes, count, nested_bytes, nested_count, - total_depth, bounds, opacity_compatible, is_safe, affects_transparency, - max_root_blend_mode, root_has_backdrop_filter, root_is_unbounded, - std::move(rtree))); + std::move(storage_), std::move(offsets_), count, nested_bytes, + nested_count, total_depth, bounds, opacity_compatible, is_safe, + affects_transparency, max_root_blend_mode, root_has_backdrop_filter, + root_is_unbounded, std::move(rtree))); } static constexpr DlRect kEmpty = DlRect(); @@ -141,10 +136,7 @@ void DisplayListBuilder::Init(bool prepare_rtree) { } DisplayListBuilder::~DisplayListBuilder() { - uint8_t* ptr = storage_.get(); - if (ptr) { - DisplayList::DisposeOps(ptr, ptr + used_); - } + DisplayList::DisposeOps(storage_, offsets_); } DlISize DisplayListBuilder::GetBaseLayerDimensions() const { @@ -390,7 +382,7 @@ void DisplayListBuilder::SetAttributesFromPaint( void DisplayListBuilder::checkForDeferredSave() { if (current_info().has_deferred_save_op) { - size_t save_offset = used_; + size_t save_offset = storage_.size(); Push(0); current_info().save_offset = save_offset; current_info().save_depth = depth_; @@ -435,7 +427,7 @@ void DisplayListBuilder::saveLayer(const DlRect& bounds, // Snapshot these values before we do any work as we need the values // from before the method was called, but some of the operations below // might update them. - size_t save_offset = used_; + size_t save_offset = storage_.size(); uint32_t save_depth = depth_; // A backdrop will affect up to the entire surface, bounded by the clip @@ -568,7 +560,7 @@ void DisplayListBuilder::Restore() { } if (!current_info().has_deferred_save_op) { - SaveOpBase* op = reinterpret_cast(storage_.get() + + SaveOpBase* op = reinterpret_cast(storage_.base() + current_info().save_offset); FML_CHECK(op->type == DisplayListOpType::kSave || op->type == DisplayListOpType::kSaveLayer || @@ -605,7 +597,7 @@ void DisplayListBuilder::RestoreLayer() { SkRect content_bounds = current_layer().layer_local_accumulator.bounds(); SaveLayerOpBase* layer_op = reinterpret_cast( - storage_.get() + current_info().save_offset); + storage_.base() + current_info().save_offset); FML_CHECK(layer_op->type == DisplayListOpType::kSaveLayer || layer_op->type == DisplayListOpType::kSaveLayerBackdrop); diff --git a/display_list/dl_builder.h b/display_list/dl_builder.h index 0dfb365c11993..f8132d819d62d 100644 --- a/display_list/dl_builder.h +++ b/display_list/dl_builder.h @@ -497,8 +497,7 @@ class DisplayListBuilder final : public virtual DlCanvas, void checkForDeferredSave(); DisplayListStorage storage_; - size_t used_ = 0u; - size_t allocated_ = 0u; + std::vector offsets_; uint32_t render_op_count_ = 0u; uint32_t depth_ = 0u; // Most rendering ops will use 1 depth value, but some attributes may diff --git a/display_list/dl_op_records.h b/display_list/dl_op_records.h index d08602994e876..ca8e5156d2559 100644 --- a/display_list/dl_op_records.h +++ b/display_list/dl_op_records.h @@ -58,8 +58,9 @@ struct DLOp { static constexpr uint32_t kDepthInc = 0; static constexpr uint32_t kRenderOpInc = 0; - DisplayListOpType type : 8; - uint32_t size : 24; + explicit DLOp(DisplayListOpType type) : type(type) {} + + const DisplayListOpType type; DisplayListCompare equals(const DLOp* other) const { return DisplayListCompare::kUseBulkCompare; @@ -67,34 +68,35 @@ struct DLOp { }; // 4 byte header + 4 byte payload packs into minimum 8 bytes -#define DEFINE_SET_BOOL_OP(name) \ - struct Set##name##Op final : DLOp { \ - static constexpr auto kType = DisplayListOpType::kSet##name; \ - \ - explicit Set##name##Op(bool value) : value(value) {} \ - \ - const bool value; \ - \ - void dispatch(DlOpReceiver& receiver) const { \ - receiver.set##name(value); \ - } \ +#define DEFINE_SET_BOOL_OP(name) \ + struct Set##name##Op final : DLOp { \ + static constexpr auto kType = DisplayListOpType::kSet##name; \ + \ + explicit Set##name##Op(bool value) : DLOp(kType), value(value) {} \ + \ + const bool value; \ + \ + void dispatch(DlOpReceiver& receiver) const { \ + receiver.set##name(value); \ + } \ }; DEFINE_SET_BOOL_OP(AntiAlias) DEFINE_SET_BOOL_OP(InvertColors) #undef DEFINE_SET_BOOL_OP // 4 byte header + 4 byte payload packs into minimum 8 bytes -#define DEFINE_SET_ENUM_OP(name) \ - struct SetStroke##name##Op final : DLOp { \ - static constexpr auto kType = DisplayListOpType::kSetStroke##name; \ - \ - explicit SetStroke##name##Op(DlStroke##name value) : value(value) {} \ - \ - const DlStroke##name value; \ - \ - void dispatch(DlOpReceiver& receiver) const { \ - receiver.setStroke##name(value); \ - } \ +#define DEFINE_SET_ENUM_OP(name) \ + struct SetStroke##name##Op final : DLOp { \ + static constexpr auto kType = DisplayListOpType::kSetStroke##name; \ + \ + explicit SetStroke##name##Op(DlStroke##name value) \ + : DLOp(kType), value(value) {} \ + \ + const DlStroke##name value; \ + \ + void dispatch(DlOpReceiver& receiver) const { \ + receiver.setStroke##name(value); \ + } \ }; DEFINE_SET_ENUM_OP(Cap) DEFINE_SET_ENUM_OP(Join) @@ -104,7 +106,7 @@ DEFINE_SET_ENUM_OP(Join) struct SetStyleOp final : DLOp { static constexpr auto kType = DisplayListOpType::kSetStyle; - explicit SetStyleOp(DlDrawStyle style) : style(style) {} + explicit SetStyleOp(DlDrawStyle style) : DLOp(kType), style(style) {} const DlDrawStyle style; @@ -116,7 +118,7 @@ struct SetStyleOp final : DLOp { struct SetStrokeWidthOp final : DLOp { static constexpr auto kType = DisplayListOpType::kSetStrokeWidth; - explicit SetStrokeWidthOp(float width) : width(width) {} + explicit SetStrokeWidthOp(float width) : DLOp(kType), width(width) {} const float width; @@ -128,7 +130,7 @@ struct SetStrokeWidthOp final : DLOp { struct SetStrokeMiterOp final : DLOp { static constexpr auto kType = DisplayListOpType::kSetStrokeMiter; - explicit SetStrokeMiterOp(float limit) : limit(limit) {} + explicit SetStrokeMiterOp(float limit) : DLOp(kType), limit(limit) {} const float limit; @@ -141,7 +143,7 @@ struct SetStrokeMiterOp final : DLOp { struct SetColorOp final : DLOp { static constexpr auto kType = DisplayListOpType::kSetColor; - explicit SetColorOp(DlColor color) : color(color) {} + explicit SetColorOp(DlColor color) : DLOp(kType), color(color) {} const DlColor color; @@ -151,7 +153,7 @@ struct SetColorOp final : DLOp { struct SetBlendModeOp final : DLOp { static constexpr auto kType = DisplayListOpType::kSetBlendMode; - explicit SetBlendModeOp(DlBlendMode mode) : mode(mode) {} + explicit SetBlendModeOp(DlBlendMode mode) : DLOp(kType), mode(mode) {} const DlBlendMode mode; @@ -170,7 +172,7 @@ struct SetBlendModeOp final : DLOp { struct Clear##name##Op final : DLOp { \ static constexpr auto kType = DisplayListOpType::kClear##name; \ \ - Clear##name##Op() {} \ + Clear##name##Op() : DLOp(kType) {} \ \ void dispatch(DlOpReceiver& receiver) const { \ receiver.set##name(nullptr); \ @@ -179,7 +181,7 @@ struct SetBlendModeOp final : DLOp { struct SetPod##name##Op final : DLOp { \ static constexpr auto kType = DisplayListOpType::kSetPod##name; \ \ - SetPod##name##Op() {} \ + SetPod##name##Op() : DLOp(kType) {} \ \ void dispatch(DlOpReceiver& receiver) const { \ const Dl##name* filter = reinterpret_cast(this + 1); \ @@ -198,7 +200,8 @@ struct SetImageColorSourceOp : DLOp { static constexpr auto kType = DisplayListOpType::kSetImageColorSource; explicit SetImageColorSourceOp(const DlImageColorSource* source) - : source(source->image(), + : DLOp(kType), + source(source->image(), source->horizontal_tile_mode(), source->vertical_tile_mode(), source->sampling(), @@ -218,7 +221,8 @@ struct SetRuntimeEffectColorSourceOp : DLOp { explicit SetRuntimeEffectColorSourceOp( const DlRuntimeEffectColorSource* source) - : source(source->runtime_effect(), + : DLOp(kType), + source(source->runtime_effect(), source->samplers(), source->uniform_data()) {} @@ -239,7 +243,7 @@ struct SetSharedImageFilterOp : DLOp { static constexpr auto kType = DisplayListOpType::kSetSharedImageFilter; explicit SetSharedImageFilterOp(const DlImageFilter* filter) - : filter(filter->shared()) {} + : DLOp(kType), filter(filter->shared()) {} const std::shared_ptr filter; @@ -259,10 +263,14 @@ struct SaveOpBase : DLOp { static constexpr uint32_t kDepthInc = 0; static constexpr uint32_t kRenderOpInc = 1; - SaveOpBase() : options(), restore_index(0) {} + explicit SaveOpBase(DisplayListOpType type) + : DLOp(type), options(), restore_index(0), total_content_depth(0) {} - explicit SaveOpBase(const SaveLayerOptions& options) - : options(options), restore_index(0), total_content_depth(0) {} + SaveOpBase(DisplayListOpType type, const SaveLayerOptions& options) + : DLOp(type), + options(options), + restore_index(0), + total_content_depth(0) {} // options parameter is only used by saveLayer operations, but since // it packs neatly into the empty space created by laying out the rest @@ -276,7 +284,7 @@ struct SaveOpBase : DLOp { struct SaveOp final : SaveOpBase { static constexpr auto kType = DisplayListOpType::kSave; - SaveOp() : SaveOpBase() {} + SaveOp() : SaveOpBase(kType) {} void dispatch(DlOpReceiver& receiver) const { receiver.save(total_content_depth); @@ -285,8 +293,10 @@ struct SaveOp final : SaveOpBase { // The base struct for all saveLayer() ops // 16 byte SaveOpBase + 20 byte payload packs into 36 bytes struct SaveLayerOpBase : SaveOpBase { - SaveLayerOpBase(const SaveLayerOptions& options, const DlRect& rect) - : SaveOpBase(options), rect(rect) {} + SaveLayerOpBase(DisplayListOpType type, + const SaveLayerOptions& options, + const DlRect& rect) + : SaveOpBase(type, options), rect(rect) {} DlRect rect; DlBlendMode max_blend_mode = DlBlendMode::kClear; @@ -297,7 +307,7 @@ struct SaveLayerOp final : SaveLayerOpBase { static constexpr auto kType = DisplayListOpType::kSaveLayer; SaveLayerOp(const SaveLayerOptions& options, const DlRect& rect) - : SaveLayerOpBase(options, rect) {} + : SaveLayerOpBase(kType, options, rect) {} void dispatch(DlOpReceiver& receiver) const { receiver.saveLayer(rect, options, total_content_depth, max_blend_mode); @@ -312,7 +322,7 @@ struct SaveLayerBackdropOp final : SaveLayerOpBase { const DlRect& rect, const DlImageFilter* backdrop, std::optional backdrop_id) - : SaveLayerOpBase(options, rect), + : SaveLayerOpBase(kType, options, rect), backdrop(backdrop->shared()), backdrop_id_(backdrop_id) {} @@ -338,7 +348,7 @@ struct RestoreOp final : DLOp { static constexpr uint32_t kDepthInc = 0; static constexpr uint32_t kRenderOpInc = 1; - RestoreOp() {} + RestoreOp() : DLOp(kType) {} void dispatch(DlOpReceiver& receiver) const { // receiver.restore(); @@ -348,13 +358,16 @@ struct RestoreOp final : DLOp { struct TransformClipOpBase : DLOp { static constexpr uint32_t kDepthInc = 0; static constexpr uint32_t kRenderOpInc = 1; + + explicit TransformClipOpBase(DisplayListOpType type) : DLOp(type) {} }; // 4 byte header + 8 byte payload uses 12 bytes but is rounded up to 16 bytes // (4 bytes unused) struct TranslateOp final : TransformClipOpBase { static constexpr auto kType = DisplayListOpType::kTranslate; - TranslateOp(DlScalar tx, DlScalar ty) : tx(tx), ty(ty) {} + TranslateOp(DlScalar tx, DlScalar ty) + : TransformClipOpBase(kType), tx(tx), ty(ty) {} const DlScalar tx; const DlScalar ty; @@ -368,7 +381,8 @@ struct TranslateOp final : TransformClipOpBase { struct ScaleOp final : TransformClipOpBase { static constexpr auto kType = DisplayListOpType::kScale; - ScaleOp(DlScalar sx, DlScalar sy) : sx(sx), sy(sy) {} + ScaleOp(DlScalar sx, DlScalar sy) + : TransformClipOpBase(kType), sx(sx), sy(sy) {} const DlScalar sx; const DlScalar sy; @@ -381,7 +395,8 @@ struct ScaleOp final : TransformClipOpBase { struct RotateOp final : TransformClipOpBase { static constexpr auto kType = DisplayListOpType::kRotate; - explicit RotateOp(DlScalar degrees) : degrees(degrees) {} + explicit RotateOp(DlScalar degrees) + : TransformClipOpBase(kType), degrees(degrees) {} const DlScalar degrees; @@ -394,7 +409,8 @@ struct RotateOp final : TransformClipOpBase { struct SkewOp final : TransformClipOpBase { static constexpr auto kType = DisplayListOpType::kSkew; - SkewOp(DlScalar sx, DlScalar sy) : sx(sx), sy(sy) {} + SkewOp(DlScalar sx, DlScalar sy) + : TransformClipOpBase(kType), sx(sx), sy(sy) {} const DlScalar sx; const DlScalar sy; @@ -411,7 +427,9 @@ struct Transform2DAffineOp final : TransformClipOpBase { // clang-format off Transform2DAffineOp(DlScalar mxx, DlScalar mxy, DlScalar mxt, DlScalar myx, DlScalar myy, DlScalar myt) - : mxx(mxx), mxy(mxy), mxt(mxt), myx(myx), myy(myy), myt(myt) {} + : TransformClipOpBase(kType), + mxx(mxx), mxy(mxy), mxt(mxt), + myx(myx), myy(myy), myt(myt) {} // clang-format on const DlScalar mxx, mxy, mxt; @@ -433,7 +451,8 @@ struct TransformFullPerspectiveOp final : TransformClipOpBase { DlScalar myx, DlScalar myy, DlScalar myz, DlScalar myt, DlScalar mzx, DlScalar mzy, DlScalar mzz, DlScalar mzt, DlScalar mwx, DlScalar mwy, DlScalar mwz, DlScalar mwt) - : mxx(mxx), mxy(mxy), mxz(mxz), mxt(mxt), + : TransformClipOpBase(kType), + mxx(mxx), mxy(mxy), mxz(mxz), mxt(mxt), myx(myx), myy(myy), myz(myz), myt(myt), mzx(mzx), mzy(mzy), mzz(mzz), mzt(mzt), mwx(mwx), mwy(mwy), mwz(mwz), mwt(mwt) {} @@ -456,7 +475,7 @@ struct TransformFullPerspectiveOp final : TransformClipOpBase { struct TransformResetOp final : TransformClipOpBase { static constexpr auto kType = DisplayListOpType::kTransformReset; - TransformResetOp() = default; + TransformResetOp() : TransformClipOpBase(kType) {} void dispatch(DlOpReceiver& receiver) const { // receiver.transformReset(); @@ -478,7 +497,7 @@ struct TransformResetOp final : TransformClipOpBase { static constexpr auto kType = DisplayListOpType::kClip##clipop##shapename; \ \ Clip##clipop##shapename##Op(shapetype shape, bool is_aa) \ - : is_aa(is_aa), shape(shape) {} \ + : TransformClipOpBase(kType), is_aa(is_aa), shape(shape) {} \ \ const bool is_aa; \ const shapetype shape; \ @@ -501,7 +520,7 @@ DEFINE_CLIP_SHAPE_OP(RoundRect, DlRoundRect, Difference) static constexpr auto kType = DisplayListOpType::kClip##clipop##Path; \ \ Clip##clipop##PathOp(const DlPath& path, bool is_aa) \ - : is_aa(is_aa), path(path) {} \ + : TransformClipOpBase(kType), is_aa(is_aa), path(path) {} \ \ const bool is_aa; \ const DlPath path; \ @@ -523,13 +542,15 @@ DEFINE_CLIP_PATH_OP(Difference) struct DrawOpBase : DLOp { static constexpr uint32_t kDepthInc = 1; static constexpr uint32_t kRenderOpInc = 1; + + explicit DrawOpBase(DisplayListOpType type) : DLOp(type) {} }; // 4 byte header + no payload uses minimum 8 bytes (4 bytes unused) struct DrawPaintOp final : DrawOpBase { static constexpr auto kType = DisplayListOpType::kDrawPaint; - DrawPaintOp() {} + DrawPaintOp() : DrawOpBase(kType) {} void dispatch(DlOpReceiver& receiver) const { // receiver.drawPaint(); @@ -540,7 +561,8 @@ struct DrawPaintOp final : DrawOpBase { struct DrawColorOp final : DrawOpBase { static constexpr auto kType = DisplayListOpType::kDrawColor; - DrawColorOp(DlColor color, DlBlendMode mode) : color(color), mode(mode) {} + DrawColorOp(DlColor color, DlBlendMode mode) + : DrawOpBase(kType), color(color), mode(mode) {} const DlColor color; const DlBlendMode mode; @@ -556,17 +578,18 @@ struct DrawColorOp final : DrawOpBase { // SkOval is same as DlRect // DlRoundRect is 48 more bytes, using 52 bytes which rounds up to 56 bytes // total (4 bytes unused) -#define DEFINE_DRAW_1ARG_OP(op_name, arg_type, arg_name) \ - struct Draw##op_name##Op final : DrawOpBase { \ - static constexpr auto kType = DisplayListOpType::kDraw##op_name; \ - \ - explicit Draw##op_name##Op(arg_type arg_name) : arg_name(arg_name) {} \ - \ - const arg_type arg_name; \ - \ - void dispatch(DlOpReceiver& receiver) const { \ - receiver.draw##op_name(arg_name); \ - } \ +#define DEFINE_DRAW_1ARG_OP(op_name, arg_type, arg_name) \ + struct Draw##op_name##Op final : DrawOpBase { \ + static constexpr auto kType = DisplayListOpType::kDraw##op_name; \ + \ + explicit Draw##op_name##Op(arg_type arg_name) \ + : DrawOpBase(kType), arg_name(arg_name) {} \ + \ + const arg_type arg_name; \ + \ + void dispatch(DlOpReceiver& receiver) const { \ + receiver.draw##op_name(arg_name); \ + } \ }; DEFINE_DRAW_1ARG_OP(Rect, DlRect, rect) DEFINE_DRAW_1ARG_OP(Oval, DlRect, oval) @@ -578,7 +601,7 @@ DEFINE_DRAW_1ARG_OP(RoundRect, DlRoundRect, rrect) struct DrawPathOp final : DrawOpBase { static constexpr auto kType = DisplayListOpType::kDrawPath; - explicit DrawPathOp(const DlPath& path) : path(path) {} + explicit DrawPathOp(const DlPath& path) : DrawOpBase(kType), path(path) {} const DlPath path; @@ -603,7 +626,7 @@ struct DrawPathOp final : DrawOpBase { static constexpr auto kType = DisplayListOpType::kDraw##op_name; \ \ Draw##op_name##Op(type1 name1, type2 name2) \ - : name1(name1), name2(name2) {} \ + : DrawOpBase(kType), name1(name1), name2(name2) {} \ \ const type1 name1; \ const type2 name2; \ @@ -625,7 +648,11 @@ struct DrawDashedLineOp final : DrawOpBase { const DlPoint& p1, DlScalar on_length, DlScalar off_length) - : p0(p0), p1(p1), on_length(on_length), off_length(off_length) {} + : DrawOpBase(kType), + p0(p0), + p1(p1), + on_length(on_length), + off_length(off_length) {} const DlPoint p0; const DlPoint p1; @@ -642,7 +669,11 @@ struct DrawArcOp final : DrawOpBase { static constexpr auto kType = DisplayListOpType::kDrawArc; DrawArcOp(DlRect bounds, DlScalar start, DlScalar sweep, bool center) - : bounds(bounds), start(start), sweep(sweep), center(center) {} + : DrawOpBase(kType), + bounds(bounds), + start(start), + sweep(sweep), + center(center) {} const DlRect bounds; const DlScalar start; @@ -664,7 +695,8 @@ struct DrawArcOp final : DrawOpBase { struct Draw##name##Op final : DrawOpBase { \ static constexpr auto kType = DisplayListOpType::kDraw##name; \ \ - explicit Draw##name##Op(uint32_t count) : count(count) {} \ + explicit Draw##name##Op(uint32_t count) \ + : DrawOpBase(kType), count(count) {} \ \ const uint32_t count; \ \ @@ -684,7 +716,7 @@ struct DrawVerticesOp final : DrawOpBase { explicit DrawVerticesOp(const std::shared_ptr& vertices, DlBlendMode mode) - : mode(mode), vertices(vertices) {} + : DrawOpBase(kType), mode(mode), vertices(vertices) {} const DlBlendMode mode; const std::shared_ptr vertices; @@ -696,29 +728,32 @@ struct DrawVerticesOp final : DrawOpBase { // 4 byte header + 40 byte payload uses 44 bytes but is rounded up to 48 bytes // (4 bytes unused) -#define DEFINE_DRAW_IMAGE_OP(name, with_attributes) \ - struct name##Op final : DrawOpBase { \ - static constexpr auto kType = DisplayListOpType::k##name; \ - \ - name##Op(const sk_sp& image, \ - const DlPoint& point, \ - DlImageSampling sampling) \ - : point(point), sampling(sampling), image(std::move(image)) {} \ - \ - const DlPoint point; \ - const DlImageSampling sampling; \ - const sk_sp image; \ - \ - void dispatch(DlOpReceiver& receiver) const { \ - receiver.drawImage(image, point, sampling, with_attributes); \ - } \ - \ - DisplayListCompare equals(const name##Op* other) const { \ - return (point == other->point && sampling == other->sampling && \ - image->Equals(other->image)) \ - ? DisplayListCompare::kEqual \ - : DisplayListCompare::kNotEqual; \ - } \ +#define DEFINE_DRAW_IMAGE_OP(name, with_attributes) \ + struct name##Op final : DrawOpBase { \ + static constexpr auto kType = DisplayListOpType::k##name; \ + \ + name##Op(const sk_sp& image, \ + const DlPoint& point, \ + DlImageSampling sampling) \ + : DrawOpBase(kType), \ + point(point), \ + sampling(sampling), \ + image(std::move(image)) {} \ + \ + const DlPoint point; \ + const DlImageSampling sampling; \ + const sk_sp image; \ + \ + void dispatch(DlOpReceiver& receiver) const { \ + receiver.drawImage(image, point, sampling, with_attributes); \ + } \ + \ + DisplayListCompare equals(const name##Op* other) const { \ + return (point == other->point && sampling == other->sampling && \ + image->Equals(other->image)) \ + ? DisplayListCompare::kEqual \ + : DisplayListCompare::kNotEqual; \ + } \ }; DEFINE_DRAW_IMAGE_OP(DrawImage, false) DEFINE_DRAW_IMAGE_OP(DrawImageWithAttr, true) @@ -735,7 +770,8 @@ struct DrawImageRectOp final : DrawOpBase { DlImageSampling sampling, bool render_with_attributes, DlCanvas::SrcRectConstraint constraint) - : src(src), + : DrawOpBase(kType), + src(src), dst(dst), sampling(sampling), render_with_attributes(render_with_attributes), @@ -765,33 +801,37 @@ struct DrawImageRectOp final : DrawOpBase { }; // 4 byte header + 44 byte payload packs efficiently into 48 bytes -#define DEFINE_DRAW_IMAGE_NINE_OP(name, render_with_attributes) \ - struct name##Op final : DrawOpBase { \ - static constexpr auto kType = DisplayListOpType::k##name; \ - static constexpr uint32_t kDepthInc = 9; \ - \ - name##Op(const sk_sp& image, \ - const DlIRect& center, \ - const DlRect& dst, \ - DlFilterMode mode) \ - : center(center), dst(dst), mode(mode), image(std::move(image)) {} \ - \ - const DlIRect center; \ - const DlRect dst; \ - const DlFilterMode mode; \ - const sk_sp image; \ - \ - void dispatch(DlOpReceiver& receiver) const { \ - receiver.drawImageNine(image, center, dst, mode, \ - render_with_attributes); \ - } \ - \ - DisplayListCompare equals(const name##Op* other) const { \ - return (center == other->center && dst == other->dst && \ - mode == other->mode && image->Equals(other->image)) \ - ? DisplayListCompare::kEqual \ - : DisplayListCompare::kNotEqual; \ - } \ +#define DEFINE_DRAW_IMAGE_NINE_OP(name, render_with_attributes) \ + struct name##Op final : DrawOpBase { \ + static constexpr auto kType = DisplayListOpType::k##name; \ + static constexpr uint32_t kDepthInc = 9; \ + \ + name##Op(const sk_sp& image, \ + const DlIRect& center, \ + const DlRect& dst, \ + DlFilterMode mode) \ + : DrawOpBase(kType), \ + center(center), \ + dst(dst), \ + mode(mode), \ + image(std::move(image)) {} \ + \ + const DlIRect center; \ + const DlRect dst; \ + const DlFilterMode mode; \ + const sk_sp image; \ + \ + void dispatch(DlOpReceiver& receiver) const { \ + receiver.drawImageNine(image, center, dst, mode, \ + render_with_attributes); \ + } \ + \ + DisplayListCompare equals(const name##Op* other) const { \ + return (center == other->center && dst == other->dst && \ + mode == other->mode && image->Equals(other->image)) \ + ? DisplayListCompare::kEqual \ + : DisplayListCompare::kNotEqual; \ + } \ }; DEFINE_DRAW_IMAGE_NINE_OP(DrawImageNine, false) DEFINE_DRAW_IMAGE_NINE_OP(DrawImageNineWithAttr, true) @@ -805,13 +845,15 @@ DEFINE_DRAW_IMAGE_NINE_OP(DrawImageNineWithAttr, true) // DlColor list only packs well if the count is even, otherwise there // can be 4 unusued bytes at the end. struct DrawAtlasBaseOp : DrawOpBase { - DrawAtlasBaseOp(const sk_sp& atlas, + DrawAtlasBaseOp(DisplayListOpType type, + const sk_sp& atlas, int count, DlBlendMode mode, DlImageSampling sampling, bool has_colors, bool render_with_attributes) - : count(count), + : DrawOpBase(type), + count(count), mode_index(static_cast(mode)), has_colors(has_colors), render_with_attributes(render_with_attributes), @@ -854,7 +896,8 @@ struct DrawAtlasOp final : DrawAtlasBaseOp { DlImageSampling sampling, bool has_colors, bool render_with_attributes) - : DrawAtlasBaseOp(atlas, + : DrawAtlasBaseOp(kType, + atlas, count, mode, sampling, @@ -894,7 +937,8 @@ struct DrawAtlasCulledOp final : DrawAtlasBaseOp { bool has_colors, const DlRect& cull_rect, bool render_with_attributes) - : DrawAtlasBaseOp(atlas, + : DrawAtlasBaseOp(kType, + atlas, count, mode, sampling, @@ -931,7 +975,7 @@ struct DrawDisplayListOp final : DrawOpBase { explicit DrawDisplayListOp(const sk_sp& display_list, DlScalar opacity) - : opacity(opacity), display_list(display_list) {} + : DrawOpBase(kType), opacity(opacity), display_list(display_list) {} DlScalar opacity; const sk_sp display_list; @@ -954,7 +998,7 @@ struct DrawTextBlobOp final : DrawOpBase { static constexpr auto kType = DisplayListOpType::kDrawTextBlob; DrawTextBlobOp(const sk_sp& blob, DlScalar x, DlScalar y) - : x(x), y(y), blob(blob) {} + : DrawOpBase(kType), x(x), y(y), blob(blob) {} const DlScalar x; const DlScalar y; @@ -971,7 +1015,7 @@ struct DrawTextFrameOp final : DrawOpBase { DrawTextFrameOp(const std::shared_ptr& text_frame, DlScalar x, DlScalar y) - : x(x), y(y), text_frame(text_frame) {} + : DrawOpBase(kType), x(x), y(y), text_frame(text_frame) {} const DlScalar x; const DlScalar y; @@ -991,7 +1035,11 @@ struct DrawTextFrameOp final : DrawOpBase { DlColor color, \ DlScalar elevation, \ DlScalar dpr) \ - : color(color), elevation(elevation), dpr(dpr), path(path) {} \ + : DrawOpBase(kType), \ + color(color), \ + elevation(elevation), \ + dpr(dpr), \ + path(path) {} \ \ const DlColor color; \ const DlScalar elevation; \ diff --git a/display_list/dl_storage.cc b/display_list/dl_storage.cc new file mode 100644 index 0000000000000..eb53f0acbd617 --- /dev/null +++ b/display_list/dl_storage.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/dl_storage.h" + +namespace flutter { + +static constexpr inline bool is_power_of_two(int value) { + return (value & (value - 1)) == 0; +} + +void DisplayListStorage::realloc(size_t count) { + ptr_.reset(static_cast(std::realloc(ptr_.release(), count))); + FML_CHECK(ptr_); + allocated_ = count; +} + +uint8_t* DisplayListStorage::allocate(size_t needed) { + if (used_ + needed > allocated_) { + static_assert(is_power_of_two(kDLPageSize), + "This math needs updating for non-pow2."); + // Next greater multiple of DL_BUILDER_PAGE. + size_t new_size = (used_ + needed + kDLPageSize) & ~(kDLPageSize - 1); + size_t old_size = allocated_; + realloc(new_size); + FML_CHECK(ptr_.get()); + FML_CHECK(allocated_ == new_size); + FML_CHECK(allocated_ >= old_size); + FML_CHECK(used_ + needed <= allocated_); + memset(ptr_.get() + used_, 0, allocated_ - old_size); + } + uint8_t* ret = ptr_.get() + used_; + used_ += needed; + FML_CHECK(used_ <= allocated_); + return ret; +} + +DisplayListStorage::DisplayListStorage(DisplayListStorage&& source) { + ptr_ = std::move(source.ptr_); + used_ = source.used_; + allocated_ = source.allocated_; + source.used_ = 0u; + source.allocated_ = 0u; +} + +} // namespace flutter diff --git a/display_list/dl_storage.h b/display_list/dl_storage.h new file mode 100644 index 0000000000000..39967d88e6712 --- /dev/null +++ b/display_list/dl_storage.h @@ -0,0 +1,54 @@ +// 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_DL_STORAGE_H_ +#define FLUTTER_DISPLAY_LIST_DL_STORAGE_H_ + +#include + +#include "flutter/fml/logging.h" + +namespace flutter { + +// Manages a buffer allocated with malloc. +class DisplayListStorage { + public: + static const constexpr size_t kDLPageSize = 4096u; + + DisplayListStorage() = default; + DisplayListStorage(DisplayListStorage&&); + + /// Returns a pointer to the base of the storage. + uint8_t* base() { return ptr_.get(); } + const uint8_t* base() const { return ptr_.get(); } + + /// Trims the storage to the currently allocated size and invalidates + /// any outstanding pointers into the storage. + void trim() { realloc(used_); } + + /// Returns the currently allocated size + size_t size() const { return used_; } + + /// Returns the maximum currently allocated space + size_t capacity() const { return allocated_; } + + /// Ensures the indicated number of bytes are available and returns + /// a pointer to that memory within the storage while also invalidating + /// any other outstanding pointers into the storage. + uint8_t* allocate(size_t needed); + + private: + void realloc(size_t count); + + struct FreeDeleter { + void operator()(uint8_t* p) { std::free(p); } + }; + std::unique_ptr ptr_; + size_t used_ = 0u; + size_t allocated_ = 0u; +}; + +} // namespace flutter + +#endif // FLUTTER_DISPLAY_LIST_DL_STORAGE_H_ diff --git a/display_list/dl_storage_unittests.cc b/display_list/dl_storage_unittests.cc new file mode 100644 index 0000000000000..78d6dcef0cbc4 --- /dev/null +++ b/display_list/dl_storage_unittests.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/dl_storage.h" + +#include "flutter/testing/testing.h" + +namespace flutter { +namespace testing { + +TEST(DisplayListStorage, DefaultConstructed) { + DisplayListStorage storage; + EXPECT_EQ(storage.base(), nullptr); + EXPECT_EQ(storage.size(), 0u); + EXPECT_EQ(storage.capacity(), 0u); +} + +TEST(DisplayListStorage, Allocation) { + DisplayListStorage storage; + EXPECT_NE(storage.allocate(10u), nullptr); + EXPECT_NE(storage.base(), nullptr); + EXPECT_EQ(storage.size(), 10u); + EXPECT_EQ(storage.capacity(), DisplayListStorage::kDLPageSize); +} + +TEST(DisplayListStorage, PostMove) { + DisplayListStorage original; + EXPECT_NE(original.allocate(10u), nullptr); + + DisplayListStorage moved = std::move(original); + + // NOLINTBEGIN(bugprone-use-after-move) + // NOLINTBEGIN(clang-analyzer-cplusplus.Move) + EXPECT_EQ(original.base(), nullptr); + EXPECT_EQ(original.size(), 0u); + EXPECT_EQ(original.capacity(), 0u); + // NOLINTEND(clang-analyzer-cplusplus.Move) + // NOLINTEND(bugprone-use-after-move) + + EXPECT_NE(moved.base(), nullptr); + EXPECT_EQ(moved.size(), 10u); + EXPECT_EQ(moved.capacity(), DisplayListStorage::kDLPageSize); +} + +} // namespace testing +} // namespace flutter From 3dced3ba812d48c9da1b387ed41fd085889812b5 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Thu, 24 Oct 2024 11:52:45 -0700 Subject: [PATCH 2/3] remove unnnecessary debugging prints --- display_list/display_list_unittests.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index 8264e79e0e888..95470007e524d 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -459,15 +459,15 @@ TEST_F(DisplayListTest, BuildRestoresAttributes) { DlOpReceiver& receiver = ToReceiver(builder); receiver.setAntiAlias(true); - FML_LOG(ERROR) << *builder.Build(); + builder.Build(); check_defaults(builder, cull_rect); receiver.setInvertColors(true); - FML_LOG(ERROR) << *builder.Build(); + builder.Build(); check_defaults(builder, cull_rect); receiver.setColor(DlColor::kRed()); - FML_LOG(ERROR) << *builder.Build(); + builder.Build(); check_defaults(builder, cull_rect); receiver.setBlendMode(DlBlendMode::kColorBurn); From 53e3ee903a9b97a29ea5e0d5b051b8887eff0ece Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Thu, 24 Oct 2024 13:17:34 -0700 Subject: [PATCH 3/3] reorganize the way the data is transferred during Build --- display_list/display_list.cc | 2 +- display_list/display_list_unittests.cc | 19 +++++++++++++++++++ display_list/dl_builder.cc | 14 +++++++++----- display_list/dl_storage.cc | 15 +++++++++++++++ display_list/dl_storage.h | 14 ++++++++++---- 5 files changed, 54 insertions(+), 10 deletions(-) diff --git a/display_list/display_list.cc b/display_list/display_list.cc index 557c3802d7828..fc823dee83209 100644 --- a/display_list/display_list.cc +++ b/display_list/display_list.cc @@ -60,7 +60,7 @@ DisplayList::DisplayList(DisplayListStorage&& storage, root_is_unbounded_(root_is_unbounded), max_root_blend_mode_(max_root_blend_mode), rtree_(std::move(rtree)) { - FML_DCHECK(offsets_.capacity() == offsets_.size()); + FML_DCHECK(storage_.capacity() == storage_.size()); } DisplayList::~DisplayList() { diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index 95470007e524d..6ead9386770ec 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -253,6 +253,25 @@ TEST_F(DisplayListTest, EmptyRebuild) { ASSERT_TRUE(dl2->Equals(dl3)); } +TEST_F(DisplayListTest, NopReusedBuildIsReallyEmpty) { + DisplayListBuilder builder; + builder.DrawRect(DlRect::MakeLTRB(0.0f, 0.0f, 10.0f, 10.0f), DlPaint()); + + { + auto dl1 = builder.Build(); + EXPECT_EQ(dl1->op_count(), 1u); + EXPECT_GT(dl1->bytes(), sizeof(DisplayList)); + EXPECT_EQ(dl1->GetBounds(), DlRect::MakeLTRB(0.0f, 0.0f, 10.0f, 10.0f)); + } + + { + auto dl2 = builder.Build(); + EXPECT_EQ(dl2->op_count(), 0u); + EXPECT_EQ(dl2->bytes(), sizeof(DisplayList)); + EXPECT_EQ(dl2->GetBounds(), DlRect()); + } +} + TEST_F(DisplayListTest, GeneralReceiverInitialValues) { DisplayListGeneralReceiver receiver; diff --git a/display_list/dl_builder.cc b/display_list/dl_builder.cc index ba70463fc1fc8..06164781fb7cc 100644 --- a/display_list/dl_builder.cc +++ b/display_list/dl_builder.cc @@ -103,12 +103,16 @@ sk_sp DisplayListBuilder::Build() { Init(rtree != nullptr); storage_.trim(); - offsets_.shrink_to_fit(); + DisplayListStorage storage; + std::vector offsets; + std::swap(offsets, offsets_); + std::swap(storage, storage_); + return sk_sp(new DisplayList( - std::move(storage_), std::move(offsets_), count, nested_bytes, - nested_count, total_depth, bounds, opacity_compatible, is_safe, - affects_transparency, max_root_blend_mode, root_has_backdrop_filter, - root_is_unbounded, std::move(rtree))); + std::move(storage), std::move(offsets), count, nested_bytes, nested_count, + total_depth, bounds, opacity_compatible, is_safe, affects_transparency, + max_root_blend_mode, root_has_backdrop_filter, root_is_unbounded, + std::move(rtree))); } static constexpr DlRect kEmpty = DlRect(); diff --git a/display_list/dl_storage.cc b/display_list/dl_storage.cc index eb53f0acbd617..9a36d461980f0 100644 --- a/display_list/dl_storage.cc +++ b/display_list/dl_storage.cc @@ -44,4 +44,19 @@ DisplayListStorage::DisplayListStorage(DisplayListStorage&& source) { source.allocated_ = 0u; } +void DisplayListStorage::reset() { + ptr_.reset(); + used_ = 0u; + allocated_ = 0u; +} + +DisplayListStorage& DisplayListStorage::operator=(DisplayListStorage&& source) { + ptr_ = std::move(source.ptr_); + used_ = source.used_; + allocated_ = source.allocated_; + source.used_ = 0u; + source.allocated_ = 0u; + return *this; +} + } // namespace flutter diff --git a/display_list/dl_storage.h b/display_list/dl_storage.h index 39967d88e6712..89108d75b5fa2 100644 --- a/display_list/dl_storage.h +++ b/display_list/dl_storage.h @@ -23,10 +23,6 @@ class DisplayListStorage { uint8_t* base() { return ptr_.get(); } const uint8_t* base() const { return ptr_.get(); } - /// Trims the storage to the currently allocated size and invalidates - /// any outstanding pointers into the storage. - void trim() { realloc(used_); } - /// Returns the currently allocated size size_t size() const { return used_; } @@ -38,6 +34,15 @@ class DisplayListStorage { /// any other outstanding pointers into the storage. uint8_t* allocate(size_t needed); + /// Trims the storage to the currently allocated size and invalidates + /// any outstanding pointers into the storage. + void trim() { realloc(used_); } + + /// Resets the storage and allocation of the object to an empty state + void reset(); + + DisplayListStorage& operator=(DisplayListStorage&& other); + private: void realloc(size_t count); @@ -45,6 +50,7 @@ class DisplayListStorage { void operator()(uint8_t* p) { std::free(p); } }; std::unique_ptr ptr_; + size_t used_ = 0u; size_t allocated_ = 0u; };