From d78fce8ce60202d6f6abc0eaff79826cb5837ae0 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Sat, 10 Dec 2022 19:12:21 -0800 Subject: [PATCH 1/2] Conceptual prototype for RTree culling in DisplayList --- display_list/display_list.cc | 26 +- display_list/display_list_builder.cc | 18 +- display_list/display_list_builder.h | 6 +- display_list/display_list_ops.h | 559 +++++++++++++++--------- display_list/display_list_test_utils.cc | 18 +- display_list/display_list_unittests.cc | 2 +- 6 files changed, 401 insertions(+), 228 deletions(-) diff --git a/display_list/display_list.cc b/display_list/display_list.cc index 202333459656f..b4a1df7264a74 100644 --- a/display_list/display_list.cc +++ b/display_list/display_list.cc @@ -75,14 +75,34 @@ void DisplayList::ComputeRTree() { void DisplayList::Dispatch(Dispatcher& dispatcher, uint8_t* ptr, uint8_t* end) const { + DispatchContext context = { + .dispatcher = dispatcher, + + .cur_offset = 0, + .next_render_offset = 0, + + .next_restore_offset = std::numeric_limits::max(), + }; + + uint8_t* op_base = storage_.get(); while (ptr < end) { + context.cur_offset = ptr - op_base; + // The following line would be used to cull render ops by setting + // it to the next rendering op that matched in the RTree instead + // of setting it to cur_offset. For now it is using the cur_offset + // as the next_render_offset to test the op_needed tests in the + // DLOps, but if there is no RTree culling going on, it would be + // just as valid to leave the next_render_offset as 0 for the + // entire run (in fact, that scenario has also been tested and + // works just fine to avoid any culling). + context.next_render_offset = context.cur_offset; auto op = reinterpret_cast(ptr); ptr += op->size; FML_DCHECK(ptr <= end); switch (op->type) { -#define DL_OP_DISPATCH(name) \ - case DisplayListOpType::k##name: \ - static_cast(op)->dispatch(dispatcher); \ +#define DL_OP_DISPATCH(name) \ + case DisplayListOpType::k##name: \ + static_cast(op)->dispatch(context); \ break; FOR_EACH_DISPLAY_LIST_OP(DL_OP_DISPATCH) diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index 7aa654730e245..96a84a4703f0a 100644 --- a/display_list/display_list_builder.cc +++ b/display_list/display_list_builder.cc @@ -423,7 +423,9 @@ void DisplayListBuilder::setAttributesFromPaint( void DisplayListBuilder::checkForDeferredSave() { if (current_layer_->has_deferred_save_op_) { + size_t save_offset = used_; Push(0, 1); + current_layer_->save_offset = save_offset; current_layer_->has_deferred_save_op_ = false; } } @@ -436,8 +438,14 @@ void DisplayListBuilder::save() { void DisplayListBuilder::restore() { if (layer_stack_.size() > 1) { + SaveOpBase* op; if (!current_layer_->has_deferred_save_op_) { + op = reinterpret_cast(storage_.get() + + current_layer_->save_offset); + op->restore_offset = used_; Push(0, 1); + } else { + op = nullptr; } // Grab the current layer info before we push the restore // on the stack. @@ -445,6 +453,10 @@ void DisplayListBuilder::restore() { layer_stack_.pop_back(); current_layer_ = &layer_stack_.back(); if (layer_info.has_layer) { + // Layers are never deferred for now, we need to update the + // following code if we ever do saveLayer culling... + FML_DCHECK(!layer_info.has_deferred_save_op_); + FML_DCHECK(op != nullptr); if (layer_info.is_group_opacity_compatible()) { // We are now going to go back and modify the matching saveLayer // call to add the option indicating it can distribute an opacity @@ -458,8 +470,6 @@ void DisplayListBuilder::restore() { // in the DisplayList are only allowed *during* the build phase. // Once built, the DisplayList records must remain read only to // ensure consistency of rendering and |Equals()| behavior. - SaveLayerOp* op = reinterpret_cast( - storage_.get() + layer_info.save_layer_offset); op->options = op->options.with_can_distribute_opacity(); } } else { @@ -486,11 +496,11 @@ void DisplayListBuilder::saveLayer(const SkRect* bounds, size_t save_layer_offset = used_; if (backdrop) { bounds // - ? Push(0, 1, *bounds, options, backdrop) + ? Push(0, 1, options, *bounds, backdrop) : Push(0, 1, options, backdrop); } else { bounds // - ? Push(0, 1, *bounds, options) + ? Push(0, 1, options, *bounds) : Push(0, 1, options); } CheckLayerOpacityCompatibility(options.renders_with_attributes()); diff --git a/display_list/display_list_builder.h b/display_list/display_list_builder.h index 2f2cd863bd96c..a1e3643b8608e 100644 --- a/display_list/display_list_builder.h +++ b/display_list/display_list_builder.h @@ -384,9 +384,9 @@ class DisplayListBuilder final : public virtual Dispatcher, struct LayerInfo { LayerInfo(const SkM44& matrix, const SkRect& clip_bounds, - size_t save_layer_offset = 0, + size_t save_offset = 0, bool has_layer = false) - : save_layer_offset(save_layer_offset), + : save_offset(save_offset), has_layer(has_layer), cannot_inherit_opacity(false), has_compatible_op(false), @@ -407,7 +407,7 @@ class DisplayListBuilder final : public virtual Dispatcher, // the records inside the saveLayer that may impact how the saveLayer // is handled (e.g., |cannot_inherit_opacity| == false). // This offset is only valid if |has_layer| is true. - size_t save_layer_offset; + size_t save_offset; bool has_deferred_save_op_ = false; diff --git a/display_list/display_list_ops.h b/display_list/display_list_ops.h index 061f29365df45..4f475677a7377 100644 --- a/display_list/display_list_ops.h +++ b/display_list/display_list_ops.h @@ -15,6 +15,28 @@ namespace flutter { +typedef uint64_t dl_offset; + +struct DispatchContext { + Dispatcher& dispatcher; + + dl_offset cur_offset; + dl_offset next_render_offset; + + dl_offset next_restore_offset; + + struct SaveInfo { + SaveInfo(dl_offset previous_restore_offset, bool save_was_needed) + : previous_restore_offset(previous_restore_offset), + save_was_needed(save_was_needed) {} + + dl_offset previous_restore_offset; + bool save_was_needed; + }; + + std::vector save_infos; +}; + // Most Ops can be bulk compared using memcmp because they contain // only numeric values or constructs that are constructed from numeric // values. @@ -69,8 +91,8 @@ struct DLOp { \ const bool value; \ \ - void dispatch(Dispatcher& dispatcher) const { \ - dispatcher.set##name(value); \ + void dispatch(DispatchContext& ctx) const { \ + ctx.dispatcher.set##name(value); \ } \ }; DEFINE_SET_BOOL_OP(AntiAlias) @@ -87,8 +109,8 @@ DEFINE_SET_BOOL_OP(InvertColors) \ const DlStroke##name value; \ \ - void dispatch(Dispatcher& dispatcher) const { \ - dispatcher.setStroke##name(value); \ + void dispatch(DispatchContext& ctx) const { \ + ctx.dispatcher.setStroke##name(value); \ } \ }; DEFINE_SET_ENUM_OP(Cap) @@ -103,7 +125,7 @@ struct SetStyleOp final : DLOp { const DlDrawStyle style; - void dispatch(Dispatcher& dispatcher) const { dispatcher.setStyle(style); } + void dispatch(DispatchContext& ctx) const { ctx.dispatcher.setStyle(style); } }; // 4 byte header + 4 byte payload packs into minimum 8 bytes struct SetStrokeWidthOp final : DLOp { @@ -113,8 +135,8 @@ struct SetStrokeWidthOp final : DLOp { const float width; - void dispatch(Dispatcher& dispatcher) const { - dispatcher.setStrokeWidth(width); + void dispatch(DispatchContext& ctx) const { + ctx.dispatcher.setStrokeWidth(width); } }; // 4 byte header + 4 byte payload packs into minimum 8 bytes @@ -125,8 +147,8 @@ struct SetStrokeMiterOp final : DLOp { const float limit; - void dispatch(Dispatcher& dispatcher) const { - dispatcher.setStrokeMiter(limit); + void dispatch(DispatchContext& ctx) const { + ctx.dispatcher.setStrokeMiter(limit); } }; @@ -138,7 +160,7 @@ struct SetColorOp final : DLOp { const DlColor color; - void dispatch(Dispatcher& dispatcher) const { dispatcher.setColor(color); } + void dispatch(DispatchContext& ctx) const { ctx.dispatcher.setColor(color); } }; // 4 byte header + 4 byte payload packs into minimum 8 bytes struct SetBlendModeOp final : DLOp { @@ -148,7 +170,9 @@ struct SetBlendModeOp final : DLOp { const DlBlendMode mode; - void dispatch(Dispatcher& dispatcher) const { dispatcher.setBlendMode(mode); } + void dispatch(DispatchContext& ctx) const { + ctx.dispatcher.setBlendMode(mode); + } }; // Clear: 4 byte header + unused 4 byte payload uses 8 bytes @@ -162,8 +186,8 @@ struct SetBlendModeOp final : DLOp { \ Clear##name##Op() {} \ \ - void dispatch(Dispatcher& dispatcher) const { \ - dispatcher.set##name(nullptr); \ + void dispatch(DispatchContext& ctx) const { \ + ctx.dispatcher.set##name(nullptr); \ } \ }; \ struct Set##name##Op final : DLOp { \ @@ -173,8 +197,8 @@ struct SetBlendModeOp final : DLOp { \ sk_sp field; \ \ - void dispatch(Dispatcher& dispatcher) const { \ - dispatcher.set##name(field); \ + void dispatch(DispatchContext& ctx) const { \ + ctx.dispatcher.set##name(field); \ } \ }; DEFINE_SET_CLEAR_SKREF_OP(Blender, blender) @@ -195,8 +219,8 @@ DEFINE_SET_CLEAR_SKREF_OP(Blender, blender) \ Clear##name##Op() {} \ \ - void dispatch(Dispatcher& dispatcher) const { \ - dispatcher.set##name(nullptr); \ + void dispatch(DispatchContext& ctx) const { \ + ctx.dispatcher.set##name(nullptr); \ } \ }; \ struct SetPod##name##Op final : DLOp { \ @@ -204,9 +228,9 @@ DEFINE_SET_CLEAR_SKREF_OP(Blender, blender) \ SetPod##name##Op() {} \ \ - void dispatch(Dispatcher& dispatcher) const { \ + void dispatch(DispatchContext& ctx) const { \ const Dl##name* filter = reinterpret_cast(this + 1); \ - dispatcher.set##name(filter); \ + ctx.dispatcher.set##name(filter); \ } \ }; \ struct SetSk##name##Op final : DLOp { \ @@ -216,9 +240,9 @@ DEFINE_SET_CLEAR_SKREF_OP(Blender, blender) \ sk_sp field; \ \ - void dispatch(Dispatcher& dispatcher) const { \ + void dispatch(DispatchContext& ctx) const { \ DlUnknown##name dl_filter(field); \ - dispatcher.set##name(&dl_filter); \ + ctx.dispatcher.set##name(&dl_filter); \ } \ }; DEFINE_SET_CLEAR_DLATTR_OP(ColorFilter, ColorFilter, filter) @@ -242,8 +266,8 @@ struct SetImageColorSourceOp : DLOp { const DlImageColorSource source; - void dispatch(Dispatcher& dispatcher) const { - dispatcher.setColorSource(&source); + void dispatch(DispatchContext& ctx) const { + ctx.dispatcher.setColorSource(&source); } }; @@ -259,8 +283,8 @@ struct SetRuntimeEffectColorSourceOp : DLOp { const DlRuntimeEffectColorSource source; - void dispatch(Dispatcher& dispatcher) const { - dispatcher.setColorSource(&source); + void dispatch(DispatchContext& ctx) const { + ctx.dispatcher.setColorSource(&source); } DisplayListCompare equals(const SetRuntimeEffectColorSourceOp* other) const { @@ -278,8 +302,8 @@ struct SetSharedImageFilterOp : DLOp { const std::shared_ptr filter; - void dispatch(Dispatcher& dispatcher) const { - dispatcher.setImageFilter(filter.get()); + void dispatch(DispatchContext& ctx) const { + ctx.dispatcher.setImageFilter(filter.get()); } DisplayListCompare equals(const SetSharedImageFilterOp* other) const { @@ -288,53 +312,80 @@ struct SetSharedImageFilterOp : DLOp { } }; -// 4 byte header + no payload uses minimum 8 bytes (4 bytes unused) -struct SaveOp final : DLOp { +// The base object for all save() and saveLayer() ops +// 4 byte header + 20 bytes payload packs neatly into 24 bytes +struct SaveOpBase : DLOp { + SaveOpBase() : options(), restore_offset(0) {} + + SaveOpBase(const SaveLayerOptions options) + : options(options), restore_offset(0) {} + + // options parameter is only used by saveLayer operations, but since + // it packs neatly into the empty space created by laying out the 64-bit + // offsets, it can be stored for free and defaulted to 0 for save operations. + SaveLayerOptions options; + dl_offset restore_offset; + + inline bool save_needed(DispatchContext& ctx) const { + bool needed = ctx.next_render_offset <= restore_offset; + ctx.save_infos.emplace_back(ctx.next_restore_offset, needed); + ctx.next_restore_offset = restore_offset; + return needed; + } +}; +// 24 byte SaveOpBase with no additional data (options is unsed here) +struct SaveOp final : SaveOpBase { static const auto kType = DisplayListOpType::kSave; - SaveOp() {} + SaveOp() : SaveOpBase() {} - void dispatch(Dispatcher& dispatcher) const { dispatcher.save(); } + void dispatch(DispatchContext& ctx) const { + if (save_needed(ctx)) { + ctx.dispatcher.save(); + } + } }; -// 4 byte header + 4 byte payload packs into minimum 8 bytes -struct SaveLayerOp final : DLOp { +// 24 byte SaveOpBase with no additional data +struct SaveLayerOp final : SaveOpBase { static const auto kType = DisplayListOpType::kSaveLayer; - explicit SaveLayerOp(const SaveLayerOptions options) : options(options) {} + explicit SaveLayerOp(const SaveLayerOptions options) : SaveOpBase(options) {} - SaveLayerOptions options; - - void dispatch(Dispatcher& dispatcher) const { - dispatcher.saveLayer(nullptr, options); + void dispatch(DispatchContext& ctx) const { + if (save_needed(ctx)) { + ctx.dispatcher.saveLayer(nullptr, options); + } } }; -// 4 byte header + 20 byte payload packs evenly into 24 bytes -struct SaveLayerBoundsOp final : DLOp { +// 24 byte SaveOpBase + 16 byte payload packs evenly into 40 bytes +struct SaveLayerBoundsOp final : SaveOpBase { static const auto kType = DisplayListOpType::kSaveLayerBounds; - SaveLayerBoundsOp(SkRect rect, const SaveLayerOptions options) - : options(options), rect(rect) {} + SaveLayerBoundsOp(const SaveLayerOptions options, const SkRect& rect) + : SaveOpBase(options), rect(rect) {} - SaveLayerOptions options; const SkRect rect; - void dispatch(Dispatcher& dispatcher) const { - dispatcher.saveLayer(&rect, options); + void dispatch(DispatchContext& ctx) const { + if (save_needed(ctx)) { + ctx.dispatcher.saveLayer(&rect, options); + } } }; -// 4 byte header + 20 byte payload packs into minimum 24 bytes -struct SaveLayerBackdropOp final : DLOp { +// 24 byte SaveOpBase + 16 byte payload packs into minimum 40 bytes +struct SaveLayerBackdropOp final : SaveOpBase { static const auto kType = DisplayListOpType::kSaveLayerBackdrop; explicit SaveLayerBackdropOp(const SaveLayerOptions options, const DlImageFilter* backdrop) - : options(options), backdrop(backdrop->shared()) {} + : SaveOpBase(options), backdrop(backdrop->shared()) {} - SaveLayerOptions options; const std::shared_ptr backdrop; - void dispatch(Dispatcher& dispatcher) const { - dispatcher.saveLayer(nullptr, options, backdrop.get()); + void dispatch(DispatchContext& ctx) const { + if (save_needed(ctx)) { + ctx.dispatcher.saveLayer(nullptr, options, backdrop.get()); + } } DisplayListCompare equals(const SaveLayerBackdropOp* other) const { @@ -343,21 +394,22 @@ struct SaveLayerBackdropOp final : DLOp { : DisplayListCompare::kNotEqual; } }; -// 4 byte header + 36 byte payload packs evenly into 36 bytes -struct SaveLayerBackdropBoundsOp final : DLOp { +// 24 byte SaveOpBase + 32 byte payload packs into minimum 56 bytes +struct SaveLayerBackdropBoundsOp final : SaveOpBase { static const auto kType = DisplayListOpType::kSaveLayerBackdropBounds; - SaveLayerBackdropBoundsOp(SkRect rect, - const SaveLayerOptions options, + SaveLayerBackdropBoundsOp(const SaveLayerOptions options, + const SkRect& rect, const DlImageFilter* backdrop) - : options(options), rect(rect), backdrop(backdrop->shared()) {} + : SaveOpBase(options), rect(rect), backdrop(backdrop->shared()) {} - SaveLayerOptions options; const SkRect rect; const std::shared_ptr backdrop; - void dispatch(Dispatcher& dispatcher) const { - dispatcher.saveLayer(&rect, options, backdrop.get()); + void dispatch(DispatchContext& ctx) const { + if (save_needed(ctx)) { + ctx.dispatcher.saveLayer(&rect, options, backdrop.get()); + } } DisplayListCompare equals(const SaveLayerBackdropBoundsOp* other) const { @@ -373,12 +425,24 @@ struct RestoreOp final : DLOp { RestoreOp() {} - void dispatch(Dispatcher& dispatcher) const { dispatcher.restore(); } + void dispatch(DispatchContext& ctx) const { + DispatchContext::SaveInfo& info = ctx.save_infos.back(); + if (info.save_was_needed) { + ctx.dispatcher.restore(); + } + ctx.next_restore_offset = info.previous_restore_offset; + ctx.save_infos.pop_back(); + } }; +struct TransformClipOpBase : DLOp { + inline bool op_needed(const DispatchContext& context) const { + return context.next_render_offset <= context.next_restore_offset; + } +}; // 4 byte header + 8 byte payload uses 12 bytes but is rounded up to 16 bytes // (4 bytes unused) -struct TranslateOp final : DLOp { +struct TranslateOp final : TransformClipOpBase { static const auto kType = DisplayListOpType::kTranslate; TranslateOp(SkScalar tx, SkScalar ty) : tx(tx), ty(ty) {} @@ -386,11 +450,15 @@ struct TranslateOp final : DLOp { const SkScalar tx; const SkScalar ty; - void dispatch(Dispatcher& dispatcher) const { dispatcher.translate(tx, ty); } + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + ctx.dispatcher.translate(tx, ty); + } + } }; // 4 byte header + 8 byte payload uses 12 bytes but is rounded up to 16 bytes // (4 bytes unused) -struct ScaleOp final : DLOp { +struct ScaleOp final : TransformClipOpBase { static const auto kType = DisplayListOpType::kScale; ScaleOp(SkScalar sx, SkScalar sy) : sx(sx), sy(sy) {} @@ -398,21 +466,29 @@ struct ScaleOp final : DLOp { const SkScalar sx; const SkScalar sy; - void dispatch(Dispatcher& dispatcher) const { dispatcher.scale(sx, sy); } + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + ctx.dispatcher.scale(sx, sy); + } + } }; // 4 byte header + 4 byte payload packs into minimum 8 bytes -struct RotateOp final : DLOp { +struct RotateOp final : TransformClipOpBase { static const auto kType = DisplayListOpType::kRotate; explicit RotateOp(SkScalar degrees) : degrees(degrees) {} const SkScalar degrees; - void dispatch(Dispatcher& dispatcher) const { dispatcher.rotate(degrees); } + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + ctx.dispatcher.rotate(degrees); + } + } }; // 4 byte header + 8 byte payload uses 12 bytes but is rounded up to 16 bytes // (4 bytes unused) -struct SkewOp final : DLOp { +struct SkewOp final : TransformClipOpBase { static const auto kType = DisplayListOpType::kSkew; SkewOp(SkScalar sx, SkScalar sy) : sx(sx), sy(sy) {} @@ -420,11 +496,15 @@ struct SkewOp final : DLOp { const SkScalar sx; const SkScalar sy; - void dispatch(Dispatcher& dispatcher) const { dispatcher.skew(sx, sy); } + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + ctx.dispatcher.skew(sx, sy); + } + } }; // 4 byte header + 24 byte payload uses 28 bytes but is rounded up to 32 bytes // (4 bytes unused) -struct Transform2DAffineOp final : DLOp { +struct Transform2DAffineOp final : TransformClipOpBase { static const auto kType = DisplayListOpType::kTransform2DAffine; // clang-format off @@ -436,14 +516,16 @@ struct Transform2DAffineOp final : DLOp { const SkScalar mxx, mxy, mxt; const SkScalar myx, myy, myt; - void dispatch(Dispatcher& dispatcher) const { - dispatcher.transform2DAffine(mxx, mxy, mxt, // - myx, myy, myt); + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + ctx.dispatcher.transform2DAffine(mxx, mxy, mxt, // + myx, myy, myt); + } } }; // 4 byte header + 64 byte payload uses 68 bytes which is rounded up to 72 bytes // (4 bytes unused) -struct TransformFullPerspectiveOp final : DLOp { +struct TransformFullPerspectiveOp final : TransformClipOpBase { static const auto kType = DisplayListOpType::kTransformFullPerspective; // clang-format off @@ -463,21 +545,27 @@ struct TransformFullPerspectiveOp final : DLOp { const SkScalar mzx, mzy, mzz, mzt; const SkScalar mwx, mwy, mwz, mwt; - void dispatch(Dispatcher& dispatcher) const { - dispatcher.transformFullPerspective(mxx, mxy, mxz, mxt, // - myx, myy, myz, myt, // - mzx, mzy, mzz, mzt, // - mwx, mwy, mwz, mwt); + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + ctx.dispatcher.transformFullPerspective(mxx, mxy, mxz, mxt, // + myx, myy, myz, myt, // + mzx, mzy, mzz, mzt, // + mwx, mwy, mwz, mwt); + } } }; // 4 byte header with no payload. -struct TransformResetOp final : DLOp { +struct TransformResetOp final : TransformClipOpBase { static const auto kType = DisplayListOpType::kTransformReset; TransformResetOp() = default; - void dispatch(Dispatcher& dispatcher) const { dispatcher.transformReset(); } + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + ctx.dispatcher.transformReset(); + } + } }; // 4 byte header + 4 byte common payload packs into minimum 8 bytes @@ -491,7 +579,7 @@ struct TransformResetOp final : DLOp { // packing into more bytes than needed (even when they are declared as // packed bit fields!) #define DEFINE_CLIP_SHAPE_OP(shapetype, clipop) \ - struct Clip##clipop##shapetype##Op final : DLOp { \ + struct Clip##clipop##shapetype##Op final : TransformClipOpBase { \ static const auto kType = DisplayListOpType::kClip##clipop##shapetype; \ \ Clip##clipop##shapetype##Op(Sk##shapetype shape, bool is_aa) \ @@ -500,8 +588,10 @@ struct TransformResetOp final : DLOp { const bool is_aa; \ const Sk##shapetype shape; \ \ - void dispatch(Dispatcher& dispatcher) const { \ - dispatcher.clip##shapetype(shape, SkClipOp::k##clipop, is_aa); \ + void dispatch(DispatchContext& ctx) const { \ + if (op_needed(ctx)) { \ + ctx.dispatcher.clip##shapetype(shape, SkClipOp::k##clipop, is_aa); \ + } \ } \ }; DEFINE_CLIP_SHAPE_OP(Rect, Intersect) @@ -511,7 +601,7 @@ DEFINE_CLIP_SHAPE_OP(RRect, Difference) #undef DEFINE_CLIP_SHAPE_OP #define DEFINE_CLIP_PATH_OP(clipop) \ - struct Clip##clipop##PathOp final : DLOp { \ + struct Clip##clipop##PathOp final : TransformClipOpBase { \ static const auto kType = DisplayListOpType::kClip##clipop##Path; \ \ Clip##clipop##PathOp(SkPath path, bool is_aa) \ @@ -520,8 +610,10 @@ DEFINE_CLIP_SHAPE_OP(RRect, Difference) const bool is_aa; \ const SkPath path; \ \ - void dispatch(Dispatcher& dispatcher) const { \ - dispatcher.clipPath(path, SkClipOp::k##clipop, is_aa); \ + void dispatch(DispatchContext& ctx) const { \ + if (op_needed(ctx)) { \ + ctx.dispatcher.clipPath(path, SkClipOp::k##clipop, is_aa); \ + } \ } \ \ DisplayListCompare equals(const Clip##clipop##PathOp* other) const { \ @@ -534,17 +626,27 @@ DEFINE_CLIP_PATH_OP(Intersect) DEFINE_CLIP_PATH_OP(Difference) #undef DEFINE_CLIP_PATH_OP +struct DrawOpBase : DLOp { + inline bool op_needed(const DispatchContext& ctx) const { + return ctx.cur_offset >= ctx.next_render_offset; + } +}; + // 4 byte header + no payload uses minimum 8 bytes (4 bytes unused) -struct DrawPaintOp final : DLOp { +struct DrawPaintOp final : DrawOpBase { static const auto kType = DisplayListOpType::kDrawPaint; DrawPaintOp() {} - void dispatch(Dispatcher& dispatcher) const { dispatcher.drawPaint(); } + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + ctx.dispatcher.drawPaint(); + } + } }; // 4 byte header + 8 byte payload uses 12 bytes but is rounded up to 16 bytes // (4 bytes unused) -struct DrawColorOp final : DLOp { +struct DrawColorOp final : DrawOpBase { static const auto kType = DisplayListOpType::kDrawColor; DrawColorOp(DlColor color, DlBlendMode mode) : color(color), mode(mode) {} @@ -552,8 +654,10 @@ struct DrawColorOp final : DLOp { const DlColor color; const DlBlendMode mode; - void dispatch(Dispatcher& dispatcher) const { - dispatcher.drawColor(color, mode); + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + ctx.dispatcher.drawColor(color, mode); + } } }; @@ -563,15 +667,17 @@ struct DrawColorOp final : DLOp { // SkOval is same as SkRect // SkRRect is 52 more bytes, which packs efficiently into 56 bytes total #define DEFINE_DRAW_1ARG_OP(op_name, arg_type, arg_name) \ - struct Draw##op_name##Op final : DLOp { \ + struct Draw##op_name##Op final : DrawOpBase { \ static const 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(Dispatcher& dispatcher) const { \ - dispatcher.draw##op_name(arg_name); \ + void dispatch(DispatchContext& ctx) const { \ + if (op_needed(ctx)) { \ + ctx.dispatcher.draw##op_name(arg_name); \ + } \ } \ }; DEFINE_DRAW_1ARG_OP(Rect, SkRect, rect) @@ -581,14 +687,18 @@ DEFINE_DRAW_1ARG_OP(RRect, SkRRect, rrect) // 4 byte header + 16 byte payload uses 20 bytes but is rounded up to 24 bytes // (4 bytes unused) -struct DrawPathOp final : DLOp { +struct DrawPathOp final : DrawOpBase { static const auto kType = DisplayListOpType::kDrawPath; explicit DrawPathOp(SkPath path) : path(path) {} const SkPath path; - void dispatch(Dispatcher& dispatcher) const { dispatcher.drawPath(path); } + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + ctx.dispatcher.drawPath(path); + } + } DisplayListCompare equals(const DrawPathOp* other) const { return path == other->path ? DisplayListCompare::kEqual @@ -603,7 +713,7 @@ struct DrawPathOp final : DLOp { // 2 x SkRRect is 104 more bytes, using 108 and rounding up to 112 bytes total // (4 bytes unused) #define DEFINE_DRAW_2ARG_OP(op_name, type1, name1, type2, name2) \ - struct Draw##op_name##Op final : DLOp { \ + struct Draw##op_name##Op final : DrawOpBase { \ static const auto kType = DisplayListOpType::kDraw##op_name; \ \ Draw##op_name##Op(type1 name1, type2 name2) \ @@ -612,8 +722,10 @@ struct DrawPathOp final : DLOp { const type1 name1; \ const type2 name2; \ \ - void dispatch(Dispatcher& dispatcher) const { \ - dispatcher.draw##op_name(name1, name2); \ + void dispatch(DispatchContext& ctx) const { \ + if (op_needed(ctx)) { \ + ctx.dispatcher.draw##op_name(name1, name2); \ + } \ } \ }; DEFINE_DRAW_2ARG_OP(Line, SkPoint, p0, SkPoint, p1) @@ -622,7 +734,7 @@ DEFINE_DRAW_2ARG_OP(DRRect, SkRRect, outer, SkRRect, inner) #undef DEFINE_DRAW_2ARG_OP // 4 byte header + 28 byte payload packs efficiently into 32 bytes -struct DrawArcOp final : DLOp { +struct DrawArcOp final : DrawOpBase { static const auto kType = DisplayListOpType::kDrawArc; DrawArcOp(SkRect bounds, SkScalar start, SkScalar sweep, bool center) @@ -633,8 +745,10 @@ struct DrawArcOp final : DLOp { const SkScalar sweep; const bool center; - void dispatch(Dispatcher& dispatcher) const { - dispatcher.drawArc(bounds, start, sweep, center); + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + ctx.dispatcher.drawArc(bounds, start, sweep, center); + } } }; @@ -644,18 +758,20 @@ struct DrawArcOp final : DLOp { // so this op will always pack efficiently // The point type is packed into 3 different OpTypes to avoid expanding // the fixed payload beyond the 8 bytes -#define DEFINE_DRAW_POINTS_OP(name, mode) \ - struct Draw##name##Op final : DLOp { \ - static const auto kType = DisplayListOpType::kDraw##name; \ - \ - explicit Draw##name##Op(uint32_t count) : count(count) {} \ - \ - const uint32_t count; \ - \ - void dispatch(Dispatcher& dispatcher) const { \ - const SkPoint* pts = reinterpret_cast(this + 1); \ - dispatcher.drawPoints(SkCanvas::PointMode::mode, count, pts); \ - } \ +#define DEFINE_DRAW_POINTS_OP(name, mode) \ + struct Draw##name##Op final : DrawOpBase { \ + static const auto kType = DisplayListOpType::kDraw##name; \ + \ + explicit Draw##name##Op(uint32_t count) : count(count) {} \ + \ + const uint32_t count; \ + \ + void dispatch(DispatchContext& ctx) const { \ + if (op_needed(ctx)) { \ + const SkPoint* pts = reinterpret_cast(this + 1); \ + ctx.dispatcher.drawPoints(SkCanvas::PointMode::mode, count, pts); \ + } \ + } \ }; DEFINE_DRAW_POINTS_OP(Points, kPoints_PointMode); DEFINE_DRAW_POINTS_OP(Lines, kLines_PointMode); @@ -669,21 +785,24 @@ DEFINE_DRAW_POINTS_OP(Polygon, kPolygon_PointMode); // Note that the DlVertices object ends with an array of 16-bit // indices so the alignment can be up to 6 bytes off leading to // up to 6 bytes of overhead -struct DrawVerticesOp final : DLOp { +struct DrawVerticesOp final : DrawOpBase { static const auto kType = DisplayListOpType::kDrawVertices; DrawVerticesOp(DlBlendMode mode) : mode(mode) {} const DlBlendMode mode; - void dispatch(Dispatcher& dispatcher) const { - const DlVertices* vertices = reinterpret_cast(this + 1); - dispatcher.drawVertices(vertices, mode); + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + const DlVertices* vertices = + reinterpret_cast(this + 1); + ctx.dispatcher.drawVertices(vertices, mode); + } } }; // 4 byte header + 12 byte payload packs efficiently into 16 bytes -struct DrawSkVerticesOp final : DLOp { +struct DrawSkVerticesOp final : DrawOpBase { static const auto kType = DisplayListOpType::kDrawSkVertices; DrawSkVerticesOp(sk_sp vertices, SkBlendMode mode) @@ -692,29 +811,33 @@ struct DrawSkVerticesOp final : DLOp { const SkBlendMode mode; const sk_sp vertices; - void dispatch(Dispatcher& dispatcher) const { - dispatcher.drawSkVertices(vertices, mode); + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + ctx.dispatcher.drawSkVertices(vertices, mode); + } } }; // 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 : DLOp { \ - static const auto kType = DisplayListOpType::k##name; \ - \ - name##Op(const sk_sp image, \ - const SkPoint& point, \ - DlImageSampling sampling) \ - : point(point), sampling(sampling), image(std::move(image)) {} \ - \ - const SkPoint point; \ - const DlImageSampling sampling; \ - const sk_sp image; \ - \ - void dispatch(Dispatcher& dispatcher) const { \ - dispatcher.drawImage(image, point, sampling, with_attributes); \ - } \ +#define DEFINE_DRAW_IMAGE_OP(name, with_attributes) \ + struct name##Op final : DrawOpBase { \ + static const auto kType = DisplayListOpType::k##name; \ + \ + name##Op(const sk_sp image, \ + const SkPoint& point, \ + DlImageSampling sampling) \ + : point(point), sampling(sampling), image(std::move(image)) {} \ + \ + const SkPoint point; \ + const DlImageSampling sampling; \ + const sk_sp image; \ + \ + void dispatch(DispatchContext& ctx) const { \ + if (op_needed(ctx)) { \ + ctx.dispatcher.drawImage(image, point, sampling, with_attributes); \ + } \ + } \ }; DEFINE_DRAW_IMAGE_OP(DrawImage, false) DEFINE_DRAW_IMAGE_OP(DrawImageWithAttr, true) @@ -722,7 +845,7 @@ DEFINE_DRAW_IMAGE_OP(DrawImageWithAttr, true) // 4 byte header + 72 byte payload uses 76 bytes but is rounded up to 80 bytes // (4 bytes unused) -struct DrawImageRectOp final : DLOp { +struct DrawImageRectOp final : DrawOpBase { static const auto kType = DisplayListOpType::kDrawImageRect; DrawImageRectOp(const sk_sp image, @@ -745,15 +868,17 @@ struct DrawImageRectOp final : DLOp { const SkCanvas::SrcRectConstraint constraint; const sk_sp image; - void dispatch(Dispatcher& dispatcher) const { - dispatcher.drawImageRect(image, src, dst, sampling, render_with_attributes, - constraint); + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + ctx.dispatcher.drawImageRect(image, src, dst, sampling, + render_with_attributes, constraint); + } } }; // 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 : DLOp { \ + struct name##Op final : DrawOpBase { \ static const auto kType = DisplayListOpType::k##name; \ \ name##Op(const sk_sp image, \ @@ -767,9 +892,11 @@ struct DrawImageRectOp final : DLOp { const DlFilterMode filter; \ const sk_sp image; \ \ - void dispatch(Dispatcher& dispatcher) const { \ - dispatcher.drawImageNine(image, center, dst, filter, \ - render_with_attributes); \ + void dispatch(DispatchContext& ctx) const { \ + if (op_needed(ctx)) { \ + ctx.dispatcher.drawImageNine(image, center, dst, filter, \ + render_with_attributes); \ + } \ } \ }; DEFINE_DRAW_IMAGE_NINE_OP(DrawImageNine, false) @@ -777,7 +904,7 @@ DEFINE_DRAW_IMAGE_NINE_OP(DrawImageNineWithAttr, true) #undef DEFINE_DRAW_IMAGE_NINE_OP // 4 byte header + 60 byte payload packs evenly into 64 bytes -struct DrawImageLatticeOp final : DLOp { +struct DrawImageLatticeOp final : DrawOpBase { static const auto kType = DisplayListOpType::kDrawImageLattice; DrawImageLatticeOp(const sk_sp image, @@ -806,20 +933,22 @@ struct DrawImageLatticeOp final : DLOp { const SkRect dst; const sk_sp image; - void dispatch(Dispatcher& dispatcher) const { - const int* xDivs = reinterpret_cast(this + 1); - const int* yDivs = reinterpret_cast(xDivs + x_count); - const SkColor* colors = - (cell_count == 0) ? nullptr - : reinterpret_cast(yDivs + y_count); - const SkCanvas::Lattice::RectType* types = - (cell_count == 0) - ? nullptr - : reinterpret_cast(colors + - cell_count); - dispatcher.drawImageLattice( - image, {xDivs, yDivs, types, x_count, y_count, &src, colors}, dst, - filter, with_paint); + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + const int* xDivs = reinterpret_cast(this + 1); + const int* yDivs = reinterpret_cast(xDivs + x_count); + const SkColor* colors = + (cell_count == 0) ? nullptr + : reinterpret_cast(yDivs + y_count); + const SkCanvas::Lattice::RectType* types = + (cell_count == 0) + ? nullptr + : reinterpret_cast( + colors + cell_count); + ctx.dispatcher.drawImageLattice( + image, {xDivs, yDivs, types, x_count, y_count, &src, colors}, dst, + filter, with_paint); + } } }; @@ -830,7 +959,7 @@ struct DrawImageLatticeOp final : DLOp { // SkRect list is also a multiple of 16 bytes so it also packs well // DlColor list only packs well if the count is even, otherwise there // can be 4 unusued bytes at the end. -struct DrawAtlasBaseOp : DLOp { +struct DrawAtlasBaseOp : DrawOpBase { DrawAtlasBaseOp(const sk_sp atlas, int count, DlBlendMode mode, @@ -870,14 +999,16 @@ struct DrawAtlasOp final : DrawAtlasBaseOp { has_colors, render_with_attributes) {} - void dispatch(Dispatcher& dispatcher) const { - const SkRSXform* xform = reinterpret_cast(this + 1); - const SkRect* tex = reinterpret_cast(xform + count); - const DlColor* colors = - has_colors ? reinterpret_cast(tex + count) : nullptr; - const DlBlendMode mode = static_cast(mode_index); - dispatcher.drawAtlas(atlas, xform, tex, colors, count, mode, sampling, - nullptr, render_with_attributes); + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + const SkRSXform* xform = reinterpret_cast(this + 1); + const SkRect* tex = reinterpret_cast(xform + count); + const DlColor* colors = + has_colors ? reinterpret_cast(tex + count) : nullptr; + const DlBlendMode mode = static_cast(mode_index); + ctx.dispatcher.drawAtlas(atlas, xform, tex, colors, count, mode, sampling, + nullptr, render_with_attributes); + } } }; @@ -905,19 +1036,21 @@ struct DrawAtlasCulledOp final : DrawAtlasBaseOp { const SkRect cull_rect; - void dispatch(Dispatcher& dispatcher) const { - const SkRSXform* xform = reinterpret_cast(this + 1); - const SkRect* tex = reinterpret_cast(xform + count); - const DlColor* colors = - has_colors ? reinterpret_cast(tex + count) : nullptr; - const DlBlendMode mode = static_cast(mode_index); - dispatcher.drawAtlas(atlas, xform, tex, colors, count, mode, sampling, - &cull_rect, render_with_attributes); + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + const SkRSXform* xform = reinterpret_cast(this + 1); + const SkRect* tex = reinterpret_cast(xform + count); + const DlColor* colors = + has_colors ? reinterpret_cast(tex + count) : nullptr; + const DlBlendMode mode = static_cast(mode_index); + ctx.dispatcher.drawAtlas(atlas, xform, tex, colors, count, mode, sampling, + &cull_rect, render_with_attributes); + } } }; // 4 byte header + 12 byte payload packs evenly into 16 bytes -struct DrawSkPictureOp final : DLOp { +struct DrawSkPictureOp final : DrawOpBase { static const auto kType = DisplayListOpType::kDrawSkPicture; DrawSkPictureOp(sk_sp picture, bool render_with_attributes) @@ -927,13 +1060,15 @@ struct DrawSkPictureOp final : DLOp { const bool render_with_attributes; const sk_sp picture; - void dispatch(Dispatcher& dispatcher) const { - dispatcher.drawPicture(picture, nullptr, render_with_attributes); + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + ctx.dispatcher.drawPicture(picture, nullptr, render_with_attributes); + } } }; // 4 byte header + 52 byte payload packs evenly into 56 bytes -struct DrawSkPictureMatrixOp final : DLOp { +struct DrawSkPictureMatrixOp final : DrawOpBase { static const auto kType = DisplayListOpType::kDrawSkPictureMatrix; DrawSkPictureMatrixOp(sk_sp picture, @@ -947,14 +1082,16 @@ struct DrawSkPictureMatrixOp final : DLOp { const sk_sp picture; const SkMatrix matrix; - void dispatch(Dispatcher& dispatcher) const { - dispatcher.drawPicture(picture, &matrix, render_with_attributes); + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + ctx.dispatcher.drawPicture(picture, &matrix, render_with_attributes); + } } }; // 4 byte header + ptr aligned payload uses 12 bytes round up to 16 // (4 bytes unused) -struct DrawDisplayListOp final : DLOp { +struct DrawDisplayListOp final : DrawOpBase { static const auto kType = DisplayListOpType::kDrawDisplayList; explicit DrawDisplayListOp(const sk_sp display_list) @@ -962,8 +1099,10 @@ struct DrawDisplayListOp final : DLOp { sk_sp display_list; - void dispatch(Dispatcher& dispatcher) const { - dispatcher.drawDisplayList(display_list); + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + ctx.dispatcher.drawDisplayList(display_list); + } } DisplayListCompare equals(const DrawDisplayListOp* other) const { @@ -975,7 +1114,7 @@ struct DrawDisplayListOp final : DLOp { // 4 byte header + 8 payload bytes + an aligned pointer take 24 bytes // (4 unused to align the pointer) -struct DrawTextBlobOp final : DLOp { +struct DrawTextBlobOp final : DrawOpBase { static const auto kType = DisplayListOpType::kDrawTextBlob; DrawTextBlobOp(const sk_sp blob, SkScalar x, SkScalar y) @@ -985,31 +1124,35 @@ struct DrawTextBlobOp final : DLOp { const SkScalar y; const sk_sp blob; - void dispatch(Dispatcher& dispatcher) const { - dispatcher.drawTextBlob(blob, x, y); + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + ctx.dispatcher.drawTextBlob(blob, x, y); + } } }; // 4 byte header + 28 byte payload packs evenly into 32 bytes -#define DEFINE_DRAW_SHADOW_OP(name, transparent_occluder) \ - struct Draw##name##Op final : DLOp { \ - static const auto kType = DisplayListOpType::kDraw##name; \ - \ - Draw##name##Op(const SkPath& path, \ - DlColor color, \ - SkScalar elevation, \ - SkScalar dpr) \ - : color(color), elevation(elevation), dpr(dpr), path(path) {} \ - \ - const DlColor color; \ - const SkScalar elevation; \ - const SkScalar dpr; \ - const SkPath path; \ - \ - void dispatch(Dispatcher& dispatcher) const { \ - dispatcher.drawShadow(path, color, elevation, transparent_occluder, \ - dpr); \ - } \ +#define DEFINE_DRAW_SHADOW_OP(name, transparent_occluder) \ + struct Draw##name##Op final : DrawOpBase { \ + static const auto kType = DisplayListOpType::kDraw##name; \ + \ + Draw##name##Op(const SkPath& path, \ + DlColor color, \ + SkScalar elevation, \ + SkScalar dpr) \ + : color(color), elevation(elevation), dpr(dpr), path(path) {} \ + \ + const DlColor color; \ + const SkScalar elevation; \ + const SkScalar dpr; \ + const SkPath path; \ + \ + void dispatch(DispatchContext& ctx) const { \ + if (op_needed(ctx)) { \ + ctx.dispatcher.drawShadow(path, color, elevation, \ + transparent_occluder, dpr); \ + } \ + } \ }; DEFINE_DRAW_SHADOW_OP(Shadow, false) DEFINE_DRAW_SHADOW_OP(ShadowTransparentOccluder, true) diff --git a/display_list/display_list_test_utils.cc b/display_list/display_list_test_utils.cc index 144fe140d7490..9d8ee65bb9c03 100644 --- a/display_list/display_list_test_utils.cc +++ b/display_list/display_list_test_utils.cc @@ -304,7 +304,7 @@ std::vector CreateAllSaveRestoreOps() { return { {"Save(Layer)+Restore", { - {5, 104, 5, 104, + {5, 112, 5, 112, [](DisplayListBuilder& b) { b.saveLayer(nullptr, SaveLayerOptions::kNoAttributes, &kTestCFImageFilter1); @@ -320,7 +320,7 @@ std::vector CreateAllSaveRestoreOps() { // attributes to be distributed to the children. To prevent those // cases we include at least one clip operation and 2 overlapping // rendering primitives between each save/restore pair. - {5, 88, 5, 88, + {5, 96, 5, 96, [](DisplayListBuilder& b) { b.save(); b.clipRect({0, 0, 25, 25}, SkClipOp::kIntersect, true); @@ -328,7 +328,7 @@ std::vector CreateAllSaveRestoreOps() { b.drawRect({10, 10, 20, 20}); b.restore(); }}, - {5, 88, 5, 88, + {5, 96, 5, 96, [](DisplayListBuilder& b) { b.saveLayer(nullptr, false); b.clipRect({0, 0, 25, 25}, SkClipOp::kIntersect, true); @@ -336,7 +336,7 @@ std::vector CreateAllSaveRestoreOps() { b.drawRect({10, 10, 20, 20}); b.restore(); }}, - {5, 88, 5, 88, + {5, 96, 5, 96, [](DisplayListBuilder& b) { b.saveLayer(nullptr, true); b.clipRect({0, 0, 25, 25}, SkClipOp::kIntersect, true); @@ -344,7 +344,7 @@ std::vector CreateAllSaveRestoreOps() { b.drawRect({10, 10, 20, 20}); b.restore(); }}, - {5, 104, 5, 104, + {5, 112, 5, 112, [](DisplayListBuilder& b) { b.saveLayer(&kTestBounds, false); b.clipRect({0, 0, 25, 25}, SkClipOp::kIntersect, true); @@ -352,7 +352,7 @@ std::vector CreateAllSaveRestoreOps() { b.drawRect({10, 10, 20, 20}); b.restore(); }}, - {5, 104, 5, 104, + {5, 112, 5, 112, [](DisplayListBuilder& b) { b.saveLayer(&kTestBounds, true); b.clipRect({0, 0, 25, 25}, SkClipOp::kIntersect, true); @@ -369,7 +369,7 @@ std::vector CreateAllSaveRestoreOps() { // b.drawRect({10, 10, 20, 20}); // b.restore(); // }}, - {5, 104, 5, 104, + {5, 112, 5, 112, [](DisplayListBuilder& b) { b.saveLayer(nullptr, SaveLayerOptions::kWithAttributes, &kTestCFImageFilter1); @@ -378,7 +378,7 @@ std::vector CreateAllSaveRestoreOps() { b.drawRect({10, 10, 20, 20}); b.restore(); }}, - {5, 120, 5, 120, + {5, 128, 5, 128, [](DisplayListBuilder& b) { b.saveLayer(&kTestBounds, SaveLayerOptions::kNoAttributes, &kTestCFImageFilter1); @@ -387,7 +387,7 @@ std::vector CreateAllSaveRestoreOps() { b.drawRect({10, 10, 20, 20}); b.restore(); }}, - {5, 120, 5, 120, + {5, 128, 5, 128, [](DisplayListBuilder& b) { b.saveLayer(&kTestBounds, SaveLayerOptions::kWithAttributes, &kTestCFImageFilter1); diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index 2a9e5d7cf7e27..d1f54649a3b88 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -1106,7 +1106,7 @@ TEST(DisplayList, FlutterSvgIssue661BoundsWereEmpty) { // This is the more practical result. The bounds are "almost" 0,0,100x100 EXPECT_EQ(display_list->bounds().roundOut(), SkIRect::MakeWH(100, 100)); EXPECT_EQ(display_list->op_count(), 19u); - EXPECT_EQ(display_list->bytes(), sizeof(DisplayList) + 304u); + EXPECT_EQ(display_list->bytes(), sizeof(DisplayList) + 352u); } TEST(DisplayList, TranslateAffectsCurrentTransform) { From 146fba569ad4232bc68edce9e5556eee95401d76 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Sat, 10 Dec 2022 19:19:17 -0800 Subject: [PATCH 2/2] remove potentially ambiguous null ptr --- display_list/display_list_builder.cc | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index 96a84a4703f0a..3ae9a6943018c 100644 --- a/display_list/display_list_builder.cc +++ b/display_list/display_list_builder.cc @@ -438,14 +438,11 @@ void DisplayListBuilder::save() { void DisplayListBuilder::restore() { if (layer_stack_.size() > 1) { - SaveOpBase* op; + SaveOpBase* op = reinterpret_cast(storage_.get() + + current_layer_->save_offset); if (!current_layer_->has_deferred_save_op_) { - op = reinterpret_cast(storage_.get() + - current_layer_->save_offset); op->restore_offset = used_; Push(0, 1); - } else { - op = nullptr; } // Grab the current layer info before we push the restore // on the stack. @@ -456,7 +453,6 @@ void DisplayListBuilder::restore() { // Layers are never deferred for now, we need to update the // following code if we ever do saveLayer culling... FML_DCHECK(!layer_info.has_deferred_save_op_); - FML_DCHECK(op != nullptr); if (layer_info.is_group_opacity_compatible()) { // We are now going to go back and modify the matching saveLayer // call to add the option indicating it can distribute an opacity