Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit e63f578

Browse files
committed
distinguish local and dest/device clip bounds and enhance tests accordingly
1 parent a40df52 commit e63f578

File tree

8 files changed

+377
-124
lines changed

8 files changed

+377
-124
lines changed

display_list/display_list_builder.cc

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,15 @@ void DisplayListBuilder::clipPath(const SkPath& path,
644644
Push<ClipDifferencePathOp>(0, 1, path, is_aa);
645645
}
646646
}
647+
SkRect DisplayListBuilder::getLocalClipBounds() {
648+
SkM44 inverse;
649+
if (current_layer_->matrix.invert(&inverse)) {
650+
SkRect devBounds;
651+
current_layer_->clip_bounds.roundOut(&devBounds);
652+
return inverse.asM33().mapRect(devBounds);
653+
}
654+
return kMaxCullRect_;
655+
}
647656

648657
void DisplayListBuilder::drawPaint() {
649658
Push<DrawPaintOp>(0, 1);

display_list/display_list_builder.h

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,14 +193,27 @@ class DisplayListBuilder final : public virtual Dispatcher,
193193
void transform(const SkMatrix& matrix) { transform(&matrix); }
194194
void transform(const SkM44& matrix44) { transform(&matrix44); }
195195

196+
/// Returns the 4x4 full perspective transform representing all transform
197+
/// operations executed so far in this DisplayList within the enclosing
198+
/// save stack.
196199
SkM44 getTransformFullPerspective() { return current_layer_->matrix; }
200+
/// Returns the 3x3 partial perspective transform representing all transform
201+
/// operations executed so far in this DisplayList within the enclosing
202+
/// save stack.
197203
SkMatrix getTransform() { return current_layer_->matrix.asM33(); }
198204

199205
void clipRect(const SkRect& rect, SkClipOp clip_op, bool is_aa) override;
200206
void clipRRect(const SkRRect& rrect, SkClipOp clip_op, bool is_aa) override;
201207
void clipPath(const SkPath& path, SkClipOp clip_op, bool is_aa) override;
202208

203-
SkRect getClipBounds() { return current_layer_->clip_bounds; }
209+
/// Conservative estimate of the bounds of all outstanding clip operations
210+
/// measured in the coordinate space within which this DisplayList will
211+
/// be rendered.
212+
SkRect getDestinationClipBounds() { return current_layer_->clip_bounds; }
213+
/// Conservative estimate of the bounds of all outstanding clip operations
214+
/// transformed into the local coordinate space in which currently
215+
/// recorded rendering operations are interpreted.
216+
SkRect getLocalClipBounds();
204217

205218
void drawPaint() override;
206219
void drawPaint(const DlPaint& paint);

display_list/display_list_unittests.cc

Lines changed: 141 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -2032,88 +2032,179 @@ TEST(DisplayList, FullTransformAffectsCurrentTransform) {
20322032
ASSERT_EQ(curMatrix, matrix);
20332033
}
20342034

2035-
TEST(DisplayList, ClipRectAffectsCurrentClipBounds) {
2035+
TEST(DisplayList, ClipRectAffectsClipBounds) {
20362036
DisplayListBuilder builder;
2037-
SkRect clip = SkRect::MakeLTRB(10.2, 11.3, 20.4, 25.7);
2038-
builder.clipRect(clip, SkClipOp::kIntersect, false);
2039-
SkRect curBounds = builder.getClipBounds();
2040-
ASSERT_EQ(curBounds, clip);
2037+
SkRect clipBounds = SkRect::MakeLTRB(10.2, 11.3, 20.4, 25.7);
2038+
SkRect clipExpandedBounds = SkRect::MakeLTRB(10, 11, 21, 26);
2039+
builder.clipRect(clipBounds, SkClipOp::kIntersect, false);
2040+
2041+
// Save initial return values for testing restored values
2042+
SkRect initialLocalBounds = builder.getLocalClipBounds();
2043+
SkRect initialDestinationBounds = builder.getDestinationClipBounds();
2044+
ASSERT_EQ(initialLocalBounds, clipExpandedBounds);
2045+
ASSERT_EQ(initialDestinationBounds, clipBounds);
2046+
2047+
builder.save();
20412048
builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, false);
2042-
// CurrentTransform has changed
2043-
ASSERT_NE(builder.getClipBounds(), clip);
2044-
// Previous return values have not
2045-
ASSERT_EQ(curBounds, clip);
2049+
// Both clip bounds have changed
2050+
ASSERT_NE(builder.getLocalClipBounds(), clipExpandedBounds);
2051+
ASSERT_NE(builder.getDestinationClipBounds(), clipBounds);
2052+
// Previous return values have not changed
2053+
ASSERT_EQ(initialLocalBounds, clipExpandedBounds);
2054+
ASSERT_EQ(initialDestinationBounds, clipBounds);
2055+
builder.restore();
2056+
2057+
// save/restore returned the values to their original values
2058+
ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds);
2059+
ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds);
2060+
2061+
builder.save();
2062+
builder.scale(2, 2);
2063+
SkRect scaledExpandedBounds = SkRect::MakeLTRB(5, 5.5, 10.5, 13);
2064+
ASSERT_EQ(builder.getLocalClipBounds(), scaledExpandedBounds);
2065+
// Destination bounds are unaffected by transform
2066+
ASSERT_EQ(builder.getDestinationClipBounds(), clipBounds);
2067+
builder.restore();
2068+
2069+
// save/restore returned the values to their original values
2070+
ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds);
2071+
ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds);
20462072
}
20472073

