store vertices arrays inline with object

Also unify some of naming (esp. around texCoords)

BUG=skia:6366

Change-Id: I5a6793f029cccf0cd0a2c1d180b259ce4eab526f
Reviewed-on: https://skia-review.googlesource.com/9705
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>

7 files changed, 197 insertions, 182 deletions

diff --git a/gm/vertices.cpp b/gm/vertices.cpp
index 864ac3dc8a..0d798270c4 100644
--- a/gm/vertices.cpp
+++ b/gm/vertices.cpp
@@ -214,7 +214,8 @@ static void draw_batching(SkCanvas* canvas, bool useObject) {
     // Triangle fans can't batch so we convert to regular triangles,
     static constexpr int kNumTris = kMeshIndexCnt - 2;
     SkVertices::Builder builder(SkCanvas::kTriangles_VertexMode, kMeshVertexCnt, 3 * kNumTris,
-                                SkVertices::kHasColors_Flag | SkVertices::kHasTexs_Flag);
+                                SkVertices::kHasColors_BuilderFlag |
+                                SkVertices::kHasTexCoords_BuilderFlag);
 
     SkPoint* pts = builder.positions();
     SkPoint* texs = builder.texCoords();
diff --git a/include/core/SkVertices.h b/include/core/SkVertices.h
index af4e3bc960..8d6f211a45 100644
--- a/include/core/SkVertices.h
+++ b/include/core/SkVertices.h
@@ -16,14 +16,10 @@
 #include "SkRefCnt.h"
 
 /**
- * An immutable set of vertex data that can be used with SkCanvas::drawVertices. Clients are
- * encouraged to provide a bounds on the vertex positions if they can compute one more cheaply than
- * looping over the positions.
+ * An immutable set of vertex data that can be used with SkCanvas::drawVertices.
  */
 class SkVertices : public SkNVRefCnt<SkVertices> {
 public:
-    ~SkVertices() { sk_free((void*)fPositions); }
-
     /**
      *  Create a vertices by copying the specified arrays. texs and colors may be nullptr,
      *  and indices is ignored if indexCount == 0.
@@ -42,72 +38,93 @@ public:
         return MakeCopy(mode, vertexCount, positions, texs, colors, 0, nullptr);
     }
 
-    enum Flags {
-        kHasTexs_Flag    = 1 << 0,
-        kHasColors_Flag  = 1 << 1,
+    struct Sizes;
+
+    enum BuilderFlags {
+        kHasTexCoords_BuilderFlag   = 1 << 0,
+        kHasColors_BuilderFlag      = 1 << 1,
     };
     class Builder {
     public:
         Builder(SkCanvas::VertexMode mode, int vertexCount, int indexCount, uint32_t flags);
-        ~Builder();
 
-        bool isValid() const { return fPositions != nullptr; }
+        bool isValid() const { return fVertices != nullptr; }
 
-        int vertexCount() const { return fVertexCnt; }
-        int indexCount() const { return fIndexCnt; }
-        SkPoint* positions() { return fPositions; }
-        SkPoint* texCoords() { return fTexs; }
-        SkColor* colors() { return fColors; }
-        uint16_t* indices() { return fIndices; }
+        // if the builder is invalid, these will return 0
+        int vertexCount() const;
+        int indexCount() const;
+        SkPoint* positions();
+        SkPoint* texCoords();   // returns null if there are no texCoords
+        SkColor* colors();      // returns null if there are no colors
+        uint16_t* indices();    // returns null if there are no indices
 
+        // Detach the built vertices object. After the first call, this will always return null.
         sk_sp<SkVertices> detach();
 
     private:
-        SkPoint* fPositions;  // owner of storage, use sk_free
-        SkPoint* fTexs;
-        SkColor* fColors;
-        uint16_t* fIndices;
-        int fVertexCnt;
-        int fIndexCnt;
-        SkCanvas::VertexMode fMode;
+        Builder(SkCanvas::VertexMode mode, int vertexCount, int indexCount, const Sizes&);
+
+        void init(SkCanvas::VertexMode mode, int vertexCount, int indexCount, const Sizes&);
+
+        // holds a partially complete object. only completed in detach()
+        sk_sp<SkVertices> fVertices;
+
+        friend class SkVertices;
     };
 
+    uint32_t uniqueID() const { return fUniqueID; }
     SkCanvas::VertexMode mode() const { return fMode; }
+    const SkRect& bounds() const { return fBounds; }
+
+    bool hasColors() const { return SkToBool(this->colors()); }
+    bool hasTexCoords() const { return SkToBool(this->texCoords()); }
+    bool hasIndices() const { return SkToBool(this->indices()); }
 
-    uint32_t uniqueID() const { return fUniqueID; }
     int vertexCount() const { return fVertexCnt; }
-    bool hasColors() const { return SkToBool(fColors); }
-    bool hasTexCoords() const { return SkToBool(fTexs); }
     const SkPoint* positions() const { return fPositions; }
     const SkPoint* texCoords() const { return fTexs; }
     const SkColor* colors() const { return fColors; }
 
-    bool isIndexed() const { return SkToBool(fIndexCnt); }
     int indexCount() const { return fIndexCnt; }
     const uint16_t* indices() const { return fIndices; }
 
-    size_t size() const {
-        return fVertexCnt * (sizeof(SkPoint) * (this->hasTexCoords() ? 2 : 1) + sizeof(SkColor)) +
-               fIndexCnt * sizeof(uint16_t);
-    }
+    // returns approximate byte size of the vertices object
+    size_t approximateSize() const;
 
-    const SkRect& bounds() const { return fBounds; }
+    /**
+     *  Recreate a vertices from a buffer previously created by calling encode().
+     *  Returns null if the data is corrupt or the length is incorrect for the contents.
+     */
+    static sk_sp<SkVertices> Decode(const void* buffer, size_t length);
 
-    static sk_sp<SkVertices> Decode(const void*, size_t);
+    /**
+     *  Pack the vertices object into a byte buffer. This can be used to recreate the vertices
+     *  by calling Decode() with the buffer.
+     */
     sk_sp<SkData> encode() const;
 
 private:
     SkVertices() {}
 
-    const SkPoint* fPositions;  // owner of storage, use sk_free
-    const SkPoint* fTexs;
-    const SkColor* fColors;
-    const uint16_t* fIndices;
-    SkRect fBounds;
+    static sk_sp<SkVertices> Alloc(int vCount, int iCount, uint32_t builderFlags,
+                                   size_t* arraySize);
+
+    // we store this first, to pair with the refcnt in our base-class, so we don't have an
+    // unnecessary pad between it and the (possibly 8-byte aligned) ptrs.
     uint32_t fUniqueID;
-    int fVertexCnt;
-    int fIndexCnt;
+
+    // these point inside our allocation, so none of these can be "freed"
+    SkPoint*    fPositions;
+    SkPoint*    fTexs;
+    SkColor*    fColors;
+    uint16_t*   fIndices;
+
+    SkRect  fBounds;    // computed to be the union of the fPositions[]
+    int     fVertexCnt;
+    int     fIndexCnt;
+
     SkCanvas::VertexMode fMode;
+    // below here is where the actual array data is stored.
 };
 
 #endif
