Prevent EndLayerOps when Begin was rejected

am: 3c53ec51ef

Change-Id: I075b68a1b7bdee4cbebd734748dd485ff6cd67b3

libs/hwui/RecordingCanvas.cpp

@@ -149,26 +149,26 @@ int RecordingCanvas::saveLayer(float left, float top, float right, float bottom,
 
     // Map visible bounds back to layer space, and intersect with parameter bounds
     Rect layerBounds = visibleBounds;
+    if (CC_LIKELY(!layerBounds.isEmpty())) {
+        // if non-empty, can safely map by the inverse transform
         Matrix4 inverse;
         inverse.loadInverse(*previous.transform);
         inverse.mapRect(layerBounds);
         layerBounds.doIntersect(unmappedBounds);
+    }
 
     int saveValue = mState.save((int) flags);
     Snapshot& snapshot = *mState.writableSnapshot();
 
     // layerBounds is in original bounds space, but clipped by current recording clip
-    if (layerBounds.isEmpty() || unmappedBounds.isEmpty()) {
+    if (!layerBounds.isEmpty() && !unmappedBounds.isEmpty()) {
-        // Don't bother recording layer, since it's been rejected
         if (CC_LIKELY(clippedLayer)) {
-            snapshot.resetClip(0, 0, 0, 0);
+            auto previousClip = getRecordedClip(); // capture before new snapshot clip has changed
-        }
+            if (addOp(alloc().create_trivial<BeginLayerOp>(
-        return saveValue;
+                    unmappedBounds,
-    }
+                    *previous.transform, // transform to *draw* with
-
+                    previousClip, // clip to *draw* with
-    if (CC_LIKELY(clippedLayer)) {
+                    refPaint(paint))) >= 0) {
-        auto previousClip = getRecordedClip(); // note: done before new snapshot's clip has changed
-
                 snapshot.flags |= Snapshot::kFlagIsLayer | Snapshot::kFlagIsFboLayer;
                 snapshot.initializeViewport(unmappedBounds.getWidth(), unmappedBounds.getHeight());
                 snapshot.transform->loadTranslate(-unmappedBounds.left, -unmappedBounds.top, 0.0f);
@@ -177,22 +177,25 @@ int RecordingCanvas::saveLayer(float left, float top, float right, float bottom,
                 clip.translate(-unmappedBounds.left, -unmappedBounds.top);
                 snapshot.resetClip(clip.left, clip.top, clip.right, clip.bottom);
                 snapshot.roundRectClipState = nullptr;
-
+                return saveValue;
-        addOp(alloc().create_trivial<BeginLayerOp>(
+            }
-                unmappedBounds,
-                *previous.transform, // transform to *draw* with
-                previousClip, // clip to *draw* with
-                refPaint(paint)));
         } else {
-        snapshot.flags |= Snapshot::kFlagIsLayer;
+            if (addOp(alloc().create_trivial<BeginUnclippedLayerOp>(
-
-        addOp(alloc().create_trivial<BeginUnclippedLayerOp>(
                     unmappedBounds,
                     *mState.currentSnapshot()->transform,
                     getRecordedClip(),
-                refPaint(paint)));
+                    refPaint(paint))) >= 0) {
+                snapshot.flags |= Snapshot::kFlagIsLayer;
+                return saveValue;
+            }
+        }
     }
 
+    // Layer not needed, so skip recording it...
+    if (CC_LIKELY(clippedLayer)) {
+        // ... and set empty clip to reject inner content, if possible
+        snapshot.resetClip(0, 0, 0, 0);
+    }
     return saveValue;
 }
 
@@ -619,7 +622,7 @@ void RecordingCanvas::callDrawGLFunction(Functor* functor,
             functor));
 }
 
-size_t RecordingCanvas::addOp(RecordedOp* op) {
+int RecordingCanvas::addOp(RecordedOp* op) {
     // skip op with empty clip
     if (op->localClip && op->localClip->rect.isEmpty()) {
         // NOTE: this rejection happens after op construction/content ref-ing, so content ref'd

libs/hwui/RecordingCanvas.h

@@ -208,7 +208,7 @@ private:
     void drawSimpleRects(const float* rects, int vertexCount, const SkPaint* paint);
 
 
-    size_t addOp(RecordedOp* op);
+    int addOp(RecordedOp* op);
 // ----------------------------------------------------------------------------
 // lazy object copy
 // ----------------------------------------------------------------------------

libs/hwui/tests/unit/RecordingCanvasTests.cpp

@@ -545,6 +545,21 @@ TEST(RecordingCanvas, saveLayer_rotateClipped) {
     EXPECT_EQ(3, count);
 }
 
+TEST(RecordingCanvas, saveLayer_rejectBegin) {
+    auto dl = TestUtils::createDisplayList<RecordingCanvas>(200, 200, [](RecordingCanvas& canvas) {
+        canvas.save(SaveFlags::MatrixClip);
+        canvas.translate(0, -20); // avoid identity case
+        // empty clip rect should force layer + contents to be rejected
+        canvas.clipRect(0, -20, 200, -20, SkRegion::kIntersect_Op);
+        canvas.saveLayerAlpha(0, 0, 200, 200, 128, SaveFlags::ClipToLayer);
+        canvas.drawRect(0, 0, 200, 200, SkPaint());
+        canvas.restore();
+        canvas.restore();
+    });
+
+    ASSERT_EQ(0u, dl->getOps().size()) << "Begin/Rect/End should all be rejected.";
+}
+
 TEST(RecordingCanvas, drawRenderNode_rejection) {
     auto child = TestUtils::createNode(50, 50, 150, 150,
             [](RenderProperties& props, RecordingCanvas& canvas) {