From cfd204a78554f46651071f56ce518a83b8b3942b Mon Sep 17 00:00:00 2001 From: Michael Ludwig Date: Fri, 23 Jul 2021 11:48:34 -0400 Subject: [PATCH] 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 Reviewed-by: Derek Sollenberger Commit-Queue: Michael Ludwig --- RELEASE_NOTES.txt | 4 +++- gm/complexclip4.cpp | 16 +++++++--------- include/android/SkAndroidFrameworkUtils.h | 10 +++------- include/core/SkCanvas.h | 9 +++++---- include/utils/SkNWayCanvas.h | 1 + src/android/SkAndroidFrameworkUtils.cpp | 14 ++------------ src/core/SkCanvas.cpp | 15 +++++++++++++-- src/core/SkCanvasPriv.h | 4 ++-- src/core/SkPictureFlat.h | 4 +++- src/core/SkPicturePlayback.cpp | 3 +++ src/core/SkPictureRecord.cpp | 7 +++++++ src/core/SkPictureRecord.h | 1 + src/core/SkRecordDraw.cpp | 8 ++++++-- src/core/SkRecorder.cpp | 5 +++++ src/core/SkRecorder.h | 1 + src/core/SkRecords.h | 2 ++ src/gpu/v1/Device_v1.h | 7 ++++++- src/utils/SkNWayCanvas.cpp | 8 ++++++++ tools/debugger/DebugCanvas.cpp | 11 +++++------ tools/debugger/DebugCanvas.h | 3 ++- tools/debugger/DrawCommand.cpp | 5 +++++ tools/debugger/DrawCommand.h | 10 ++++++++++ 22 files changed, 100 insertions(+), 48 deletions(-) diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt index 42a3a3d0e562..de4fe088faaf 100644 --- a/RELEASE_NOTES.txt +++ b/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 * * * diff --git a/gm/complexclip4.cpp b/gm/complexclip4.cpp index 93dec8b19ada..92376af06516 100644 --- a/gm/complexclip4.cpp +++ b/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; diff --git a/include/android/SkAndroidFrameworkUtils.h b/include/android/SkAndroidFrameworkUtils.h index dc3a6ebc0740..577bfab72a0c 100644 --- a/include/android/SkAndroidFrameworkUtils.h +++ b/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. diff --git a/include/core/SkCanvas.h b/include/core/SkCanvas.h index 157889f7ca64..aad456562c37 100644 --- a/include/core/SkCanvas.h +++ b/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, 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; } diff --git a/include/utils/SkNWayCanvas.h b/include/utils/SkNWayCanvas.h index 22d426af5c3b..9f6655e60004 100644 --- a/include/utils/SkNWayCanvas.h +++ b/include/utils/SkNWayCanvas.h @@ -70,6 +70,7 @@ class SK_API SkNWayCanvas : public SkCanvasVirtualEnforcer { void onClipPath(const SkPath&, SkClipOp, ClipEdgeStyle) override; void onClipShader(sk_sp, 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; diff --git a/src/android/SkAndroidFrameworkUtils.cpp b/src/android/SkAndroidFrameworkUtils.cpp index ecdf735c7226..123c5921c5b2 100644 --- a/src/android/SkAndroidFrameworkUtils.cpp +++ b/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) { diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp index 61bbead27028..d487b0d2307d 100644 --- a/src/core/SkCanvas.cpp +++ b/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) { diff --git a/src/core/SkCanvasPriv.h b/src/core/SkCanvasPriv.h index 35158ec76fbc..66b2eb624b29 100644 --- a/src/core/SkCanvasPriv.h +++ b/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*); diff --git a/src/core/SkPictureFlat.h b/src/core/SkPictureFlat.h index 7853f9080497..3e5cea4c540d 100644 --- a/src/core/SkPictureFlat.h +++ b/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 { diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp index 375de3b9f148..93411833a91e 100644 --- a/src/core/SkPicturePlayback.cpp +++ b/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: { diff --git a/src/core/SkPictureRecord.cpp b/src/core/SkPictureRecord.cpp index d32b64bab784..5918138f6a94 100644 --- a/src/core/SkPictureRecord.cpp +++ b/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; diff --git a/src/core/SkPictureRecord.h b/src/core/SkPictureRecord.h index 1b154a79ce54..b40fc446fc10 100644 --- a/src/core/SkPictureRecord.h +++ b/src/core/SkPictureRecord.h @@ -204,6 +204,7 @@ class SkPictureRecord : public SkCanvasVirtualEnforcer { void onClipPath(const SkPath&, SkClipOp, ClipEdgeStyle) override; void onClipShader(sk_sp, SkClipOp) override; void onClipRegion(const SkRegion&, SkClipOp) override; + void onResetClip() override; void onDrawPicture(const SkPicture*, const SkMatrix*, const SkPaint*) override; diff --git a/src/core/SkRecordDraw.cpp b/src/core/SkRecordDraw.cpp index 5a2befd4441e..0f3de30eaa94 100644 --- a/src/core/SkRecordDraw.cpp +++ b/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, } } } - diff --git a/src/core/SkRecorder.cpp b/src/core/SkRecorder.cpp index 5b2fd46f2bb6..1d96ea97a24a 100644 --- a/src/core/SkRecorder.cpp +++ b/src/core/SkRecorder.cpp @@ -384,6 +384,11 @@ void SkRecorder::onClipRegion(const SkRegion& deviceRgn, SkClipOp op) { this->append(deviceRgn, op); } +void SkRecorder::onResetClip() { + INHERITED(onResetClip); + this->append(); +} + sk_sp SkRecorder::onNewSurface(const SkImageInfo&, const SkSurfaceProps&) { return nullptr; } diff --git a/src/core/SkRecorder.h b/src/core/SkRecorder.h index a8159aaf31e5..7b810832436d 100644 --- a/src/core/SkRecorder.h +++ b/src/core/SkRecorder.h @@ -105,6 +105,7 @@ class SkRecorder final : public SkCanvasVirtualEnforcer { void onClipPath(const SkPath& path, SkClipOp, ClipEdgeStyle) override; void onClipShader(sk_sp, SkClipOp) override; void onClipRegion(const SkRegion& deviceRgn, SkClipOp) override; + void onResetClip() override; void onDrawPicture(const SkPicture*, const SkMatrix*, const SkPaint*) override; diff --git a/src/core/SkRecords.h b/src/core/SkRecords.h index 73eeebcab9c7..85ebe283a730 100644 --- a/src/core/SkRecords.h +++ b/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 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, diff --git a/src/gpu/v1/Device_v1.h b/src/gpu/v1/Device_v1.h index 203307a1f11c..0b007ceada9a 100644 --- a/src/gpu/v1/Device_v1.h +++ b/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; diff --git a/src/utils/SkNWayCanvas.cpp b/src/utils/SkNWayCanvas.cpp index 3f72d7001d31..c3b118a51985 100644 --- a/src/utils/SkNWayCanvas.cpp +++ b/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()) { diff --git a/tools/debugger/DebugCanvas.cpp b/tools/debugger/DebugCanvas.cpp index 929866054822..98b30b239dd2 100644 --- a/tools/debugger/DebugCanvas.cpp +++ b/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 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); diff --git a/tools/debugger/DebugCanvas.h b/tools/debugger/DebugCanvas.h index b3bc09657ab6..9ae1bf51ca6f 100644 --- a/tools/debugger/DebugCanvas.h +++ b/tools/debugger/DebugCanvas.h @@ -184,8 +184,9 @@ class DebugCanvas : public SkCanvasVirtualEnforcer { void onClipPath(const SkPath&, SkClipOp, ClipEdgeStyle) override; void onClipShader(sk_sp, 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; diff --git a/tools/debugger/DrawCommand.cpp b/tools/debugger/DrawCommand.cpp index a499392e5de4..e638d6fe6598 100644 --- a/tools/debugger/DrawCommand.cpp +++ b/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; } diff --git a/tools/debugger/DrawCommand.h b/tools/debugger/DrawCommand.h index 56f4baebaa90..c21fd06fa506 100644 --- a/tools/debugger/DrawCommand.h +++ b/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);