From 70cf50c24ac86efe373ab035c656c811344b0f0c Mon Sep 17 00:00:00 2001 From: Michael Ludwig Date: Wed, 21 Jul 2021 17:02:39 +0000 Subject: [PATCH] Emulate replace clip ops using SkAndroidFrameworkUtils::ResetClip At the JNI bridge, Region::Op::kReplace_Op is detected and sent to new functions on Canvas.h -> emulateReplaceClip[Rect|Path]. The SkiaCanvas implementation calls SkAndroidFrameworkUtils::ResetClip to turn the SkCanvas' clip wide open (modulo the device clip restriction), followed by an intersect op with the actual clip geometry. SkiaCanvas is also updated to record the replace ops in its Clip struct for partial saves. This should result in no visible behavioral change at the Java Canvas level, but allows the removal of the SK_SUPPORT_DEPRECATED_CLIPOPS flag. Test: Updated SkiaPipeline.clip_replace unit test to use the new emulateReplaceClipRect function instead of SkClipOp::kReplace. Builds and recently added CTS tests for clip ops pass. Bug: b/151725198 Bug: b/143352151 Change-Id: I8fb8765b4ead92bbe8eceadb52710a5673b6cf6b --- libs/hwui/DisplayListOps.in | 1 + libs/hwui/RecordingCanvas.cpp | 16 +++++++++++++++ libs/hwui/RecordingCanvas.h | 2 ++ libs/hwui/SkiaCanvas.cpp | 16 +++++++++++++++ libs/hwui/SkiaCanvas.h | 3 +++ libs/hwui/canvas/CanvasFrontend.cpp | 7 ++----- libs/hwui/hwui/Canvas.h | 6 ++++++ libs/hwui/jni/android_graphics_Canvas.cpp | 23 +++++++++------------- libs/hwui/pipeline/skia/DumpOpsCanvas.h | 2 ++ libs/hwui/tests/unit/SkiaPipelineTests.cpp | 2 +- 10 files changed, 58 insertions(+), 20 deletions(-) diff --git a/libs/hwui/DisplayListOps.in b/libs/hwui/DisplayListOps.in index fb3e21fc15719..4ec782f6fec03 100644 --- a/libs/hwui/DisplayListOps.in +++ b/libs/hwui/DisplayListOps.in @@ -27,6 +27,7 @@ X(ClipPath) X(ClipRect) X(ClipRRect) X(ClipRegion) +X(ResetClip) X(DrawPaint) X(DrawBehind) X(DrawPath) diff --git a/libs/hwui/RecordingCanvas.cpp b/libs/hwui/RecordingCanvas.cpp index c945f274bbf4e..a285462eef742 100644 --- a/libs/hwui/RecordingCanvas.cpp +++ b/libs/hwui/RecordingCanvas.cpp @@ -187,6 +187,11 @@ struct ClipRegion final : Op { SkClipOp op; void draw(SkCanvas* c, const SkMatrix&) const { c->clipRegion(region, op); } }; +struct ResetClip final : Op { + static const auto kType = Type::ResetClip; + ResetClip() {} + void draw(SkCanvas* c, const SkMatrix&) const { SkAndroidFrameworkUtils::ResetClip(c); } +}; struct DrawPaint final : Op { static const auto kType = Type::DrawPaint; @@ -662,6 +667,9 @@ void DisplayListData::clipRRect(const SkRRect& rrect, SkClipOp op, bool aa) { void DisplayListData::clipRegion(const SkRegion& region, SkClipOp op) { this->push(0, region, op); } +void DisplayListData::resetClip() { + this->push(0); +} void DisplayListData::drawPaint(const SkPaint& paint) { this->push(0, paint); @@ -969,6 +977,14 @@ void RecordingCanvas::onClipRegion(const SkRegion& region, SkClipOp op) { fDL->clipRegion(region, op); this->INHERITED::onClipRegion(region, op); } +void RecordingCanvas::onResetClip() { + // This is part of "replace op" emulation, but rely on the following intersection + // clip to potentially mark the clip as complex. If we are already complex, we do + // not reset the complexity so that we don't break the contract that no higher + // save point has a complex clip when "not complex". + fDL->resetClip(); + this->INHERITED::onResetClip(); +} void RecordingCanvas::onDrawPaint(const SkPaint& paint) { fDL->drawPaint(paint); diff --git a/libs/hwui/RecordingCanvas.h b/libs/hwui/RecordingCanvas.h index 4fae6a13a25ac..212b4e72dcb23 100644 --- a/libs/hwui/RecordingCanvas.h +++ b/libs/hwui/RecordingCanvas.h @@ -97,6 +97,7 @@ private: void clipRect(const SkRect&, SkClipOp, bool aa); void clipRRect(const SkRRect&, SkClipOp, bool aa); void clipRegion(const SkRegion&, SkClipOp); + void resetClip(); void drawPaint(const SkPaint&); void drawBehind(const SkPaint&); @@ -169,6 +170,7 @@ public: void onClipRRect(const SkRRect&, SkClipOp, ClipEdgeStyle) override; void onClipPath(const SkPath&, SkClipOp, ClipEdgeStyle) override; void onClipRegion(const SkRegion&, SkClipOp) override; + void onResetClip() override; void onDrawPaint(const SkPaint&) override; void onDrawBehind(const SkPaint&) override; diff --git a/libs/hwui/SkiaCanvas.cpp b/libs/hwui/SkiaCanvas.cpp index d6b6e162757c0..53c6db0cdf3a2 100644 --- a/libs/hwui/SkiaCanvas.cpp +++ b/libs/hwui/SkiaCanvas.cpp @@ -396,6 +396,22 @@ bool SkiaCanvas::clipPath(const SkPath* path, SkClipOp op) { return !mCanvas->isClipEmpty(); } +bool SkiaCanvas::replaceClipRect_deprecated(float left, float top, float right, float bottom) { + SkRect rect = SkRect::MakeLTRB(left, top, right, bottom); + + // Emulated clip rects are not recorded for partial saves, since + // partial saves have been removed from the public API. + SkAndroidFrameworkUtils::ResetClip(mCanvas); + mCanvas->clipRect(rect, SkClipOp::kIntersect); + return !mCanvas->isClipEmpty(); +} + +bool SkiaCanvas::replaceClipPath_deprecated(const SkPath* path) { + SkAndroidFrameworkUtils::ResetClip(mCanvas); + mCanvas->clipPath(*path, SkClipOp::kIntersect, true); + return !mCanvas->isClipEmpty(); +} + // ---------------------------------------------------------------------------- // Canvas state operations: Filters // ---------------------------------------------------------------------------- diff --git a/libs/hwui/SkiaCanvas.h b/libs/hwui/SkiaCanvas.h index f2d7bda364993..715007cdcd3b1 100644 --- a/libs/hwui/SkiaCanvas.h +++ b/libs/hwui/SkiaCanvas.h @@ -94,6 +94,9 @@ public: virtual bool quickRejectPath(const SkPath& path) const override; virtual bool clipRect(float left, float top, float right, float bottom, SkClipOp op) override; virtual bool clipPath(const SkPath* path, SkClipOp op) override; + virtual bool replaceClipRect_deprecated(float left, float top, float right, + float bottom) override; + virtual bool replaceClipPath_deprecated(const SkPath* path) override; virtual PaintFilter* getPaintFilter() override; virtual void setPaintFilter(sk_sp paintFilter) override; diff --git a/libs/hwui/canvas/CanvasFrontend.cpp b/libs/hwui/canvas/CanvasFrontend.cpp index 8f261c83b8d3c..dd01a5b15da50 100644 --- a/libs/hwui/canvas/CanvasFrontend.cpp +++ b/libs/hwui/canvas/CanvasFrontend.cpp @@ -54,13 +54,10 @@ bool CanvasStateHelper::internalSave(SaveEntry saveEntry) { return false; } -// Assert that the cast from SkClipOp to SkRegion::Op is valid +// Assert that the cast from SkClipOp to SkRegion::Op is valid. +// SkClipOp is a subset of SkRegion::Op and only supports difference and intersect. static_assert(static_cast(SkClipOp::kDifference) == SkRegion::Op::kDifference_Op); static_assert(static_cast(SkClipOp::kIntersect) == SkRegion::Op::kIntersect_Op); -static_assert(static_cast(SkClipOp::kUnion_deprecated) == SkRegion::Op::kUnion_Op); -static_assert(static_cast(SkClipOp::kXOR_deprecated) == SkRegion::Op::kXOR_Op); -static_assert(static_cast(SkClipOp::kReverseDifference_deprecated) == SkRegion::Op::kReverseDifference_Op); -static_assert(static_cast(SkClipOp::kReplace_deprecated) == SkRegion::Op::kReplace_Op); void CanvasStateHelper::internalClipRect(const SkRect& rect, SkClipOp op) { clip().opRect(rect, transform(), mInitialBounds, (SkRegion::Op)op, false); diff --git a/libs/hwui/hwui/Canvas.h b/libs/hwui/hwui/Canvas.h index 70a558b3d69d0..82777646f3a21 100644 --- a/libs/hwui/hwui/Canvas.h +++ b/libs/hwui/hwui/Canvas.h @@ -185,6 +185,12 @@ public: virtual bool clipRect(float left, float top, float right, float bottom, SkClipOp op) = 0; virtual bool clipPath(const SkPath* path, SkClipOp op) = 0; + // Resets clip to wide open, used to emulate the now-removed SkClipOp::kReplace on + // apps with compatibility < P. Canvases for version P and later are restricted to + // intersect and difference at the Java level, matching SkClipOp's definition. + // NOTE: These functions are deprecated and will be removed in a future release + virtual bool replaceClipRect_deprecated(float left, float top, float right, float bottom) = 0; + virtual bool replaceClipPath_deprecated(const SkPath* path) = 0; // filters virtual PaintFilter* getPaintFilter() = 0; diff --git a/libs/hwui/jni/android_graphics_Canvas.cpp b/libs/hwui/jni/android_graphics_Canvas.cpp index a4cf02812553d..0ef80ee107082 100644 --- a/libs/hwui/jni/android_graphics_Canvas.cpp +++ b/libs/hwui/jni/android_graphics_Canvas.cpp @@ -188,14 +188,10 @@ static jboolean quickRejectPath(CRITICAL_JNI_PARAMS_COMMA jlong canvasHandle, jl return result ? JNI_TRUE : JNI_FALSE; } -// SkRegion::Op and SkClipOp are numerically identical, so we can freely cast -// from one to the other (though SkClipOp is destined to become a strict subset) +// SkClipOp is a strict subset of SkRegion::Op and is castable back and forth for their +// shared operations (intersect and difference). static_assert(SkRegion::kDifference_Op == static_cast(SkClipOp::kDifference), ""); static_assert(SkRegion::kIntersect_Op == static_cast(SkClipOp::kIntersect), ""); -static_assert(SkRegion::kUnion_Op == static_cast(SkClipOp::kUnion_deprecated), ""); -static_assert(SkRegion::kXOR_Op == static_cast(SkClipOp::kXOR_deprecated), ""); -static_assert(SkRegion::kReverseDifference_Op == static_cast(SkClipOp::kReverseDifference_deprecated), ""); -static_assert(SkRegion::kReplace_Op == static_cast(SkClipOp::kReplace_deprecated), ""); static jboolean clipRect(CRITICAL_JNI_PARAMS_COMMA jlong canvasHandle, jfloat l, jfloat t, jfloat r, jfloat b, jint opHandle) { @@ -203,16 +199,16 @@ static jboolean clipRect(CRITICAL_JNI_PARAMS_COMMA jlong canvasHandle, jfloat l, SkRegion::Op rgnOp = static_cast(opHandle); bool nonEmptyClip; switch (rgnOp) { - case SkRegion::Op::kReplace_Op: - // For now replace can still be handled as an SkClipOp but will be emulated in the - // future - [[fallthrough]]; case SkRegion::Op::kIntersect_Op: case SkRegion::Op::kDifference_Op: // Intersect and difference are supported clip operations nonEmptyClip = get_canvas(canvasHandle)->clipRect(l, t, r, b, static_cast(rgnOp)); break; + case SkRegion::Op::kReplace_Op: + // Replace is emulated to support legacy apps older than P + nonEmptyClip = get_canvas(canvasHandle)->replaceClipRect_deprecated(l, t, r, b); + break; default: // All other operations would expand the clip and are no longer supported, // so log and skip (to avoid breaking legacy apps). @@ -230,14 +226,13 @@ static jboolean clipPath(CRITICAL_JNI_PARAMS_COMMA jlong canvasHandle, jlong pat SkPath* path = reinterpret_cast(pathHandle); bool nonEmptyClip; switch (rgnOp) { - case SkRegion::Op::kReplace_Op: - // For now replace can still be handled as an SkClipOp but will be emulated in the - // future - [[fallthrough]]; case SkRegion::Op::kIntersect_Op: case SkRegion::Op::kDifference_Op: nonEmptyClip = get_canvas(canvasHandle)->clipPath(path, static_cast(rgnOp)); break; + case SkRegion::Op::kReplace_Op: + nonEmptyClip = get_canvas(canvasHandle)->replaceClipPath_deprecated(path); + break; default: ALOGW("Ignoring unsupported clip operation %d", opHandle); SkRect clipBounds; // ignored diff --git a/libs/hwui/pipeline/skia/DumpOpsCanvas.h b/libs/hwui/pipeline/skia/DumpOpsCanvas.h index 3580bed45a1f9..3f89c07124075 100644 --- a/libs/hwui/pipeline/skia/DumpOpsCanvas.h +++ b/libs/hwui/pipeline/skia/DumpOpsCanvas.h @@ -52,6 +52,8 @@ protected: mOutput << mIdent << "clipRegion" << std::endl; } + void onResetClip() override { mOutput << mIdent << "resetClip" << std::endl; } + void onDrawPaint(const SkPaint&) override { mOutput << mIdent << "drawPaint" << std::endl; } void onDrawPath(const SkPath&, const SkPaint&) override { diff --git a/libs/hwui/tests/unit/SkiaPipelineTests.cpp b/libs/hwui/tests/unit/SkiaPipelineTests.cpp index 8c999c41bf7bb..57a7fe3fd4667 100644 --- a/libs/hwui/tests/unit/SkiaPipelineTests.cpp +++ b/libs/hwui/tests/unit/SkiaPipelineTests.cpp @@ -382,7 +382,7 @@ RENDERTHREAD_SKIA_PIPELINE_TEST(SkiaPipeline, clip_replace) { std::vector> nodes; nodes.push_back(TestUtils::createSkiaNode( 20, 20, 30, 30, [](RenderProperties& props, SkiaRecordingCanvas& canvas) { - canvas.clipRect(0, -20, 10, 30, SkClipOp::kReplace_deprecated); + canvas.replaceClipRect_deprecated(0, -20, 10, 30); canvas.drawColor(SK_ColorWHITE, SkBlendMode::kSrcOver); }));