Modify SkDeferredCanvas so that it uses its inherited SkCanvas to track matrix and...

Removed 'virtual' from a few canvas methods that no longer need it thanks to this change.

BUG=http://code.google.com/p/skia/issues/detail?id=506
TEST=Canvas unit test
REVIEW=http://codereview.appspot.com/5697052/

git-svn-id: http://skia.googlecode.com/svn/trunk@3261 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/include/core/SkCanvas.h b/include/core/SkCanvas.h
index 058979d..84d1038 100644 (file)
--- a/include/core/SkCanvas.h
+++ b/include/core/SkCanvas.h
@@ -283,7 +283,7 @@ public:
     /** Returns the number of matrix/clip states on the SkCanvas' private stack.
         This will equal # save() calls - # restore() calls.
     */
-    virtual int getSaveCount() const;
+    int getSaveCount() const;
 
     /** Efficient way to pop any calls to save() that happened after the save
         count reached saveCount. It is an error for saveCount to be less than
@@ -850,7 +850,7 @@ public:
         This does not account for the translate in any of the devices.
         @return The current matrix on the canvas.
     */
-    virtual const SkMatrix& getTotalMatrix() const;
+    const SkMatrix& getTotalMatrix() const;
 
     enum ClipType {
         kEmpty_ClipType = 0,
@@ -936,6 +936,11 @@ protected:
     virtual void commonDrawBitmap(const SkBitmap&, const SkIRect*,
                                   const SkMatrix&, const SkPaint& paint);
 
+    // Clip rectangle bounds. Called internally by saveLayer.
+    // returns false if the entire rectangle is entirely clipped out
+    bool clipRectBounds(const SkRect* bounds, SaveFlags flags,
+                         SkIRect* intersection);
+
 private:
     class MCRec;
 

diff --git a/include/utils/SkDeferredCanvas.h b/include/utils/SkDeferredCanvas.h
index e06ea81..87797ac 100644 (file)
--- a/include/utils/SkDeferredCanvas.h
+++ b/include/utils/SkDeferredCanvas.h
@@ -80,14 +80,13 @@ public:
     virtual int saveLayer(const SkRect* bounds, const SkPaint* paint,
                           SaveFlags flags) SK_OVERRIDE;
     virtual void restore() SK_OVERRIDE;
-    virtual int getSaveCount() const SK_OVERRIDE;
+    virtual bool isDrawingToLayer() const SK_OVERRIDE;
     virtual bool translate(SkScalar dx, SkScalar dy) SK_OVERRIDE;
     virtual bool scale(SkScalar sx, SkScalar sy) SK_OVERRIDE;
     virtual bool rotate(SkScalar degrees) SK_OVERRIDE;
     virtual bool skew(SkScalar sx, SkScalar sy) SK_OVERRIDE;
     virtual bool concat(const SkMatrix& matrix) SK_OVERRIDE;
     virtual void setMatrix(const SkMatrix& matrix) SK_OVERRIDE;
-    virtual const SkMatrix& getTotalMatrix() const SK_OVERRIDE;
     virtual bool clipRect(const SkRect& rect, SkRegion::Op op,
                           bool doAntiAlias) SK_OVERRIDE;
     virtual bool clipPath(const SkPath& path, SkRegion::Op op,

diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 6e061c2..5f97d68 100644 (file)
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -707,19 +707,12 @@ static bool bounds_affects_clip(SkCanvas::SaveFlags flags) {
     return (flags & SkCanvas::kClipToLayer_SaveFlag) != 0;
 }
 
-int SkCanvas::saveLayer(const SkRect* bounds, const SkPaint* paint,
-                        SaveFlags flags) {
-    // do this before we create the layer. We don't call the public save() since
-    // that would invoke a possibly overridden virtual
-    int count = this->internalSave(flags);
-
-    fDeviceCMDirty = true;
-
+bool SkCanvas::clipRectBounds(const SkRect* bounds, SaveFlags flags,
+                               SkIRect* intersection) {
     SkIRect clipBounds;
     if (!this->getClipDeviceBounds(&clipBounds)) {
-        return count;
+        return false;
     }
-
     SkIRect ir;
     if (NULL != bounds) {
         SkRect r;
@@ -731,16 +724,36 @@ int SkCanvas::saveLayer(const SkRect* bounds, const SkPaint* paint,
             if (bounds_affects_clip(flags)) {
                 fMCRec->fRasterClip->setEmpty();
             }
-            return count;
+            return false;
         }
     } else {    // no user bounds, so just use the clip
         ir = clipBounds;
     }
 
     fClipStack.clipDevRect(ir, SkRegion::kIntersect_Op);
+
     // early exit if the clip is now empty
     if (bounds_affects_clip(flags) &&
         !fMCRec->fRasterClip->op(ir, SkRegion::kIntersect_Op)) {
+        return false;
+    }
+
+    if (intersection) {
+        *intersection = ir;
+    }
+    return true;
+}
+
+int SkCanvas::saveLayer(const SkRect* bounds, const SkPaint* paint,
+                        SaveFlags flags) {
+    // do this before we create the layer. We don't call the public save() since
+    // that would invoke a possibly overridden virtual
+    int count = this->internalSave(flags);
+
+    fDeviceCMDirty = true;
+
+    SkIRect ir;
+    if (!this->clipRectBounds(bounds, flags, &ir)) {
         return count;
     }
 
@@ -1044,8 +1057,8 @@ static bool clipPathHelper(const SkCanvas* canvas, SkRasterClip* currClip,
             return currClip->op(clip, op);
         }
     } else {
-        const SkBitmap& bm = canvas->getDevice()->accessBitmap(false);
-        base.setRect(0, 0, bm.width(), bm.height());
+        const SkDevice* device = canvas->getDevice();
+        base.setRect(0, 0, device->width(), device->height());
 
         if (SkRegion::kReplace_Op == op) {
             return currClip->setPath(devPath, base, doAA);

diff --git a/src/core/SkPictureRecord.cpp b/src/core/SkPictureRecord.cpp
index c56c104..ba49b72 100644 (file)
--- a/src/core/SkPictureRecord.cpp
+++ b/src/core/SkPictureRecord.cpp
@@ -62,7 +62,9 @@ int SkPictureRecord::saveLayer(const SkRect* bounds, const SkPaint* paint,
         clip starts out the size of the picture, which is often much larger
         than the size of the actual device we'll use during playback).
      */
-    return this->INHERITED::save(flags);
+    int count = this->INHERITED::save(flags);
+    this->clipRectBounds(bounds, flags, NULL);
+    return count;
 }
 
 bool SkPictureRecord::isDrawingToLayer() const {

diff --git a/src/utils/SkDeferredCanvas.cpp b/src/utils/SkDeferredCanvas.cpp
index 00a26b9..1254392 100644 (file)
--- a/src/utils/SkDeferredCanvas.cpp
+++ b/src/utils/SkDeferredCanvas.cpp
@@ -101,7 +101,6 @@ void SkDeferredCanvas::init()
 void SkDeferredCanvas::validate() const
 {
     SkASSERT(getDevice());
-    SkASSERT(INHERITED::getTotalMatrix().isIdentity());
 }
 
 SkCanvas* SkDeferredCanvas::drawingCanvas() const
@@ -216,78 +215,87 @@ bool SkDeferredCanvas::isFullFrame(const SkRect* rect,
 
 int SkDeferredCanvas::save(SaveFlags flags)
 {
-    return drawingCanvas()->save(flags);
+    drawingCanvas()->save(flags);
+    return this->INHERITED::save(flags);
 }
 
 int SkDeferredCanvas::saveLayer(const SkRect* bounds, const SkPaint* paint,
                           SaveFlags flags)
 {
-    return drawingCanvas()->saveLayer(bounds, paint, flags);
+    drawingCanvas()->saveLayer(bounds, paint, flags);
+    int count = this->INHERITED::save(flags);
+    this->clipRectBounds(bounds, flags, NULL);
+    return count;
 }
 
 void SkDeferredCanvas::restore()
 {
     drawingCanvas()->restore();
+    this->INHERITED::restore();
 }
 
-int SkDeferredCanvas::getSaveCount() const
+bool SkDeferredCanvas::isDrawingToLayer() const
 {
-    return drawingCanvas()->getSaveCount();
+    return drawingCanvas()->isDrawingToLayer();
 }
 
 bool SkDeferredCanvas::translate(SkScalar dx, SkScalar dy)
 {
-    return drawingCanvas()->translate(dx, dy);
+    drawingCanvas()->translate(dx, dy);
+    return this->INHERITED::translate(dx, dy);
 }
 
 bool SkDeferredCanvas::scale(SkScalar sx, SkScalar sy)
 {
-    return drawingCanvas()->scale(sx, sy);
+    drawingCanvas()->scale(sx, sy);
+    return this->INHERITED::scale(sx, sy);
 }
 
 bool SkDeferredCanvas::rotate(SkScalar degrees)
 {
-    return drawingCanvas()->rotate(degrees);
+    drawingCanvas()->rotate(degrees);
+    return this->INHERITED::rotate(degrees);
 }
 
 bool SkDeferredCanvas::skew(SkScalar sx, SkScalar sy)
 {
-    return drawingCanvas()->skew(sx, sy);
+    drawingCanvas()->skew(sx, sy);
+    return this->INHERITED::skew(sx, sy);
 }
 
 bool SkDeferredCanvas::concat(const SkMatrix& matrix)
 {
-    return drawingCanvas()->concat(matrix);
+    drawingCanvas()->concat(matrix);
+    return this->INHERITED::concat(matrix);
 }
 
 void SkDeferredCanvas::setMatrix(const SkMatrix& matrix)
 {
     drawingCanvas()->setMatrix(matrix);
-}
-
-const SkMatrix& SkDeferredCanvas::getTotalMatrix() const 
-{
-    return drawingCanvas()->getTotalMatrix();
+    this->INHERITED::setMatrix(matrix);
 }
 
 bool SkDeferredCanvas::clipRect(const SkRect& rect,
                                 SkRegion::Op op,
                                 bool doAntiAlias)
 {
-    return drawingCanvas()->clipRect(rect, op, doAntiAlias);
+    drawingCanvas()->clipRect(rect, op, doAntiAlias);
+    return this->INHERITED::clipRect(rect, op, doAntiAlias);
 }
 
 bool SkDeferredCanvas::clipPath(const SkPath& path,
                                 SkRegion::Op op,
                                 bool doAntiAlias)
 {
-    return drawingCanvas()->clipPath(path, op, doAntiAlias);
+    drawingCanvas()->clipPath(path, op, doAntiAlias);
+    return this->INHERITED::clipPath(path, op, doAntiAlias);
 }
 
 bool SkDeferredCanvas::clipRegion(const SkRegion& deviceRgn,
                                   SkRegion::Op op)
 {
-    return drawingCanvas()->clipRegion(deviceRgn, op);
+    drawingCanvas()->clipRegion(deviceRgn, op);
+    return this->INHERITED::clipRegion(deviceRgn, op);
 }
 
 void SkDeferredCanvas::clear(SkColor color)
@@ -454,14 +462,14 @@ void SkDeferredCanvas::drawVertices(VertexMode vmode, int vertexCount,
 
 SkBounder* SkDeferredCanvas::setBounder(SkBounder* bounder)
 {
-    INHERITED::setBounder(bounder); // So non-virtual getBounder works
-    return drawingCanvas()->setBounder(bounder);
+    drawingCanvas()->setBounder(bounder);
+    return INHERITED::setBounder(bounder);
 }
 
 SkDrawFilter* SkDeferredCanvas::setDrawFilter(SkDrawFilter* filter)
 {
-    INHERITED::setDrawFilter(filter); // So non-virtual getDrawFilter works
-    return drawingCanvas()->setDrawFilter(filter); 
+    drawingCanvas()->setDrawFilter(filter); 
+    return INHERITED::setDrawFilter(filter); // So non-virtual getDrawFilter works
 }
 
 SkCanvas* SkDeferredCanvas::canvasForDrawIter() {
@@ -482,8 +490,7 @@ SkDeferredCanvas::DeferredDevice::DeferredDevice(
     fImmediateDevice = immediateDevice; // ref counted via fImmediateCanvas
     fImmediateCanvas = SkNEW_ARGS(SkCanvas, (fImmediateDevice));
     fRecordingCanvas = fPicture.beginRecording(fImmediateDevice->width(),
-        fImmediateDevice->height(),
-        SkPicture::kUsePathBoundsForClip_RecordingFlag);
+        fImmediateDevice->height(), 0);
 }
 
 SkDeferredCanvas::DeferredDevice::~DeferredDevice()
@@ -518,8 +525,7 @@ void SkDeferredCanvas::DeferredDevice::contentsCleared()
             // old one, hence purging deferred draw ops.
             fRecordingCanvas = fPicture.beginRecording(
                 fImmediateDevice->width(),
-                fImmediateDevice->height(),
-                SkPicture::kUsePathBoundsForClip_RecordingFlag);
+                fImmediateDevice->height(), 0);
 
             // Restore pre-purge state
             if (!clipRegion.isEmpty()) {
@@ -548,8 +554,7 @@ void SkDeferredCanvas::DeferredDevice::flushPending()
     }
     fPicture.draw(fImmediateCanvas);
     fRecordingCanvas = fPicture.beginRecording(fImmediateDevice->width(), 
-        fImmediateDevice->height(),
-        SkPicture::kUsePathBoundsForClip_RecordingFlag);
+        fImmediateDevice->height(), 0);
 }
 
 void SkDeferredCanvas::DeferredDevice::flush()

diff --git a/tests/CanvasTest.cpp b/tests/CanvasTest.cpp
index 54b728b..0c4683e 100644 (file)
--- a/tests/CanvasTest.cpp
+++ b/tests/CanvasTest.cpp
@@ -224,27 +224,25 @@ TEST_STEP(NAME, NAME##TestStep )
 // Basic test steps for most virtual methods in SkCanvas that draw or affect 
 // the state of the canvas.
 
-// The following test steps are commented-out because they currently fail
-// Issue: http://code.google.com/p/skia/issues/detail?id=506
-//SIMPLE_TEST_STEP(SaveMatrix, save(SkCanvas::kMatrix_SaveFlag));
-//SIMPLE_TEST_STEP(SaveClip, save(SkCanvas::kClip_SaveFlag));
-//SIMPLE_TEST_STEP(SaveMatrixClip, save(SkCanvas::kMatrixClip_SaveFlag));
-//SIMPLE_TEST_STEP(SaveLayer, saveLayer(NULL, NULL));
-//SIMPLE_TEST_STEP(BoundedSaveLayer, saveLayer(&kTestRect, NULL));
-//SIMPLE_TEST_STEP(PaintSaveLayer, saveLayer(NULL, &kTestPaint));
-//SIMPLE_TEST_STEP_WITH_ASSERT(Translate,
-//    translate(SkIntToScalar(1), SkIntToScalar(2)));
-//SIMPLE_TEST_STEP_WITH_ASSERT(Scale,
-//    scale(SkIntToScalar(1), SkIntToScalar(2)));
-//SIMPLE_TEST_STEP_WITH_ASSERT(Rotate, rotate(SkIntToScalar(1)));
-//SIMPLE_TEST_STEP_WITH_ASSERT(Skew,
-//    skew(SkIntToScalar(1), SkIntToScalar(2)));
-//SIMPLE_TEST_STEP_WITH_ASSERT(Concat, concat(kTestMatrix));
-//SIMPLE_TEST_STEP(SetMatrix, setMatrix(kTestMatrix));
-//SIMPLE_TEST_STEP_WITH_ASSERT(ClipRect, clipRect(kTestRect));
-//SIMPLE_TEST_STEP_WITH_ASSERT(ClipPath, clipPath(kTestPath));
-//SIMPLE_TEST_STEP_WITH_ASSERT(ClipRegion,
-//    clipRegion(kTestRegion, SkRegion::kReplace_Op));
+SIMPLE_TEST_STEP(SaveMatrix, save(SkCanvas::kMatrix_SaveFlag));
+SIMPLE_TEST_STEP(SaveClip, save(SkCanvas::kClip_SaveFlag));
+SIMPLE_TEST_STEP(SaveMatrixClip, save(SkCanvas::kMatrixClip_SaveFlag));
+SIMPLE_TEST_STEP(SaveLayer, saveLayer(NULL, NULL));
+SIMPLE_TEST_STEP(BoundedSaveLayer, saveLayer(&kTestRect, NULL));
+SIMPLE_TEST_STEP(PaintSaveLayer, saveLayer(NULL, &kTestPaint));
+SIMPLE_TEST_STEP_WITH_ASSERT(Translate,
+    translate(SkIntToScalar(1), SkIntToScalar(2)));
+SIMPLE_TEST_STEP_WITH_ASSERT(Scale,
+    scale(SkIntToScalar(1), SkIntToScalar(2)));
+SIMPLE_TEST_STEP_WITH_ASSERT(Rotate, rotate(SkIntToScalar(1)));
+SIMPLE_TEST_STEP_WITH_ASSERT(Skew,
+    skew(SkIntToScalar(1), SkIntToScalar(2)));
+SIMPLE_TEST_STEP_WITH_ASSERT(Concat, concat(kTestMatrix));
+SIMPLE_TEST_STEP(SetMatrix, setMatrix(kTestMatrix));
+SIMPLE_TEST_STEP_WITH_ASSERT(ClipRect, clipRect(kTestRect));
+SIMPLE_TEST_STEP_WITH_ASSERT(ClipPath, clipPath(kTestPath));
+SIMPLE_TEST_STEP_WITH_ASSERT(ClipRegion,
+    clipRegion(kTestRegion, SkRegion::kReplace_Op));
 SIMPLE_TEST_STEP(Clear, clear(kTestColor));
 SIMPLE_TEST_STEP(DrawPaint, drawPaint(kTestPaint));
 SIMPLE_TEST_STEP(DrawPointsPoints, drawPoints(SkCanvas::kPoints_PointMode,
@@ -341,9 +339,6 @@ static void SaveRestoreTestStep(SkCanvas* canvas,
 }
 TEST_STEP(SaveRestore, SaveRestoreTestStep);
 
-// The following test step is commented-out because it currently fails
-// Issue: http://code.google.com/p/skia/issues/detail?id=506
-/*
 static void DrawLayerTestStep(SkCanvas* canvas, 
                               skiatest::Reporter* reporter,
                               CanvasTestStep* testStep) {
@@ -376,7 +371,6 @@ static void DrawLayerTestStep(SkCanvas* canvas,
         testStep->assertMessage());
 }
 TEST_STEP(DrawLayer, DrawLayerTestStep);
-*/
 
 static void AssertCanvasStatesEqual(skiatest::Reporter* reporter,
                                     const SkCanvas* canvas1,