2048-
TEST(DisplayList, ClipRRectAffectsCurrentClipBounds) {
2074+
TEST(DisplayList, ClipRRectAffectsClipBounds) {
20492075
DisplayListBuilder builder;
2050-
SkRRect clip = SkRRect::MakeRectXY({10.2, 11.3, 20.4, 25.7}, 3, 2);
2076+
SkRect clipBounds = SkRect::MakeLTRB(10.2, 11.3, 20.4, 25.7);
2077+
SkRect clipExpandedBounds = SkRect::MakeLTRB(10, 11, 21, 26);
2078+
SkRRect clip = SkRRect::MakeRectXY(clipBounds, 3, 2);
20512079
builder.clipRRect(clip, SkClipOp::kIntersect, false);
2052-
SkRect curBounds = builder.getClipBounds();
2053-
ASSERT_EQ(curBounds, clip.getBounds());
2080+
2081+
// Save initial return values for testing restored values
2082+
SkRect initialLocalBounds = builder.getLocalClipBounds();
2083+
SkRect initialDestinationBounds = builder.getDestinationClipBounds();
2084+
ASSERT_EQ(initialLocalBounds, clipExpandedBounds);
2085+
ASSERT_EQ(initialDestinationBounds, clipBounds);
2086+
2087+
builder.save();
20542088
builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, false);
2055-
// CurrentTransform has changed
2056-
ASSERT_NE(builder.getClipBounds(), clip.getBounds());
2057-
// Previous return values have not
2058-
ASSERT_EQ(curBounds, clip.getBounds());
2089+
// Both clip bounds have changed
2090+
ASSERT_NE(builder.getLocalClipBounds(), clipExpandedBounds);
2091+
ASSERT_NE(builder.getDestinationClipBounds(), clipBounds);
2092+
// Previous return values have not changed
2093+
ASSERT_EQ(initialLocalBounds, clipExpandedBounds);
2094+
ASSERT_EQ(initialDestinationBounds, clipBounds);
2095+
builder.restore();
2096+
2097+
// save/restore returned the values to their original values
2098+
ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds);
2099+
ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds);
2100+
2101+
builder.save();
2102+
builder.scale(2, 2);
2103+
SkRect scaledExpandedBounds = SkRect::MakeLTRB(5, 5.5, 10.5, 13);
2104+
ASSERT_EQ(builder.getLocalClipBounds(), scaledExpandedBounds);
2105+
// Destination bounds are unaffected by transform
2106+
ASSERT_EQ(builder.getDestinationClipBounds(), clipBounds);
2107+
builder.restore();
2108+
2109+
// save/restore returned the values to their original values
2110+
ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds);
2111+
ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds);
20592112
}
20602113

