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
diff --git a/include/core/SkCanvas.h b/include/core/SkCanvas.h
index 5088d7d..f514d18 100644
--- a/include/core/SkCanvas.h
+++ b/include/core/SkCanvas.h
@@ -1265,6 +1265,11 @@
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 @@
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 @@
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 618431a..aad5bf4 100644
--- a/include/core/SkDevice.h
+++ b/include/core/SkDevice.h
@@ -372,8 +372,6 @@
// 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 5545227..91fc2f0 100644
--- a/include/gpu/SkGpuDevice.h
+++ b/include/gpu/SkGpuDevice.h
@@ -152,7 +152,6 @@
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 56a0962..686f179 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 @@
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 62d60da..9978615 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -81,12 +81,9 @@
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 @@
*/
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 @@
*/
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 @@
////////////////////////////////////////////////////////////////////////////
-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 @@
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 @@
{
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 @@
{
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 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 @@
}
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 @@
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 @@
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 @@
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::quickReject(const SkRect& rect) const {
+
if (!rect.isFinite())
return true;
diff --git a/src/core/SkDeviceLooper.cpp b/src/core/SkDeviceLooper.cpp
index a8350cc..9346465 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 cae8420..b171691 100644
--- a/src/core/SkPictureRecord.cpp
+++ b/src/core/SkPictureRecord.cpp
@@ -788,7 +788,7 @@
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 @@
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 f820c5a..dab91d8 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 @@
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 @@
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 @@
// 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 @@
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 @@
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 @@
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 @@
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 8a06818..ec38990 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 @@
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 @@
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 @@
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 f6c16d1..86578a5 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::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 64e3784..776cd52 100644
--- a/tests/AAClipTest.cpp
+++ b/tests/AAClipTest.cpp
@@ -371,7 +371,6 @@
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 @@
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 3bc91d7..05aec09 100644
--- a/tests/PictureTest.cpp
+++ b/tests/PictureTest.cpp
@@ -1570,8 +1570,8 @@
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);