Rename/simplify SkCanvas::resetClip() and make recordable

AndroidFramework uses both their own custom display list (which could
handle resetClip with android-side changes) AND conventional picture
recording. In order for replace op emulation to work when they have
been recorded into a picture, we need to make it virtual and supported
in SkPicture.

This also renames the API to ResetClip() from ReplaceClip() and does not
have any additional arguments. Based on AF's usage pattern, it only n
needs to reset the clip to the surface bounds or the device clip
restriction, it seems best to reduce the API as much as possible before
it's adopted.

Bug: skia:10209
Change-Id: I37adb097c84a642f4254b8c0f9d4c7fea8d9abdf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/430897
Reviewed-by: Mike Reed <[email protected]>
Reviewed-by: Derek Sollenberger <[email protected]>
Commit-Queue: Michael Ludwig <[email protected]>

RELEASE_NOTES.txt

@@ -6,7 +6,9 @@ This file includes a list of high level updates for each milestone release.
 
 Milestone 94
 ------------
-  *
+  * Added virtual onResetClip() to SkCanvas for Android Framework, to emulate the soon-to-be-removed
+    expanding clip ops guarded by SK_SUPPORT_DEPRECATED_CLIPOPS.
+    https://review.skia.org/430897
 
 * * *
 

gm/complexclip4.cpp