2061-
TEST(DisplayList, ClipPathAffectsCurrentClipBounds) {
2114+
TEST(DisplayList, ClipPathAffectsClipBounds) {
20622115
DisplayListBuilder builder;
20632116
SkPath clip = SkPath().addCircle(10.2, 11.3, 2).addCircle(20.4, 25.7, 2);
2117+
SkRect clipBounds = SkRect::MakeLTRB(8.2, 9.3, 22.4, 27.7);
2118+
SkRect clipExpandedBounds = SkRect::MakeLTRB(8, 9, 23, 28);
20642119
builder.clipPath(clip, SkClipOp::kIntersect, false);
2065-
SkRect curBounds = builder.getClipBounds();
2066-
ASSERT_EQ(curBounds, clip.getBounds());
2120+
2121+
// Save initial return values for testing restored values
2122+
SkRect initialLocalBounds = builder.getLocalClipBounds();
2123+
SkRect initialDestinationBounds = builder.getDestinationClipBounds();
2124+
ASSERT_EQ(initialLocalBounds, clipExpandedBounds);
2125+
ASSERT_EQ(initialDestinationBounds, clipBounds);
2126+
2127+
builder.save();
20672128
builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, false);
2068-
// CurrentTransform has changed
2069-
ASSERT_NE(builder.getClipBounds(), clip.getBounds());
2070-
// Previous return values have not
2071-
ASSERT_EQ(curBounds, clip.getBounds());
2129+
// Both clip bounds have changed
2130+
ASSERT_NE(builder.getLocalClipBounds(), clipExpandedBounds);
2131+
ASSERT_NE(builder.getDestinationClipBounds(), clipBounds);
2132+
// Previous return values have not changed
2133+
ASSERT_EQ(initialLocalBounds, clipExpandedBounds);
2134+
ASSERT_EQ(initialDestinationBounds, clipBounds);
2135+
builder.restore();
2136+
2137+
// save/restore returned the values to their original values
2138+
ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds);
2139+
ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds);
2140+
2141+
builder.save();
2142+
builder.scale(2, 2);
2143+
SkRect scaledExpandedBounds = SkRect::MakeLTRB(4, 4.5, 11.5, 14);
2144+
ASSERT_EQ(builder.getLocalClipBounds(), scaledExpandedBounds);
2145+
// Destination bounds are unaffected by transform
2146+
ASSERT_EQ(builder.getDestinationClipBounds(), clipBounds);
2147+
builder.restore();
2148+
2149+
// save/restore returned the values to their original values
2150+
ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds);
2151+
ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds);
20722152
}
20732153

2074-
TEST(DisplayList, DiffClipRectDoesNotAffectCurrentClipBounds) {
2154+
TEST(DisplayList, DiffClipRectDoesNotAffectClipBounds) {
20752155
DisplayListBuilder builder;
20762156
SkRect diff_clip = SkRect::MakeLTRB(0, 0, 15, 15);
2077-
SkRect clip = SkRect::MakeLTRB(10.2, 11.3, 20.4, 25.7);
2078-
builder.clipRect(clip, SkClipOp::kIntersect, false);
2157+
SkRect clipBounds = SkRect::MakeLTRB(10.2, 11.3, 20.4, 25.7);
2158+
SkRect clipExpandedBounds = SkRect::MakeLTRB(10, 11, 21, 26);
2159+
builder.clipRect(clipBounds, SkClipOp::kIntersect, false);
2160+
2161+
// Save initial return values for testing after kDifference clip
2162+
SkRect initialLocalBounds = builder.getLocalClipBounds();
2163+
SkRect initialDestinationBounds = builder.getDestinationClipBounds();
2164+
ASSERT_EQ(initialLocalBounds, clipExpandedBounds);
2165+
ASSERT_EQ(initialDestinationBounds, clipBounds);
2166+
20792167
builder.clipRect(diff_clip, SkClipOp::kDifference, false);
2080-
SkRect curBounds = builder.getClipBounds();
2081-
ASSERT_EQ(curBounds, clip);
2082-
builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, false);
2083-
// CurrentTransform has changed
2084-
ASSERT_NE(builder.getClipBounds(), clip);
2085-
// Previous return values have not
2086-
ASSERT_EQ(curBounds, clip);
2168+
ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds);
2169+
ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds);
20872170
}
20882171

