diff options
author | reed <reed@google.com> | 2014-09-09 12:51:10 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2014-09-09 12:51:10 -0700 |
commit | 6f09709519b79a1159f3826645f1c5fbc101ee11 (patch) | |
tree | 2c929112d91010c2a8ea664e5991519d858252a0 | |
parent | 6bc2c94de334efb40e1a09e31112262dec77532b (diff) |
Revert of allow canvas to force conservative clips (for speed) (patchset #7 id:120001 of https://codereview.chromium.org/541593005/)
Reason for revert:
multipicturedraw failed on nvprmsaa -- don't know why yet
Original issue's description:
> Allow SkCanvas to be initialized to force conservative rasterclips. This has the following effects:
>
> 1. Queries to the current clip will be conservatively large. This can mean the quickReject may return false more often.
>
> 2. The conservative clips mean less work is done.
>
> 3. Enabled by default for Gpu, Record, and NoSaveLayer canvases.
>
> 4. API is private for now.
>
> Committed: https://skia.googlesource.com/skia/+/27a5e656c3d6ef22f9cb34de18e1b960da3aa241
R=robertphillips@google.com, bsalomon@google.com, mtklein@google.com, junov@google.com
TBR=bsalomon@google.com, junov@google.com, mtklein@google.com, robertphillips@google.com
NOTREECHECKS=true
NOTRY=true
Author: reed@google.com
Review URL: https://codereview.chromium.org/554033003
-rw-r--r-- | include/core/SkCanvas.h | 17 | ||||
-rw-r--r-- | include/core/SkDevice.h | 2 | ||||
-rw-r--r-- | include/gpu/SkGpuDevice.h | 1 | ||||
-rw-r--r-- | include/utils/SkNoSaveLayerCanvas.h | 18 | ||||
-rw-r--r-- | src/core/SkCanvas.cpp | 183 | ||||
-rw-r--r-- | src/core/SkDeviceLooper.cpp | 7 | ||||
-rw-r--r-- | src/core/SkPictureRecord.cpp | 6 | ||||
-rw-r--r-- | src/core/SkRasterClip.cpp | 138 | ||||
-rw-r--r-- | src/core/SkRasterClip.h | 10 | ||||
-rw-r--r-- | src/core/SkRecorder.cpp | 6 | ||||
-rw-r--r-- | tests/AAClipTest.cpp | 7 | ||||
-rw-r--r-- | tests/PictureTest.cpp | 4 |
12 files changed, 166 insertions, 233 deletions
diff --git a/include/core/SkCanvas.h b/include/core/SkCanvas.h index 5088d7ded5..f514d18b5e 100644 --- a/include/core/SkCanvas.h +++ b/include/core/SkCanvas.h @@ -1265,6 +1265,11 @@ protected: SkIRect* intersection, const SkImageFilter* imageFilter = NULL); + // Called by child classes that override clipPath and clipRRect to only + // track fast conservative clip bounds, rather than exact clips. + void updateClipConservativelyUsingBounds(const SkRect&, SkRegion::Op, + bool inverseFilled); + // notify our surface (if we have one) that we are about to draw, so it // can perform copy-on-write or invalidate any cached images void predrawNotify(); @@ -1304,22 +1309,13 @@ private: friend class SkDebugCanvas; // needs experimental fAllowSimplifyClip friend class SkDeferredDevice; // needs getTopDevice() friend class SkSurface_Raster; // needs getDevice() - friend class SkRecorder; // InitFlags - friend class SkNoSaveLayerCanvas; // InitFlags - enum InitFlags { - kDefault_InitFlags = 0, - kConservativeRasterClip_InitFlag = 1 << 0, - }; - SkCanvas(int width, int height, InitFlags flags); - SkCanvas(SkBaseDevice*, InitFlags flags); - // needs gettotalclip() friend SkCanvasState* SkCanvasStateUtils::CaptureCanvasState(SkCanvas*); SkBaseDevice* createLayerDevice(const SkImageInfo&); - SkBaseDevice* init(SkBaseDevice*, InitFlags); + SkBaseDevice* init(SkBaseDevice*); /** * DEPRECATED @@ -1369,7 +1365,6 @@ private: mutable bool fCachedLocalClipBoundsDirty; bool fAllowSoftClip; bool fAllowSimplifyClip; - bool fConservativeRasterClip; const SkRect& getLocalClipBounds() const { if (fCachedLocalClipBoundsDirty) { diff --git a/include/core/SkDevice.h b/include/core/SkDevice.h index 618431a2de..aad5bf4737 100644 --- a/include/core/SkDevice.h +++ b/include/core/SkDevice.h @@ -372,8 +372,6 @@ private: // TODO: move to SkBitmapDevice virtual void replaceBitmapBackendForRasterSurface(const SkBitmap&) {} - virtual bool forceConservativeRasterClip() const { return false; } - // just called by SkCanvas when built as a layer void setOrigin(int x, int y) { fOrigin.set(x, y); } // just called by SkCanvas for saveLayer diff --git a/include/gpu/SkGpuDevice.h b/include/gpu/SkGpuDevice.h index 5545227129..91fc2f0bb6 100644 --- a/include/gpu/SkGpuDevice.h +++ b/include/gpu/SkGpuDevice.h @@ -152,7 +152,6 @@ private: virtual SkSurface* newSurface(const SkImageInfo&) SK_OVERRIDE; virtual SkImageFilter::Cache* getImageFilterCache() SK_OVERRIDE; - virtual bool forceConservativeRasterClip() const SK_OVERRIDE { return true; } // sets the render target, clip, and matrix on GrContext. Use forceIdenity to override // SkDraw's matrix and draw in device coords. diff --git a/include/utils/SkNoSaveLayerCanvas.h b/include/utils/SkNoSaveLayerCanvas.h index 56a09622a0..686f179901 100644 --- a/include/utils/SkNoSaveLayerCanvas.h +++ b/include/utils/SkNoSaveLayerCanvas.h @@ -16,8 +16,7 @@ // It also simplifies the clipping calls to only use rectangles. class SK_API SkNoSaveLayerCanvas : public SkCanvas { public: - SkNoSaveLayerCanvas(SkBaseDevice* device) : INHERITED(device, kConservativeRasterClip_InitFlag) - {} + SkNoSaveLayerCanvas(SkBaseDevice* device) : INHERITED(device) {} protected: virtual SaveLayerStrategy willSaveLayer(const SkRect* bounds, const SkPaint* paint, @@ -26,6 +25,21 @@ protected: return kNoLayer_SaveLayerStrategy; } + // disable aa for speed + virtual void onClipRect(const SkRect& rect, SkRegion::Op op, ClipEdgeStyle) SK_OVERRIDE { + this->INHERITED::onClipRect(rect, op, kHard_ClipEdgeStyle); + } + + // for speed, just respect the bounds, and disable AA. May give us a few + // false positives and negatives. + virtual void onClipPath(const SkPath& path, SkRegion::Op op, ClipEdgeStyle) SK_OVERRIDE { + this->updateClipConservativelyUsingBounds(path.getBounds(), op, + path.isInverseFillType()); + } + virtual void onClipRRect(const SkRRect& rrect, SkRegion::Op op, ClipEdgeStyle) SK_OVERRIDE { + this->updateClipConservativelyUsingBounds(rrect.getBounds(), op, false); + } + private: typedef SkCanvas INHERITED; }; diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp index 62d60da178..99786151ab 100644 --- a/src/core/SkCanvas.cpp +++ b/src/core/SkCanvas.cpp @@ -81,12 +81,9 @@ struct DeviceCM { const SkMatrix* fMatrix; SkPaint* fPaint; // may be null (in the future) - DeviceCM(SkBaseDevice* device, int x, int y, const SkPaint* paint, SkCanvas* canvas, - bool conservativeRasterClip) - : fNext(NULL) - , fClip(conservativeRasterClip) - { - if (NULL != device) { + DeviceCM(SkBaseDevice* device, int x, int y, const SkPaint* paint, SkCanvas* canvas) + : fNext(NULL) { + if (device) { device->ref(); device->onAttachToCanvas(canvas); } @@ -154,10 +151,11 @@ private: */ class SkCanvas::MCRec { public: - SkRasterClip fRasterClip; SkMatrix fMatrix; + SkRasterClip fRasterClip; SkDrawFilter* fFilter; // the current filter (or null) - DeviceCM* fLayer; + + DeviceCM* fLayer; /* If there are any layers in the stack, this points to the top-most one that is at or below this level in the stack (so we know what bitmap/device to draw into from this level. This value is NOT @@ -166,21 +164,22 @@ public: */ DeviceCM* fTopLayer; - MCRec(bool conservativeRasterClip) : fRasterClip(conservativeRasterClip) { - fMatrix.reset(); - fFilter = NULL; - fLayer = NULL; - fTopLayer = NULL; - - // don't bother initializing fNext - inc_rec(); - } - MCRec(const MCRec& prev) : fRasterClip(prev.fRasterClip) { - fMatrix = prev.fMatrix; - fFilter = SkSafeRef(prev.fFilter); + MCRec(const MCRec* prev) { + if (prev) { + fMatrix = prev->fMatrix; + fRasterClip = prev->fRasterClip; + + fFilter = prev->fFilter; + SkSafeRef(fFilter); + + fTopLayer = prev->fTopLayer; + } else { // no prev + fMatrix.reset(); + fFilter = NULL; + fTopLayer = NULL; + } fLayer = NULL; - fTopLayer = prev.fTopLayer; - + // don't bother initializing fNext inc_rec(); } @@ -382,8 +381,7 @@ bool AutoDrawLooper::doNext(SkDrawFilter::Type drawType) { //////////////////////////////////////////////////////////////////////////// -SkBaseDevice* SkCanvas::init(SkBaseDevice* device, InitFlags flags) { - fConservativeRasterClip = SkToBool(flags & kConservativeRasterClip_InitFlag); +SkBaseDevice* SkCanvas::init(SkBaseDevice* device) { fCachedLocalClipBounds.setEmpty(); fCachedLocalClipBoundsDirty = true; fAllowSoftClip = true; @@ -393,14 +391,10 @@ SkBaseDevice* SkCanvas::init(SkBaseDevice* device, InitFlags flags) { fCullCount = 0; fMetaData = NULL; - if (device && device->forceConservativeRasterClip()) { - fConservativeRasterClip = true; - } - fMCRec = (MCRec*)fMCStack.push_back(); - new (fMCRec) MCRec(fConservativeRasterClip); + new (fMCRec) MCRec(NULL); - fMCRec->fLayer = SkNEW_ARGS(DeviceCM, (NULL, 0, 0, NULL, NULL, fConservativeRasterClip)); + fMCRec->fLayer = SkNEW_ARGS(DeviceCM, (NULL, 0, 0, NULL, NULL)); fMCRec->fTopLayer = fMCRec->fLayer; fSurfaceBase = NULL; @@ -418,54 +412,25 @@ SkCanvas::SkCanvas() { inc_canvas(); - this->init(NULL, kDefault_InitFlags); + this->init(NULL); } -static SkBitmap make_nopixels(int width, int height) { - SkBitmap bitmap; - bitmap.setInfo(SkImageInfo::MakeUnknown(width, height)); - return bitmap; -} - -class SkNoPixelsBitmapDevice : public SkBitmapDevice { -public: - SkNoPixelsBitmapDevice(int width, int height) : INHERITED(make_nopixels(width, height)) {} - -private: - - typedef SkBitmapDevice INHERITED; -}; - SkCanvas::SkCanvas(int width, int height) : fMCStack(sizeof(MCRec), fMCRecStorage, sizeof(fMCRecStorage)) { inc_canvas(); - - this->init(SkNEW_ARGS(SkNoPixelsBitmapDevice, (width, height)), kDefault_InitFlags)->unref(); -} - -SkCanvas::SkCanvas(int width, int height, InitFlags flags) - : fMCStack(sizeof(MCRec), fMCRecStorage, sizeof(fMCRecStorage)) -{ - inc_canvas(); - - this->init(SkNEW_ARGS(SkNoPixelsBitmapDevice, (width, height)), flags)->unref(); -} -SkCanvas::SkCanvas(SkBaseDevice* device, InitFlags flags) - : fMCStack(sizeof(MCRec), fMCRecStorage, sizeof(fMCRecStorage)) -{ - inc_canvas(); - - this->init(device, flags); + SkBitmap bitmap; + bitmap.setInfo(SkImageInfo::MakeUnknown(width, height)); + this->init(SkNEW_ARGS(SkBitmapDevice, (bitmap)))->unref(); } SkCanvas::SkCanvas(SkBaseDevice* device) : fMCStack(sizeof(MCRec), fMCRecStorage, sizeof(fMCRecStorage)) { inc_canvas(); - - this->init(device, kDefault_InitFlags); + + this->init(device); } SkCanvas::SkCanvas(const SkBitmap& bitmap) @@ -473,7 +438,7 @@ SkCanvas::SkCanvas(const SkBitmap& bitmap) { inc_canvas(); - this->init(SkNEW_ARGS(SkBitmapDevice, (bitmap)), kDefault_InitFlags)->unref(); + this->init(SkNEW_ARGS(SkBitmapDevice, (bitmap)))->unref(); } SkCanvas::~SkCanvas() { @@ -769,7 +734,7 @@ int SkCanvas::internalSave() { int saveCount = this->getSaveCount(); // record this before the actual save MCRec* newTop = (MCRec*)fMCStack.push_back(); - new (newTop) MCRec(*fMCRec); // balanced in restore() + new (newTop) MCRec(fMCRec); // balanced in restore() fMCRec = newTop; fClipStack.save(); @@ -901,8 +866,7 @@ int SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, Save } device->setOrigin(ir.fLeft, ir.fTop); - DeviceCM* layer = SkNEW_ARGS(DeviceCM, - (device, ir.fLeft, ir.fTop, paint, this, fConservativeRasterClip)); + DeviceCM* layer = SkNEW_ARGS(DeviceCM, (device, ir.fLeft, ir.fTop, paint, this)); device->unref(); layer->fNext = fMCRec->fTopLayer; @@ -1322,7 +1286,7 @@ void SkCanvas::onClipRect(const SkRect& rect, SkRegion::Op op, ClipEdgeStyle edg fMCRec->fMatrix.mapRect(&r, rect); fClipStack.clipDevRect(r, op, kSoft_ClipEdgeStyle == edgeStyle); - fMCRec->fRasterClip.op(r, this->getBaseLayerSize(), op, kSoft_ClipEdgeStyle == edgeStyle); + fMCRec->fRasterClip.op(r, op, kSoft_ClipEdgeStyle == edgeStyle); } else { // since we're rotated or some such thing, we convert the rect to a path // and clip against that, since it can handle any matrix. However, to @@ -1457,6 +1421,82 @@ void SkCanvas::onClipPath(const SkPath& path, SkRegion::Op op, ClipEdgeStyle edg rasterclip_path(&fMCRec->fRasterClip, this, devPath, op, edgeStyle); } +void SkCanvas::updateClipConservativelyUsingBounds(const SkRect& bounds, SkRegion::Op op, + bool inverseFilled) { + // This is for updating the clip conservatively using only bounds + // information. + // Contract: + // The current clip must contain the true clip. The true + // clip is the clip that would have normally been computed + // by calls to clipPath and clipRRect + // Objective: + // Keep the current clip as small as possible without + // breaking the contract, using only clip bounding rectangles + // (for performance). + + // N.B.: This *never* calls back through a virtual on canvas, so subclasses + // don't have to worry about getting caught in a loop. Thus anywhere + // we call a virtual method, we explicitly prefix it with + // SkCanvas:: to be sure to call the base-class. + + if (inverseFilled) { + switch (op) { + case SkRegion::kIntersect_Op: + case SkRegion::kDifference_Op: + // These ops can only shrink the current clip. So leaving + // the clip unchanged conservatively respects the contract. + break; + case SkRegion::kUnion_Op: + case SkRegion::kReplace_Op: + case SkRegion::kReverseDifference_Op: + case SkRegion::kXOR_Op: { + // These ops can grow the current clip up to the extents of + // the input clip, which is inverse filled, so we just set + // the current clip to the device bounds. + SkRect deviceBounds; + SkIRect deviceIBounds; + this->getDevice()->getGlobalBounds(&deviceIBounds); + deviceBounds = SkRect::Make(deviceIBounds); + + // set the clip in device space + SkMatrix savedMatrix = this->getTotalMatrix(); + this->SkCanvas::setMatrix(SkMatrix::I()); + this->SkCanvas::onClipRect(deviceBounds, SkRegion::kReplace_Op, + kHard_ClipEdgeStyle); + this->setMatrix(savedMatrix); + break; + } + default: + SkASSERT(0); // unhandled op? + } + } else { + // Not inverse filled + switch (op) { + case SkRegion::kIntersect_Op: + case SkRegion::kUnion_Op: + case SkRegion::kReplace_Op: + this->SkCanvas::onClipRect(bounds, op, kHard_ClipEdgeStyle); + break; + case SkRegion::kDifference_Op: + // Difference can only shrink the current clip. + // Leaving clip unchanged conservatively fullfills the contract. + break; + case SkRegion::kReverseDifference_Op: + // To reverse, we swap in the bounds with a replace op. + // As with difference, leave it unchanged. + this->SkCanvas::onClipRect(bounds, SkRegion::kReplace_Op, kHard_ClipEdgeStyle); + break; + case SkRegion::kXOR_Op: + // Be conservative, based on (A XOR B) always included in (A union B), + // which is always included in (bounds(A) union bounds(B)) + this->SkCanvas::onClipRect(bounds, SkRegion::kUnion_Op, kHard_ClipEdgeStyle); + break; + default: + SkASSERT(0); // unhandled op? + } + } +} + void SkCanvas::clipRegion(const SkRegion& rgn, SkRegion::Op op) { this->onClipRegion(rgn, op); } @@ -1485,7 +1525,7 @@ void SkCanvas::validateClip() const { SkIRect ir; ir.set(0, 0, device->width(), device->height()); - SkRasterClip tmpClip(ir, fConservativeRasterClip); + SkRasterClip tmpClip(ir); SkClipStack::B2TIter iter(fClipStack); const SkClipStack::Element* element; @@ -1529,6 +1569,7 @@ bool SkCanvas::isClipRect() const { } bool SkCanvas::quickReject(const SkRect& rect) const { + if (!rect.isFinite()) return true; diff --git a/src/core/SkDeviceLooper.cpp b/src/core/SkDeviceLooper.cpp index a8350cc027..9346465dfc 100644 --- a/src/core/SkDeviceLooper.cpp +++ b/src/core/SkDeviceLooper.cpp @@ -10,10 +10,9 @@ SkDeviceLooper::SkDeviceLooper(const SkBitmap& base, const SkRasterClip& rc, const SkIRect& bounds, bool aa) - : fBaseBitmap(base) - , fBaseRC(rc) - , fSubsetRC(rc.isForceConservativeRects()) - , fDelta(aa ? kAA_Delta : kBW_Delta) +: fBaseBitmap(base) +, fBaseRC(rc) +, fDelta(aa ? kAA_Delta : kBW_Delta) { // sentinels that next() has not yet been called, and so our mapper functions // should not be called either. diff --git a/src/core/SkPictureRecord.cpp b/src/core/SkPictureRecord.cpp index cae8420e95..b17169183a 100644 --- a/src/core/SkPictureRecord.cpp +++ b/src/core/SkPictureRecord.cpp @@ -788,7 +788,7 @@ size_t SkPictureRecord::recordClipRect(const SkRect& rect, SkRegion::Op op, bool void SkPictureRecord::onClipRRect(const SkRRect& rrect, SkRegion::Op op, ClipEdgeStyle edgeStyle) { this->recordClipRRect(rrect, op, kSoft_ClipEdgeStyle == edgeStyle); - this->INHERITED::onClipRRect(rrect, op, edgeStyle); + this->updateClipConservativelyUsingBounds(rrect.getBounds(), op, false); } size_t SkPictureRecord::recordClipRRect(const SkRRect& rrect, SkRegion::Op op, bool doAA) { @@ -810,7 +810,9 @@ size_t SkPictureRecord::recordClipRRect(const SkRRect& rrect, SkRegion::Op op, b void SkPictureRecord::onClipPath(const SkPath& path, SkRegion::Op op, ClipEdgeStyle edgeStyle) { int pathID = this->addPathToHeap(path); this->recordClipPath(pathID, op, kSoft_ClipEdgeStyle == edgeStyle); - this->INHERITED::onClipPath(path, op, edgeStyle); + + this->updateClipConservativelyUsingBounds(path.getBounds(), op, + path.isInverseFillType()); } size_t SkPictureRecord::recordClipPath(int pathID, SkRegion::Op op, bool doAA) { diff --git a/src/core/SkRasterClip.cpp b/src/core/SkRasterClip.cpp index f820c5a0a4..dab91d8f4c 100644 --- a/src/core/SkRasterClip.cpp +++ b/src/core/SkRasterClip.cpp @@ -6,12 +6,18 @@ */ #include "SkRasterClip.h" -#include "SkPath.h" + + +SkRasterClip::SkRasterClip() { + fIsBW = true; + fIsEmpty = true; + fIsRect = false; + SkDEBUGCODE(this->validate();) +} SkRasterClip::SkRasterClip(const SkRasterClip& src) { AUTO_RASTERCLIP_VALIDATE(src); - fForceConservativeRects = src.fForceConservativeRects; fIsBW = src.fIsBW; if (fIsBW) { fBW = src.fBW; @@ -24,22 +30,13 @@ SkRasterClip::SkRasterClip(const SkRasterClip& src) { SkDEBUGCODE(this->validate();) } -SkRasterClip::SkRasterClip(const SkIRect& bounds, bool forceConservativeRects) : fBW(bounds) { - fForceConservativeRects = forceConservativeRects; +SkRasterClip::SkRasterClip(const SkIRect& bounds) : fBW(bounds) { fIsBW = true; fIsEmpty = this->computeIsEmpty(); // bounds might be empty, so compute fIsRect = !fIsEmpty; SkDEBUGCODE(this->validate();) } -SkRasterClip::SkRasterClip(bool forceConservativeRects) { - fForceConservativeRects = forceConservativeRects; - fIsBW = true; - fIsEmpty = true; - fIsRect = false; - SkDEBUGCODE(this->validate();) -} - SkRasterClip::~SkRasterClip() { SkDEBUGCODE(this->validate();) } @@ -73,84 +70,9 @@ bool SkRasterClip::setRect(const SkIRect& rect) { return fIsRect; } -///////////////////////////////////////////////////////////////////////////////////// - -bool SkRasterClip::setConservativeRect(const SkRect& r, const SkIRect& clipR, bool isInverse) { - SkIRect ir; - r.roundOut(&ir); - - SkRegion::Op op; - if (isInverse) { - op = SkRegion::kDifference_Op; - } else { - op = SkRegion::kIntersect_Op; - } - fBW.setRect(clipR); - fBW.op(ir, op); - return this->updateCacheAndReturnNonEmpty(); -} - -///////////////////////////////////////////////////////////////////////////////////// - -enum MutateResult { - kDoNothing_MutateResult, - kReplaceClippedAgainstGlobalBounds_MutateResult, - kContinue_MutateResult, -}; - -static MutateResult mutate_conservative_op(SkRegion::Op* op, bool inverseFilled) { - if (inverseFilled) { - switch (*op) { - case SkRegion::kIntersect_Op: - case SkRegion::kDifference_Op: - // These ops can only shrink the current clip. So leaving - // the clip unchanged conservatively respects the contract. - return kDoNothing_MutateResult; - case SkRegion::kUnion_Op: - case SkRegion::kReplace_Op: - case SkRegion::kReverseDifference_Op: - case SkRegion::kXOR_Op: { - // These ops can grow the current clip up to the extents of - // the input clip, which is inverse filled, so we just set - // the current clip to the device bounds. - *op = SkRegion::kReplace_Op; - return kReplaceClippedAgainstGlobalBounds_MutateResult; - } - } - } else { - // Not inverse filled - switch (*op) { - case SkRegion::kIntersect_Op: - case SkRegion::kUnion_Op: - case SkRegion::kReplace_Op: - return kContinue_MutateResult; - case SkRegion::kDifference_Op: - // Difference can only shrink the current clip. - // Leaving clip unchanged conservatively fullfills the contract. - return kDoNothing_MutateResult; - case SkRegion::kReverseDifference_Op: - // To reverse, we swap in the bounds with a replace op. - // As with difference, leave it unchanged. - *op = SkRegion::kReplace_Op; - return kContinue_MutateResult; - case SkRegion::kXOR_Op: - // Be conservative, based on (A XOR B) always included in (A union B), - // which is always included in (bounds(A) union bounds(B)) - *op = SkRegion::kUnion_Op; - return kContinue_MutateResult; - } - } - SkFAIL("should not get here"); - return kDoNothing_MutateResult; -} - bool SkRasterClip::setPath(const SkPath& path, const SkRegion& clip, bool doAA) { AUTO_RASTERCLIP_VALIDATE(*this); - if (fForceConservativeRects) { - return this->setConservativeRect(path.getBounds(), clip.getBounds(), path.isInverseFillType()); - } - if (this->isBW() && !doAA) { (void)fBW.setPath(path, clip); } else { @@ -168,22 +90,7 @@ bool SkRasterClip::op(const SkPath& path, const SkISize& size, SkRegion::Op op, // base is used to limit the size (and therefore memory allocation) of the // region that results from scan converting devPath. SkRegion base; - - if (fForceConservativeRects) { - SkIRect ir; - switch (mutate_conservative_op(&op, path.isInverseFillType())) { - case kDoNothing_MutateResult: - return !this->isEmpty(); - case kReplaceClippedAgainstGlobalBounds_MutateResult: - ir = SkIRect::MakeSize(size); - break; - case kContinue_MutateResult: - path.getBounds().roundOut(&ir); - break; - } - return this->op(ir, op); - } - + if (SkRegion::kIntersect_Op == op) { // since we are intersect, we can do better (tighter) with currRgn's // bounds, than just using the device. However, if currRgn is complex, @@ -195,7 +102,7 @@ bool SkRasterClip::op(const SkPath& path, const SkISize& size, SkRegion::Op op, return this->setPath(path, this->bwRgn(), doAA); } else { base.setRect(this->getBounds()); - SkRasterClip clip(fForceConservativeRects); + SkRasterClip clip; clip.setPath(path, base, doAA); return this->op(clip, op); } @@ -205,7 +112,7 @@ bool SkRasterClip::op(const SkPath& path, const SkISize& size, SkRegion::Op op, if (SkRegion::kReplace_Op == op) { return this->setPath(path, base, doAA); } else { - SkRasterClip clip(fForceConservativeRects); + SkRasterClip clip; clip.setPath(path, base, doAA); return this->op(clip, op); } @@ -275,24 +182,9 @@ static bool nearly_integral(SkScalar x) { return x - SkScalarFloorToScalar(x) < domain; } -bool SkRasterClip::op(const SkRect& r, const SkISize& size, SkRegion::Op op, bool doAA) { +bool SkRasterClip::op(const SkRect& r, SkRegion::Op op, bool doAA) { AUTO_RASTERCLIP_VALIDATE(*this); - if (fForceConservativeRects) { - SkIRect ir; - switch (mutate_conservative_op(&op, false)) { - case kDoNothing_MutateResult: - return !this->isEmpty(); - case kReplaceClippedAgainstGlobalBounds_MutateResult: - ir = SkIRect::MakeSize(size); - break; - case kContinue_MutateResult: - r.roundOut(&ir); - break; - } - return this->op(ir, op); - } - if (fIsBW && doAA) { // check that the rect really needs aa, or is it close enought to // integer boundaries that we can just treat it as a BW rect? @@ -359,9 +251,7 @@ const SkRegion& SkRasterClip::forceGetBW() { void SkRasterClip::convertToAA() { AUTO_RASTERCLIP_VALIDATE(*this); - - SkASSERT(!fForceConservativeRects); - + SkASSERT(fIsBW); fAA.setRegion(fBW); fIsBW = false; diff --git a/src/core/SkRasterClip.h b/src/core/SkRasterClip.h index 8a0681805c..ec38990a0c 100644 --- a/src/core/SkRasterClip.h +++ b/src/core/SkRasterClip.h @@ -13,13 +13,11 @@ class SkRasterClip { public: - SkRasterClip(bool forceConservativeRects = false); - SkRasterClip(const SkIRect&, bool forceConservativeRects = false); + SkRasterClip(); + SkRasterClip(const SkIRect&); SkRasterClip(const SkRasterClip&); ~SkRasterClip(); - bool isForceConservativeRects() const { return fForceConservativeRects; } - bool isBW() const { return fIsBW; } bool isAA() const { return !fIsBW; } const SkRegion& bwRgn() const { SkASSERT(fIsBW); return fBW; } @@ -43,7 +41,7 @@ public: bool op(const SkIRect&, SkRegion::Op); bool op(const SkRegion&, SkRegion::Op); - bool op(const SkRect&, const SkISize&, SkRegion::Op, bool doAA); + bool op(const SkRect&, SkRegion::Op, bool doAA); bool op(const SkPath&, const SkISize&, SkRegion::Op, bool doAA); void translate(int dx, int dy, SkRasterClip* dst) const; @@ -77,7 +75,6 @@ public: private: SkRegion fBW; SkAAClip fAA; - bool fForceConservativeRects; bool fIsBW; // these 2 are caches based on querying the right obj based on fIsBW bool fIsEmpty; @@ -110,7 +107,6 @@ private: bool setPath(const SkPath& path, const SkRegion& clip, bool doAA); bool setPath(const SkPath& path, const SkIRect& clip, bool doAA); bool op(const SkRasterClip&, SkRegion::Op); - bool setConservativeRect(const SkRect& r, const SkIRect& clipR, bool isInverse); }; class SkAutoRasterClipValidate : SkNoncopyable { diff --git a/src/core/SkRecorder.cpp b/src/core/SkRecorder.cpp index f6c16d1c30..86578a53b8 100644 --- a/src/core/SkRecorder.cpp +++ b/src/core/SkRecorder.cpp @@ -11,7 +11,7 @@ // SkCanvas will fail in mysterious ways if it doesn't know the real width and height. SkRecorder::SkRecorder(SkRecord* record, int width, int height) - : SkCanvas(width, height, SkCanvas::kConservativeRasterClip_InitFlag) + : SkCanvas(width, height) , fRecord(record) , fSaveLayerCount(0) {} @@ -280,12 +280,12 @@ void SkRecorder::onClipRect(const SkRect& rect, SkRegion::Op op, ClipEdgeStyle e } void SkRecorder::onClipRRect(const SkRRect& rrect, SkRegion::Op op, ClipEdgeStyle edgeStyle) { - INHERITED(onClipRRect, rrect, op, edgeStyle); + INHERITED(updateClipConservativelyUsingBounds, rrect.getBounds(), op, false); APPEND(ClipRRect, this->devBounds(), rrect, op, edgeStyle == kSoft_ClipEdgeStyle); } void SkRecorder::onClipPath(const SkPath& path, SkRegion::Op op, ClipEdgeStyle edgeStyle) { - INHERITED(onClipPath, path, op, edgeStyle); + INHERITED(updateClipConservativelyUsingBounds, path.getBounds(), op, path.isInverseFillType()); APPEND(ClipPath, this->devBounds(), delay_copy(path), op, edgeStyle == kSoft_ClipEdgeStyle); } diff --git a/tests/AAClipTest.cpp b/tests/AAClipTest.cpp index 64e378455a..776cd5267a 100644 --- a/tests/AAClipTest.cpp +++ b/tests/AAClipTest.cpp @@ -371,7 +371,6 @@ static bool operator==(const SkRasterClip& a, const SkRasterClip& b) { static void did_dx_affect(skiatest::Reporter* reporter, const SkScalar dx[], size_t count, bool changed) { - const SkISize baseSize = SkISize::Make(10, 10); SkIRect ir = { 0, 0, 10, 10 }; for (size_t i = 0; i < count; ++i) { @@ -382,11 +381,11 @@ static void did_dx_affect(skiatest::Reporter* reporter, const SkScalar dx[], SkRasterClip rc1(ir); SkRasterClip rc2(ir); - rc0.op(r, baseSize, SkRegion::kIntersect_Op, false); + rc0.op(r, SkRegion::kIntersect_Op, false); r.offset(dx[i], 0); - rc1.op(r, baseSize, SkRegion::kIntersect_Op, true); + rc1.op(r, SkRegion::kIntersect_Op, true); r.offset(-2*dx[i], 0); - rc2.op(r, baseSize, SkRegion::kIntersect_Op, true); + rc2.op(r, SkRegion::kIntersect_Op, true); REPORTER_ASSERT(reporter, changed != (rc0 == rc1)); REPORTER_ASSERT(reporter, changed != (rc0 == rc2)); diff --git a/tests/PictureTest.cpp b/tests/PictureTest.cpp index 3bc91d78a7..05aec092c3 100644 --- a/tests/PictureTest.cpp +++ b/tests/PictureTest.cpp @@ -1570,8 +1570,8 @@ static void test_clip_bound_opt(skiatest::Reporter* reporter) { path2.addOval(rect3); SkIRect clipBounds; SkPictureRecorder recorder; - - // Testing conservative-raster-clip that is enabled by PictureRecord + // Minimalist test set for 100% code coverage of + // SkPictureRecord::updateClipConservativelyUsingBounds { SkCanvas* canvas = recorder.beginRecording(10, 10); canvas->clipPath(invPath, SkRegion::kIntersect_Op); |