Don't use getDeviceClipBounds() to bound pic ops.

The values returned by SkCanvas::getDeviceClipBounds() are in the right
space, but have extra constraints on them that are not desirable for
bounding the logical bounds of draw operations:

  - they are integral
  - they are non-negative

We've been intersecting the bounds of each operation with these bounds,
which means we're mixing these bogus constraints into the bounds of each
recorded operation.  This percolates up to the SkPicutre cull rect too.

The most egregious way to see the problem is to record a draw op
entirely in negative space... it'll come back with empty logical bounds
rather than its correct (negative-space) bounds.  I've added a test
for this, and another test I also think should be passing but left
making it so as a follow up.

I've had to disable a couple tests asserting clips affect the bounds. :/

A possible follow-up might go back to using the clips to tighten the
bounds of the ops, just so long as we take the original user bounds and
map them with the CTM through to device space ourselves, rather than
relying on the recording canvas' clip stack.  I think this means we'd
need to maintain our own stack of device-space float SkRect clip bounds
while calculating these op bounds.

Bug: skia:7735
Change-Id: I6bf15f6b2a9ba4329a4eeae7f9d57aa8729ec1bb
Reviewed-on: https://skia-review.googlesource.com/126002
Commit-Queue: Mike Klein <mtklein@chromium.org>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Mike Klein <mtklein@chromium.org>
Reviewed-by: Brian Osman <brianosman@google.com>

1 files changed, 6 insertions, 0 deletions

diff --git a/tests/RecordDrawTest.cpp b/tests/RecordDrawTest.cpp
index 216865056a..de9d2333cd 100644
--- a/tests/RecordDrawTest.cpp
+++ b/tests/RecordDrawTest.cpp
@@ -130,6 +130,8 @@ static bool sloppy_rect_eq(SkRect a, SkRect b) {
     return outset.contains(b) && !inset.contains(b);
 }
 
+// TODO This would be nice, but we can't get it right today.
+#if 0
 DEF_TEST(RecordDraw_BasicBounds, r) {
     SkRecord record;
     SkRecorder recorder(&record, W, H);
@@ -146,6 +148,7 @@ DEF_TEST(RecordDraw_BasicBounds, r) {
         REPORTER_ASSERT(r, sloppy_rect_eq(SkRect::MakeWH(400, 480), bounds[i]));
     }
 }
+#endif
 
 // A regression test for crbug.com/409110.
 DEF_TEST(RecordDraw_TextBounds, r) {
@@ -232,6 +235,8 @@ DEF_TEST(RecordDraw_SaveLayerAffectsClipBounds, r) {
     REPORTER_ASSERT(r, sloppy_rect_eq(bounds[3], SkRect::MakeLTRB(0, 0, 50, 50)));
 }
 
+// TODO This would be nice, but we can't get it right today.
+#if 0
 // When a saveLayer provides an explicit bound and has a complex paint (e.g., one that
 // affects transparent black), that bound should serve to shrink the area of the required
 // backing store.
@@ -253,6 +258,7 @@ DEF_TEST(RecordDraw_SaveLayerBoundsAffectsClipBounds, r) {
     REPORTER_ASSERT(r, sloppy_rect_eq(bounds[1], SkRect::MakeLTRB(20, 20, 30, 30)));
     REPORTER_ASSERT(r, sloppy_rect_eq(bounds[2], SkRect::MakeLTRB(10, 10, 40, 40)));
 }
+#endif
 
 DEF_TEST(RecordDraw_drawImage, r){
     class SkCanvasMock : public SkCanvas {