From 548de7451e752ce0d6fd842c1bb4f04af4c0afdc Mon Sep 17 00:00:00 2001 From: Brian Osman Date: Fri, 24 Apr 2020 12:02:25 -0400 Subject: [PATCH] Change Marker IDs to be strings They are hashed to uint32_t at the API boundary (SkCanvas, SkVertices), but making them functionally strings will make the SkSL interaction much nicer. Change-Id: I0979871bf3d21373812129eb7e994987b3030e00 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/285664 Commit-Queue: Brian Osman Reviewed-by: Brian Salomon --- gm/vertices.cpp | 15 ++-- include/core/SkCanvas.h | 20 ++--- include/core/SkVertices.h | 29 ++++--- include/utils/SkNWayCanvas.h | 2 +- samplecode/Sample3D.cpp | 8 +- src/core/SkCanvas.cpp | 18 ++--- src/core/SkPicturePlayback.cpp | 5 +- src/core/SkPictureRecord.cpp | 9 ++- src/core/SkPictureRecord.h | 2 +- src/core/SkRecordDraw.cpp | 2 +- src/core/SkRecorder.cpp | 4 +- src/core/SkRecorder.h | 2 +- src/core/SkRecords.h | 2 +- src/core/SkVertices.cpp | 131 +++++++++++++++++++++++-------- src/gpu/ops/GrDrawVerticesOp.cpp | 27 ++++--- src/utils/SkNWayCanvas.cpp | 6 +- tests/CanvasTest.cpp | 4 +- 17 files changed, 178 insertions(+), 108 deletions(-) diff --git a/gm/vertices.cpp b/gm/vertices.cpp index 268c40c87a..683ca464a4 100644 --- a/gm/vertices.cpp +++ b/gm/vertices.cpp @@ -517,8 +517,8 @@ DEF_SIMPLE_GM(vertices_custom_colors, canvas, 400, 200) { } } -static sk_sp make_cone(Attr::Usage u, uint32_t id) { - Attr attr(Attr::Type::kFloat3, u, id); +static sk_sp make_cone(Attr::Usage u, const char* markerName) { + Attr attr(Attr::Type::kFloat3, u, markerName); constexpr int kPerimeterVerts = 64; // +1 for the center, +1 to repeat the first perimeter point (so we draw a complete circle) @@ -547,12 +547,9 @@ static sk_sp make_cone(Attr::Usage u, uint32_t id) { DEF_SIMPLE_GM(vertices_custom_matrices, canvas, 400, 300) { ToolUtils::draw_checkerboard(canvas); - enum MatrixMarkers { - kDeviceSpace = 0, - kViewSpace, - kWorldSpace, - kLocalSpace, - }; + const char* kViewSpace = "local_to_view"; + const char* kWorldSpace = "local_to_world"; + const char* kLocalSpace = "local_to_local"; auto draw = [=](SkScalar cx, SkScalar cy, sk_sp vertices, const char* prog, SkScalar squish = 1.0f) { @@ -588,7 +585,7 @@ DEF_SIMPLE_GM(vertices_custom_matrices, canvas, 400, 300) { })"; // raw, local vectors, normals, and positions should all look the same (no real transform) - draw(50, 50, make_cone(Attr::Usage::kRaw, 0), vectorProg); + draw(50, 50, make_cone(Attr::Usage::kRaw, nullptr), vectorProg); draw(150, 50, make_cone(Attr::Usage::kVector, kLocalSpace), vectorProg); draw(250, 50, make_cone(Attr::Usage::kNormalVector, kLocalSpace), vectorProg); draw(350, 50, make_cone(Attr::Usage::kPosition, kLocalSpace), vectorProg); diff --git a/include/core/SkCanvas.h b/include/core/SkCanvas.h index afd07c3c3c..9489154f14 100644 --- a/include/core/SkCanvas.h +++ b/include/core/SkCanvas.h @@ -880,23 +880,19 @@ public: void concat(const SkMatrix& matrix); void concat(const SkM44&); - typedef uint32_t MarkerID; - /** - * Record a marker (provided by caller) for the current CTM. This does not - * change anything about the ctm or clip, but does "note" this matrix value, so it can - * be referenced by custom effects (who access it by specifying the same id). + * Record a marker (provided by caller) for the current CTM. This does not change anything + * about the ctm or clip, but does "name" this matrix value, so it can be referenced by + * custom effects (who access it by specifying the same name). * - * Within a save frame, marking with the same id more than once just replaces the previous - * value. However, between save frames, marking with the same id does not lose the marker + * Within a save frame, marking with the same name more than once just replaces the previous + * value. However, between save frames, marking with the same name does not lose the marker * in the previous save frame. It is "visible" when the current save() is balanced with * a restore(). - * - * NOTE: id==0 is reserved. */ - void markCTM(MarkerID id); + void markCTM(const char* name); - bool findMarkedCTM(MarkerID id, SkM44*) const; + bool findMarkedCTM(const char* name, SkM44*) const; /** Replaces SkMatrix with matrix. Unlike concat(), any prior matrix state is overwritten. @@ -2500,7 +2496,7 @@ protected: virtual void willRestore() {} virtual void didRestore() {} - virtual void onMarkCTM(MarkerID) {} + virtual void onMarkCTM(const char*) {} virtual void didConcat44(const SkM44&) {} virtual void didConcat(const SkMatrix& ) {} virtual void didSetMatrix(const SkMatrix& ) {} diff --git a/include/core/SkVertices.h b/include/core/SkVertices.h index 957c9ae7ce..20a0fa811e 100644 --- a/include/core/SkVertices.h +++ b/include/core/SkVertices.h @@ -57,6 +57,14 @@ public: static constexpr int kMaxCustomAttributes = 8; + /** + * EXPERIMENTAL - An SkVertices object can be constructed with a custom collection of vertex + * attributes. Each attribute is described by a single Attribute struct. Type defines the CPU + * type of the data. Usage determines what transformation (if any) is applied to that data in + * the vertex shader. For positions or vectors, markerName identifies what matrix is used in + * the vertex shader to transform the data. Those names should match a named transform on the + * CTM stack, created by calling SkCanvas::markCTM(). + */ struct Attribute { enum class Type : uint8_t { kFloat, @@ -84,10 +92,11 @@ public: kPosition, }; - Attribute(Type t = Type::kFloat, Usage u = Usage::kRaw, uint32_t id = 0) - : fType(t) - , fUsage(u) - , fMarkerID(id) {} + /** + * markerName is not copied by the Attribute, so it must outlive this struct. + * It is copied when this Attribute is passed to the Builder constructor. + */ + Attribute(Type t = Type::kFloat, Usage u = Usage::kRaw, const char* markerName = nullptr); bool operator==(const Attribute& that) const { return fType == that.fType && fUsage == that.fUsage && fMarkerID == that.fMarkerID; @@ -101,9 +110,10 @@ public: size_t bytesPerVertex() const; bool isValid() const; - Type fType; - Usage fUsage; - uint32_t fMarkerID; + Type fType; + Usage fUsage; + uint32_t fMarkerID; + const char* fMarkerName; // Preserved for serialization and debugging }; enum BuilderFlags { @@ -188,6 +198,7 @@ private: uint32_t fUniqueID; // these point inside our allocation, so none of these can be "freed" + Attribute* fAttributes; // [attributeCount] or null SkPoint* fPositions; // [vertexCount] uint16_t* fIndices; // [indexCount] or null void* fCustomData; // [customDataSize * vertexCount] or null @@ -197,9 +208,7 @@ private: SkRect fBounds; // computed to be the union of the fPositions[] int fVertexCount; int fIndexCount; - - Attribute fAttributes[kMaxCustomAttributes]; - int fAttributeCount; + int fAttributeCount; VertexMode fMode; // below here is where the actual array data is stored. diff --git a/include/utils/SkNWayCanvas.h b/include/utils/SkNWayCanvas.h index 8d5b7ad0c0..5c85c024e1 100644 --- a/include/utils/SkNWayCanvas.h +++ b/include/utils/SkNWayCanvas.h @@ -30,7 +30,7 @@ protected: bool onDoSaveBehind(const SkRect*) override; void willRestore() override; - void onMarkCTM(MarkerID) override; + void onMarkCTM(const char*) override; void didConcat44(const SkM44&) override; void didConcat(const SkMatrix&) override; void didSetMatrix(const SkMatrix&) override; diff --git a/samplecode/Sample3D.cpp b/samplecode/Sample3D.cpp index a323209f01..402ec68ab4 100644 --- a/samplecode/Sample3D.cpp +++ b/samplecode/Sample3D.cpp @@ -82,9 +82,7 @@ protected: SkV3 fCOA { 0, 0, 0 }; SkV3 fUp { 0, 1, 0 }; - enum { - kWorldID = 42, - }; + const char* kLocalToWorld = "local_to_world"; public: void concatCamera(SkCanvas* canvas, const SkRect& area, SkScalar zscale) { @@ -98,7 +96,7 @@ public: SkM44 localToWorld(SkCanvas* canvas) { SkM44 worldToDevice; - SkAssertResult(canvas->findMarkedCTM(kWorldID, &worldToDevice)); + SkAssertResult(canvas->findMarkedCTM(kLocalToWorld, &worldToDevice)); return inv(worldToDevice) * canvas->getLocalToDevice(); } }; @@ -289,7 +287,7 @@ public: canvas->concat(trans); // "World" space - content is centered at the origin, in device scale (+-200) - canvas->markCTM(kWorldID); + canvas->markCTM(kLocalToWorld); canvas->concat(m * inv(trans)); this->drawContent(canvas, f.fColor, index++, drawFront); diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp index 68f92db040..7352117294 100644 --- a/src/core/SkCanvas.cpp +++ b/src/core/SkCanvas.cpp @@ -1541,16 +1541,16 @@ void SkCanvas::resetMatrix() { this->setMatrix(SkMatrix::I()); } -void SkCanvas::markCTM(MarkerID id) { - if (id == 0) return; - - this->onMarkCTM(id); - - fMarkerStack->setMarker(id, this->getLocalToDevice(), fMCRec); +void SkCanvas::markCTM(const char* name) { + if (name && name[0]) { + fMarkerStack->setMarker(SkOpts::hash_fn(name, strlen(name), 0), + this->getLocalToDevice(), fMCRec); + this->onMarkCTM(name); + } } -bool SkCanvas::findMarkedCTM(MarkerID id, SkM44* mx) const { - return fMarkerStack->findMarker(id, mx); +bool SkCanvas::findMarkedCTM(const char* name, SkM44* mx) const { + return name && name[0] && fMarkerStack->findMarker(SkOpts::hash_fn(name, strlen(name), 0), mx); } ////////////////////////////////////////////////////////////////////////////// @@ -1963,7 +1963,7 @@ void SkCanvas::drawVertices(const SkVertices* vertices, SkBlendMode mode, const return; } // If we can't provide any of the asked-for matrices, we can't draw this - if (attr.fMarkerID && !this->findMarkedCTM(attr.fMarkerID, nullptr)) { + if (attr.fMarkerID && !fMarkerStack->findMarker(attr.fMarkerID, nullptr)) { return; } } diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp index 9264c6040d..07ea8cd3bf 100644 --- a/src/core/SkPicturePlayback.cpp +++ b/src/core/SkPicturePlayback.cpp @@ -566,9 +566,10 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader, } } break; case MARK_CTM: { - uint32_t id = reader->readInt(); + SkString name; + reader->readString(&name); BREAK_ON_READ_ERROR(reader); - canvas->markCTM(id); + canvas->markCTM(name.c_str()); } break; case RESTORE: canvas->restore(); diff --git a/src/core/SkPictureRecord.cpp b/src/core/SkPictureRecord.cpp index e0a59b942c..a9e053eb4a 100644 --- a/src/core/SkPictureRecord.cpp +++ b/src/core/SkPictureRecord.cpp @@ -63,13 +63,14 @@ void SkPictureRecord::recordSave() { this->validate(initialOffset, size); } -void SkPictureRecord::onMarkCTM(MarkerID id) { - size_t size = sizeof(kUInt32Size) + sizeof(uint32_t); // op + id +void SkPictureRecord::onMarkCTM(const char* name) { + size_t nameLen = fWriter.WriteStringSize(name); + size_t size = sizeof(kUInt32Size) + nameLen; // op + name size_t initialOffset = this->addDraw(MARK_CTM, &size); - fWriter.write32(id); + fWriter.writeString(name); this->validate(initialOffset, size); - this->INHERITED::onMarkCTM(id); + this->INHERITED::onMarkCTM(name); } SkCanvas::SaveLayerStrategy SkPictureRecord::getSaveLayerStrategy(const SaveLayerRec& rec) { diff --git a/src/core/SkPictureRecord.h b/src/core/SkPictureRecord.h index b7c6cb858a..1e22021585 100644 --- a/src/core/SkPictureRecord.h +++ b/src/core/SkPictureRecord.h @@ -162,7 +162,7 @@ protected: bool onDoSaveBehind(const SkRect*) override; void willRestore() override; - void onMarkCTM(MarkerID) override; + void onMarkCTM(const char*) override; void didConcat44(const SkM44&) override; void didConcat(const SkMatrix&) override; void didSetMatrix(const SkMatrix&) override; diff --git a/src/core/SkRecordDraw.cpp b/src/core/SkRecordDraw.cpp index bf8e97735b..a8d88dbc98 100644 --- a/src/core/SkRecordDraw.cpp +++ b/src/core/SkRecordDraw.cpp @@ -93,7 +93,7 @@ template <> void Draw::draw(const DrawBehind& r) { SkCanvasPriv::DrawBehind(fCanvas, r.paint); } -DRAW(MarkCTM, markCTM(r.id)); +DRAW(MarkCTM, markCTM(r.name.c_str())); DRAW(SetMatrix, setMatrix(SkMatrix::Concat(fInitialCTM, r.matrix))); DRAW(Concat44, concat(r.matrix)); DRAW(Concat, concat(r.matrix)); diff --git a/src/core/SkRecorder.cpp b/src/core/SkRecorder.cpp index 5ca83c693d..b11bd1ae4b 100644 --- a/src/core/SkRecorder.cpp +++ b/src/core/SkRecorder.cpp @@ -331,8 +331,8 @@ void SkRecorder::didRestore() { this->append(this->getTotalMatrix()); } -void SkRecorder::onMarkCTM(MarkerID id) { - this->append(id); +void SkRecorder::onMarkCTM(const char* name) { + this->append(SkString(name)); } void SkRecorder::didConcat44(const SkM44& m) { diff --git a/src/core/SkRecorder.h b/src/core/SkRecorder.h index 6a7969ea18..96e77c50e3 100644 --- a/src/core/SkRecorder.h +++ b/src/core/SkRecorder.h @@ -67,7 +67,7 @@ public: void willRestore() override {} void didRestore() override; - void onMarkCTM(MarkerID) override; + void onMarkCTM(const char*) override; void didConcat44(const SkM44&) override; void didConcat(const SkMatrix&) override; void didSetMatrix(const SkMatrix&) override; diff --git a/src/core/SkRecords.h b/src/core/SkRecords.h index c8b5442b08..8d89252e4b 100644 --- a/src/core/SkRecords.h +++ b/src/core/SkRecords.h @@ -193,7 +193,7 @@ RECORD(SaveBehind, 0, Optional subset); RECORD(MarkCTM, 0, - uint32_t id); + SkString name); RECORD(SetMatrix, 0, TypedMatrix matrix); RECORD(Concat, 0, diff --git a/src/core/SkVertices.cpp b/src/core/SkVertices.cpp index 08db370627..2d6992c593 100644 --- a/src/core/SkVertices.cpp +++ b/src/core/SkVertices.cpp @@ -9,6 +9,7 @@ #include "include/core/SkData.h" #include "include/private/SkTo.h" +#include "src/core/SkOpts.h" #include "src/core/SkReader32.h" #include "src/core/SkSafeMath.h" #include "src/core/SkSafeRange.h" @@ -27,6 +28,14 @@ static int32_t next_id() { return id; } +SkVertices::Attribute::Attribute(Type t, Usage u, const char* markerName) + : fType(t) + , fUsage(u) + , fMarkerName((markerName && markerName[0]) ? markerName : nullptr) { + fMarkerID = fMarkerName ? SkOpts::hash_fn(fMarkerName, strlen(fMarkerName), 0) : 0; + SkASSERT(!fMarkerName || fMarkerID != 0); +} + int SkVertices::Attribute::channelCount() const { SkASSERT(this->isValid()); switch (fUsage) { @@ -99,14 +108,21 @@ struct SkVertices::Sizes { Sizes(const Desc& desc) { desc.validate(); - for (int i = 0; i < desc.fAttributeCount; ++i) { - if (!desc.fAttributes[i].isValid()) { - return; - } - } - SkSafeMath safe; + fNameSize = 0; + for (int i = 0; i < desc.fAttributeCount; ++i) { + const Attribute& attr(desc.fAttributes[i]); + if (!attr.isValid()) { + return; + } + if (attr.fMarkerName) { + fNameSize = safe.add(fNameSize, strlen(attr.fMarkerName)); + } + } + fNameSize = SkAlign4(fNameSize); + + fAttrSize = safe.mul(desc.fAttributeCount, sizeof(Attribute)); fVSize = safe.mul(desc.fVertexCount, sizeof(SkPoint)); fDSize = safe.mul(custom_data_size(desc.fAttributes, desc.fAttributeCount), desc.fVertexCount); @@ -137,14 +153,16 @@ struct SkVertices::Sizes { } fTotal = safe.add(sizeof(SkVertices), + safe.add(fAttrSize, + safe.add(fNameSize, safe.add(fVSize, safe.add(fDSize, safe.add(fTSize, safe.add(fCSize, - fISize))))); + fISize))))))); if (safe.ok()) { - fArrays = fTotal - sizeof(SkVertices); // just the sum of the arrays + fArrays = fVSize + fDSize + fTSize + fCSize + fISize; // just the sum of the arrays } else { sk_bzero(this, sizeof(*this)); } @@ -153,7 +171,9 @@ struct SkVertices::Sizes { 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 + D + T + C + I) + size_t fAttrSize; // size of attributes + size_t fNameSize; // size of attribute marker names + size_t fArrays; // size of all the data arrays (V + D + T + C + I) size_t fVSize; size_t fDSize; // size of all customData = [customDataSize * fVertexCount] size_t fTSize; @@ -211,20 +231,32 @@ void SkVertices::Builder::init(const Desc& desc) { return new_ptr; }; - fVertices->fPositions = (SkPoint*) advance(sizes.fVSize); - fVertices->fCustomData = (void*) advance(sizes.fDSize); - fVertices->fTexs = (SkPoint*) advance(sizes.fTSize); - fVertices->fColors = (SkColor*) advance(sizes.fCSize); - fVertices->fIndices = (uint16_t*)advance(sizes.fISize); - - fVertices->fVertexCount = desc.fVertexCount; - fVertices->fIndexCount = desc.fIndexCount; + fVertices->fAttributes = (Attribute*)advance(sizes.fAttrSize); + char* markerNames = advance(sizes.fNameSize); + // Copy the attributes into our block of memory (immediately after the SkVertices) sk_careful_memcpy(fVertices->fAttributes, desc.fAttributes, desc.fAttributeCount * sizeof(Attribute)); - fVertices->fAttributeCount = desc.fAttributeCount; - fVertices->fMode = desc.fMode; + // Now copy the marker names, and fix up the pointers in our attributes + for (int i = 0; i < desc.fAttributeCount; ++i) { + Attribute& attr(fVertices->fAttributes[i]); + if (attr.fMarkerName) { + attr.fMarkerName = strcpy(markerNames, attr.fMarkerName); + markerNames += (strlen(markerNames) + 1); + } + } + + fVertices->fPositions = (SkPoint*) advance(sizes.fVSize); + fVertices->fCustomData = (void*) advance(sizes.fDSize); + fVertices->fTexs = (SkPoint*) advance(sizes.fTSize); + fVertices->fColors = (SkColor*) advance(sizes.fCSize); + fVertices->fIndices = (uint16_t*)advance(sizes.fISize); + + fVertices->fVertexCount = desc.fVertexCount; + fVertices->fIndexCount = desc.fIndexCount; + fVertices->fAttributeCount = desc.fAttributeCount; + fVertices->fMode = desc.fMode; // We defer assigning fBounds and fUniqueID until detach() is called } @@ -309,7 +341,7 @@ sk_sp SkVertices::MakeCopy(VertexMode mode, int vertexCount, } size_t SkVertices::approximateSize() const { - return sizeof(SkVertices) + this->getSizes().fArrays; + return this->getSizes().fTotal; } SkVertices::Sizes SkVertices::getSizes() const { @@ -335,21 +367,26 @@ bool SkVerticesPriv::hasUsage(SkVertices::Attribute::Usage u) const { /////////////////////////////////////////////////////////////////////////////////////////////////// enum EncodedVerticesVersions { - kAttribute_Version = 3, // Adds array of custom attribute types + kNamedMarkers_Version = 4, // Marker IDs changed to strings - kCurrent_Version = kAttribute_Version + kCurrent_Version = kNamedMarkers_Version }; -struct Header_v3 { +struct EncodedAttribute { + SkVertices::Attribute::Type fType; + SkVertices::Attribute::Usage fUsage; + bool fHasMarkerName; +}; + +struct Header_v4 { uint32_t fPacked; int32_t fVertexCount; int32_t fIndexCount; int32_t fAttributeCount; - SkVertices::Attribute fAttributes[SkVertices::kMaxCustomAttributes]; - // [pos] + [customData] + [texs] + [colors] + [indices] + // [EncodedAttributes] + [MarkerNames] + [pos] + [customData] + [texs] + [colors] + [indices] }; -#define kCurrentHeaderSize sizeof(Header_v3) +#define kCurrentHeaderSize sizeof(Header_v4) // storage = packed | vertex_count | index_count | attr_count // | pos[] | custom[] | texs[] | colors[] | indices[] @@ -373,22 +410,43 @@ sk_sp SkVertices::encode() const { } packed |= kCurrent_Version << kVersion_Shift; + size_t attrSize = SkAlign4(sizeof(EncodedAttribute) * fAttributeCount); + for (int i = 0; i < fAttributeCount; ++i) { + if (fAttributes[i].fMarkerName) { + attrSize += SkWriter32::WriteStringSize(fAttributes[i].fMarkerName); + } + } + Sizes sizes = this->getSizes(); SkASSERT(!sizes.fBuilderTriFanISize); // need to force alignment to 4 for SkWriter32 -- will pad w/ 0s as needed - const size_t size = SkAlign4(kCurrentHeaderSize + sizes.fArrays); + const size_t size = SkAlign4(kCurrentHeaderSize + attrSize + sizes.fArrays); sk_sp data = SkData::MakeUninitialized(size); SkWriter32 writer(data->writable_data(), data->size()); + // Header writer.write32(packed); writer.write32(fVertexCount); writer.write32(fIndexCount); writer.write32(fAttributeCount); - // Write all custom attribute slots to simplify alignment - writer.write(fAttributes, sizeof(fAttributes)); + // Encoded attributes (may not be 4 byte aligned) + EncodedAttribute* encodedAttrs = + (EncodedAttribute*)writer.reservePad(fAttributeCount * sizeof(EncodedAttribute)); + for (int i = 0; i < fAttributeCount; ++i) { + encodedAttrs[i] = {fAttributes[i].fType, fAttributes[i].fUsage, + SkToBool(fAttributes[i].fMarkerName)}; + } + // Marker names + for (int i = 0; i < fAttributeCount; ++i) { + if (fAttributes[i].fMarkerName) { + writer.writeString(fAttributes[i].fMarkerName); + } + } + + // Data arrays writer.write(fPositions, sizes.fVSize); writer.write(fCustomData, sizes.fDSize); writer.write(fTexs, sizes.fTSize); @@ -400,7 +458,7 @@ sk_sp SkVertices::encode() const { } sk_sp SkVertices::Decode(const void* data, size_t length) { - if (length < sizeof(Header_v3)) { + if (length < sizeof(Header_v4)) { return nullptr; } @@ -420,13 +478,22 @@ sk_sp SkVertices::Decode(const void* data, size_t length) { if (!safe // Invalid header fields || attrCount > kMaxCustomAttributes // Too many custom attributes? - || version < kAttribute_Version // Old (unsupported) version + || version < kNamedMarkers_Version // Old (unsupported) version || (attrCount > 0 && (hasTexs || hasColors))) { // Overspecified (incompatible features) return nullptr; } + if (!reader.isAvailable(attrCount * sizeof(EncodedAttribute))) { + return nullptr; + } + Attribute attrs[kMaxCustomAttributes]; - reader.read(attrs, sizeof(attrs)); + const EncodedAttribute* encodedAttrs = + (const EncodedAttribute*)reader.skip(attrCount * sizeof(EncodedAttribute)); + for (int i = 0; i < attrCount; ++i) { + attrs[i] = Attribute(encodedAttrs[i].fType, encodedAttrs[i].fUsage, + encodedAttrs[i].fHasMarkerName ? reader.readString() : nullptr); + } const Desc desc{ mode, vertexCount, indexCount, hasTexs, hasColors, attrCount ? attrs : nullptr, attrCount diff --git a/src/gpu/ops/GrDrawVerticesOp.cpp b/src/gpu/ops/GrDrawVerticesOp.cpp index 0b083c8172..b3d17797f7 100644 --- a/src/gpu/ops/GrDrawVerticesOp.cpp +++ b/src/gpu/ops/GrDrawVerticesOp.cpp @@ -58,15 +58,15 @@ static GrSLType SkVerticesAttributeToGrSLType(const SkVertices::Attribute& a) { SkUNREACHABLE; } -// Container for a collection of [MarkerID, Matrix] pairs. For a GrDrawVerticesOp whose custom -// attributes reference some set of MarkerIDs, this stores the actual values of those matrices, +// Container for a collection of [uint32_t, Matrix] pairs. For a GrDrawVerticesOp whose custom +// attributes reference some set of IDs, this stores the actual values of those matrices, // at the time the Op is created. class MarkedMatrices { public: - // For each MarkerID required by 'info', fetch the value of that matrix from 'matrixProvider' + // For each ID required by 'info', fetch the value of that matrix from 'matrixProvider' void gather(const SkVerticesPriv& info, const SkMatrixProvider& matrixProvider) { for (int i = 0; i < info.attributeCount(); ++i) { - if (SkCanvas::MarkerID id = info.attributes()[i].fMarkerID) { + if (uint32_t id = info.attributes()[i].fMarkerID) { if (std::none_of(fMatrices.begin(), fMatrices.end(), [id](const auto& m) { return m.first == id; })) { SkM44 matrix; @@ -78,7 +78,7 @@ public: } } - SkM44 get(SkCanvas::MarkerID id) const { + SkM44 get(uint32_t id) const { for (const auto& m : fMatrices) { if (m.first == id) { return m.second; @@ -95,7 +95,7 @@ private: // If we expected many MarkerIDs, this should be a hash table. As it is, we're bounded by // SkVertices::kMaxCustomAttributes (which is 8). Realistically, we're never going to see // more than 1 or 2 unique MarkerIDs, so rely on linear search when inserting and fetching. - std::vector> fMatrices; + std::vector> fMatrices; }; class VerticesGP : public GrGeometryProcessor { @@ -204,7 +204,7 @@ public: } } if (!matrixHandle.isValid()) { - SkString uniName = SkStringPrintf("customMatrix_%d%s", customAttr.fMarkerID, + SkString uniName = SkStringPrintf("customMatrix_%x%s", customAttr.fMarkerID, normal ? "_IT" : ""); matrixHandle = uniformHandler->addUniform( nullptr, kVertex_GrShaderFlag, @@ -306,11 +306,12 @@ public: b->add32(key); b->add32(GrColorSpaceXform::XformKey(vgp.fColorSpaceXform.get())); + uint32_t usageBits = 0; for (int i = 0; i < vgp.fCustomAttributeCount; ++i) { - SkASSERT(SkTFitsIn(vgp.fCustomAttributes[i].fMarkerID)); - b->add32(vgp.fCustomAttributes[i].fMarkerID << 16 | - (uint32_t)vgp.fCustomAttributes[i].fUsage); + b->add32(vgp.fCustomAttributes[i].fMarkerID); + usageBits = (usageBits << 8) | (uint32_t)vgp.fCustomAttributes[i].fUsage; } + b->add32(usageBits); } void setData(const GrGLSLProgramDataManager& pdman, @@ -363,9 +364,9 @@ public: GrGLSLColorSpaceXformHelper fColorSpaceHelper; struct MarkedUniform { - SkCanvas::MarkerID fID; - bool fNormal; - UniformHandle fUniform; + uint32_t fID; + bool fNormal; + UniformHandle fUniform; }; std::vector fCustomMatrixUniforms; diff --git a/src/utils/SkNWayCanvas.cpp b/src/utils/SkNWayCanvas.cpp index c4ff3919e3..4e623073c5 100644 --- a/src/utils/SkNWayCanvas.cpp +++ b/src/utils/SkNWayCanvas.cpp @@ -93,12 +93,12 @@ void SkNWayCanvas::willRestore() { this->INHERITED::willRestore(); } -void SkNWayCanvas::onMarkCTM(MarkerID id) { +void SkNWayCanvas::onMarkCTM(const char* name) { Iter iter(fList); while (iter.next()) { - iter->markCTM(id); + iter->markCTM(name); } - this->INHERITED::onMarkCTM(id); + this->INHERITED::onMarkCTM(name); } void SkNWayCanvas::didConcat44(const SkM44& m) { diff --git a/tests/CanvasTest.cpp b/tests/CanvasTest.cpp index 0c85e2bc44..c6c47d8fc1 100644 --- a/tests/CanvasTest.cpp +++ b/tests/CanvasTest.cpp @@ -684,8 +684,8 @@ DEF_TEST(canvas_markctm, reporter) { SkCanvas canvas(10, 10); SkM44 m; - uint32_t id_a = 1, - id_b = 2; + const char* id_a = "a"; + const char* id_b = "b"; REPORTER_ASSERT(reporter, !canvas.findMarkedCTM(id_a, nullptr)); REPORTER_ASSERT(reporter, !canvas.findMarkedCTM(id_b, nullptr));