Fix cull nesting assertion.
Cull rects are in local coordinates and cannot be compared directly.
No wonder it was so hard enforcing this in Blink :o
This moves the validation logic into SkCanvas, using a device-space
cull stack (debug build only).
There are still some Blink bugs causing violations, so for now I'd like
to keep this as an error message only.
R=reed@google.com, robertphillips@google.com
Author: fmalita@chromium.org
Review URL: https://codereview.chromium.org/200923008
git-svn-id: http://skia.googlecode.com/svn/trunk@13885 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/include/core/SkCanvas.h b/include/core/SkCanvas.h
index fa433b3..f5d4fe6 100644
--- a/include/core/SkCanvas.h
+++ b/include/core/SkCanvas.h
@@ -1063,20 +1063,13 @@
* is not enforced, but the information might be used to quick-reject command blocks,
* so an incorrect bounding box may result in incomplete rendering.
*/
- void pushCull(const SkRect& cullRect) {
- ++fCullCount;
- this->onPushCull(cullRect);
- }
+ void pushCull(const SkRect& cullRect);
/**
* Terminates the current culling block, and restores the previous one (if any).
*/
- void popCull() {
- if (fCullCount > 0) {
- --fCullCount;
- this->onPopCull();
- }
- }
+ void popCull();
+
//////////////////////////////////////////////////////////////////////////
/** Get the current bounder object.
@@ -1394,6 +1387,9 @@
};
#ifdef SK_DEBUG
+ // The cull stack rects are in device-space
+ SkTDArray<SkIRect> fCullStack;
+ void validateCull(const SkIRect&);
void validateClip() const;
#else
void validateClip() const {}
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index cd2065b..787d89d 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -1162,6 +1162,60 @@
}
/////////////////////////////////////////////////////////////////////////////
+#ifdef SK_DEBUG
+// Ensure that cull rects are monotonically nested in device space.
+void SkCanvas::validateCull(const SkIRect& devCull) {
+ if (fCullStack.isEmpty()
+ || devCull.isEmpty()
+ || fCullStack.top().contains(devCull)) {
+ return;
+ }
+
+ SkDEBUGF(("Invalid cull: [%d %d %d %d] (previous cull: [%d %d %d %d])\n",
+ devCull.x(), devCull.y(), devCull.right(), devCull.bottom(),
+ fCullStack.top().x(), fCullStack.top().y(),
+ fCullStack.top().right(), fCullStack.top().bottom()));
+
+#ifdef ASSERT_NESTED_CULLING
+ SkDEBUGFAIL("Invalid cull.");
+#endif
+}
+#endif
+
+void SkCanvas::pushCull(const SkRect& cullRect) {
+ ++fCullCount;
+ this->onPushCull(cullRect);
+
+#ifdef SK_DEBUG
+ // Map the cull rect into device space.
+ SkRect mappedCull;
+ this->getTotalMatrix().mapRect(&mappedCull, cullRect);
+
+ // Take clipping into account.
+ SkIRect devClip, devCull;
+ mappedCull.roundOut(&devCull);
+ this->getClipDeviceBounds(&devClip);
+ if (!devCull.intersect(devClip)) {
+ devCull.setEmpty();
+ }
+
+ this->validateCull(devCull);
+ fCullStack.push(devCull); // balanced in popCull
+#endif
+}
+
+void SkCanvas::popCull() {
+ SkASSERT(fCullStack.count() == fCullCount);
+
+ if (fCullCount > 0) {
+ --fCullCount;
+ this->onPopCull();
+
+ SkDEBUGCODE(fCullStack.pop());
+ }
+}
+
+/////////////////////////////////////////////////////////////////////////////
void SkCanvas::internalDrawBitmap(const SkBitmap& bitmap,
const SkMatrix& matrix, const SkPaint* paint) {
diff --git a/src/core/SkPictureRecord.cpp b/src/core/SkPictureRecord.cpp
index 55670ba..ce21d95 100644
--- a/src/core/SkPictureRecord.cpp
+++ b/src/core/SkPictureRecord.cpp
@@ -1559,18 +1559,6 @@
// [op/size] [rect] [skip offset]
static const uint32_t kPushCullOpSize = 2 * kUInt32Size + sizeof(SkRect);
void SkPictureRecord::onPushCull(const SkRect& cullRect) {
- // Skip identical cull rects.
- if (!fCullOffsetStack.isEmpty()) {
- const SkRect& prevCull = fWriter.readTAt<SkRect>(fCullOffsetStack.top() - sizeof(SkRect));
- if (prevCull == cullRect) {
- // Skipped culls are tracked on the stack, but they point to the previous offset.
- fCullOffsetStack.push(fCullOffsetStack.top());
- return;
- }
-
- SkASSERT(prevCull.contains(cullRect));
- }
-
uint32_t size = kPushCullOpSize;
size_t initialOffset = this->addDraw(PUSH_CULL, &size);
// PUSH_CULL's size should stay constant (used to rewind).
@@ -1588,11 +1576,6 @@
uint32_t cullSkipOffset = fCullOffsetStack.top();
fCullOffsetStack.pop();
- // Skipped push, do the same for pop.
- if (!fCullOffsetStack.isEmpty() && cullSkipOffset == fCullOffsetStack.top()) {
- return;
- }
-
// Collapse empty push/pop pairs.
if ((size_t)(cullSkipOffset + kUInt32Size) == fWriter.bytesWritten()) {
SkASSERT(fWriter.bytesWritten() >= kPushCullOpSize);