Handle unbounded drawPaint/drawGLFunction operations safely

bug:26591194

Also, revert to using current clip bounds as drawColor/drawPaint bounds
for simplicity in new pipeline.

Change-Id: I1a6b3f9716b564b46df41d57dfe14475fdd24de0

7 files changed, 49 insertions, 30 deletions

diff --git a/libs/hwui/BakedOpState.cpp b/libs/hwui/BakedOpState.cpp
index 26653f77daeb..859036543b4a 100644
--- a/libs/hwui/BakedOpState.cpp
+++ b/libs/hwui/BakedOpState.cpp
@@ -82,6 +82,16 @@ ResolvedRenderState::ResolvedRenderState(LinearAllocator& allocator, Snapshot& s
     }
 }
 
+ResolvedRenderState::ResolvedRenderState(LinearAllocator& allocator, Snapshot& snapshot,
+        const Matrix4& localTransform, const ClipBase* localClip) {
+    transform.loadMultiply(*snapshot.transform, localTransform);
+    clipState = snapshot.mutateClipArea().serializeIntersectedClip(allocator,
+            localClip, *(snapshot.transform));
+    clippedBounds = clipState->rect;
+    clipSideFlags = OpClipSideFlags::Full;
+    localProjectionPathMask = nullptr;
+}
+
 ResolvedRenderState::ResolvedRenderState(LinearAllocator& allocator, Snapshot& snapshot)
         : transform(*snapshot.transform)
         , clipState(snapshot.mutateClipArea().serializeClip(allocator))
diff --git a/libs/hwui/BakedOpState.h b/libs/hwui/BakedOpState.h
index ffe2901782a9..4e3cb8a15e24 100644
--- a/libs/hwui/BakedOpState.h
+++ b/libs/hwui/BakedOpState.h
@@ -55,6 +55,10 @@ public:
     ResolvedRenderState(LinearAllocator& allocator, Snapshot& snapshot,
             const RecordedOp& recordedOp, bool expandForStroke);
 
+    // Constructor for unbounded ops *with* transform/clip
+    ResolvedRenderState(LinearAllocator& allocator, Snapshot& snapshot,
+            const Matrix4& localTransform, const ClipBase* localClip);
+
     // Constructor for unbounded ops without transform/clip (namely shadows)
     ResolvedRenderState(LinearAllocator& allocator, Snapshot& snapshot);
 
@@ -111,8 +115,14 @@ public:
         return bakedState;
     }
 
