Simplify SkLayerDrawLooper

include/effects/SkLayerDrawLooper.h

Index: include/effects/SkLayerDrawLooper.h
diff --git a/include/effects/SkLayerDrawLooper.h b/include/effects/SkLayerDrawLooper.h
index 76172dfd4b1dfcfb45296bac51a36dcff101d344..8918f83a9964f4fd1b95156843fda6744a979b2f 100644
--- a/include/effects/SkLayerDrawLooper.h
+++ b/include/effects/SkLayerDrawLooper.h
@@ -9,10 +9,13 @@
 #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();
@@ -88,13 +91,8 @@ protected:
     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 {
@@ -105,7 +103,8 @@ private:
         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&);
     };
@@ -119,35 +118,35 @@ public:
         ~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;
     };
 };