@@ -40,32 +40,31 @@ class ComplexClip4GM : public GM {
 
     // Android Framework will still support the legacy kReplace SkClipOp on older devices, so
     // this represents how to do so while also respecting the device restriction using the newer
-    // androidFramework_replaceClip() API.
+    // androidFramework_resetClip() API.
     void emulateDeviceRestriction(SkCanvas* canvas, const SkIRect& deviceRestriction) {
-        // or any other device-space rect intersection
-        SkCanvasPriv::ReplaceClip(canvas, deviceRestriction);
-        // save for later replace clip ops
-        fDeviceRestriction = deviceRestriction;
+        // TODO(michaelludwig): It may make more sense for device clip restriction to move on to
+        // the SkSurface (which would let this GM draw correctly in viewer).
+        canvas->androidFramework_setDeviceClipRestriction(deviceRestriction);
     }
 
     void emulateClipRectReplace(SkCanvas* canvas,
                                 const SkRect& clipRect,
                                 bool aa) {
-        SkCanvasPriv::ReplaceClip(canvas, fDeviceRestriction);
+        SkCanvasPriv::ResetClip(canvas);
         canvas->clipRect(clipRect, SkClipOp::kIntersect, aa);
     }
 
     void emulateClipRRectReplace(SkCanvas* canvas,
                                  const SkRRect& clipRRect,
                                  bool aa) {
-        SkCanvasPriv::ReplaceClip(canvas, fDeviceRestriction);
+        SkCanvasPriv::ResetClip(canvas);
         canvas->clipRRect(clipRRect, SkClipOp::kIntersect, aa);
     }
 
     void emulateClipPathReplace(SkCanvas* canvas,
                                 const SkPath& path,
                                 bool aa) {
-        SkCanvasPriv::ReplaceClip(canvas, fDeviceRestriction);
+        SkCanvasPriv::ResetClip(canvas);
         canvas->clipPath(path, SkClipOp::kIntersect, aa);
     }
 
@@ -124,7 +123,6 @@ class ComplexClip4GM : public GM {
         canvas->restore();
     }
 private:
-    SkIRect fDeviceRestriction;
     bool    fDoAAClip;
 
     using INHERITED = GM;

include/android/SkAndroidFrameworkUtils.h

@@ -42,13 +42,9 @@ class SkAndroidFrameworkUtils {
 
     static int SaveBehind(SkCanvas* canvas, const SkRect* subset);
 
-    // Operating within the canvas' clip stack, this resets the geometry of the clip to be an
-    // intersection with the device-space 'rect'. If 'rect' is null, this will use the rect that
-    // was last set using androidFramework_setDeviceClipRestriction on the canvas. If that was never
-    // set, it will restrict the clip to the canvas' dimensions.
-    //
-    // TODO: Eventually, make 'rect' non-optional and no longer store the restriction per canvas.
-    static void ReplaceClip(SkCanvas* canvas, const SkIRect* rect = nullptr);
+    // Operating within the canvas' clip stack, this resets the geometry of the clip to be wide
+    // open modula any device clip restriction that was set outside of the clip stack.
+    static void ResetClip(SkCanvas* canvas);
 
     /**
      * Unrolls a chain of nested SkPaintFilterCanvas to return the base wrapped canvas.

include/core/SkCanvas.h

@@ -2268,6 +2268,7 @@ class SK_API SkCanvas {
     virtual void onClipPath(const SkPath& path, SkClipOp op, ClipEdgeStyle edgeStyle);
     virtual void onClipShader(sk_sp<SkShader>, SkClipOp);
     virtual void onClipRegion(const SkRegion& deviceRgn, SkClipOp op);
+    virtual void onResetClip();
 
     virtual void onDiscard();
 
@@ -2430,11 +2431,11 @@ class SK_API SkCanvas {
     bool androidFramework_isClipAA() const;
 
     /**
-     * Reset the clip to be just the intersection with the global-space 'rect'. This operates within
-     * the save/restore stack of the canvas, so restore() will bring back any saved clip. However,
-     * since 'rect' is already in global space, it is not modified by the canvas matrix.
+     * Reset the clip to be wide-open (modulo any separately specified device clip restriction).
+     * This operate within the save/restore clip stack so it can be undone by restoring to an
+     * earlier save point.
      */
-    void androidFramework_replaceClip(const SkIRect& rect);
+    void internal_private_resetClip();
 
     virtual SkPaintFilterCanvas* internal_private_asPaintFilterCanvas() const { return nullptr; }
 

include/utils/SkNWayCanvas.h

@@ -70,6 +70,7 @@ class SK_API SkNWayCanvas : public SkCanvasVirtualEnforcer<SkNoDrawCanvas> {
     void onClipPath(const SkPath&, SkClipOp, ClipEdgeStyle) override;
     void onClipShader(sk_sp<SkShader>, SkClipOp) override;
     void onClipRegion(const SkRegion&, SkClipOp) override;
+    void onResetClip() override;
 
     void onDrawPicture(const SkPicture*, const SkMatrix*, const SkPaint*) override;
     void onDrawDrawable(SkDrawable*, const SkMatrix*) override;

src/android/SkAndroidFrameworkUtils.cpp

@@ -34,18 +34,8 @@ int SkAndroidFrameworkUtils::SaveBehind(SkCanvas* canvas, const SkRect* subset)
     return canvas->only_axis_aligned_saveBehind(subset);
 }
 
-void SkAndroidFrameworkUtils::ReplaceClip(SkCanvas* canvas, const SkIRect* rect) {
-    SkIRect deviceRestriction;
-    if (!rect) {
-        if (canvas->fClipRestrictionRect.isEmpty()) {
-            deviceRestriction = canvas->imageInfo().bounds();
-        } else {
-            deviceRestriction = canvas->fClipRestrictionRect;
-        }
-    } else {
-        deviceRestriction = *rect;
-    }
-    canvas->androidFramework_replaceClip(deviceRestriction);
+void SkAndroidFrameworkUtils::ResetClip(SkCanvas* canvas) {
+    canvas->internal_private_resetClip();
 }
 
 SkCanvas* SkAndroidFrameworkUtils::getBaseWrappedCanvas(SkCanvas* canvas) {

src/core/SkCanvas.cpp

@@ -1445,11 +1445,22 @@ void SkCanvas::androidFramework_setDeviceClipRestriction(const SkIRect& rect) {
     this->topDevice()->androidFramework_setDeviceClipRestriction(&fClipRestrictionRect);
 }
 
-void SkCanvas::androidFramework_replaceClip(const SkIRect& rect) {
+void SkCanvas::internal_private_resetClip() {
     this->checkForDeferredSave();
+    this->onResetClip();
+}
+
+void SkCanvas::onResetClip() {
+    SkIRect deviceRestriction = this->imageInfo().bounds();
+    if (!fClipRestrictionRect.isEmpty()) {
+        // Always respect the device clip restriction, particularly when expanding the clip.
+        if (!deviceRestriction.intersect(fClipRestrictionRect)) {
+            deviceRestriction = SkIRect::MakeEmpty();
+        }
+    }
 
     AutoUpdateQRBounds aqr(this);
-    this->topDevice()->replaceClip(rect);
+    this->topDevice()->replaceClip(deviceRestriction);
 }
 
 void SkCanvas::clipRRect(const SkRRect& rrect, SkClipOp op, bool doAA) {

src/core/SkCanvasPriv.h

@@ -46,8 +46,8 @@ class SkCanvasPriv {
     }
 
     // Exposed for testing on non-Android framework builds
-    static void ReplaceClip(SkCanvas* canvas, const SkIRect& rect) {
-        canvas->androidFramework_replaceClip(rect);
+    static void ResetClip(SkCanvas* canvas) {
+        canvas->internal_private_resetClip();
     }
 
     static GrSurfaceDrawContext* TopDeviceSurfaceDrawContext(SkCanvas*);

src/core/SkPictureFlat.h

@@ -114,7 +114,9 @@ enum DrawType {
     DRAW_IMAGE_LATTICE2,
     DRAW_EDGEAA_IMAGE_SET2,
 
-    LAST_DRAWTYPE_ENUM = DRAW_EDGEAA_IMAGE_SET2,
+    RESET_CLIP,
+
+    LAST_DRAWTYPE_ENUM = RESET_CLIP,
 };
 
 enum DrawVertexFlags {

src/core/SkPicturePlayback.cpp

@@ -159,6 +159,9 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
 
             canvas->clipShader(paint.refShader(), clipOp);
         } break;
+        case RESET_CLIP:
+            SkCanvasPriv::ResetClip(canvas);
+            break;
         case PUSH_CULL: break;  // Deprecated, safe to ignore both push and pop.
         case POP_CULL:  break;
         case CONCAT: {

src/core/SkPictureRecord.cpp

@@ -434,6 +434,13 @@ size_t SkPictureRecord::recordClipRegion(const SkRegion& region, SkClipOp op) {
     return offset;
 }
 
+void SkPictureRecord::onResetClip() {
+    size_t size = sizeof(kUInt32Size);
+    size_t initialOffset = this->addDraw(RESET_CLIP, &size);
+    this->validate(initialOffset, size);
+    this->INHERITED::onResetClip();
+}
+
 void SkPictureRecord::onDrawPaint(const SkPaint& paint) {
     // op + paint index
     size_t size = 2 * kUInt32Size;

src/core/SkPictureRecord.h

@@ -204,6 +204,7 @@ class SkPictureRecord : public SkCanvasVirtualEnforcer<SkCanvas> {
     void onClipPath(const SkPath&, SkClipOp, ClipEdgeStyle) override;
     void onClipShader(sk_sp<SkShader>, SkClipOp) override;
     void onClipRegion(const SkRegion&, SkClipOp) override;
+    void onResetClip() override;
 
     void onDrawPicture(const SkPicture*, const SkMatrix*, const SkPaint*) override;
 

src/core/SkRecordDraw.cpp

@@ -105,6 +105,10 @@ DRAW(ClipRect, clipRect(r.rect, r.opAA.op(), r.opAA.aa()));
 DRAW(ClipRegion, clipRegion(r.region, r.op));
 DRAW(ClipShader, clipShader(r.shader, r.op));
 
+template <> void Draw::draw(const ResetClip& r) {
+    SkCanvasPriv::ResetClip(fCanvas);
+}
+
 DRAW(DrawArc, drawArc(r.oval, r.startAngle, r.sweepAngle, r.useCenter, r.paint));
 DRAW(DrawDRRect, drawDRRect(r.outer, r.inner, r.paint));
 DRAW(DrawImage, drawImage(r.image.get(), r.left, r.top, r.sampling, r.paint));
@@ -270,7 +274,7 @@ class FillBounds : SkNoncopyable {
 
     void trackBounds(const MarkCTM&)           { this->pushControl(); }
     void trackBounds(const SetMatrix&)         { this->pushControl(); }
-    void trackBounds(const SetM44&)         { this->pushControl(); }
+    void trackBounds(const SetM44&)            { this->pushControl(); }
     void trackBounds(const Concat&)            { this->pushControl(); }
     void trackBounds(const Concat44&)          { this->pushControl(); }
     void trackBounds(const Scale&)             { this->pushControl(); }
@@ -280,6 +284,7 @@ class FillBounds : SkNoncopyable {
     void trackBounds(const ClipPath&)          { this->pushControl(); }
     void trackBounds(const ClipRegion&)        { this->pushControl(); }
     void trackBounds(const ClipShader&)        { this->pushControl(); }
+    void trackBounds(const ResetClip&)         { this->pushControl(); }
 
 
     // For all other ops, we can calculate and store the bounds directly now.
@@ -550,4 +555,3 @@ void SkRecordFillBounds(const SkRect& cullRect, const SkRecord& record,
         }
     }
 }
-

src/core/SkRecorder.cpp

@@ -384,6 +384,11 @@ void SkRecorder::onClipRegion(const SkRegion& deviceRgn, SkClipOp op) {
     this->append<SkRecords::ClipRegion>(deviceRgn, op);
 }
 
+void SkRecorder::onResetClip() {
+    INHERITED(onResetClip);
+    this->append<SkRecords::ResetClip>();
+}
+
 sk_sp<SkSurface> SkRecorder::onNewSurface(const SkImageInfo&, const SkSurfaceProps&) {
     return nullptr;
 }

src/core/SkRecorder.h

@@ -105,6 +105,7 @@ class SkRecorder final : public SkCanvasVirtualEnforcer<SkNoDrawCanvas> {
     void onClipPath(const SkPath& path, SkClipOp, ClipEdgeStyle) override;
     void onClipShader(sk_sp<SkShader>, SkClipOp) override;
     void onClipRegion(const SkRegion& deviceRgn, SkClipOp) override;
+    void onResetClip() override;
 
     void onDrawPicture(const SkPicture*, const SkMatrix*, const SkPaint*) override;
 

src/core/SkRecords.h

@@ -57,6 +57,7 @@ namespace SkRecords {
     M(ClipRect)                                                     \
     M(ClipRegion)                                                   \
     M(ClipShader)                                                   \
+    M(ResetClip)                                                    \
     M(DrawArc)                                                      \
     M(DrawDrawable)                                                 \
     M(DrawImage)                                                    \
@@ -218,6 +219,7 @@ RECORD(ClipRegion, 0,
 RECORD(ClipShader, 0,
         sk_sp<SkShader> shader;
         SkClipOp op);
+RECORD(ResetClip, 0);
 
 // While not strictly required, if you have an SkPaint, it's fastest to put it first.
 RECORD(DrawArc, kDraw_Tag|kHasPaint_Tag,

src/gpu/v1/Device_v1.h

@@ -185,7 +185,12 @@ class Device : public BaseDevice  {
     bool onClipIsAA() const override;
 
     void onSetDeviceClipRestriction(SkIRect* mutableClipRestriction) override {
-        SkASSERT(mutableClipRestriction->isEmpty());
+        if (!mutableClipRestriction->isEmpty()) {
+            // Just apply the clip restriction as a device-space intersection. No need to
+            // remember it for expanding clip ops since those shouldn't be used with GrClipStack.
+            fClip.clipRect(this->globalToDevice().asM33(), SkRect::Make(*mutableClipRestriction),
+                           GrAA::kNo, SkClipOp::kIntersect);
+        }
     }
     bool onClipIsWideOpen() const override {
         return fClip.clipState() == GrClipStack::ClipState::kWideOpen;

src/utils/SkNWayCanvas.cpp

@@ -169,6 +169,14 @@ void SkNWayCanvas::onClipRegion(const SkRegion& deviceRgn, SkClipOp op) {
     this->INHERITED::onClipRegion(deviceRgn, op);
 }
 
+void SkNWayCanvas::onResetClip() {
+    Iter iter(fList);
+    while (iter.next()) {
+        SkCanvasPriv::ResetClip(iter.get());
+    }
+    this->INHERITED::onResetClip();
+}
+
 void SkNWayCanvas::onDrawPaint(const SkPaint& paint) {
     Iter iter(fList);
     while (iter.next()) {

tools/debugger/DebugCanvas.cpp

@@ -133,13 +133,8 @@ void DebugCanvas::drawTo(SkCanvas* originalCanvas, int index, int m) {
 
     int saveCount = originalCanvas->save();
 
-    SkRect windowRect = SkRect::MakeWH(SkIntToScalar(originalCanvas->getBaseLayerSize().width()),
-                                       SkIntToScalar(originalCanvas->getBaseLayerSize().height()));
-
     originalCanvas->resetMatrix();
-    if (!windowRect.isEmpty()) {
-        SkCanvasPriv::ReplaceClip(originalCanvas, windowRect.roundOut());
-    }
+    SkCanvasPriv::ResetClip(originalCanvas);
 
     DebugPaintFilterCanvas filterCanvas(originalCanvas);
     SkCanvas* finalCanvas = fOverdrawViz ? &filterCanvas : originalCanvas;
@@ -383,6 +378,10 @@ void DebugCanvas::onClipShader(sk_sp<SkShader> cs, SkClipOp op) {
     this->addDrawCommand(new ClipShaderCommand(std::move(cs), op));
 }
 
+void DebugCanvas::onResetClip() {
+    this->addDrawCommand(new ResetClipCommand());
+}
+
 void DebugCanvas::didConcat44(const SkM44& m) {
     this->addDrawCommand(new Concat44Command(m));
     this->INHERITED::didConcat44(m);

tools/debugger/DebugCanvas.h

@@ -184,8 +184,9 @@ class DebugCanvas : public SkCanvasVirtualEnforcer<SkCanvas> {
     void onClipPath(const SkPath&, SkClipOp, ClipEdgeStyle) override;
     void onClipShader(sk_sp<SkShader>, SkClipOp) override;
     void onClipRegion(const SkRegion& region, SkClipOp) override;
-    void onDrawShadowRec(const SkPath&, const SkDrawShadowRec&) override;
+    void onResetClip() override;
 
+    void onDrawShadowRec(const SkPath&, const SkDrawShadowRec&) override;
     void onDrawDrawable(SkDrawable*, const SkMatrix*) override;
     void onDrawPicture(const SkPicture*, const SkMatrix*, const SkPaint*) override;
 

tools/debugger/DrawCommand.cpp

@@ -218,6 +218,7 @@ const char* DrawCommand::GetCommandString(OpType type) {
         case kClipRegion_OpType: return "ClipRegion";
         case kClipRect_OpType: return "ClipRect";
         case kClipRRect_OpType: return "ClipRRect";
+        case kResetClip_OpType: return "ResetClip";
         case kConcat_OpType: return "Concat";
         case kConcat44_OpType: return "Concat44";
         case kDrawAnnotation_OpType: return "DrawAnnotation";
@@ -1094,6 +1095,10 @@ void ClipShaderCommand::toJSON(SkJSONWriter& writer, UrlDataManager& urlDataMana
     writer.appendString(DEBUGCANVAS_ATTRIBUTE_REGIONOP, regionop_name(fOp));
 }
 
+ResetClipCommand::ResetClipCommand() : INHERITED(kResetClip_OpType) {}
+
+void ResetClipCommand::execute(SkCanvas* canvas) const { SkCanvasPriv::ResetClip(canvas); }
+
 ConcatCommand::ConcatCommand(const SkMatrix& matrix) : INHERITED(kConcat_OpType) {
     fMatrix = matrix;
 }

tools/debugger/DrawCommand.h

@@ -35,6 +35,7 @@ class DrawCommand {
         kClipRect_OpType,
         kClipRRect_OpType,
         kClipShader_OpType,
+        kResetClip_OpType,
         kConcat_OpType,
         kConcat44_OpType,
         kDrawAnnotation_OpType,
@@ -212,6 +213,15 @@ class ClipShaderCommand : public DrawCommand {
     using INHERITED = DrawCommand;
 };
 
+class ResetClipCommand : public DrawCommand {
+public:
+    ResetClipCommand();
+    void execute(SkCanvas* canvas) const override;
+
+private:
+    using INHERITED = DrawCommand;
+};
+
 class ConcatCommand : public DrawCommand {
 public:
     ConcatCommand(const SkMatrix& matrix);