2089-
TEST(DisplayList, DiffClipRRectAffectsCurrentClipBounds) {
2172+
TEST(DisplayList, DiffClipRRectDoesNotAffectClipBounds) {
20902173
DisplayListBuilder builder;
20912174
SkRRect diff_clip = SkRRect::MakeRectXY({0, 0, 15, 15}, 1, 1);
2175+
SkRect clipBounds = SkRect::MakeLTRB(10.2, 11.3, 20.4, 25.7);
2176+
SkRect clipExpandedBounds = SkRect::MakeLTRB(10, 11, 21, 26);
20922177
SkRRect clip = SkRRect::MakeRectXY({10.2, 11.3, 20.4, 25.7}, 3, 2);
20932178
builder.clipRRect(clip, SkClipOp::kIntersect, false);
2179+
2180+
// Save initial return values for testing after kDifference clip
2181+
SkRect initialLocalBounds = builder.getLocalClipBounds();
2182+
SkRect initialDestinationBounds = builder.getDestinationClipBounds();
2183+
ASSERT_EQ(initialLocalBounds, clipExpandedBounds);
2184+
ASSERT_EQ(initialDestinationBounds, clipBounds);
2185+
20942186
builder.clipRRect(diff_clip, SkClipOp::kDifference, false);
2095-
SkRect curBounds = builder.getClipBounds();
2096-
ASSERT_EQ(curBounds, clip.getBounds());
2097-
builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, false);
2098-
// CurrentTransform has changed
2099-
ASSERT_NE(builder.getClipBounds(), clip.getBounds());
2100-
// Previous return values have not
2101-
ASSERT_EQ(curBounds, clip.getBounds());
2187+
ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds);
2188+
ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds);
21022189
}
21032190

2104-
TEST(DisplayList, DiffClipPathAffectsCurrentClipBounds) {
2191+
TEST(DisplayList, DiffClipPathDoesNotAffectClipBounds) {
21052192
DisplayListBuilder builder;
21062193
SkPath diff_clip = SkPath().addRect({0, 0, 15, 15});
21072194
SkPath clip = SkPath().addCircle(10.2, 11.3, 2).addCircle(20.4, 25.7, 2);
2195+
SkRect clipBounds = SkRect::MakeLTRB(8.2, 9.3, 22.4, 27.7);
2196+
SkRect clipExpandedBounds = SkRect::MakeLTRB(8, 9, 23, 28);
21082197
builder.clipPath(clip, SkClipOp::kIntersect, false);
2198+
2199+
// Save initial return values for testing after kDifference clip
2200+
SkRect initialLocalBounds = builder.getLocalClipBounds();
2201+
SkRect initialDestinationBounds = builder.getDestinationClipBounds();
2202+
ASSERT_EQ(initialLocalBounds, clipExpandedBounds);
2203+
ASSERT_EQ(initialDestinationBounds, clipBounds);
2204+
21092205
builder.clipPath(diff_clip, SkClipOp::kDifference, false);
2110-
SkRect curBounds = builder.getClipBounds();
2111-
ASSERT_EQ(curBounds, clip.getBounds());
2112-
builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, false);
2113-
// CurrentTransform has changed
2114-
ASSERT_NE(builder.getClipBounds(), clip.getBounds());
2115-
// Previous return values have not
2116-
ASSERT_EQ(curBounds, clip.getBounds());
2206+
ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds);
2207+
ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds);
21172208
}
21182209

21192210
} // namespace testing

lib/ui/painting.dart

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4515,15 +4515,27 @@ class Canvas extends NativeFieldWrapperClass1 {
45154515
}
45164516
void _clipPath(Path path, bool doAntiAlias) native 'Canvas_clipPath';
45174517