diff --git a/src/core/SkVertices.cpp b/src/core/SkVertices.cpp
index 936d70dc09..1980b7ea5c 100644
--- a/src/core/SkVertices.cpp
+++ b/src/core/SkVertices.cpp
@@ -20,202 +20,199 @@ static int32_t next_id() {
     return id;
 }
 
-static size_t compute_arrays_size(int vertexCount, int indexCount, uint32_t builderFlags) {
-    if (vertexCount < 0 || indexCount < 0) {
-        return 0;   // signal error
-    }
+struct SkVertices::Sizes {
+    Sizes(int vertexCount, int indexCount, bool hasTexs, bool hasColors) {
+        int64_t vSize = (int64_t)vertexCount * sizeof(SkPoint);
+        int64_t tSize = hasTexs ? (int64_t)vertexCount * sizeof(SkPoint) : 0;
+        int64_t cSize = hasColors ? (int64_t)vertexCount * sizeof(SkColor) : 0;
+        int64_t iSize = (int64_t)indexCount * sizeof(uint16_t);
+
+        int64_t total = sizeof(SkVertices) + vSize + tSize + cSize + iSize;
+        if (!sk_64_isS32(total)) {
+            sk_bzero(this, sizeof(*this));
+        } else {
+            fTotal = SkToSizeT(total);
+            fVSize = SkToSizeT(vSize);
+            fTSize = SkToSizeT(tSize);
+            fCSize = SkToSizeT(cSize);
+            fISize = SkToSizeT(iSize);
+            fArrays = fTotal - sizeof(SkVertices);  // just the sum of the arrays
+        }
+    }
+
+    bool isValid() const { return fTotal != 0; }
+
+    size_t fTotal;  // size of entire SkVertices allocation (obj + arrays)
+    size_t fArrays; // size of all the arrays (V + T + C + I)
+    size_t fVSize;
+    size_t fTSize;
+    size_t fCSize;
+    size_t fISize;
+};
 
-    uint64_t size = vertexCount * sizeof(SkPoint);
-    if (builderFlags & SkVertices::kHasTexs_Flag) {
-        size += vertexCount * sizeof(SkPoint);
-    }
-    if (builderFlags & SkVertices::kHasColors_Flag) {
-        size += vertexCount * sizeof(SkColor);
-    }
-    size += indexCount * sizeof(uint16_t);
-    if (!sk_64_isS32(size)) {
-        return 0;   // signal error
-    }
-    return (size_t)size;
+SkVertices::Builder::Builder(SkCanvas::VertexMode mode, int vertexCount, int indexCount,
+                             uint32_t builderFlags) {
+    bool hasTexs = SkToBool(builderFlags & SkVertices::kHasTexCoords_BuilderFlag);
+    bool hasColors = SkToBool(builderFlags & SkVertices::kHasColors_BuilderFlag);
+    this->init(mode, vertexCount, indexCount,
+               SkVertices::Sizes(vertexCount, indexCount, hasTexs, hasColors));
 }
 
 SkVertices::Builder::Builder(SkCanvas::VertexMode mode, int vertexCount, int indexCount,
-                             uint32_t flags) {
-    fPositions = nullptr;   // signal that we have nothing to cleanup
-    fColors = nullptr;
-    fTexs = nullptr;
-    fIndices = nullptr;
-    fVertexCnt = 0;
-    fIndexCnt = 0;
-
-    size_t size = compute_arrays_size(vertexCount, indexCount, flags);
-    if (0 == size) {
-        return;
+                             const SkVertices::Sizes& sizes) {
+    this->init(mode, vertexCount, indexCount, sizes);
+}
+
+void SkVertices::Builder::init(SkCanvas::VertexMode mode, int vertexCount, int indexCount,
+                               const SkVertices::Sizes& sizes) {
+    if (!sizes.isValid()) {
+        return; // fVertices will already be null
     }
 
-    char* ptr = (char*)sk_malloc_throw(sk_64_asS32(size));
+    void* storage = ::operator new (sizes.fTotal);
+    fVertices.reset(new (storage) SkVertices);
 
-    fMode = mode;
-    fVertexCnt = vertexCount;
-    fIndexCnt = indexCount;
-    fPositions = (SkPoint*)ptr;  // owner
-    ptr += vertexCount * sizeof(SkPoint);
+    // need to point past the object to store the arrays
+    char* ptr = (char*)fVertices.get() + sizeof(SkVertices);
 
-    if (flags & kHasTexs_Flag) {
-        fTexs = (SkPoint*)ptr;
-        ptr += vertexCount * sizeof(SkPoint);
-    }
-    if (flags & kHasColors_Flag) {
-        fColors = (SkColor*)ptr;
-        ptr += vertexCount * sizeof(SkColor);
-    }
-    if (indexCount) {
-        fIndices = (uint16_t*)ptr;
+    fVertices->fPositions = (SkPoint*)ptr;                          ptr += sizes.fVSize;
+    fVertices->fTexs = sizes.fTSize ? (SkPoint*)ptr : nullptr;      ptr += sizes.fTSize;
+    fVertices->fColors = sizes.fCSize ? (SkColor*)ptr : nullptr;    ptr += sizes.fCSize;
+    fVertices->fIndices = sizes.fISize ? (uint16_t*)ptr : nullptr;
+    fVertices->fVertexCnt = vertexCount;
+    fVertices->fIndexCnt = indexCount;
+    fVertices->fMode = mode;
+    // We defer assigning fBounds and fUniqueID until detach() is called
+}
+
+sk_sp<SkVertices> SkVertices::Builder::detach() {
+    if (fVertices) {
+        fVertices->fBounds.set(fVertices->fPositions, fVertices->fVertexCnt);
+        fVertices->fUniqueID = next_id();
+        return std::move(fVertices);        // this will null fVertices after the return
     }
+    return nullptr;
 }
 
-SkVertices::Builder::~Builder() {
-    sk_free(fPositions);
+int SkVertices::Builder::vertexCount() const {
+    return fVertices ? fVertices->vertexCount() : 0;
 }
 
-sk_sp<SkVertices> SkVertices::Builder::detach() {
-    if (!fPositions) {
-        return nullptr;
-    }
+int SkVertices::Builder::indexCount() const {
+    return fVertices ? fVertices->indexCount() : 0;
+}
 
-    SkVertices* obj = new SkVertices;
-    obj->fPositions = fPositions;  // owner of storage, use sk_free
-    obj->fTexs = fTexs;
-    obj->fColors = fColors;
-    obj->fIndices = fIndices;
-    obj->fBounds.set(fPositions, fVertexCnt);
-    obj->fUniqueID = next_id();
-    obj->fVertexCnt = fVertexCnt;
-    obj->fIndexCnt = fIndexCnt;
-    obj->fMode = fMode;
+SkPoint* SkVertices::Builder::positions() {
+    return fVertices ? const_cast<SkPoint*>(fVertices->positions()) : nullptr;
+}
 
-    fPositions = nullptr;   // so we don't free the memory, now that obj owns it
+SkPoint* SkVertices::Builder::texCoords() {
+    return fVertices ? const_cast<SkPoint*>(fVertices->texCoords()) : nullptr;
+}
+
+SkColor* SkVertices::Builder::colors() {
+    return fVertices ? const_cast<SkColor*>(fVertices->colors()) : nullptr;
+}
 
-    return sk_sp<SkVertices>(obj);
+uint16_t* SkVertices::Builder::indices() {
+    return fVertices ? const_cast<uint16_t*>(fVertices->indices()) : nullptr;
 }
 
+///////////////////////////////////////////////////////////////////////////////////////////////////
+
 sk_sp<SkVertices> SkVertices::MakeCopy(SkCanvas::VertexMode mode, int vertexCount,
                                        const SkPoint pos[], const SkPoint texs[],
                                        const SkColor colors[], int indexCount,
                                        const uint16_t indices[]) {
-    uint32_t flags = 0;
-    if (texs) {
-        flags |= kHasTexs_Flag;
-    }
-    if (colors) {
-        flags |= kHasColors_Flag;
-    }
-    Builder builder(mode, vertexCount, indexCount, flags);
-    if (!builder.isValid()) {
+    Sizes sizes(vertexCount, indexCount, texs != nullptr, colors != nullptr);
+    if (!sizes.isValid()) {
         return nullptr;
     }
 
-    memcpy(builder.positions(), pos, vertexCount * sizeof(SkPoint));
-    if (texs) {
-        memcpy(builder.texCoords(), texs, vertexCount * sizeof(SkPoint));
-    }
-    if (colors) {
-        memcpy(builder.colors(), colors, vertexCount * sizeof(SkColor));
-    }
-    if (indices) {
-        memcpy(builder.indices(), indices, indexCount * sizeof(uint16_t));
-    }
+    Builder builder(mode, vertexCount, indexCount, sizes);
+    SkASSERT(builder.isValid());
+
+    sk_careful_memcpy(builder.positions(), pos, sizes.fVSize);
+    sk_careful_memcpy(builder.texCoords(), texs, sizes.fTSize);
+    sk_careful_memcpy(builder.colors(), colors, sizes.fCSize);
+    sk_careful_memcpy(builder.indices(), indices, sizes.fISize);
+
     return builder.detach();
 }
 
+size_t SkVertices::approximateSize() const {
+    Sizes sizes(fVertexCnt, fIndexCnt, this->hasTexCoords(), this->hasColors());
+    SkASSERT(sizes.isValid());
+    return sizeof(SkVertices) + sizes.fArrays;
+}
+
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
-// storage = flags | vertex_count | index_count | pos[] | texs[] | colors[] | indices[]
+// storage = packed | vertex_count | index_count | pos[] | texs[] | colors[] | indices[]
+//         = header + arrays
 
 #define kMode_Mask          0x0FF
 #define kHasTexs_Mask       0x100
 #define kHasColors_Mask     0x200
+#define kHeaderSize         (3 * sizeof(uint32_t))
 
 sk_sp<SkData> SkVertices::encode() const {
-    uint32_t flags = static_cast<uint32_t>(fMode);
-    SkASSERT((flags & ~kMode_Mask) == 0);
-    if (fTexs) {
-        flags |= kHasTexs_Mask;
+    // packed has room for addtional flags in the future (e.g. versioning)
+    uint32_t packed = static_cast<uint32_t>(fMode);
+    SkASSERT((packed & ~kMode_Mask) == 0);  // our mode fits in the mask bits
+    if (this->hasTexCoords()) {
+        packed |= kHasTexs_Mask;
     }
-    if (fColors) {
-        flags |= kHasColors_Mask;
+    if (this->hasColors()) {
+        packed |= kHasColors_Mask;
     }
 
-    size_t size = sizeof(uint32_t) * 3; // flags | verts_count | indices_count
-    size += fVertexCnt * sizeof(SkPoint);
-    if (fTexs) {
-        size += fVertexCnt * sizeof(SkPoint);
-    }
-    if (fColors) {
-        size += fVertexCnt * sizeof(SkColor);
-    }
-    size += fIndexCnt * sizeof(uint16_t);
+    Sizes sizes(fVertexCnt, fIndexCnt, this->hasTexCoords(), this->hasColors());
+    SkASSERT(sizes.isValid());
+    const size_t size = kHeaderSize + sizes.fArrays;
 
     sk_sp<SkData> data = SkData::MakeUninitialized(size);
     SkWriter32 writer(data->writable_data(), data->size());
 
-    writer.write32(flags);
+    writer.write32(packed);
     writer.write32(fVertexCnt);
     writer.write32(fIndexCnt);
-    writer.write(fPositions, fVertexCnt * sizeof(SkPoint));
-    if (fTexs) {
-        writer.write(fTexs, fVertexCnt * sizeof(SkPoint));
-    }
-    if (fColors) {
-        writer.write(fColors, fVertexCnt * sizeof(SkColor));
-    }
-    writer.write(fIndices, fIndexCnt * sizeof(uint16_t));
+    writer.write(fPositions, sizes.fVSize);
+    writer.write(fTexs, sizes.fTSize);
+    writer.write(fColors, sizes.fCSize);
+    writer.write(fIndices, sizes.fISize);
 
     return data;
 }
 
 sk_sp<SkVertices> SkVertices::Decode(const void* data, size_t length) {
-    if (length < 3 * sizeof(uint32_t)) {
-        return nullptr; // buffer too small
+    if (length < kHeaderSize) {
+        return nullptr;
     }
 
     SkReader32 reader(data, length);
 
-    uint32_t storageFlags = reader.readInt();
-    SkCanvas::VertexMode mode = static_cast<SkCanvas::VertexMode>(storageFlags & kMode_Mask);
-    int vertexCount = reader.readInt();
-    int indexCount = reader.readInt();
-    uint32_t builderFlags = 0;
-    if (storageFlags & kHasTexs_Mask) {
-        builderFlags |= SkVertices::kHasTexs_Flag;
-    }
-    if (storageFlags & kHasColors_Mask) {
-        builderFlags |= SkVertices::kHasColors_Flag;
-    }
+    const uint32_t packed = reader.readInt();
+    const int vertexCount = reader.readInt();
+    const int indexCount = reader.readInt();
 
-    size_t size = compute_arrays_size(vertexCount, indexCount, builderFlags);
-    if (0 == size) {
+    const SkCanvas::VertexMode mode = static_cast<SkCanvas::VertexMode>(packed & kMode_Mask);
+    const bool hasTexs = SkToBool(packed & kHasTexs_Mask);
+    const bool hasColors = SkToBool(packed & kHasColors_Mask);
+    Sizes sizes(vertexCount, indexCount, hasTexs, hasColors);
+    if (!sizes.isValid()) {
         return nullptr;
     }
-
-    length -= 3 * sizeof(uint32_t); // already read the header
-    if (length < size) {    // buffer too small
+    if (kHeaderSize + sizes.fArrays != length) {
         return nullptr;
     }
 
-    Builder builder(mode, vertexCount, indexCount, builderFlags);
-    if (!builder.isValid()) {
-        return nullptr;
-    }
+    Builder builder(mode, vertexCount, indexCount, sizes);
 
-    reader.read(builder.positions(), vertexCount * sizeof(SkPoint));
-    if (builderFlags & SkVertices::kHasTexs_Flag) {
-        reader.read(builder.texCoords(), vertexCount * sizeof(SkPoint));
-    }
-    if (builderFlags & SkVertices::kHasColors_Flag) {
-        reader.read(builder.colors(), vertexCount * sizeof(SkColor));
-    }
-    reader.read(builder.indices(), indexCount * sizeof(uint16_t));
+    reader.read(builder.positions(), sizes.fVSize);
+    reader.read(builder.texCoords(), sizes.fTSize);
+    reader.read(builder.colors(), sizes.fCSize);
+    reader.read(builder.indices(), sizes.fISize);
     
     return builder.detach();
 }
diff --git a/src/gpu/ops/GrDrawVerticesOp.cpp b/src/gpu/ops/GrDrawVerticesOp.cpp
index 5d8837880f..aa13cae816 100644
--- a/src/gpu/ops/GrDrawVerticesOp.cpp
+++ b/src/gpu/ops/GrDrawVerticesOp.cpp
@@ -257,7 +257,7 @@ bool GrDrawVerticesOp::onCombineIfPossible(GrOp* t, const GrCaps& caps) {
         return false;
     }
 
-    if (fMeshes[0].fVertices->isIndexed() != that->fMeshes[0].fVertices->isIndexed()) {
+    if (fMeshes[0].fVertices->hasIndices() != that->fMeshes[0].fVertices->hasIndices()) {
         return false;
     }
 
diff --git a/src/gpu/ops/GrDrawVerticesOp.h b/src/gpu/ops/GrDrawVerticesOp.h
index c837734798..eddf20185b 100644
--- a/src/gpu/ops/GrDrawVerticesOp.h
+++ b/src/gpu/ops/GrDrawVerticesOp.h
@@ -97,7 +97,7 @@ private:
 
     bool isIndexed() const {
         // Consistency enforced in onCombineIfPossible.
-        return fMeshes[0].fVertices->isIndexed();
+        return fMeshes[0].fVertices->hasIndices();
     }
 
     bool requiresPerVertexColors() const {
diff --git a/src/utils/SkShadowUtils.cpp b/src/utils/SkShadowUtils.cpp
index eb015aa8f9..402dca5c92 100644
--- a/src/utils/SkShadowUtils.cpp
+++ b/src/utils/SkShadowUtils.cpp
@@ -232,12 +232,12 @@ private:
                 i = fCount++;
             } else {
                 i = gRandom.nextULessThan(MAX_ENTRIES);
-                fSize -= fEntries[i].fVertices->size();
+                fSize -= fEntries[i].fVertices->approximateSize();
             }
             fEntries[i].fFactory = factory;
             fEntries[i].fVertices = vertices;
             fEntries[i].fMatrix = matrix;
-            fSize += vertices->size();
+            fSize += vertices->approximateSize();
             return vertices;
         }
 
diff --git a/tests/VerticesTest.cpp b/tests/VerticesTest.cpp
index 8cf55142f3..9621d80cd0 100644
--- a/tests/VerticesTest.cpp
+++ b/tests/VerticesTest.cpp
@@ -53,8 +53,8 @@ DEF_TEST(Vertices, reporter) {
     int vCount = 4;
     int iCount = 6;
 
-    const uint32_t texFlags[] = { 0, SkVertices::kHasTexs_Flag };
-    const uint32_t colFlags[] = { 0, SkVertices::kHasColors_Flag };
+    const uint32_t texFlags[] = { 0, SkVertices::kHasTexCoords_BuilderFlag };
+    const uint32_t colFlags[] = { 0, SkVertices::kHasColors_BuilderFlag };
     for (auto texF : texFlags) {
         for (auto colF : colFlags) {
             uint32_t flags = texF | colF;