diff --git a/display_list/benchmarking/dl_complexity_gl.cc b/display_list/benchmarking/dl_complexity_gl.cc index 53b70e2c24e47..a1f4c79de5d19 100644 --- a/display_list/benchmarking/dl_complexity_gl.cc +++ b/display_list/benchmarking/dl_complexity_gl.cc @@ -49,7 +49,7 @@ unsigned int DisplayListGLComplexityCalculator::GLHelper::BatchedComplexity() { } void DisplayListGLComplexityCalculator::GLHelper::saveLayer( - const SkRect* bounds, + const SkRect& bounds, const SaveLayerOptions options, const DlImageFilter* backdrop) { if (IsComplex()) { @@ -615,7 +615,8 @@ void DisplayListGLComplexityCalculator::GLHelper::drawDisplayList( } GLHelper helper(Ceiling() - CurrentComplexityScore()); if (opacity < SK_Scalar1 && !display_list->can_apply_group_opacity()) { - helper.saveLayer(nullptr, SaveLayerOptions::kWithAttributes, nullptr); + auto bounds = display_list->bounds(); + helper.saveLayer(bounds, SaveLayerOptions::kWithAttributes, nullptr); } display_list->Dispatch(helper); AccumulateComplexity(helper.ComplexityScore()); diff --git a/display_list/benchmarking/dl_complexity_gl.h b/display_list/benchmarking/dl_complexity_gl.h index d388c6f6a2547..c30926902ade3 100644 --- a/display_list/benchmarking/dl_complexity_gl.h +++ b/display_list/benchmarking/dl_complexity_gl.h @@ -35,7 +35,7 @@ class DisplayListGLComplexityCalculator explicit GLHelper(unsigned int ceiling) : ComplexityCalculatorHelper(ceiling) {} - void saveLayer(const SkRect* bounds, + void saveLayer(const SkRect& bounds, const SaveLayerOptions options, const DlImageFilter* backdrop) override; diff --git a/display_list/benchmarking/dl_complexity_metal.cc b/display_list/benchmarking/dl_complexity_metal.cc index c6b6547afee56..d24c1c643425e 100644 --- a/display_list/benchmarking/dl_complexity_metal.cc +++ b/display_list/benchmarking/dl_complexity_metal.cc @@ -63,7 +63,7 @@ DisplayListMetalComplexityCalculator::MetalHelper::BatchedComplexity() { } void DisplayListMetalComplexityCalculator::MetalHelper::saveLayer( - const SkRect* bounds, + const SkRect& bounds, const SaveLayerOptions options, const DlImageFilter* backdrop) { if (IsComplex()) { @@ -559,7 +559,8 @@ void DisplayListMetalComplexityCalculator::MetalHelper::drawDisplayList( } MetalHelper helper(Ceiling() - CurrentComplexityScore()); if (opacity < SK_Scalar1 && !display_list->can_apply_group_opacity()) { - helper.saveLayer(nullptr, SaveLayerOptions::kWithAttributes, nullptr); + auto bounds = display_list->bounds(); + helper.saveLayer(bounds, SaveLayerOptions::kWithAttributes, nullptr); } display_list->Dispatch(helper); AccumulateComplexity(helper.ComplexityScore()); diff --git a/display_list/benchmarking/dl_complexity_metal.h b/display_list/benchmarking/dl_complexity_metal.h index 4663b47836cd0..303e9c645d996 100644 --- a/display_list/benchmarking/dl_complexity_metal.h +++ b/display_list/benchmarking/dl_complexity_metal.h @@ -35,7 +35,7 @@ class DisplayListMetalComplexityCalculator explicit MetalHelper(unsigned int ceiling) : ComplexityCalculatorHelper(ceiling) {} - void saveLayer(const SkRect* bounds, + void saveLayer(const SkRect& bounds, const SaveLayerOptions options, const DlImageFilter* backdrop) override; diff --git a/display_list/display_list.h b/display_list/display_list.h index d7225865edb92..fe12048418972 100644 --- a/display_list/display_list.h +++ b/display_list/display_list.h @@ -88,9 +88,7 @@ namespace flutter { \ V(Save) \ V(SaveLayer) \ - V(SaveLayerBounds) \ V(SaveLayerBackdrop) \ - V(SaveLayerBackdropBounds) \ V(Restore) \ \ V(Translate) \ @@ -165,6 +163,7 @@ class SaveLayerOptions { SaveLayerOptions without_optimizations() const { SaveLayerOptions options; options.fRendersWithAttributes = fRendersWithAttributes; + options.fBoundsFromCaller = fBoundsFromCaller; return options; } @@ -182,6 +181,33 @@ class SaveLayerOptions { return options; } + // Returns true iff the bounds for the saveLayer operation were provided + // by the caller, otherwise the bounds will have been computed by the + // DisplayListBuilder and provided for reference. + bool bounds_from_caller() const { return fBoundsFromCaller; } + SaveLayerOptions with_bounds_from_caller() const { + SaveLayerOptions options(this); + options.fBoundsFromCaller = true; + return options; + } + SaveLayerOptions without_bounds_from_caller() const { + SaveLayerOptions options(this); + options.fBoundsFromCaller = false; + return options; + } + bool bounds_were_calculated() const { return !fBoundsFromCaller; } + + // Returns true iff the bounds for the saveLayer do not fully cover the + // contained rendering operations. This will only occur if the original + // caller supplied bounds and those bounds were not a strict superset + // of the content bounds computed by the DisplayListBuilder. + bool content_is_clipped() const { return fContentIsClipped; } + SaveLayerOptions with_content_is_clipped() const { + SaveLayerOptions options(this); + options.fContentIsClipped = true; + return options; + } + SaveLayerOptions& operator=(const SaveLayerOptions& other) { flags_ = other.flags_; return *this; @@ -198,6 +224,8 @@ class SaveLayerOptions { struct { unsigned fRendersWithAttributes : 1; unsigned fCanDistributeOpacity : 1; + unsigned fBoundsFromCaller : 1; + unsigned fContentIsClipped : 1; }; uint32_t flags_; }; diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index 242b34359a5e8..cc1912e877220 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -1227,7 +1227,7 @@ class SaveLayerOptionsExpector : public virtual DlOpReceiver, explicit SaveLayerOptionsExpector(std::vector expected) : expected_(std::move(expected)) {} - void saveLayer(const SkRect* bounds, + void saveLayer(const SkRect& bounds, const SaveLayerOptions options, const DlImageFilter* backdrop) override { EXPECT_EQ(options, expected_[save_layer_count_]); @@ -1555,7 +1555,7 @@ TEST_F(DisplayListTest, 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) + 384u); + EXPECT_EQ(display_list->bytes(), sizeof(DisplayList) + 400u); } TEST_F(DisplayListTest, TranslateAffectsCurrentTransform) { @@ -3332,5 +3332,454 @@ TEST_F(DisplayListTest, ImpellerPathPreferenceIsHonored) { } } +class SaveLayerBoundsExpector : public virtual DlOpReceiver, + public IgnoreAttributeDispatchHelper, + public IgnoreClipDispatchHelper, + public IgnoreTransformDispatchHelper, + public IgnoreDrawDispatchHelper { + public: + explicit SaveLayerBoundsExpector() {} + + SaveLayerBoundsExpector& addComputedExpectation(const SkRect& bounds) { + expected_.emplace_back(BoundsExpectation{ + .bounds = bounds, + .options = SaveLayerOptions(), + }); + return *this; + } + + SaveLayerBoundsExpector& addSuppliedExpectation(const SkRect& bounds, + bool clipped = false) { + SaveLayerOptions options; + options = options.with_bounds_from_caller(); + if (clipped) { + options = options.with_content_is_clipped(); + } + expected_.emplace_back(BoundsExpectation{ + .bounds = bounds, + .options = options, + }); + return *this; + } + + void saveLayer(const SkRect& bounds, + const SaveLayerOptions options, + const DlImageFilter* backdrop) override { + ASSERT_LT(save_layer_count_, expected_.size()); + auto expected = expected_[save_layer_count_]; + EXPECT_EQ(options.bounds_from_caller(), + expected.options.bounds_from_caller()) + << "expected bounds index " << save_layer_count_; + EXPECT_EQ(options.content_is_clipped(), + expected.options.content_is_clipped()) + << "expected bounds index " << save_layer_count_; + if (!SkScalarNearlyEqual(bounds.fLeft, expected.bounds.fLeft) || + !SkScalarNearlyEqual(bounds.fTop, expected.bounds.fTop) || + !SkScalarNearlyEqual(bounds.fRight, expected.bounds.fRight) || + !SkScalarNearlyEqual(bounds.fBottom, expected.bounds.fBottom)) { + EXPECT_EQ(bounds, expected.bounds) + << "expected bounds index " << save_layer_count_; + } + save_layer_count_++; + } + + bool all_bounds_checked() const { + return save_layer_count_ == expected_.size(); + } + + private: + struct BoundsExpectation { + const SkRect bounds; + const SaveLayerOptions options; + }; + + std::vector expected_; + size_t save_layer_count_ = 0; +}; + +TEST_F(DisplayListTest, SaveLayerBoundsComputationOfSimpleRect) { + SkRect rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f); + + DisplayListBuilder builder; + builder.SaveLayer(nullptr, nullptr); + { // + builder.DrawRect(rect, DlPaint()); + } + builder.Restore(); + auto display_list = builder.Build(); + + SaveLayerBoundsExpector expector; + expector.addComputedExpectation(rect); + display_list->Dispatch(expector); + EXPECT_TRUE(expector.all_bounds_checked()); +} + +TEST_F(DisplayListTest, SaveLayerBoundsComputationOfMaskBlurredRect) { + SkRect rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f); + DlPaint draw_paint; + auto mask_filter = DlBlurMaskFilter::Make(DlBlurStyle::kNormal, 2.0f); + draw_paint.setMaskFilter(mask_filter); + + DisplayListBuilder builder; + builder.SaveLayer(nullptr, nullptr); + { // + builder.DrawRect(rect, draw_paint); + } + builder.Restore(); + auto display_list = builder.Build(); + + SaveLayerBoundsExpector expector; + expector.addComputedExpectation(rect.makeOutset(6.0f, 6.0f)); + display_list->Dispatch(expector); + EXPECT_TRUE(expector.all_bounds_checked()); +} + +TEST_F(DisplayListTest, SaveLayerBoundsComputationOfImageBlurredRect) { + SkRect rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f); + DlPaint draw_paint; + auto image_filter = DlBlurImageFilter::Make(2.0f, 3.0f, DlTileMode::kDecal); + draw_paint.setImageFilter(image_filter); + + DisplayListBuilder builder; + builder.SaveLayer(nullptr, nullptr); + { // + builder.DrawRect(rect, draw_paint); + } + builder.Restore(); + auto display_list = builder.Build(); + + SaveLayerBoundsExpector expector; + expector.addComputedExpectation(rect.makeOutset(6.0f, 9.0f)); + display_list->Dispatch(expector); + EXPECT_TRUE(expector.all_bounds_checked()); +} + +TEST_F(DisplayListTest, SaveLayerBoundsComputationOfStrokedRect) { + SkRect rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f); + DlPaint draw_paint; + draw_paint.setStrokeWidth(5.0f); + draw_paint.setDrawStyle(DlDrawStyle::kStroke); + + DisplayListBuilder builder; + builder.SaveLayer(nullptr, nullptr); + { // + builder.DrawRect(rect, draw_paint); + } + builder.Restore(); + auto display_list = builder.Build(); + + SaveLayerBoundsExpector expector; + expector.addComputedExpectation(rect.makeOutset(2.5f, 2.5f)); + display_list->Dispatch(expector); + EXPECT_TRUE(expector.all_bounds_checked()); +} + +TEST_F(DisplayListTest, TranslatedSaveLayerBoundsComputationOfSimpleRect) { + SkRect rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f); + + DisplayListBuilder builder; + builder.Translate(10.0f, 10.0f); + builder.SaveLayer(nullptr, nullptr); + { // + builder.DrawRect(rect, DlPaint()); + } + builder.Restore(); + auto display_list = builder.Build(); + + SaveLayerBoundsExpector expector; + expector.addComputedExpectation(rect); + display_list->Dispatch(expector); + EXPECT_TRUE(expector.all_bounds_checked()); +} + +TEST_F(DisplayListTest, ScaledSaveLayerBoundsComputationOfSimpleRect) { + SkRect rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f); + + DisplayListBuilder builder; + builder.Scale(10.0f, 10.0f); + builder.SaveLayer(nullptr, nullptr); + { // + builder.DrawRect(rect, DlPaint()); + } + builder.Restore(); + auto display_list = builder.Build(); + + SaveLayerBoundsExpector expector; + expector.addComputedExpectation(rect); + display_list->Dispatch(expector); + EXPECT_TRUE(expector.all_bounds_checked()); +} + +TEST_F(DisplayListTest, RotatedSaveLayerBoundsComputationOfSimpleRect) { + SkRect rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f); + + DisplayListBuilder builder; + builder.Rotate(45.0f); + builder.SaveLayer(nullptr, nullptr); + { // + builder.DrawRect(rect, DlPaint()); + } + builder.Restore(); + auto display_list = builder.Build(); + + SaveLayerBoundsExpector expector; + expector.addComputedExpectation(rect); + display_list->Dispatch(expector); + EXPECT_TRUE(expector.all_bounds_checked()); +} + +TEST_F(DisplayListTest, TransformResetSaveLayerBoundsComputationOfSimpleRect) { + SkRect rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f); + SkRect rect_doubled = SkMatrix::Scale(2.0f, 2.0f).mapRect(rect); + + DisplayListBuilder builder; + builder.Scale(10.0f, 10.0f); + builder.SaveLayer(nullptr, nullptr); + builder.TransformReset(); + builder.Scale(20.0f, 20.0f); + // Net local transform for saveLayer is Scale(2, 2) + { // + builder.DrawRect(rect, DlPaint()); + } + builder.Restore(); + auto display_list = builder.Build(); + + SaveLayerBoundsExpector expector; + expector.addComputedExpectation(rect_doubled); + display_list->Dispatch(expector); + EXPECT_TRUE(expector.all_bounds_checked()); +} + +TEST_F(DisplayListTest, SaveLayerBoundsComputationOfTranslatedSimpleRect) { + SkRect rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f); + + DisplayListBuilder builder; + builder.SaveLayer(nullptr, nullptr); + { // + builder.Translate(10.0f, 10.0f); + builder.DrawRect(rect, DlPaint()); + } + builder.Restore(); + auto display_list = builder.Build(); + + SaveLayerBoundsExpector expector; + expector.addComputedExpectation(rect.makeOffset(10.0f, 10.0f)); + display_list->Dispatch(expector); + EXPECT_TRUE(expector.all_bounds_checked()); +} + +TEST_F(DisplayListTest, SaveLayerBoundsComputationOfScaledSimpleRect) { + SkRect rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f); + + DisplayListBuilder builder; + builder.SaveLayer(nullptr, nullptr); + { // + builder.Scale(10.0f, 10.0f); + builder.DrawRect(rect, DlPaint()); + } + builder.Restore(); + auto display_list = builder.Build(); + + SaveLayerBoundsExpector expector; + expector.addComputedExpectation( + SkRect::MakeLTRB(1000.0f, 1000.0f, 2000.0f, 2000.0f)); + display_list->Dispatch(expector); + EXPECT_TRUE(expector.all_bounds_checked()); +} + +TEST_F(DisplayListTest, SaveLayerBoundsComputationOfRotatedSimpleRect) { + SkRect rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f); + + DisplayListBuilder builder; + builder.SaveLayer(nullptr, nullptr); + { // + builder.Rotate(45.0f); + builder.DrawRect(rect, DlPaint()); + } + builder.Restore(); + auto display_list = builder.Build(); + + SkMatrix matrix = SkMatrix::RotateDeg(45.0f); + SaveLayerBoundsExpector expector; + expector.addComputedExpectation(matrix.mapRect(rect)); + display_list->Dispatch(expector); + EXPECT_TRUE(expector.all_bounds_checked()); +} + +TEST_F(DisplayListTest, SaveLayerBoundsComputationOfNestedSimpleRect) { + SkRect rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f); + + DisplayListBuilder builder; + builder.SaveLayer(nullptr, nullptr); + { // + builder.SaveLayer(nullptr, nullptr); + { // + builder.DrawRect(rect, DlPaint()); + } + builder.Restore(); + } + builder.Restore(); + auto display_list = builder.Build(); + + SaveLayerBoundsExpector expector; + expector.addComputedExpectation(rect); + expector.addComputedExpectation(rect); + display_list->Dispatch(expector); + EXPECT_TRUE(expector.all_bounds_checked()); +} + +TEST_F(DisplayListTest, FloodingSaveLayerBoundsComputationOfSimpleRect) { + SkRect rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f); + DlPaint save_paint; + auto color_filter = + DlBlendColorFilter::Make(DlColor::kRed(), DlBlendMode::kSrc); + ASSERT_TRUE(color_filter->modifies_transparent_black()); + save_paint.setColorFilter(color_filter); + SkRect clip_rect = rect.makeOutset(100.0f, 100.0f); + ASSERT_NE(clip_rect, rect); + ASSERT_TRUE(clip_rect.contains(rect)); + + DisplayListBuilder builder; + builder.ClipRect(clip_rect); + builder.SaveLayer(nullptr, &save_paint); + { // + builder.DrawRect(rect, DlPaint()); + } + builder.Restore(); + auto display_list = builder.Build(); + + SaveLayerBoundsExpector expector; + expector.addComputedExpectation(rect); + display_list->Dispatch(expector); + EXPECT_TRUE(expector.all_bounds_checked()); +} + +TEST_F(DisplayListTest, NestedFloodingSaveLayerBoundsComputationOfSimpleRect) { + SkRect rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f); + DlPaint save_paint; + auto color_filter = + DlBlendColorFilter::Make(DlColor::kRed(), DlBlendMode::kSrc); + ASSERT_TRUE(color_filter->modifies_transparent_black()); + save_paint.setColorFilter(color_filter); + SkRect clip_rect = rect.makeOutset(100.0f, 100.0f); + ASSERT_NE(clip_rect, rect); + ASSERT_TRUE(clip_rect.contains(rect)); + + DisplayListBuilder builder; + builder.ClipRect(clip_rect); + builder.SaveLayer(nullptr, nullptr); + { + builder.SaveLayer(nullptr, &save_paint); + { // + builder.DrawRect(rect, DlPaint()); + } + builder.Restore(); + } + builder.Restore(); + auto display_list = builder.Build(); + + SaveLayerBoundsExpector expector; + expector.addComputedExpectation(clip_rect); + expector.addComputedExpectation(rect); + display_list->Dispatch(expector); + EXPECT_TRUE(expector.all_bounds_checked()); +} + +TEST_F(DisplayListTest, SaveLayerBoundsComputationOfFloodingImageFilter) { + SkRect rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f); + DlPaint draw_paint; + auto color_filter = + DlBlendColorFilter::Make(DlColor::kRed(), DlBlendMode::kSrc); + ASSERT_TRUE(color_filter->modifies_transparent_black()); + auto image_filter = DlColorFilterImageFilter::Make(color_filter); + draw_paint.setImageFilter(image_filter); + SkRect clip_rect = rect.makeOutset(100.0f, 100.0f); + ASSERT_NE(clip_rect, rect); + ASSERT_TRUE(clip_rect.contains(rect)); + + DisplayListBuilder builder; + builder.ClipRect(clip_rect); + builder.SaveLayer(nullptr, nullptr); + { // + builder.DrawRect(rect, draw_paint); + } + builder.Restore(); + auto display_list = builder.Build(); + + SaveLayerBoundsExpector expector; + expector.addComputedExpectation(clip_rect); + display_list->Dispatch(expector); + EXPECT_TRUE(expector.all_bounds_checked()); +} + +TEST_F(DisplayListTest, SaveLayerBoundsComputationOfFloodingColorFilter) { + SkRect rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f); + DlPaint draw_paint; + auto color_filter = + DlBlendColorFilter::Make(DlColor::kRed(), DlBlendMode::kSrc); + ASSERT_TRUE(color_filter->modifies_transparent_black()); + draw_paint.setColorFilter(color_filter); + SkRect clip_rect = rect.makeOutset(100.0f, 100.0f); + ASSERT_NE(clip_rect, rect); + ASSERT_TRUE(clip_rect.contains(rect)); + + DisplayListBuilder builder; + builder.ClipRect(clip_rect); + builder.SaveLayer(nullptr, nullptr); + { // + builder.DrawRect(rect, draw_paint); + } + builder.Restore(); + auto display_list = builder.Build(); + + // A color filter is implicitly clipped to the draw bounds so the layer + // bounds will be the same as the draw bounds. + SaveLayerBoundsExpector expector; + expector.addComputedExpectation(rect); + display_list->Dispatch(expector); + EXPECT_TRUE(expector.all_bounds_checked()); +} + +TEST_F(DisplayListTest, SaveLayerBoundsClipDetectionSimpleUnclippedRect) { + SkRect rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f); + SkRect save_rect = SkRect::MakeLTRB(50.0f, 50.0f, 250.0f, 250.0f); + + DisplayListBuilder builder; + builder.SaveLayer(&save_rect, nullptr); + { // + builder.DrawRect(rect, DlPaint()); + } + builder.Restore(); + auto display_list = builder.Build(); + + // A color filter is implicitly clipped to the draw bounds so the layer + // bounds will be the same as the draw bounds. + SaveLayerBoundsExpector expector; + expector.addSuppliedExpectation(rect); + display_list->Dispatch(expector); + EXPECT_TRUE(expector.all_bounds_checked()); +} + +TEST_F(DisplayListTest, SaveLayerBoundsClipDetectionSimpleClippedRect) { + SkRect rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f); + SkRect save_rect = SkRect::MakeLTRB(50.0f, 50.0f, 110.0f, 110.0f); + SkRect content_rect = SkRect::MakeLTRB(100.0f, 100.0f, 110.0f, 110.0f); + + DisplayListBuilder builder; + builder.SaveLayer(&save_rect, nullptr); + { // + builder.DrawRect(rect, DlPaint()); + } + builder.Restore(); + auto display_list = builder.Build(); + + // A color filter is implicitly clipped to the draw bounds so the layer + // bounds will be the same as the draw bounds. + SaveLayerBoundsExpector expector; + expector.addSuppliedExpectation(content_rect, true); + display_list->Dispatch(expector); + EXPECT_TRUE(expector.all_bounds_checked()); +} + } // namespace testing } // namespace flutter diff --git a/display_list/dl_builder.cc b/display_list/dl_builder.cc index e6ba24ba7f3a3..cefca96901706 100644 --- a/display_list/dl_builder.cc +++ b/display_list/dl_builder.cc @@ -85,7 +85,9 @@ sk_sp DisplayListBuilder::Build() { storage_.realloc(bytes); layer_stack_.pop_back(); layer_stack_.emplace_back(); + current_layer_ = &layer_stack_.back(); tracker_.reset(); + layer_tracker_.reset(); current_ = DlPaint(); return sk_sp(new DisplayList( @@ -389,62 +391,107 @@ void DisplayListBuilder::checkForDeferredSave() { } void DisplayListBuilder::Save() { - bool is_nop = current_layer_->is_nop_; layer_stack_.emplace_back(); current_layer_ = &layer_stack_.back(); + + FML_DCHECK(layer_stack_.size() >= 2u); + // Note we can't use the previous value of current_layer_ because + // the emplace_back() may have moved the storage locations, so we + // recompute the location of the penultimate layer info here. + auto parent_layer = &layer_stack_.end()[-2]; + current_layer_->has_deferred_save_op_ = true; - current_layer_->is_nop_ = is_nop; + current_layer_->is_nop_ = parent_layer->is_nop_; + + if (parent_layer->layer_accumulator_) { + FML_DCHECK(layer_tracker_); + // If the previous layer was using an accumulator, we need to keep + // filling it with content bounds. We reuse the previous accumulator + // for this layer, but clone the associated transform so that new + // transform mutations are restricted to this save/restore context. + current_layer_->layer_accumulator_ = parent_layer->layer_accumulator_; + layer_tracker_->save(); + } else { + FML_DCHECK(!layer_tracker_); + } + tracker_.save(); accumulator()->save(); } void DisplayListBuilder::Restore() { - if (layer_stack_.size() > 1) { - SaveOpBase* op = reinterpret_cast( - storage_.get() + current_layer_->save_offset()); - if (!current_layer_->has_deferred_save_op_) { - op->restore_index = op_index_; - Push(0, 1); - } - // Grab the current layer info before we push the restore - // on the stack. - LayerInfo layer_info = layer_stack_.back(); - - tracker_.restore(); - layer_stack_.pop_back(); - current_layer_ = &layer_stack_.back(); - bool is_unbounded = layer_info.is_unbounded(); - - // Before we pop_back we will get the current layer bounds from the - // current accumulator and adjust it as required based on the filter. - std::shared_ptr filter = layer_info.filter(); - if (filter) { - const SkRect clip = tracker_.device_cull_rect(); - if (!accumulator()->restore( - [filter = filter, matrix = GetTransform()](const SkRect& input, - SkRect& output) { - SkIRect output_bounds; - bool ret = filter->map_device_bounds(input.roundOut(), matrix, - output_bounds); - output.set(output_bounds); - return ret; - }, - &clip)) { - is_unbounded = true; - } - } else { - accumulator()->restore(); - } + if (layer_stack_.size() <= 1) { + return; + } - if (is_unbounded) { - AccumulateUnbounded(); - } + SaveOpBase* op = reinterpret_cast(storage_.get() + + current_layer_->save_offset()); - if (layer_info.has_layer()) { + if (!current_layer_->has_deferred_save_op_) { + op->restore_index = op_index_; + Push(0, 1); + } + + std::shared_ptr filter = current_layer_->filter(); + { + // We should not pop the stack until we are done synching up the current + // and parent layers. + auto parent_layer = &layer_stack_.end()[-2]; + + if (current_layer_->is_save_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_); - if (layer_info.is_group_opacity_compatible()) { + FML_DCHECK(!current_layer_->has_deferred_save_op_); + FML_DCHECK(current_layer_->layer_accumulator_); + + SkRect content_bounds = current_layer_->layer_accumulator_->bounds(); + + switch (op->type) { + case DisplayListOpType::kSaveLayer: + case DisplayListOpType::kSaveLayerBackdrop: { + SaveLayerOpBase* layer_op = reinterpret_cast(op); + if (op->options.bounds_from_caller()) { + if (!content_bounds.isEmpty() && + !layer_op->rect.contains(content_bounds)) { + op->options = op->options.with_content_is_clipped(); + content_bounds.intersect(layer_op->rect); + } + } + layer_op->rect = content_bounds; + break; + } + default: + FML_UNREACHABLE(); + } + + if (layer_tracker_->getSaveCount() > 1) { + layer_tracker_->restore(); + } else { + // If this was the last layer in the tracker, then there should + // be no parent saveLayer. + FML_DCHECK(!parent_layer->layer_accumulator_); + layer_tracker_.reset(); + } + + if (parent_layer->layer_accumulator_) { + SkRect bounds_for_parent = content_bounds; + if (filter) { + if (!filter->map_local_bounds(bounds_for_parent, bounds_for_parent)) { + parent_layer->set_unbounded(); + } + } + // The content_bounds were accumulated in the base coordinate system + // of the current layer, and have been adjusted there according to + // its image filter. + // The content bounds accumulation of the parent layer is relative + // to the parent's base coordinate system, so we need to adjust + // bounds_for_parent to that coordinate space. + FML_DCHECK(layer_tracker_); + layer_tracker_->mapRect(&bounds_for_parent); + parent_layer->layer_accumulator_->accumulate(bounds_for_parent); + } + + if (current_layer_->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 // value to its children. @@ -460,15 +507,53 @@ void DisplayListBuilder::Restore() { op->options = op->options.with_can_distribute_opacity(); } } else { + if (layer_tracker_) { + FML_DCHECK(layer_tracker_->getSaveCount() > 1); + layer_tracker_->restore(); + } // For regular save() ops there was no protecting layer so we have to - // accumulate the values into the enclosing layer. - if (layer_info.cannot_inherit_opacity()) { - current_layer_->mark_incompatible(); - } else if (layer_info.has_compatible_op()) { - current_layer_->add_compatible_op(); + // accumulate the inheritance properties into the enclosing layer. + if (current_layer_->cannot_inherit_opacity()) { + parent_layer->mark_incompatible(); + } else if (current_layer_->has_compatible_op()) { + parent_layer->add_compatible_op(); } } } + + // Remember whether the outgoing layer was unbounded so we can adjust + // for it below after we apply the outgoing layer's filter to the bounds. + bool popped_was_unbounded = current_layer_->is_unbounded(); + + // parent_layer is no longer in scope, time to pop the layer. + layer_stack_.pop_back(); + tracker_.restore(); + current_layer_ = &layer_stack_.back(); + + // As we pop the accumulator, use the filter that was applied to the + // outgoing layer (saved above, if any) to adjust the bounds that + // were accumulated while that layer was active. + if (filter) { + const SkRect clip = tracker_.device_cull_rect(); + if (!accumulator()->restore( + [filter = filter, matrix = GetTransform()](const SkRect& input, + SkRect& output) { + SkIRect output_bounds; + bool ret = filter->map_device_bounds(input.roundOut(), matrix, + output_bounds); + output.set(output_bounds); + return ret; + }, + &clip)) { + popped_was_unbounded = true; + } + } else { + accumulator()->restore(); + } + + if (popped_was_unbounded) { + AccumulateUnbounded(); + } } void DisplayListBuilder::RestoreToCount(int restore_count) { FML_DCHECK(restore_count <= GetSaveCount()); @@ -476,7 +561,7 @@ void DisplayListBuilder::RestoreToCount(int restore_count) { restore(); } } -void DisplayListBuilder::saveLayer(const SkRect* bounds, +void DisplayListBuilder::saveLayer(const SkRect& bounds, const SaveLayerOptions in_options, const DlImageFilter* backdrop) { SaveLayerOptions options = in_options.without_optimizations(); @@ -489,7 +574,9 @@ void DisplayListBuilder::saveLayer(const SkRect* bounds, current_layer_->is_nop_ = true; return; } + size_t save_layer_offset = used_; + if (options.renders_with_attributes()) { // The actual flood of the outer layer clip will occur after the // (eventual) corresponding restore is called, but rather than @@ -508,17 +595,40 @@ void DisplayListBuilder::saveLayer(const SkRect* bounds, FML_DCHECK(unclipped); } CheckLayerOpacityCompatibility(true); - layer_stack_.emplace_back(save_layer_offset, true, - current_.getImageFilter()); + layer_stack_.emplace_back(save_layer_offset); + layer_stack_.back().filter_ = current_.getImageFilter(); } else { CheckLayerOpacityCompatibility(false); - layer_stack_.emplace_back(save_layer_offset, true, nullptr); + layer_stack_.emplace_back(save_layer_offset); } current_layer_ = &layer_stack_.back(); + current_layer_->is_save_layer_ = true; tracker_.save(); accumulator()->save(); + SkRect record_bounds; + if (in_options.bounds_from_caller()) { + options = options.with_bounds_from_caller(); + record_bounds = bounds; + } else { + FML_DCHECK(record_bounds.isEmpty()); + } + current_layer_->layer_accumulator_.reset(new RectBoundsAccumulator()); + if (layer_tracker_) { + layer_tracker_->save(); + layer_tracker_->setTransform(SkMatrix::I()); + } else { + SkRect cull_rect; + if (in_options.bounds_from_caller()) { + cull_rect = bounds; + } else { + cull_rect = tracker_.local_cull_rect(); + } + layer_tracker_.reset( + new DisplayListMatrixClipTracker(cull_rect, SkMatrix::I())); + } + if (backdrop) { // A backdrop will affect up to the entire surface, bounded by the clip // Accumulate should always return true here because if the @@ -526,13 +636,9 @@ void DisplayListBuilder::saveLayer(const SkRect* bounds, // when we tested the PaintResult. [[maybe_unused]] bool unclipped = AccumulateUnbounded(); FML_DCHECK(unclipped); - bounds // - ? Push(0, 1, options, *bounds, backdrop) - : Push(0, 1, options, backdrop); + Push(0, 1, options, record_bounds, backdrop); } else { - bounds // - ? Push(0, 1, options, *bounds) - : Push(0, 1, options); + Push(0, 1, options, record_bounds); } if (options.renders_with_attributes()) { @@ -558,24 +664,31 @@ void DisplayListBuilder::saveLayer(const SkRect* bounds, // The filtered bounds will be clipped to the existing clip rect when // this layer is restored. // If bounds is null then the original cull_rect will be used. - tracker_.resetCullRect(bounds); - } else if (bounds) { + tracker_.resetCullRect(in_options.bounds_from_caller() ? &bounds : nullptr); + } else if (in_options.bounds_from_caller()) { // Even though Skia claims that the bounds are only a hint, they actually // use them as the temporary layer bounds during rendering the layer, so // we set them as if a clip operation were performed. - tracker_.clipRect(*bounds, ClipOp::kIntersect, false); + tracker_.clipRect(bounds, ClipOp::kIntersect, false); } } void DisplayListBuilder::SaveLayer(const SkRect* bounds, const DlPaint* paint, const DlImageFilter* backdrop) { + SaveLayerOptions options; + SkRect temp_bounds; + if (bounds) { + options = options.with_bounds_from_caller(); + temp_bounds = *bounds; + } else { + temp_bounds.setEmpty(); + } if (paint != nullptr) { + options = options.with_renders_with_attributes(); SetAttributesFromPaint(*paint, DisplayListOpFlags::kSaveLayerWithPaintFlags); - saveLayer(bounds, SaveLayerOptions::kWithAttributes, backdrop); - } else { - saveLayer(bounds, SaveLayerOptions::kNoAttributes, backdrop); } + saveLayer(temp_bounds, options, backdrop); } void DisplayListBuilder::Translate(SkScalar tx, SkScalar ty) { @@ -584,6 +697,9 @@ void DisplayListBuilder::Translate(SkScalar tx, SkScalar ty) { checkForDeferredSave(); Push(0, 1, tx, ty); tracker_.translate(tx, ty); + if (layer_tracker_) { + layer_tracker_->translate(tx, ty); + } } } void DisplayListBuilder::Scale(SkScalar sx, SkScalar sy) { @@ -592,6 +708,9 @@ void DisplayListBuilder::Scale(SkScalar sx, SkScalar sy) { checkForDeferredSave(); Push(0, 1, sx, sy); tracker_.scale(sx, sy); + if (layer_tracker_) { + layer_tracker_->scale(sx, sy); + } } } void DisplayListBuilder::Rotate(SkScalar degrees) { @@ -599,6 +718,9 @@ void DisplayListBuilder::Rotate(SkScalar degrees) { checkForDeferredSave(); Push(0, 1, degrees); tracker_.rotate(degrees); + if (layer_tracker_) { + layer_tracker_->rotate(degrees); + } } } void DisplayListBuilder::Skew(SkScalar sx, SkScalar sy) { @@ -607,6 +729,9 @@ void DisplayListBuilder::Skew(SkScalar sx, SkScalar sy) { checkForDeferredSave(); Push(0, 1, sx, sy); tracker_.skew(sx, sy); + if (layer_tracker_) { + layer_tracker_->skew(sx, sy); + } } } @@ -629,6 +754,10 @@ void DisplayListBuilder::Transform2DAffine( myx, myy, myt); tracker_.transform2DAffine(mxx, mxy, mxt, myx, myy, myt); + if (layer_tracker_) { + layer_tracker_->transform2DAffine(mxx, mxy, mxt, + myx, myy, myt); + } } } } @@ -658,12 +787,39 @@ void DisplayListBuilder::TransformFullPerspective( myx, myy, myz, myt, mzx, mzy, mzz, mzt, mwx, mwy, mwz, mwt); + if (layer_tracker_) { + layer_tracker_->transformFullPerspective(mxx, mxy, mxz, mxt, + myx, myy, myz, myt, + mzx, mzy, mzz, mzt, + mwx, mwy, mwz, mwt); + } } } // clang-format on void DisplayListBuilder::TransformReset() { checkForDeferredSave(); Push(0, 0); + if (layer_tracker_) { + // The matrices in layer_tracker_ and tracker_ are similar, but + // start at a different base transform. The tracker_ potentially + // has some number of transform operations on it that prefix the + // operations accumulated in layer_tracker_. So we can't set them both + // to identity in parallel as they would no longer maintain their + // relationship to each other. + // Instead we reinterpret this operation as transforming by the + // inverse of the current transform. Doing so to tracker_ sets it + // to identity so we can avoid the math there, but we must do the + // math the long way for layer_tracker_. This becomes: + // layer_tracker_.transform(tracker_.inverse()); + if (!layer_tracker_->inverseTransform(tracker_)) { + // If the inverse operation failed then that means that either + // the matrix above the current layer was singular, or the matrix + // became singular while we were accumulating the current layer. + // In either case, we should no longer be accumulating any + // contents so we set the layer tracking transform to a singular one. + layer_tracker_->setTransform(SkMatrix::Scale(0.0f, 0.0f)); + } + } tracker_.setIdentity(); } void DisplayListBuilder::Transform(const SkMatrix* matrix) { @@ -1360,16 +1516,6 @@ void DisplayListBuilder::DrawShadow(const SkPath& path, } } -bool DisplayListBuilder::ComputeFilteredBounds(SkRect& bounds, - const DlImageFilter* filter) { - if (filter) { - if (!filter->map_local_bounds(bounds, bounds)) { - return false; - } - } - return true; -} - bool DisplayListBuilder::AdjustBoundsForPaint(SkRect& bounds, DisplayListAttributeFlags flags) { if (flags.ignores_paint()) { @@ -1420,8 +1566,16 @@ bool DisplayListBuilder::AdjustBoundsForPaint(SkRect& bounds, } } + // Color filter does not modify bounds even if it affects transparent + // black because it is clipped by the "mask" of the primitive. That + // property only comes into play when it is applied to something like + // a layer. + if (flags.applies_image_filter()) { - return ComputeFilteredBounds(bounds, current_.getImageFilter().get()); + auto filter = current_.getImageFilterPtr(); + if (filter && !filter->map_local_bounds(bounds, bounds)) { + return false; + } } return true; @@ -1433,6 +1587,11 @@ bool DisplayListBuilder::AccumulateUnbounded() { return false; } accumulator()->accumulate(clip, op_index_); + if (current_layer_->layer_accumulator_) { + FML_DCHECK(layer_tracker_); + current_layer_->layer_accumulator_->accumulate( + layer_tracker_->device_cull_rect()); + } return true; } @@ -1446,9 +1605,16 @@ bool DisplayListBuilder::AccumulateOpBounds(SkRect& bounds, } bool DisplayListBuilder::AccumulateBounds(SkRect& bounds) { if (!bounds.isEmpty()) { - tracker_.mapRect(&bounds); - if (bounds.intersect(tracker_.device_cull_rect())) { - accumulator()->accumulate(bounds, op_index_); + SkRect device_bounds; + tracker_.mapRect(bounds, &device_bounds); + if (device_bounds.intersect(tracker_.device_cull_rect())) { + accumulator()->accumulate(device_bounds, op_index_); + if (current_layer_->layer_accumulator_) { + FML_DCHECK(layer_tracker_); + SkRect layer_bounds; + layer_tracker_->mapRect(bounds, &layer_bounds); + current_layer_->layer_accumulator_->accumulate(layer_bounds); + } return true; } } diff --git a/display_list/dl_builder.h b/display_list/dl_builder.h index 69e48cb96d2d6..a1742f3a4305a 100644 --- a/display_list/dl_builder.h +++ b/display_list/dl_builder.h @@ -356,7 +356,7 @@ class DisplayListBuilder final : public virtual DlCanvas, // other flags will be ignored and calculated anew as the DisplayList is // built. Alternatively, use the |saveLayer(SkRect, bool)| method. // |DlOpReceiver| - void saveLayer(const SkRect* bounds, + void saveLayer(const SkRect& bounds, const SaveLayerOptions options, const DlImageFilter* backdrop) override; // |DlOpReceiver| @@ -508,25 +508,19 @@ class DisplayListBuilder final : public virtual DlCanvas, return SkScalarIsFinite(sigma) && sigma > 0.0; } - class LayerInfo { + class SaveInfo { public: - explicit LayerInfo( - size_t save_offset = 0, - bool has_layer = false, - const std::shared_ptr& filter = nullptr) - : save_offset_(save_offset), - has_layer_(has_layer), - filter_(filter) {} - - // The offset into the memory buffer where the saveLayer DLOp record - // for this saveLayer() call is placed. This may be needed if the + explicit SaveInfo(size_t save_offset = 0) : save_offset_(save_offset) {} + + // The offset into the memory buffer where the save DLOp record + // for this save() call is placed. This may be needed if the // eventual restore() call has discovered important information about // 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_offset() const { return save_offset_; } - bool has_layer() const { return has_layer_; } + bool is_save_layer() const { return is_save_layer_; } bool cannot_inherit_opacity() const { return cannot_inherit_opacity_; } bool has_compatible_op() const { return has_compatible_op_; } bool affects_transparent_layer() const { @@ -593,7 +587,7 @@ class DisplayListBuilder final : public virtual DlCanvas, private: size_t save_offset_; - bool has_layer_; + bool is_save_layer_ = false; bool cannot_inherit_opacity_ = false; bool has_compatible_op_ = false; std::shared_ptr filter_; @@ -601,13 +595,15 @@ class DisplayListBuilder final : public virtual DlCanvas, bool has_deferred_save_op_ = false; bool is_nop_ = false; bool affects_transparent_layer_ = false; + std::shared_ptr layer_accumulator_; friend class DisplayListBuilder; }; - std::vector layer_stack_; - LayerInfo* current_layer_; + std::vector layer_stack_; + SaveInfo* current_layer_; DisplayListMatrixClipTracker tracker_; + std::unique_ptr layer_tracker_; std::unique_ptr accumulator_; BoundsAccumulator* accumulator() { return accumulator_.get(); } @@ -744,12 +740,6 @@ class DisplayListBuilder final : public virtual DlCanvas, static DlColor GetEffectiveColor(const DlPaint& paint, DisplayListAttributeFlags flags); - // Computes the bounds of an operation adjusted for a given ImageFilter - // and returns whether the computation was possible. If the method - // returns false then the caller should assume the worst about the bounds. - static bool ComputeFilteredBounds(SkRect& bounds, - const DlImageFilter* filter); - // Adjusts the indicated bounds for the given flags and returns true if // the calculation was possible, or false if it could not be estimated. bool AdjustBoundsForPaint(SkRect& bounds, DisplayListAttributeFlags flags); diff --git a/display_list/dl_op_receiver.h b/display_list/dl_op_receiver.h index dff97c3c15b7a..2eec9358273a7 100644 --- a/display_list/dl_op_receiver.h +++ b/display_list/dl_op_receiver.h @@ -122,6 +122,11 @@ class DlOpReceiver { // attributes will be applied to the save layer surface while rendering // it back to the current surface. If the flag is false then this method // is equivalent to |SkCanvas::saveLayer| with a null paint object. + // + // The |options| parameter can also specify whether the bounds came from + // the caller who recorded the operation, or whether they were calculated + // by the DisplayListBuilder. + // // The |options| parameter may contain other options that indicate some // specific optimizations may be made by the underlying implementation // to avoid creating a temporary layer, these optimization options will @@ -129,11 +134,37 @@ class DlOpReceiver { // specified in calling a |DisplayListBuilder| as they will be ignored. // The |backdrop| filter, if not null, is used to initialize the new // layer before further rendering happens. - virtual void saveLayer(const SkRect* bounds, + virtual void saveLayer(const SkRect& bounds, const SaveLayerOptions options, const DlImageFilter* backdrop = nullptr) = 0; virtual void restore() = 0; + // --------------------------------------------------------------------- + // Legacy helper method for older callers that use the null-ness of + // the bounds to indicate if they should be recorded or computed. + // This method will not be called on a |DlOpReceiver| that is passed + // to the |DisplayList::Dispatch()| method, so client receivers should + // ignore it for their implementation purposes. + // + // DlOpReceiver methods are generally meant to ONLY be output from a + // previously recorded DisplayList so this method is really only used + // from testing methods that bypass the public builder APIs for legacy + // convenience or for internal white-box testing of the DisplayList + // internals. Such methods should eventually be converted to using the + // public DisplayListBuilder/DlCanvas public interfaces where possible, + // as tracked in: + // https://github.com/flutter/flutter/issues/144070 + virtual void saveLayer(const SkRect* bounds, + const SaveLayerOptions options, + const DlImageFilter* backdrop = nullptr) final { + if (bounds) { + saveLayer(*bounds, options.with_bounds_from_caller(), backdrop); + } else { + saveLayer(SkRect(), options.without_bounds_from_caller(), backdrop); + } + } + // --------------------------------------------------------------------- + virtual void translate(SkScalar tx, SkScalar ty) = 0; virtual void scale(SkScalar sx, SkScalar sy) = 0; virtual void rotate(SkScalar degrees) = 0; diff --git a/display_list/dl_op_records.h b/display_list/dl_op_records.h index d0f12caea0318..bcd895a1fcad7 100644 --- a/display_list/dl_op_records.h +++ b/display_list/dl_op_records.h @@ -312,8 +312,8 @@ struct SetSharedImageFilterOp : DLOp { } }; -// The base object for all save() and saveLayer() ops -// 4 byte header + 8 byte payload packs neatly into 16 bytes (4 bytes unused) +// The base struct for all save() and saveLayer() ops +// 4 byte header + 8 byte payload packs into 16 bytes (4 bytes unused) struct SaveOpBase : DLOp { SaveOpBase() : options(), restore_index(0) {} @@ -333,7 +333,7 @@ struct SaveOpBase : DLOp { return needed; } }; -// 24 byte SaveOpBase with no additional data (options is unsed here) +// 16 byte SaveOpBase with no additional data (options is unsed here) struct SaveOp final : SaveOpBase { static const auto kType = DisplayListOpType::kSave; @@ -345,74 +345,45 @@ struct SaveOp final : SaveOpBase { } } }; -// 16 byte SaveOpBase with no additional data -struct SaveLayerOp final : SaveOpBase { - static const auto kType = DisplayListOpType::kSaveLayer; - - explicit SaveLayerOp(const SaveLayerOptions& options) : SaveOpBase(options) {} +// The base struct for all saveLayer() ops +// 16 byte SaveOpBase + 16 byte payload packs into 32 bytes (4 bytes unused) +struct SaveLayerOpBase : SaveOpBase { + SaveLayerOpBase(const SaveLayerOptions& options, const SkRect& rect) + : SaveOpBase(options), rect(rect) {} - void dispatch(DispatchContext& ctx) const { - if (save_needed(ctx)) { - ctx.receiver.saveLayer(nullptr, options); - } - } + SkRect rect; }; -// 24 byte SaveOpBase + 16 byte payload packs evenly into 40 bytes -struct SaveLayerBoundsOp final : SaveOpBase { - static const auto kType = DisplayListOpType::kSaveLayerBounds; - - SaveLayerBoundsOp(const SaveLayerOptions& options, const SkRect& rect) - : SaveOpBase(options), rect(rect) {} +// 32 byte SaveLayerOpBase with no additional data +struct SaveLayerOp final : SaveLayerOpBase { + static const auto kType = DisplayListOpType::kSaveLayer; - const SkRect rect; + SaveLayerOp(const SaveLayerOptions& options, const SkRect& rect) + : SaveLayerOpBase(options, rect) {} void dispatch(DispatchContext& ctx) const { if (save_needed(ctx)) { - ctx.receiver.saveLayer(&rect, options); + ctx.receiver.saveLayer(rect, options); } } }; -// 24 byte SaveOpBase + 16 byte payload packs into minimum 40 bytes -struct SaveLayerBackdropOp final : SaveOpBase { +// 32 byte SaveLayerOpBase + 16 byte payload packs into minimum 48 bytes +struct SaveLayerBackdropOp final : SaveLayerOpBase { static const auto kType = DisplayListOpType::kSaveLayerBackdrop; - explicit SaveLayerBackdropOp(const SaveLayerOptions& options, - const DlImageFilter* backdrop) - : SaveOpBase(options), backdrop(backdrop->shared()) {} + SaveLayerBackdropOp(const SaveLayerOptions& options, + const SkRect& rect, + const DlImageFilter* backdrop) + : SaveLayerOpBase(options, rect), backdrop(backdrop->shared()) {} const std::shared_ptr backdrop; void dispatch(DispatchContext& ctx) const { if (save_needed(ctx)) { - ctx.receiver.saveLayer(nullptr, options, backdrop.get()); + ctx.receiver.saveLayer(rect, options, backdrop.get()); } } DisplayListCompare equals(const SaveLayerBackdropOp* other) const { - return options == other->options && Equals(backdrop, other->backdrop) - ? DisplayListCompare::kEqual - : DisplayListCompare::kNotEqual; - } -}; -// 24 byte SaveOpBase + 32 byte payload packs into minimum 56 bytes -struct SaveLayerBackdropBoundsOp final : SaveOpBase { - static const auto kType = DisplayListOpType::kSaveLayerBackdropBounds; - - SaveLayerBackdropBoundsOp(const SaveLayerOptions& options, - const SkRect& rect, - const DlImageFilter* backdrop) - : SaveOpBase(options), rect(rect), backdrop(backdrop->shared()) {} - - const SkRect rect; - const std::shared_ptr backdrop; - - void dispatch(DispatchContext& ctx) const { - if (save_needed(ctx)) { - ctx.receiver.saveLayer(&rect, options, backdrop.get()); - } - } - - DisplayListCompare equals(const SaveLayerBackdropBoundsOp* other) const { return (options == other->options && rect == other->rect && Equals(backdrop, other->backdrop)) ? DisplayListCompare::kEqual diff --git a/display_list/skia/dl_sk_dispatcher.cc b/display_list/skia/dl_sk_dispatcher.cc index 13449c38ee99d..e75ccd96da4b9 100644 --- a/display_list/skia/dl_sk_dispatcher.cc +++ b/display_list/skia/dl_sk_dispatcher.cc @@ -43,10 +43,10 @@ void DlSkCanvasDispatcher::restore() { canvas_->restore(); restore_opacity(); } -void DlSkCanvasDispatcher::saveLayer(const SkRect* bounds, +void DlSkCanvasDispatcher::saveLayer(const SkRect& bounds, const SaveLayerOptions options, const DlImageFilter* backdrop) { - if (bounds == nullptr && options.can_distribute_opacity() && + if (!options.content_is_clipped() && options.can_distribute_opacity() && backdrop == nullptr) { // We know that: // - no bounds is needed for clipping here @@ -67,8 +67,9 @@ void DlSkCanvasDispatcher::saveLayer(const SkRect* bounds, TRACE_EVENT0("flutter", "Canvas::saveLayer"); const SkPaint* paint = safe_paint(options.renders_with_attributes()); const sk_sp sk_backdrop = ToSk(backdrop); + const SkRect* sl_bounds = options.bounds_from_caller() ? &bounds : nullptr; canvas_->saveLayer( - SkCanvas::SaveLayerRec(bounds, paint, sk_backdrop.get(), 0)); + SkCanvas::SaveLayerRec(sl_bounds, paint, sk_backdrop.get(), 0)); // saveLayer will apply the current opacity on behalf of the children // so they will inherit an opaque opacity. save_opacity(SK_Scalar1); diff --git a/display_list/skia/dl_sk_dispatcher.h b/display_list/skia/dl_sk_dispatcher.h index a9177925aaabd..0db4dce710d0f 100644 --- a/display_list/skia/dl_sk_dispatcher.h +++ b/display_list/skia/dl_sk_dispatcher.h @@ -29,7 +29,7 @@ class DlSkCanvasDispatcher : public virtual DlOpReceiver, void save() override; void restore() override; - void saveLayer(const SkRect* bounds, + void saveLayer(const SkRect& bounds, const SaveLayerOptions options, const DlImageFilter* backdrop) override; diff --git a/display_list/testing/dl_test_snippets.cc b/display_list/testing/dl_test_snippets.cc index c26d95ab91e99..4757833f99ef6 100644 --- a/display_list/testing/dl_test_snippets.cc +++ b/display_list/testing/dl_test_snippets.cc @@ -252,7 +252,7 @@ std::vector CreateAllSaveRestoreOps() { return { {"Save(Layer)+Restore", { - {5, 112, 5, 112, + {5, 128, 5, 128, [](DlOpReceiver& r) { r.saveLayer(nullptr, SaveLayerOptions::kNoAttributes, &kTestCFImageFilter1); @@ -276,7 +276,7 @@ std::vector CreateAllSaveRestoreOps() { r.drawRect({10, 10, 20, 20}); r.restore(); }}, - {5, 96, 5, 96, + {5, 112, 5, 112, [](DlOpReceiver& r) { r.saveLayer(nullptr, SaveLayerOptions::kNoAttributes); r.clipRect({0, 0, 25, 25}, DlCanvas::ClipOp::kIntersect, true); @@ -284,7 +284,7 @@ std::vector CreateAllSaveRestoreOps() { r.drawRect({10, 10, 20, 20}); r.restore(); }}, - {5, 96, 5, 96, + {5, 112, 5, 112, [](DlOpReceiver& r) { r.saveLayer(nullptr, SaveLayerOptions::kWithAttributes); r.clipRect({0, 0, 25, 25}, DlCanvas::ClipOp::kIntersect, true); @@ -317,7 +317,7 @@ std::vector CreateAllSaveRestoreOps() { // r.drawRect({10, 10, 20, 20}); // r.restore(); // }}, - {5, 112, 5, 112, + {5, 128, 5, 128, [](DlOpReceiver& r) { r.saveLayer(nullptr, SaveLayerOptions::kWithAttributes, &kTestCFImageFilter1); diff --git a/display_list/utils/dl_matrix_clip_tracker.cc b/display_list/utils/dl_matrix_clip_tracker.cc index 2dd0e8fa31a39..c4023aa840089 100644 --- a/display_list/utils/dl_matrix_clip_tracker.cc +++ b/display_list/utils/dl_matrix_clip_tracker.cc @@ -224,6 +224,24 @@ void DisplayListMatrixClipTracker::setTransform(const SkM44& m44) { current_->setTransform(m44); } +bool DisplayListMatrixClipTracker::inverseTransform( + const DisplayListMatrixClipTracker& tracker_) { + if (tracker_.using_4x4_matrix()) { + SkM44 inverse; + if (tracker_.matrix_4x4().invert(&inverse)) { + transform(inverse); + return true; + } + } else { + SkMatrix inverse; + if (tracker_.matrix_3x3().invert(&inverse)) { + transform(inverse); + return true; + } + } + return false; +} + void DisplayListMatrixClipTracker::clipRRect(const SkRRect& rrect, ClipOp op, bool is_aa) { diff --git a/display_list/utils/dl_matrix_clip_tracker.h b/display_list/utils/dl_matrix_clip_tracker.h index 81976a9e858d9..ca71e78b3df34 100644 --- a/display_list/utils/dl_matrix_clip_tracker.h +++ b/display_list/utils/dl_matrix_clip_tracker.h @@ -84,7 +84,15 @@ class DisplayListMatrixClipTracker { void setTransform(const SkMatrix& matrix) { current_->setTransform(matrix); } void setTransform(const SkM44& m44); void setIdentity() { current_->setIdentity(); } + // If the matrix in |other_tracker| is invertible then transform this + // tracker by the inverse of its matrix and return true. Otherwise, + // return false and leave this tracker unmodified. + bool inverseTransform(const DisplayListMatrixClipTracker& other_tracker); + bool mapRect(SkRect* rect) const { return current_->mapRect(*rect, rect); } + bool mapRect(const SkRect& src, SkRect* mapped) { + return current_->mapRect(src, mapped); + } void clipRect(const SkRect& rect, ClipOp op, bool is_aa) { current_->clipBounds(rect, op, is_aa); diff --git a/display_list/utils/dl_receiver_utils.h b/display_list/utils/dl_receiver_utils.h index 4450cefdbe5ba..a3b5bb156de86 100644 --- a/display_list/utils/dl_receiver_utils.h +++ b/display_list/utils/dl_receiver_utils.h @@ -78,7 +78,7 @@ class IgnoreTransformDispatchHelper : public virtual DlOpReceiver { class IgnoreDrawDispatchHelper : public virtual DlOpReceiver { public: void save() override {} - void saveLayer(const SkRect* bounds, + void saveLayer(const SkRect& bounds, const SaveLayerOptions options, const DlImageFilter* backdrop) override {} void restore() override {} diff --git a/impeller/aiks/canvas.cc b/impeller/aiks/canvas.cc index 57f84496625b3..1e3fe3812b138 100644 --- a/impeller/aiks/canvas.cc +++ b/impeller/aiks/canvas.cc @@ -783,7 +783,8 @@ void Canvas::AddEntityToCurrentPass(Entity entity) { void Canvas::SaveLayer(const Paint& paint, std::optional bounds, - const std::shared_ptr& backdrop_filter) { + const std::shared_ptr& backdrop_filter, + ContentBoundsPromise bounds_promise) { TRACE_EVENT0("flutter", "Canvas::saveLayer"); Save(true, paint.blend_mode, backdrop_filter); @@ -796,7 +797,9 @@ void Canvas::SaveLayer(const Paint& paint, } auto& new_layer_pass = GetCurrentPass(); - new_layer_pass.SetBoundsLimit(bounds); + if (bounds) { + new_layer_pass.SetBoundsLimit(bounds, bounds_promise); + } if (paint.image_filter) { MipCountVisitor mip_count_visitor; diff --git a/impeller/aiks/canvas.h b/impeller/aiks/canvas.h index 867c5b8535069..36ba4639b264d 100644 --- a/impeller/aiks/canvas.h +++ b/impeller/aiks/canvas.h @@ -75,9 +75,11 @@ class Canvas { void Save(); - void SaveLayer(const Paint& paint, - std::optional bounds = std::nullopt, - const std::shared_ptr& backdrop_filter = nullptr); + void SaveLayer( + const Paint& paint, + std::optional bounds = std::nullopt, + const std::shared_ptr& backdrop_filter = nullptr, + ContentBoundsPromise bounds_promise = ContentBoundsPromise::kUnknown); bool Restore(); diff --git a/impeller/aiks/canvas_recorder.h b/impeller/aiks/canvas_recorder.h index e0089111720b8..526050547453f 100644 --- a/impeller/aiks/canvas_recorder.h +++ b/impeller/aiks/canvas_recorder.h @@ -116,9 +116,10 @@ class CanvasRecorder { void SaveLayer( const Paint& paint, std::optional bounds = std::nullopt, - const std::shared_ptr& backdrop_filter = nullptr) { + const std::shared_ptr& backdrop_filter = nullptr, + ContentBoundsPromise bounds_promise = ContentBoundsPromise::kUnknown) { return ExecuteAndSerialize(FLT_CANVAS_RECORDER_OP_ARG(SaveLayer), paint, - bounds, backdrop_filter); + bounds, backdrop_filter, bounds_promise); } bool Restore() { diff --git a/impeller/aiks/canvas_recorder_unittests.cc b/impeller/aiks/canvas_recorder_unittests.cc index 24ef24355febf..a18e092aab9e2 100644 --- a/impeller/aiks/canvas_recorder_unittests.cc +++ b/impeller/aiks/canvas_recorder_unittests.cc @@ -56,6 +56,8 @@ class Serializer { void Write(const SourceRectConstraint& src_rect_constraint) {} + void Write(const ContentBoundsPromise& promise) {} + CanvasRecorderOp last_op_; }; } // namespace diff --git a/impeller/aiks/paint_pass_delegate.cc b/impeller/aiks/paint_pass_delegate.cc index fe42af0b7c2aa..34610488f5a24 100644 --- a/impeller/aiks/paint_pass_delegate.cc +++ b/impeller/aiks/paint_pass_delegate.cc @@ -71,8 +71,9 @@ bool OpacityPeepholePassDelegate::CanElide() { // |EntityPassDelgate| bool OpacityPeepholePassDelegate::CanCollapseIntoParentPass( EntityPass* entity_pass) { - // Passes with absorbed clips can not be safely collapsed. - if (entity_pass->GetBoundsLimit().has_value()) { + // Passes with enforced bounds that clip the contents can not be safely + // collapsed. + if (entity_pass->GetBoundsLimitMightClipContent()) { return false; } diff --git a/impeller/aiks/trace_serializer.cc b/impeller/aiks/trace_serializer.cc index 09325cf432df9..7600760406b1b 100644 --- a/impeller/aiks/trace_serializer.cc +++ b/impeller/aiks/trace_serializer.cc @@ -258,4 +258,8 @@ void TraceSerializer::Write(const SourceRectConstraint& src_rect_constraint) { buffer_ << "[SourceRectConstraint] "; } +void TraceSerializer::Write(const ContentBoundsPromise& promise) { + buffer_ << "[SaveLayerBoundsPromise]"; +} + } // namespace impeller diff --git a/impeller/aiks/trace_serializer.h b/impeller/aiks/trace_serializer.h index 0f777287bd992..32abcdcddcdb4 100644 --- a/impeller/aiks/trace_serializer.h +++ b/impeller/aiks/trace_serializer.h @@ -60,6 +60,8 @@ class TraceSerializer { void Write(const SourceRectConstraint& src_rect_constraint); + void Write(const ContentBoundsPromise& promise); + private: std::stringstream buffer_; }; diff --git a/impeller/display_list/dl_dispatcher.cc b/impeller/display_list/dl_dispatcher.cc index d999c5d9eaae4..8be3b9daf39bf 100644 --- a/impeller/display_list/dl_dispatcher.cc +++ b/impeller/display_list/dl_dispatcher.cc @@ -623,12 +623,15 @@ void DlDispatcher::save() { } // |flutter::DlOpReceiver| -void DlDispatcher::saveLayer(const SkRect* bounds, +void DlDispatcher::saveLayer(const SkRect& bounds, const flutter::SaveLayerOptions options, const flutter::DlImageFilter* backdrop) { auto paint = options.renders_with_attributes() ? paint_ : Paint{}; + auto promise = options.content_is_clipped() + ? ContentBoundsPromise::kMayClipContents + : ContentBoundsPromise::kContainsContents; canvas_.SaveLayer(paint, skia_conversions::ToRect(bounds), - ToImageFilter(backdrop)); + ToImageFilter(backdrop), promise); } // |flutter::DlOpReceiver| diff --git a/impeller/display_list/dl_dispatcher.h b/impeller/display_list/dl_dispatcher.h index 4ef1da1c29aea..4643e43ad1c94 100644 --- a/impeller/display_list/dl_dispatcher.h +++ b/impeller/display_list/dl_dispatcher.h @@ -72,7 +72,7 @@ class DlDispatcher final : public flutter::DlOpReceiver { void save() override; // |flutter::DlOpReceiver| - void saveLayer(const SkRect* bounds, + void saveLayer(const SkRect& bounds, const flutter::SaveLayerOptions options, const flutter::DlImageFilter* backdrop) override; diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index d84e8f935b575..ebce3faf8b2a7 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -60,14 +60,48 @@ void EntityPass::SetDelegate(std::shared_ptr delegate) { delegate_ = std::move(delegate); } -void EntityPass::SetBoundsLimit(std::optional bounds_limit) { +void EntityPass::SetBoundsLimit(std::optional bounds_limit, + ContentBoundsPromise bounds_promise) { bounds_limit_ = bounds_limit; + bounds_promise_ = bounds_limit.has_value() ? bounds_promise + : ContentBoundsPromise::kUnknown; } std::optional EntityPass::GetBoundsLimit() const { return bounds_limit_; } +bool EntityPass::GetBoundsLimitMightClipContent() const { + switch (bounds_promise_) { + case ContentBoundsPromise::kUnknown: + // If the promise is unknown due to not having a bounds limit, + // then no clipping will occur. But if we have a bounds limit + // and it is unkown, then we can make no promises about whether + // it causes clipping of the entity pass contents and we + // conservatively return true. + return bounds_limit_.has_value(); + case ContentBoundsPromise::kContainsContents: + FML_DCHECK(bounds_limit_.has_value()); + return false; + case ContentBoundsPromise::kMayClipContents: + FML_DCHECK(bounds_limit_.has_value()); + return true; + } + FML_UNREACHABLE(); +} + +bool EntityPass::GetBoundsLimitIsSnug() const { + switch (bounds_promise_) { + case ContentBoundsPromise::kUnknown: + return false; + case ContentBoundsPromise::kContainsContents: + case ContentBoundsPromise::kMayClipContents: + FML_DCHECK(bounds_limit_.has_value()); + return true; + } + FML_UNREACHABLE(); +} + void EntityPass::AddEntity(Entity entity) { if (entity.GetBlendMode() == BlendMode::kSourceOver && entity.GetContents()->IsOpaque()) { @@ -201,6 +235,10 @@ std::optional EntityPass::GetElementsCoverage( std::optional EntityPass::GetSubpassCoverage( const EntityPass& subpass, std::optional coverage_limit) const { + if (subpass.bounds_limit_.has_value() && subpass.GetBoundsLimitIsSnug()) { + return subpass.bounds_limit_->TransformBounds(subpass.transform_); + } + std::shared_ptr image_filter = subpass.delegate_->WithImageFilter(Rect(), subpass.transform_); diff --git a/impeller/entity/entity_pass.h b/impeller/entity/entity_pass.h index 322fbd18fd62f..4198bccd0fbba 100644 --- a/impeller/entity/entity_pass.h +++ b/impeller/entity/entity_pass.h @@ -23,6 +23,23 @@ namespace impeller { class ContentContext; class EntityPassClipRecorder; +/// Specifies how much to trust the bounds rectangle provided for a list +/// of contents. Used by both |EntityPass| and |Canvas::SaveLayer|. +enum class ContentBoundsPromise { + /// @brief The caller makes no claims related to the size of the bounds. + kUnknown, + + /// @brief The caller claims the bounds are a reasonably tight estimate + /// of the coverage of the contents and should contain all of the + /// contents. + kContainsContents, + + /// @brief The caller claims the bounds are a subset of an estimate of + /// the reasonably tight bounds but likely clips off some of the + /// contents. + kMayClipContents, +}; + class EntityPass { public: /// Elements are renderable items in the `EntityPass`. Each can either be an @@ -63,12 +80,26 @@ class EntityPass { /// For consistency with Skia, we effectively treat this like a /// rectangle clip by forcing the subpass texture size to never exceed /// it. - void SetBoundsLimit(std::optional bounds_limit); + /// + /// The entity pass will assume that these bounds cause a clipping + /// effect on the layer unless this call is followed up with a + /// call to |SetBoundsClipsContent()| specifying otherwise. + void SetBoundsLimit( + std::optional bounds_limit, + ContentBoundsPromise bounds_promise = ContentBoundsPromise::kUnknown); /// @brief Get the bounds limit, which is provided by the user when creating /// a SaveLayer. std::optional GetBoundsLimit() const; + /// @brief Indicates if the bounds limit set using |SetBoundsLimit()| + /// might clip the contents of the pass. + bool GetBoundsLimitMightClipContent() const; + + /// @brief Indicates if the bounds limit set using |SetBoundsLimit()| + /// is a reasonably tight estimate of the bounds of the contents. + bool GetBoundsLimitIsSnug() const; + size_t GetSubpassesDepth() const; /// @brief Add an entity to the current entity pass. @@ -322,6 +353,7 @@ class EntityPass { bool flood_clip_ = false; bool enable_offscreen_debug_checkerboard_ = false; std::optional bounds_limit_; + ContentBoundsPromise bounds_promise_ = ContentBoundsPromise::kUnknown; std::unique_ptr clip_replay_ = std::make_unique(); int32_t required_mip_count_ = 1; diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index 9b293bd00d06f..1592bcb1198f1 100644 --- a/impeller/entity/entity_unittests.cc +++ b/impeller/entity/entity_unittests.cc @@ -106,20 +106,22 @@ class TestPassDelegate final : public EntityPassDelegate { const bool collapse_; }; -auto CreatePassWithRectPath(Rect rect, - std::optional bounds_hint, - bool collapse = false) { +auto CreatePassWithRectPath( + Rect rect, + std::optional bounds_hint, + ContentBoundsPromise bounds_promise = ContentBoundsPromise::kUnknown, + bool collapse = false) { auto subpass = std::make_unique(); Entity entity; entity.SetContents(SolidColorContents::Make( PathBuilder{}.AddRect(rect).TakePath(), Color::Red())); subpass->AddEntity(std::move(entity)); subpass->SetDelegate(std::make_unique(collapse)); - subpass->SetBoundsLimit(bounds_hint); + subpass->SetBoundsLimit(bounds_hint, bounds_promise); return subpass; } -TEST_P(EntityTest, EntityPassRespectsSubpassBoundsLimit) { +TEST_P(EntityTest, EntityPassRespectsUntrustedSubpassBoundsLimit) { EntityPass pass; auto subpass0 = CreatePassWithRectPath(Rect::MakeLTRB(0, 0, 100, 100), @@ -146,13 +148,50 @@ TEST_P(EntityTest, EntityPassRespectsSubpassBoundsLimit) { ASSERT_RECT_NEAR(coverage.value(), Rect::MakeLTRB(50, 50, 900, 900)); } +TEST_P(EntityTest, EntityPassTrustsSnugSubpassBoundsLimit) { + EntityPass pass; + + auto subpass0 = // + CreatePassWithRectPath(Rect::MakeLTRB(10, 10, 90, 90), + Rect::MakeLTRB(5, 5, 95, 95), + ContentBoundsPromise::kContainsContents); + auto subpass1 = // + CreatePassWithRectPath(Rect::MakeLTRB(500, 500, 1000, 1000), + Rect::MakeLTRB(495, 495, 1005, 1005), + ContentBoundsPromise::kContainsContents); + + auto subpass0_coverage = + pass.GetSubpassCoverage(*subpass0.get(), std::nullopt); + EXPECT_TRUE(subpass0_coverage.has_value()); + // Result should be the overridden bounds + // (we lied about them being snug, but the property is respected) + EXPECT_RECT_NEAR(subpass0_coverage.value(), Rect::MakeLTRB(5, 5, 95, 95)); + + auto subpass1_coverage = + pass.GetSubpassCoverage(*subpass1.get(), std::nullopt); + EXPECT_TRUE(subpass1_coverage.has_value()); + // Result should be the overridden bounds + // (we lied about them being snug, but the property is respected) + EXPECT_RECT_NEAR(subpass1_coverage.value(), + Rect::MakeLTRB(495, 495, 1005, 1005)); + + pass.AddSubpass(std::move(subpass0)); + pass.AddSubpass(std::move(subpass1)); + + auto coverage = pass.GetElementsCoverage(std::nullopt); + EXPECT_TRUE(coverage.has_value()); + // This result should be the union of the overridden bounds + EXPECT_RECT_NEAR(coverage.value(), Rect::MakeLTRB(5, 5, 1005, 1005)); +} + TEST_P(EntityTest, EntityPassCanMergeSubpassIntoParent) { // Both a red and a blue box should appear if the pass merging has worked // correctly. EntityPass pass; auto subpass = CreatePassWithRectPath(Rect::MakeLTRB(0, 0, 100, 100), - Rect::MakeLTRB(50, 50, 150, 150), true); + Rect::MakeLTRB(50, 50, 150, 150), + ContentBoundsPromise::kUnknown, true); pass.AddSubpass(std::move(subpass)); Entity entity; diff --git a/shell/common/dl_op_spy.cc b/shell/common/dl_op_spy.cc index bcf213bffa8e9..f3995a45cf2f2 100644 --- a/shell/common/dl_op_spy.cc +++ b/shell/common/dl_op_spy.cc @@ -29,7 +29,7 @@ void DlOpSpy::setColorSource(const DlColorSource* source) { will_draw_ = true; } void DlOpSpy::save() {} -void DlOpSpy::saveLayer(const SkRect* bounds, +void DlOpSpy::saveLayer(const SkRect& bounds, const SaveLayerOptions options, const DlImageFilter* backdrop) {} void DlOpSpy::restore() {} diff --git a/shell/common/dl_op_spy.h b/shell/common/dl_op_spy.h index 8982b98d6914b..438a2757bf631 100644 --- a/shell/common/dl_op_spy.h +++ b/shell/common/dl_op_spy.h @@ -39,7 +39,7 @@ class DlOpSpy final : public virtual DlOpReceiver, void setColor(DlColor color) override; void setColorSource(const DlColorSource* source) override; void save() override; - void saveLayer(const SkRect* bounds, + void saveLayer(const SkRect& bounds, const SaveLayerOptions options, const DlImageFilter* backdrop) override; void restore() override; diff --git a/shell/platform/embedder/fixtures/verifyb143464703_soft_noxform.png b/shell/platform/embedder/fixtures/verifyb143464703_soft_noxform.png index 35f482b381d22..659473eedd28a 100644 Binary files a/shell/platform/embedder/fixtures/verifyb143464703_soft_noxform.png and b/shell/platform/embedder/fixtures/verifyb143464703_soft_noxform.png differ diff --git a/testing/display_list_testing.cc b/testing/display_list_testing.cc index 908afd9432755..03a37553ac84f 100644 --- a/testing/display_list_testing.cc +++ b/testing/display_list_testing.cc @@ -628,7 +628,7 @@ void DisplayListStreamDispatcher::save() { startl() << "{" << std::endl; indent(); } -void DisplayListStreamDispatcher::saveLayer(const SkRect* bounds, +void DisplayListStreamDispatcher::saveLayer(const SkRect& bounds, const SaveLayerOptions options, const DlImageFilter* backdrop) { startl() << "saveLayer(" << bounds << ", " << options; diff --git a/testing/display_list_testing.h b/testing/display_list_testing.h index 0657a5a63d6f4..74bc2826228a8 100644 --- a/testing/display_list_testing.h +++ b/testing/display_list_testing.h @@ -73,7 +73,7 @@ class DisplayListStreamDispatcher final : public DlOpReceiver { void setImageFilter(const DlImageFilter* filter) override; void save() override; - void saveLayer(const SkRect* bounds, + void saveLayer(const SkRect& bounds, const SaveLayerOptions options, const DlImageFilter* backdrop) override; void restore() override;