+    static BakedOpState* tryConstructUnbounded(LinearAllocator& allocator,
+            Snapshot& snapshot, const RecordedOp& recordedOp) {
+        if (CC_UNLIKELY(snapshot.getRenderTargetClip().isEmpty())) return nullptr;
+        return allocator.create_trivial<BakedOpState>(allocator, snapshot, recordedOp);
+    }
+
     enum class StrokeBehavior {
-        // stroking is forced, regardless of style on paint
+        // stroking is forced, regardless of style on paint (such as for lines)
         Forced,
         // stroking is defined by style on paint
         StyleDefined,
@@ -167,6 +177,13 @@ private:
             , roundRectClipState(snapshot.roundRectClipState)
             , op(&recordedOp) {}
 
+    // TODO: fix this brittleness
+    BakedOpState(LinearAllocator& allocator, Snapshot& snapshot, const RecordedOp& recordedOp)
+            : computedState(allocator, snapshot, recordedOp.localMatrix, recordedOp.localClip)
+            , alpha(snapshot.alpha)
+            , roundRectClipState(snapshot.roundRectClipState)
+            , op(&recordedOp) {}
+
     BakedOpState(LinearAllocator& allocator, Snapshot& snapshot, const ShadowOp* shadowOpPtr)
             : computedState(allocator, snapshot)
             , alpha(snapshot.alpha)
diff --git a/libs/hwui/FrameBuilder.cpp b/libs/hwui/FrameBuilder.cpp
index fd5856a1c1de..2855db08d819 100644
--- a/libs/hwui/FrameBuilder.cpp
+++ b/libs/hwui/FrameBuilder.cpp
@@ -574,7 +574,7 @@ void FrameBuilder::deferCirclePropsOp(const CirclePropsOp& op) {
 }
 
 void FrameBuilder::deferFunctorOp(const FunctorOp& op) {
-    BakedOpState* bakedState = tryBakeOpState(op);
+    BakedOpState* bakedState = tryBakeUnboundedOpState(op);
     if (!bakedState) return; // quick rejected
     currentLayer().deferUnmergeableOp(mAllocator, bakedState, OpBatchType::Functor);
 }
@@ -661,7 +661,7 @@ void FrameBuilder::deferTextOp(const TextOp& op) {
 }
 
 void FrameBuilder::deferTextOnPathOp(const TextOnPathOp& op) {
-    BakedOpState* bakedState = tryBakeOpState(op);
+    BakedOpState* bakedState = tryBakeUnboundedOpState(op);
     if (!bakedState) return; // quick rejected
     currentLayer().deferUnmergeableOp(mAllocator, bakedState, textBatchId(*(op.paint)));
 }
diff --git a/libs/hwui/FrameBuilder.h b/libs/hwui/FrameBuilder.h
index f44306a931bf..8a00d336dc08 100644
--- a/libs/hwui/FrameBuilder.h
+++ b/libs/hwui/FrameBuilder.h
@@ -173,6 +173,10 @@ private:
     BakedOpState* tryBakeOpState(const RecordedOp& recordedOp) {
         return BakedOpState::tryConstruct(mAllocator, *mCanvasState.writableSnapshot(), recordedOp);
     }
+    BakedOpState* tryBakeUnboundedOpState(const RecordedOp& recordedOp) {
+        return BakedOpState::tryConstructUnbounded(mAllocator, *mCanvasState.writableSnapshot(), recordedOp);
+    }
+
 
     // should always be surrounded by a save/restore pair, and not called if DisplayList is null
     void deferNodePropsAndOps(RenderNode& node);
diff --git a/libs/hwui/RecordedOp.h b/libs/hwui/RecordedOp.h
index c37458d31029..96a57b6a8efe 100644
--- a/libs/hwui/RecordedOp.h
+++ b/libs/hwui/RecordedOp.h
@@ -257,8 +257,10 @@ struct CirclePropsOp : RecordedOp {
 };
 
 struct FunctorOp : RecordedOp {
-    FunctorOp(BASE_PARAMS_PAINTLESS, Functor* functor)
-            : SUPER_PAINTLESS(FunctorOp)
+    // Note: undefined record-time bounds, since this op fills the clip
+    // TODO: explicitly define bounds
+    FunctorOp(const Matrix4& localMatrix, const ClipBase* localClip, Functor* functor)
+            : RecordedOp(RecordedOpId::FunctorOp, Rect(), localMatrix, localClip, nullptr)
             , functor(functor) {}
     Functor* functor;
 };
@@ -385,9 +387,10 @@ struct TextOp : RecordedOp {
 };
 
 struct TextOnPathOp : RecordedOp {
-    TextOnPathOp(BASE_PARAMS, const glyph_t* glyphs, int glyphCount,
-            const SkPath* path, float hOffset, float vOffset)
-            : SUPER(TextOnPathOp)
+    // TODO: explicitly define bounds
+    TextOnPathOp(const Matrix4& localMatrix, const ClipBase* localClip, const SkPaint* paint,
+            const glyph_t* glyphs, int glyphCount, const SkPath* path, float hOffset, float vOffset)
+            : RecordedOp(RecordedOpId::TextOnPathOp, Rect(), localMatrix, localClip, paint)
             , glyphs(glyphs)
             , glyphCount(glyphCount)
             , path(path)
diff --git a/libs/hwui/RecordingCanvas.cpp b/libs/hwui/RecordingCanvas.cpp
index 11eb825a56b0..1546baf88691 100644
--- a/libs/hwui/RecordingCanvas.cpp
+++ b/libs/hwui/RecordingCanvas.cpp
@@ -24,14 +24,6 @@
 namespace android {
 namespace uirenderer {
 
-#define MIL_PIX 1000000
-static Rect sUnreasonablyLargeBounds(-MIL_PIX, -MIL_PIX, MIL_PIX, MIL_PIX);
-
-static const Rect& getConservativeOpBounds(const ClipBase* clip) {
-    // if op is clipped, that rect can be used, but otherwise just use a conservatively large rect
-    return clip ? clip->rect : sUnreasonablyLargeBounds;
-}
-
 RecordingCanvas::RecordingCanvas(size_t width, size_t height)
         : mState(*this)
         , mResourceCache(ResourceCache::getInstance()) {
@@ -249,12 +241,10 @@ void RecordingCanvas::drawColor(int color, SkXfermode::Mode mode) {
 }
 
 void RecordingCanvas::drawPaint(const SkPaint& paint) {
-    const ClipBase* clip = getRecordedClip();
-    addOp(alloc().create_trivial<RectOp>(
-            getConservativeOpBounds(clip),
-            Matrix4::identity(),
-            clip,
-            refPaint(&paint)));
+    SkRect bounds;
+    if (getClipBounds(&bounds)) {
+        drawRect(bounds.fLeft, bounds.fTop, bounds.fRight, bounds.fBottom, paint);
+    }
 }
 
 static Rect calcBoundsOfPoints(const float* points, int floatCount) {
@@ -544,11 +534,9 @@ void RecordingCanvas::drawTextOnPath(const uint16_t* glyphs, int glyphCount, con
             float hOffset, float vOffset, const SkPaint& paint) {
     if (!glyphs || glyphCount <= 0 || PaintUtils::paintWillNotDrawText(paint)) return;
     glyphs = refBuffer<glyph_t>(glyphs, glyphCount);
-    auto clip = getRecordedClip();
     addOp(alloc().create_trivial<TextOnPathOp>(
-            getConservativeOpBounds(clip), // TODO: explicitly define bounds
             *(mState.currentSnapshot()->transform),
-            clip,
+            getRecordedClip(),
             refPaint(&paint), glyphs, glyphCount, refPath(&path), hOffset, vOffset));
 }
 
@@ -599,11 +587,9 @@ void RecordingCanvas::drawLayer(DeferredLayerUpdater* layerHandle) {
 
 void RecordingCanvas::callDrawGLFunction(Functor* functor) {
     mDisplayList->functors.push_back(functor);
-    auto clip = getRecordedClip();
     addOp(alloc().create_trivial<FunctorOp>(
-            getConservativeOpBounds(clip),
             *(mState.currentSnapshot()->transform),
-            clip,
+            getRecordedClip(),
             functor));
 }
 
diff --git a/libs/hwui/tests/unit/RecordingCanvasTests.cpp b/libs/hwui/tests/unit/RecordingCanvasTests.cpp
index 5e613fd50c3e..e6d84c6681f1 100644
--- a/libs/hwui/tests/unit/RecordingCanvasTests.cpp
+++ b/libs/hwui/tests/unit/RecordingCanvasTests.cpp
@@ -223,8 +223,7 @@ TEST(RecordingCanvas, drawColor) {
     auto op = *(dl->getOps()[0]);
     EXPECT_EQ(RecordedOpId::RectOp, op.opId);
     EXPECT_EQ(nullptr, op.localClip);
-    EXPECT_TRUE(op.unmappedBounds.contains(Rect(-1000, -1000, 1000, 1000)))
-            << "no clip, unmappedBounds should resolve to be much larger than DL bounds";
+    EXPECT_TRUE(op.unmappedBounds.contains(Rect(200, 200))) << "Expect recording/clip bounds";
 }
 
 TEST(RecordingCanvas, backgroundAndImage) {