From 612d347a17fa7b1fc3ce5b15f75865d433e3857c Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Thu, 23 Dec 2021 14:29:36 -0800 Subject: [PATCH 1/2] rename the DL bounds accumulation methods to be more transparent --- display_list/display_list_canvas_unittests.cc | 4 +- display_list/display_list_unittests.cc | 11 +++ display_list/display_list_utils.cc | 80 ++++++++++--------- display_list/display_list_utils.h | 28 +++++-- 4 files changed, 78 insertions(+), 45 deletions(-) diff --git a/display_list/display_list_canvas_unittests.cc b/display_list/display_list_canvas_unittests.cc index 5c279034afd91..674ef3bc0a3a8 100644 --- a/display_list/display_list_canvas_unittests.cc +++ b/display_list/display_list_canvas_unittests.cc @@ -1039,7 +1039,7 @@ class CanvasCompareTester { { RenderWith(testP, env, tolerance, CaseParameters( - "ImageFilter == Blender Arithmetic 0.25-false", + "Blender == Arithmetic 0.25-false", [=](SkCanvas*, SkPaint& p) { p.setBlender(blender); }, [=](DisplayListBuilder& b) { b.setBlender(blender); })); } @@ -1048,7 +1048,7 @@ class CanvasCompareTester { { RenderWith(testP, env, tolerance, CaseParameters( - "ImageFilter == Blender Arithmetic 0.25-true", + "Blender == Arithmetic 0.25-true", [=](SkCanvas*, SkPaint& p) { p.setBlender(blender); }, [=](DisplayListBuilder& b) { b.setBlender(blender); })); } diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index 3dc53324c2e9c..694ede58f6210 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -1575,5 +1575,16 @@ TEST(DisplayList, SaveLayerTrueSupportsGroupOpacityWithChildSrcBlend) { EXPECT_TRUE(display_list->can_apply_group_opacity()); } +TEST(DisplayList, SaveLayerBoundsSnapshotsImageFilter) { + DisplayListBuilder builder; + builder.saveLayer(nullptr, true); + builder.drawRect({50, 50, 100, 100}); + // This image filter should be ignored since it was not set before saveLayer + builder.setImageFilter(TestImageFilter1); + builder.restore(); + SkRect bounds = builder.Build()->bounds(); + EXPECT_EQ(bounds, SkRect::MakeLTRB(50, 50, 100, 100)); +} + } // namespace testing } // namespace flutter diff --git a/display_list/display_list_utils.cc b/display_list/display_list_utils.cc index af7083620f285..8b09c5b788d65 100644 --- a/display_list/display_list_utils.cc +++ b/display_list/display_list_utils.cc @@ -330,11 +330,12 @@ void DisplayListBoundsCalculator::restore() { // the unbounded state may be contained at a higher level, so we at // least accumulate our best estimate about what we have. if (!layer_bounds.isEmpty()) { - // kUnfiltered because the layer already applied all bounds - // modifications based on the attributes that were in place - // when it was instantiated. Modifying it further base on the - // current attributes would mix attribute states. - AccumulateRect(layer_bounds, kSaveLayerFlags); + // We do not use AccumulateOpBounds because the layer info already + // applied all bounds modifications based on the attributes that + // were in place when it was created. Modifying the bounds further + // base on the current attributes would mix attribute states. + // The bounds are still transformed and clipped by this method. + AccumulateBounds(layer_bounds); } if (layer_unbounded) { AccumulateUnbounded(); @@ -354,32 +355,32 @@ void DisplayListBoundsCalculator::drawLine(const SkPoint& p0, DisplayListAttributeFlags flags = (bounds.width() > 0.0f && bounds.height() > 0.0f) ? kDrawLineFlags : kDrawHVLineFlags; - AccumulateRect(bounds, flags); + AccumulateOpBounds(bounds, flags); } void DisplayListBoundsCalculator::drawRect(const SkRect& rect) { - AccumulateRect(rect, kDrawRectFlags); + AccumulateOpBounds(rect, kDrawRectFlags); } void DisplayListBoundsCalculator::drawOval(const SkRect& bounds) { - AccumulateRect(bounds, kDrawOvalFlags); + AccumulateOpBounds(bounds, kDrawOvalFlags); } void DisplayListBoundsCalculator::drawCircle(const SkPoint& center, SkScalar radius) { - AccumulateRect(SkRect::MakeLTRB(center.fX - radius, center.fY - radius, - center.fX + radius, center.fY + radius), - kDrawCircleFlags); + AccumulateOpBounds(SkRect::MakeLTRB(center.fX - radius, center.fY - radius, + center.fX + radius, center.fY + radius), + kDrawCircleFlags); } void DisplayListBoundsCalculator::drawRRect(const SkRRect& rrect) { - AccumulateRect(rrect.getBounds(), kDrawRRectFlags); + AccumulateOpBounds(rrect.getBounds(), kDrawRRectFlags); } void DisplayListBoundsCalculator::drawDRRect(const SkRRect& outer, const SkRRect& inner) { - AccumulateRect(outer.getBounds(), kDrawDRRectFlags); + AccumulateOpBounds(outer.getBounds(), kDrawDRRectFlags); } void DisplayListBoundsCalculator::drawPath(const SkPath& path) { if (path.isInverseFillType()) { AccumulateUnbounded(); } else { - AccumulateRect(path.getBounds(), kDrawPathFlags); + AccumulateOpBounds(path.getBounds(), kDrawPathFlags); } } void DisplayListBoundsCalculator::drawArc(const SkRect& bounds, @@ -389,10 +390,10 @@ void DisplayListBoundsCalculator::drawArc(const SkRect& bounds, // This could be tighter if we compute where the start and end // angles are and then also consider the quadrants swept and // the center if specified. - AccumulateRect(bounds, - useCenter // - ? kDrawArcWithCenterFlags - : kDrawArcNoCenterFlags); + AccumulateOpBounds(bounds, + useCenter // + ? kDrawArcWithCenterFlags + : kDrawArcNoCenterFlags); } void DisplayListBoundsCalculator::drawPoints(SkCanvas::PointMode mode, uint32_t count, @@ -405,20 +406,20 @@ void DisplayListBoundsCalculator::drawPoints(SkCanvas::PointMode mode, SkRect point_bounds = ptBounds.bounds(); switch (mode) { case SkCanvas::kPoints_PointMode: - AccumulateRect(point_bounds, kDrawPointsAsPointsFlags); + AccumulateOpBounds(point_bounds, kDrawPointsAsPointsFlags); break; case SkCanvas::kLines_PointMode: - AccumulateRect(point_bounds, kDrawPointsAsLinesFlags); + AccumulateOpBounds(point_bounds, kDrawPointsAsLinesFlags); break; case SkCanvas::kPolygon_PointMode: - AccumulateRect(point_bounds, kDrawPointsAsPolygonFlags); + AccumulateOpBounds(point_bounds, kDrawPointsAsPolygonFlags); break; } } } void DisplayListBoundsCalculator::drawVertices(const sk_sp vertices, SkBlendMode mode) { - AccumulateRect(vertices->bounds(), kDrawVerticesFlags); + AccumulateOpBounds(vertices->bounds(), kDrawVerticesFlags); } void DisplayListBoundsCalculator::drawImage(const sk_sp image, const SkPoint point, @@ -429,7 +430,7 @@ void DisplayListBoundsCalculator::drawImage(const sk_sp image, DisplayListAttributeFlags flags = render_with_attributes // ? kDrawImageWithPaintFlags : kDrawImageFlags; - AccumulateRect(bounds, flags); + AccumulateOpBounds(bounds, flags); } void DisplayListBoundsCalculator::drawImageRect( const sk_sp image, @@ -441,7 +442,7 @@ void DisplayListBoundsCalculator::drawImageRect( DisplayListAttributeFlags flags = render_with_attributes ? kDrawImageRectWithPaintFlags : kDrawImageRectFlags; - AccumulateRect(dst, flags); + AccumulateOpBounds(dst, flags); } void DisplayListBoundsCalculator::drawImageNine(const sk_sp image, const SkIRect& center, @@ -451,7 +452,7 @@ void DisplayListBoundsCalculator::drawImageNine(const sk_sp image, DisplayListAttributeFlags flags = render_with_attributes ? kDrawImageNineWithPaintFlags : kDrawImageNineFlags; - AccumulateRect(dst, flags); + AccumulateOpBounds(dst, flags); } void DisplayListBoundsCalculator::drawImageLattice( const sk_sp image, @@ -462,7 +463,7 @@ void DisplayListBoundsCalculator::drawImageLattice( DisplayListAttributeFlags flags = render_with_attributes ? kDrawImageLatticeWithPaintFlags : kDrawImageLatticeFlags; - AccumulateRect(dst, flags); + AccumulateOpBounds(dst, flags); } void DisplayListBoundsCalculator::drawAtlas(const sk_sp atlas, const SkRSXform xform[], @@ -486,7 +487,7 @@ void DisplayListBoundsCalculator::drawAtlas(const sk_sp atlas, DisplayListAttributeFlags flags = render_with_attributes // ? kDrawAtlasWithPaintFlags : kDrawAtlasFlags; - AccumulateRect(atlasBounds.bounds(), flags); + AccumulateOpBounds(atlasBounds.bounds(), flags); } } void DisplayListBoundsCalculator::drawPicture(const sk_sp picture, @@ -502,16 +503,16 @@ void DisplayListBoundsCalculator::drawPicture(const sk_sp picture, DisplayListAttributeFlags flags = render_with_attributes // ? kDrawPictureWithPaintFlags : kDrawPictureFlags; - AccumulateRect(bounds, flags); + AccumulateOpBounds(bounds, flags); } void DisplayListBoundsCalculator::drawDisplayList( const sk_sp display_list) { - AccumulateRect(display_list->bounds(), kDrawDisplayListFlags); + AccumulateOpBounds(display_list->bounds(), kDrawDisplayListFlags); } void DisplayListBoundsCalculator::drawTextBlob(const sk_sp blob, SkScalar x, SkScalar y) { - AccumulateRect(blob->bounds().makeOffset(x, y), kDrawTextBlobFlags); + AccumulateOpBounds(blob->bounds().makeOffset(x, y), kDrawTextBlobFlags); } void DisplayListBoundsCalculator::drawShadow(const SkPath& path, const SkColor color, @@ -520,7 +521,7 @@ void DisplayListBoundsCalculator::drawShadow(const SkPath& path, SkScalar dpr) { SkRect shadow_bounds = DisplayListCanvasDispatcher::ComputeShadowBounds( path, elevation, dpr, matrix()); - AccumulateRect(shadow_bounds, kDrawShadowFlags); + AccumulateOpBounds(shadow_bounds, kDrawShadowFlags); } bool DisplayListBoundsCalculator::ComputeFilteredBounds(SkRect& bounds, @@ -596,18 +597,21 @@ void DisplayListBoundsCalculator::AccumulateUnbounded() { layer_infos_.back()->set_unbounded(); } } -void DisplayListBoundsCalculator::AccumulateRect( - SkRect& rect, +void DisplayListBoundsCalculator::AccumulateOpBounds( + SkRect& bounds, DisplayListAttributeFlags flags) { - if (AdjustBoundsForPaint(rect, flags)) { - matrix().mapRect(&rect); - if (!has_clip() || rect.intersect(clip_bounds())) { - accumulator_->accumulate(rect); - } + if (AdjustBoundsForPaint(bounds, flags)) { + AccumulateBounds(bounds); } else { AccumulateUnbounded(); } } +void DisplayListBoundsCalculator::AccumulateBounds(SkRect& bounds) { + matrix().mapRect(&bounds); + if (!has_clip() || bounds.intersect(clip_bounds())) { + accumulator_->accumulate(bounds); + } +} bool DisplayListBoundsCalculator::paint_nops_on_transparency() { // SkImageFilter::canComputeFastBounds tests for transparency behavior diff --git a/display_list/display_list_utils.h b/display_list/display_list_utils.h index a70b9026b5dae..2a60cadcdffbf 100644 --- a/display_list/display_list_utils.h +++ b/display_list/display_list_utils.h @@ -578,15 +578,33 @@ class DisplayListBoundsCalculator final bool paint_nops_on_transparency(); - static bool ComputeFilteredBounds(SkRect& rect, SkImageFilter* filter); + // Computes the bounds of an operation adjusted for a given ImageFilter + static bool ComputeFilteredBounds(SkRect& bounds, SkImageFilter* 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); + // Records the fact that we encountered an op that either could not + // estimate its bounds or that fills all of the destination space. void AccumulateUnbounded(); - void AccumulateRect(const SkRect& rect, DisplayListAttributeFlags flags) { - SkRect bounds = rect; - AccumulateRect(bounds, flags); + + // Records the bounds for an op after modifying them according to the + // supplied attribute flags and transforming by the current matrix. + void AccumulateOpBounds(const SkRect& bounds, + DisplayListAttributeFlags flags) { + SkRect safe_bounds = bounds; + AccumulateOpBounds(safe_bounds, flags); } - void AccumulateRect(SkRect& rect, DisplayListAttributeFlags flags); + + // Records the bounds for an op after modifying them according to the + // supplied attribute flags and transforming by the current matrix + // and clipping against the current clip. + void AccumulateOpBounds(SkRect& bounds, DisplayListAttributeFlags flags); + + // Records the given bounds after transforming by the current matrix + // and clipping against the current clip. + void AccumulateBounds(SkRect& bounds); }; } // namespace flutter From 5a1c5cd78b2f4c50ca48695dec178e2081459f9d Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Thu, 23 Dec 2021 14:40:15 -0800 Subject: [PATCH 2/2] typo in comment --- display_list/display_list_utils.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/display_list/display_list_utils.cc b/display_list/display_list_utils.cc index 8b09c5b788d65..db341c73f8948 100644 --- a/display_list/display_list_utils.cc +++ b/display_list/display_list_utils.cc @@ -333,7 +333,7 @@ void DisplayListBoundsCalculator::restore() { // We do not use AccumulateOpBounds because the layer info already // applied all bounds modifications based on the attributes that // were in place when it was created. Modifying the bounds further - // base on the current attributes would mix attribute states. + // based on the current attributes would mix attribute states. // The bounds are still transformed and clipped by this method. AccumulateBounds(layer_bounds); }