4518-
/// Returns the bounds of the current clip including a conservative combined
4519-
/// result of all clip methods executed since the creation of this [Canvas]
4520-
/// object, and respecting the save/restore history.
4521-
///
4522-
/// The returned bounds are a conservative estimate of the bounds of the actual
4523-
/// clip based on intersecting the bounds of each clip method that was executed
4524-
/// with [ClipOp.intersect] and potentially ignoring any clip method that was
4525-
/// executed with [ClipOp.difference]. These [ClipOp] arguments are only present
4526-
/// on the [clipRect] method.
4518+
/// Returns the conservative bounds of the combined result of all clip methods
4519+
/// executed within the current save stack of this [Canvas] object, as measured
4520+
/// in the local coordinate space under which rendering operations are curretnly
4521+
/// performed.
4522+
///
4523+
/// The combined clip results are rounded out to an integer pixel boundary before
4524+
/// they are transformed back into the local coordinate space which accounts for
4525+
/// the pixel roundoff in rendering operations, particularly when antialiasing.
4526+
/// Because the [Picture] may eventually be rendered into a scene within the
4527+
/// context of transforming widgets or layers, the result may thus be overly
4528+
/// conservative due to premature rounding. Using the [getDestinationClipBounds]
4529+
/// method combined with the external transforms and rounding in the true device
4530+
/// coordinate system will produce more accurate results, but this value may
4531+
/// provide a more convenient approximation to compare rendering operations to
4532+
/// the established clip.
4533+
///
4534+
/// {@template dart.ui.canvas.conservativeClipBounds}
4535+
/// The conservative estimate of the bounds is based on intersecting the bounds
4536+
/// of each clip method that was executed with [ClipOp.intersect] and potentially
4537+
/// ignoring any clip method that was executed with [ClipOp.difference]. The
4538+
/// [ClipOp] argument is only present on the [clipRect] method.
45274539
///
45284540
/// To understand how the bounds estimate can be conservative, consider the
45294541
/// following two clip method calls:
@@ -4551,12 +4563,34 @@ class Canvas extends NativeFieldWrapperClass1 {
45514563
/// and [clipPath]. The [restore] method can also modify the current clip by
45524564
/// restoring it to the same value it had before its associated [save] or
45534565
/// [saveLayer] call.
4554-
Rect getClipBounds() {
4566+
/// {@endtemplate}
4567+
Rect getLocalClipBounds() {
4568+
final Float64List bounds = Float64List(4);
4569+
_getLocalClipBounds(bounds);
4570+
return Rect.fromLTRB(bounds[0], bounds[1], bounds[2], bounds[3]);
4571+
}
4572+
void _getLocalClipBounds(Float64List bounds) native 'Canvas_getLocalClipBounds';
4573+
4574+
/// Returns the conservative bounds of the combined result of all clip methods
4575+
/// executed within the current save stack of this [Canvas] object, as measured
4576+
/// in the destination coordinate space in which the [Picture] will be rendered.
4577+
///
4578+
/// Unlike [getLocalClipBounds], the bounds are not rounded out to an integer
4579+
/// pixel boundary as the Destination coordinate space may not represent pixels
4580+
/// if the [Picture] being constructed will be further transformed when it is
4581+
/// rendered or added to a scene. In order to determine the true pixels being
4582+
/// affected, those external transforms should be applied first before rounding
4583+
/// out the result to integer pixel boundaries. Most typically, [Picture] objects
4584+
/// are rendered in a scene with a scale transform representing the Device Pixel
4585+
/// Ratio.
4586+
///
4587+
/// {@macro dart.ui.canvas.conservativeClipBounds}
4588+
Rect getDestinationClipBounds() {
45554589
final Float64List bounds = Float64List(4);
4556-
_getClipBounds(bounds);
4590+
_getDestinationClipBounds(bounds);
45574591
return Rect.fromLTRB(bounds[0], bounds[1], bounds[2], bounds[3]);
45584592
}
4559-
void _getClipBounds(Float64List bounds) native 'Canvas_getClipBounds';
4593+
void _getDestinationClipBounds(Float64List bounds) native 'Canvas_getDestinationClipBounds';
45604594

45614595
/// Paints the given [Color] onto the canvas, applying the given
45624596
/// [BlendMode], with the given color being the source and the background

0 commit comments

Comments
 (0)