Change SkCanvasState to use inheritance.

src/utils/SkCanvasStateUtils.cpp

 /*
  * Copyright 2013 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkCanvasStateUtils.h"
 
 #include "SkBitmapDevice.h"
@@ skipping 55 matching lines @@
         struct {
             int32_t textureID;
         } gpu;
     };
 };
 
 class SkCanvasState {
 public:
     SkCanvasState(SkCanvas* canvas) {
         SkASSERT(canvas);
         version = CANVAS_STATE_VERSION;

[mtklein, 2014/07/07 22:09:16]

Might want to have the derived class pass in version so that it can be part of
that class?  E.g.

class SkCanvasState_v1 : public SkCanvasState {
  static const int32 kVersion = 1;

  SkCanvasState_v1(SkCanvas* canvas)
    : INHERITED(kVersion, canvas) ...  { ... }
  ...
};

This could make the asserts in ReleaseCanvasState a little clearer:
  SkASSERT(SkCanvasState_v1::kVersion == state->version);
  SkDELETE(static_cast<SkCanvasSatate_v1*>(state));
}

[scroggo, 2014/07/08 18:45:13]

Done.

-        width = canvas->getDeviceSize().width();
-        height = canvas->getDeviceSize().height();
+        width = canvas->getBaseLayerSize().width();
+        height = canvas->getBaseLayerSize().height();
+
+    }
+
+    /**
+     * The version this struct was built with.  This field must always appear
+     * first in the struct so that when the versions don't match (and the
+     * remaining contents and size are potentially different) we can still
+     * compare the version numbers.
+     */
+    int32_t version;
+    int32_t width;
+    int32_t height;
+    int32_t alignmentPadding;
+};
+
+class SkCanvasState_v1 : public SkCanvasState {
+public:
+    SkCanvasState_v1(SkCanvas* canvas)
+    : INHERITED(canvas)
+    {
+        SkASSERT(this->version == 1);
         layerCount = 0;
         layers = NULL;
-        originalCanvas = SkRef(canvas);
-
         mcState.clipRectCount = 0;
         mcState.clipRects = NULL;
+        originalCanvas = SkRef(canvas);
     }
 
-    ~SkCanvasState() {
+    ~SkCanvasState_v1() {
         // loop through the layers and free the data allocated to the clipRects
         for (int i = 0; i < layerCount; ++i) {
             sk_free(layers[i].mcState.clipRects);
         }
 
         sk_free(mcState.clipRects);
         sk_free(layers);
 
         // it is now safe to free the canvas since there should be no remaining
         // references to the content that is referenced by this canvas (e.g. pixels)
         originalCanvas->unref();
     }
 
-    /**
-     * The version this struct was built with.  This field must always appear
-     * first in the struct so that when the versions don't match (and the
-     * remaining contents and size are potentially different) we can still
-     * compare the version numbers.
-     */
-    int32_t version;
-
-    int32_t width;
-    int32_t height;
-
     SkMCState mcState;
 
     int32_t layerCount;
     SkCanvasLayerState* layers;
-
 private:
     SkCanvas* originalCanvas;
+    typedef SkCanvasState INHERITED;
 };
 
 ////////////////////////////////////////////////////////////////////////////////
 
 class ClipValidator : public SkCanvas::ClipVisitor {
 public:
     ClipValidator() : fFailed(false) {}
     bool failed() { return fFailed; }
 
     // ClipVisitor
@@ skipping 55 matching lines @@
 
     // Check the clip can be decomposed into rectangles (i.e. no soft clips).
     ClipValidator validator;
     canvas->replayClips(&validator);
     if (validator.failed()) {
         SkErrorInternals::SetError(kInvalidOperation_SkError,
                 "CaptureCanvasState does not support canvases with antialiased clips.\n");
         return NULL;
     }
 
-    SkAutoTDelete<SkCanvasState> canvasState(SkNEW_ARGS(SkCanvasState, (canvas)));
+    SkAutoTDelete<SkCanvasState_v1> canvasState(SkNEW_ARGS(SkCanvasState_v1, (canvas)));
 
     // decompose the total matrix and clip
     setup_MC_state(&canvasState->mcState, canvas->getTotalMatrix(),
                    canvas->internal_private_getTotalClip());
 
     /*
      * decompose the layers
      *
      * storage is allocated on the stack for the first 3 layers. It is common in
      * some view systems (e.g. Android) that a few non-clipped layers are present
@@ skipping 35 matching lines @@
     }
 
     // allocate memory for the layers and then and copy them to the struct
     SkASSERT(layerWriter.bytesWritten() == layerCount * sizeof(SkCanvasLayerState));
     canvasState->layerCount = layerCount;
     canvasState->layers = (SkCanvasLayerState*) sk_malloc_throw(layerWriter.bytesWritten());
     layerWriter.flatten(canvasState->layers);
 
     // for now, just ignore any client supplied DrawFilter.
     if (canvas->getDrawFilter()) {
-//        SkDEBUGF(("CaptureCanvasState will ignore the canvases draw filter.\n"));
+//        SkDEBUGF(("CaptureCanvasState will ignore the canvas's draw filter.\n"));
     }
 
     return canvasState.detach();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 
 static void setup_canvas_from_MC_state(const SkMCState& state, SkCanvas* canvas) {
     // reconstruct the matrix
     SkMatrix matrix;
@@ skipping 38 matching lines @@
     SkAutoTUnref<SkCanvas> canvas(SkNEW_ARGS(SkCanvas, (bitmap)));
 
     // setup the matrix and clip
     setup_canvas_from_MC_state(layerState.mcState, canvas.get());
 
     return canvas.detach();
 }
 
 SkCanvas* SkCanvasStateUtils::CreateFromCanvasState(const SkCanvasState* state) {
     SkASSERT(state);
+    SkASSERT(CANVAS_STATE_VERSION == state->version);

[scroggo, 2014/07/07 21:44:02]

Since CreateFromCanvasState will always be called from the updated version of
Skia, we don't need it to be compatible with other versions yet. If/when we
update the version, CreateFromCanvasState will need to handle older versions,
but never future versions.

Does that seem reasonable? Should we update the documentation explaining that we
can only recreate SkCanvas in one direction?

[mtklein, 2014/07/07 22:09:16]

It can't hurt to get this function in the state it'll eventually be in, e.g.

switch (state->version) {
  case 1: return create_from_v1(static_cast<const SkCanvasState_v1*>(state));
  default:
    SkASSERTF(false, "Unknown SkCanvasState version %d", state->version); 
    return NULL;
}
?

That said, leaving things as you've got them here certainly seems fine until we
add v2.

 
-    // check that the versions match
-    if (CANVAS_STATE_VERSION != state->version) {
-        SkDebugf("CreateFromCanvasState version does not match the one use to create the input");
-        return NULL;
-    }
+    const SkCanvasState_v1* state_v1 = static_cast<const SkCanvasState_v1*>(state);
 
-    if (state->layerCount < 1) {
+    if (state_v1->layerCount < 1) {
         return NULL;
     }
 
     SkAutoTUnref<SkCanvasStack> canvas(SkNEW_ARGS(SkCanvasStack, (state->width, state->height)));
 
     // setup the matrix and clip on the n-way canvas
-    setup_canvas_from_MC_state(state->mcState, canvas);
+    setup_canvas_from_MC_state(state_v1->mcState, canvas);
 
     // Iterate over the layers and add them to the n-way canvas
-    for (int i = state->layerCount - 1; i >= 0; --i) {
-        SkAutoTUnref<SkCanvas> canvasLayer(create_canvas_from_canvas_layer(state->layers[i]));
+    for (int i = state_v1->layerCount - 1; i >= 0; --i) {
+        SkAutoTUnref<SkCanvas> canvasLayer(create_canvas_from_canvas_layer(state_v1->layers[i]));
         if (!canvasLayer.get()) {
             return NULL;
         }
-        canvas->pushCanvas(canvasLayer.get(), SkIPoint::Make(state->layers[i].x,
-                                                             state->layers[i].y));
+        canvas->pushCanvas(canvasLayer.get(), SkIPoint::Make(state_v1->layers[i].x,
+                                                             state_v1->layers[i].y));
     }
 
     return canvas.detach();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 
 void SkCanvasStateUtils::ReleaseCanvasState(SkCanvasState* state) {
-    SkDELETE(state);
+    SkASSERT(CANVAS_STATE_VERSION == state->version);
+    // Upcast to the correct version of SkCanvasState. This avoids having a virtual destructor on
+    // SkCanvasState. That would be strange since SkCanvasState has no other virtual functions, and
+    // instead uses the field "version" to determine how to behave.
+    SkDELETE(static_cast<SkCanvasState_v1*>(state));
 }