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>
diff --git a/gm/vertices.cpp b/gm/vertices.cpp
index 864ac3d..0d79827 100644
--- a/gm/vertices.cpp
+++ b/gm/vertices.cpp
@@ -214,7 +214,8 @@
// 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 af4e3bc..8d6f211 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 @@
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;
};
- SkCanvas::VertexMode mode() const { return fMode; }
-
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()); }
+
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 936d70d..1980b7e 100644
--- a/src/core/SkVertices.cpp
+++ b/src/core/SkVertices.cpp
@@ -20,202 +20,199 @@
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
+ }
}
- 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;
+ 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;
+};
+
+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;
- }
-
- char* ptr = (char*)sk_malloc_throw(sk_64_asS32(size));
-
- fMode = mode;
- fVertexCnt = vertexCount;
- fIndexCnt = indexCount;
- fPositions = (SkPoint*)ptr; // owner
- ptr += vertexCount * sizeof(SkPoint);
-
- 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;
- }
+ const SkVertices::Sizes& sizes) {
+ this->init(mode, vertexCount, indexCount, sizes);
}
-SkVertices::Builder::~Builder() {
- sk_free(fPositions);
+void SkVertices::Builder::init(SkCanvas::VertexMode mode, int vertexCount, int indexCount,
+ const SkVertices::Sizes& sizes) {
+ if (!sizes.isValid()) {
+ return; // fVertices will already be null
+ }
+
+ void* storage = ::operator new (sizes.fTotal);
+ fVertices.reset(new (storage) SkVertices);
+
+ // need to point past the object to store the arrays
+ char* ptr = (char*)fVertices.get() + sizeof(SkVertices);
+
+ 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 (!fPositions) {
- return nullptr;
+ if (fVertices) {
+ fVertices->fBounds.set(fVertices->fPositions, fVertices->fVertexCnt);
+ fVertices->fUniqueID = next_id();
+ return std::move(fVertices); // this will null fVertices after the return
}
-
- 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;
-
- fPositions = nullptr; // so we don't free the memory, now that obj owns it
-
- return sk_sp<SkVertices>(obj);
+ return nullptr;
}
+int SkVertices::Builder::vertexCount() const {
+ return fVertices ? fVertices->vertexCount() : 0;
+}
+
+int SkVertices::Builder::indexCount() const {
+ return fVertices ? fVertices->indexCount() : 0;
+}
+
+SkPoint* SkVertices::Builder::positions() {
+ return fVertices ? const_cast<SkPoint*>(fVertices->positions()) : nullptr;
+}
+
+SkPoint* SkVertices::Builder::texCoords() {
+ return fVertices ? const_cast<SkPoint*>(fVertices->texCoords()) : nullptr;
+}
+
+SkColor* SkVertices::Builder::colors() {
+ return fVertices ? const_cast<SkColor*>(fVertices->colors()) : nullptr;
+}
+
+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;
+ }
+ if (kHeaderSize + sizes.fArrays != length) {
return nullptr;
}
- length -= 3 * sizeof(uint32_t); // already read the header
- if (length < size) { // buffer too small
- return nullptr;
- }
+ Builder builder(mode, vertexCount, indexCount, sizes);
- Builder builder(mode, vertexCount, indexCount, builderFlags);
- if (!builder.isValid()) {
- return nullptr;
- }
-
- 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 5d88378..aa13cae 100644
--- a/src/gpu/ops/GrDrawVerticesOp.cpp
+++ b/src/gpu/ops/GrDrawVerticesOp.cpp
@@ -257,7 +257,7 @@
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 c837734..eddf201 100644
--- a/src/gpu/ops/GrDrawVerticesOp.h
+++ b/src/gpu/ops/GrDrawVerticesOp.h
@@ -97,7 +97,7 @@
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 eb015aa..402dca5 100644
--- a/src/utils/SkShadowUtils.cpp
+++ b/src/utils/SkShadowUtils.cpp
@@ -232,12 +232,12 @@
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 8cf5514..9621d80 100644
--- a/tests/VerticesTest.cpp
+++ b/tests/VerticesTest.cpp
@@ -53,8 +53,8 @@
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;