diff options
author | Mike Reed <reed@google.com> | 2017-03-16 14:38:48 -0400 |
---|---|---|
committer | Skia Commit-Bot <skia-commit-bot@chromium.org> | 2017-03-16 19:17:11 +0000 |
commit | aa9e3326f71c74e6250ae783cc3257d835624dd0 (patch) | |
tree | beaf54d29c342a15c194dd4ff23296088dbdf4f1 | |
parent | 2bf4b3a97b770811d9e0558dbbfbdb57cfafbdb7 (diff) |
Revert[4] "store vertices arrays inline with object""""
This reverts commit 0c492cfe1713d6895d1d513e754d938ff0faa5e5.
BUG=skia:
Change-Id: I63bce834fee6dd6f043b3889ac4ec287dd03d2e6
Reviewed-on: https://skia-review.googlesource.com/9809
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Reed <reed@google.com>
-rw-r--r-- | gm/vertices.cpp | 3 | ||||
-rw-r--r-- | include/core/SkVertices.h | 101 | ||||
-rw-r--r-- | src/core/SkVertices.cpp | 267 | ||||
-rw-r--r-- | src/gpu/ops/GrDrawVerticesOp.cpp | 2 | ||||
-rw-r--r-- | src/gpu/ops/GrDrawVerticesOp.h | 2 | ||||
-rw-r--r-- | src/utils/SkShadowUtils.cpp | 4 | ||||
-rw-r--r-- | tests/VerticesTest.cpp | 4 |
7 files changed, 201 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..e4b5bcb80b 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,97 @@ 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; + // these are needed since we've manually sized our allocation (see Builder::init) + friend class SkNVRefCnt<SkVertices>; + void operator delete(void* p) { ::operator delete(p); } + + 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..297b424e77 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*)storage + 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; |