Simplify SkLayerDrawLooper

include/effects/SkLayerDrawLooper.h

 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #ifndef SkLayerDrawLooper_DEFINED
 #define SkLayerDrawLooper_DEFINED
 
 #include "SkDrawLooper.h"
-#include "SkPaint.h"
-#include "SkPoint.h"
 #include "SkXfermode.h"
 
+#include <stddef.h>
+#include <vector>

[mtklein, 2015/09/25 11:47:21]

We don't use std::vector in Skia (yet?) but SkTArray<T> should be very similar.

We've also got SkTDArray<T> (D for Data, or Dumb) for POD T that can be a bit
faster, but SkTArray<T> is usually just fine and always safe.

[mdempsky, 2015/09/25 16:40:17]

Hm, right.  Somehow I thought I saw STL usage elsewhere in Skia, but looking
again it seems like only a few very specialized uses.

Will update to use a Skia vector alternative.

+
+class SkPaint;
+
 class SK_API SkLayerDrawLooper : public SkDrawLooper {
 public:
     virtual ~SkLayerDrawLooper();
 
     /**
      *  Bits specifies which aspects of the layer's paint should replace the
      *  corresponding aspects on the draw's paint.
      *  kEntirePaint_Bits means use the layer's paint completely.
      *  0 means ignore the layer's paint... except for fColorMode, which is
      *  always applied.
@@ skipping 55 matching lines @@
 
     Factory getFactory() const override { return CreateProc; }
     static SkFlattenable* CreateProc(SkReadBuffer& buffer);
 
 protected:
     SkLayerDrawLooper();
 
     void flatten(SkWriteBuffer&) const override;
 
 private:
-    struct Rec {
-        Rec*    fNext;
-        SkPaint fPaint;
-        LayerInfo fInfo;
-    };
-    Rec*    fRecs;
-    int     fCount;
+    struct Rec;
+    std::vector<Rec> fRecs;
 
     // state-machine during the init/next cycle
     class LayerDrawLooperContext : public SkDrawLooper::Context {
     public:
         explicit LayerDrawLooperContext(const SkLayerDrawLooper* looper);
 
     protected:
         bool next(SkCanvas*, SkPaint* paint) override;
 
     private:
-        Rec* fCurrRec;
+        std::vector<Rec>::const_iterator fCurr;
+        const std::vector<Rec>::const_iterator fEnd;
 
         static void ApplyInfo(SkPaint* dst, const SkPaint& src, const LayerInfo&);
     };
 
     typedef SkDrawLooper INHERITED;
 
 public:
     class SK_API Builder {
     public:
         Builder();
         ~Builder();
 
         /**
-         *  Call for each layer you want to add (from top to bottom).
+         *  Requests that the builder reserve space for at least n layers.
+         */
+        void reserve(size_t n);
+
+        /**
+         *  Call for each layer you want to add (from bottom to top).
          *  This returns a paint you can modify, but that ptr is only valid until
-         *  the next call made to addLayer().
+         *  the next call made to addLayerOnTop().
          */
-        SkPaint* addLayer(const LayerInfo&);
+        SkPaint* addLayerOnTop(const LayerInfo&);

[mtklein, 2015/09/25 11:47:21]

Can we keep the API oriented top to bottom while keeping the simplified
implementation?  Seems like we can still append to an array just as you're doing
here, but just reverse the iteration order?

[mdempsky, 2015/09/25 16:40:17]

Yeah that's possible too if you think that's better.  FWIW, the reasons I
favored addLayerOnTop are:

- I only found one use of addLayer in Chromium versus several uses of
addLayerOnTop in Blink, so converting everyone to addLayerOnTop seemed slightly
easier.

- The name is a bit more explicit about how the layer is positioned relative to
other layers (although addLayer's documentation mentions top to bottom).

- A sequence of calls to addLayerOnTop will match up in the same ordering as the
sequence of paints onto the canvas later, which seems easier / more natural for
developers to reason about.

I'm happy to look into keeping addLayer instead though.  What do you think?

 
         /**
          *  This layer will draw with the original paint, at the specified offset
          */
-        void addLayer(SkScalar dx, SkScalar dy);
+        void addLayerOnTop(SkScalar dx, SkScalar dy);
 
         /**
          *  This layer will with the original paint and no offset.
          */
-        void addLayer() { this->addLayer(0, 0); }
-
-        /// Similar to addLayer, but adds a layer to the top.
-        SkPaint* addLayerOnTop(const LayerInfo&);
+        void addLayerOnTop() { this->addLayerOnTop(0, 0); }
 
         /**
-          * Pass list of layers on to newly built looper and return it. This will
-          * also reset the builder, so it can be used to build another looper.
-          */
+         *  Pass list of layers on to newly built looper and return it. This will
+         *  also reset the builder, so it can be used to build another looper.
+         */
         SkLayerDrawLooper* detachLooper();
 
     private:
-        Rec* fRecs;
-        Rec* fTopRec;
-        int  fCount;
+        std::vector<Rec> fRecs;
     };
 };
 
 #endif