diff --git a/src/core/SkKeyHelpers.cpp b/src/core/SkKeyHelpers.cpp index 041213c15f80..b9d55aef84bf 100644 --- a/src/core/SkKeyHelpers.cpp +++ b/src/core/SkKeyHelpers.cpp @@ -620,7 +620,6 @@ void RuntimeShaderBlock::BeginBlock(const SkKeyContext& keyContext, builder->beginBlock(kCodeSnippetID); builder->addBytes(sizeof(hash), reinterpret_cast(&hash)); builder->addBytes(sizeof(uniformSize), reinterpret_cast(&uniformSize)); - builder->addPointer(shaderData.fEffect.get()); #endif // SK_GRAPHITE_ENABLED break; } diff --git a/src/core/SkPaintParamsKey.cpp b/src/core/SkPaintParamsKey.cpp index e18acbac89f9..263c983a8f6f 100644 --- a/src/core/SkPaintParamsKey.cpp +++ b/src/core/SkPaintParamsKey.cpp @@ -36,7 +36,6 @@ SkPaintParamsKeyBuilder::SkPaintParamsKeyBuilder(const SkShaderCodeDictionary* d void SkPaintParamsKeyBuilder::checkReset() { SkASSERT(!this->isLocked()); SkASSERT(this->sizeInBytes() == 0); - SkASSERT(this->numPointers() == 0); SkASSERT(fIsValid); SkASSERT(fStack.empty()); #ifdef SK_GRAPHITE_ENABLED @@ -156,10 +155,9 @@ void SkPaintParamsKeyBuilder::checkExpectations(DataPayloadType actualType, uint static int field_size(DataPayloadType type) { switch (type) { - case DataPayloadType::kByte: - case DataPayloadType::kPointerIndex: return 1; - case DataPayloadType::kInt: return 4; - case DataPayloadType::kFloat4: return 16; + case DataPayloadType::kByte: return 1; + case DataPayloadType::kInt: return 4; + case DataPayloadType::kFloat4: return 16; } SkUNREACHABLE; } @@ -195,14 +193,6 @@ void SkPaintParamsKeyBuilder::add(int numColors, const SkColor4f* colors) { this->addToKey(numColors, colors, DataPayloadType::kFloat4); } -void SkPaintParamsKeyBuilder::addPointer(const void* ptr) { - SkASSERT(fPointerData.size() <= 0xFF); - uint8_t pointerIndex = (uint8_t)fPointerData.size(); - - this->addToKey(1, &pointerIndex, DataPayloadType::kPointerIndex); - fPointerData.push_back(ptr); -} - SkPaintParamsKey SkPaintParamsKeyBuilder::lockAsKey() { if (!fStack.empty()) { // SKGPU_LOG_W("Mismatched beginBlock/endBlocks."); @@ -216,9 +206,7 @@ SkPaintParamsKey SkPaintParamsKeyBuilder::lockAsKey() { fIsValid = true; fStack.rewind(); - return SkPaintParamsKey(SkSpan(fData.begin(), fData.count()), - SkSpan(fPointerData.begin(), fPointerData.count()), - this); + return SkPaintParamsKey(SkSpan(fData.begin(), fData.count()), this); } void SkPaintParamsKeyBuilder::makeInvalid() { @@ -227,7 +215,6 @@ void SkPaintParamsKeyBuilder::makeInvalid() { fStack.rewind(); fData.rewind(); - fPointerData.rewind(); this->beginBlock(SkBuiltInCodeSnippetID::kError); this->endBlock(); @@ -237,10 +224,8 @@ void SkPaintParamsKeyBuilder::makeInvalid() { //-------------------------------------------------------------------------------------------------- SkPaintParamsKey::SkPaintParamsKey(SkSpan span, - SkSpan pointerSpan, SkPaintParamsKeyBuilder* originatingBuilder) : fData(span) - , fPointerData(pointerSpan) , fOriginatingBuilder(originatingBuilder) { fOriginatingBuilder->lock(); } @@ -257,14 +242,13 @@ SkPaintParamsKey::~SkPaintParamsKey() { } bool SkPaintParamsKey::operator==(const SkPaintParamsKey& that) const { - // Pointer data is intentionally ignored here; a cached key will not have pointer data. return fData.size() == that.fData.size() && !memcmp(fData.data(), that.fData.data(), fData.size()); } SkPaintParamsKey::BlockReader SkPaintParamsKey::reader(const SkShaderCodeDictionary* dict, int headerOffset) const { - return BlockReader(dict, fData, fPointerData, headerOffset); + return BlockReader(dict, fData, headerOffset); } #ifdef SK_DEBUG @@ -326,12 +310,10 @@ bool SkPaintParamsKey::isErrorKey() const { SkPaintParamsKey::BlockReader::BlockReader(const SkShaderCodeDictionary* dict, SkSpan parentSpan, - SkSpan pointerSpan, int offsetInParent) { Header header = read_header(parentSpan, offsetInParent); fBlock = parentSpan.subspan(offsetInParent, header.blockSize); - fPointerSpan = pointerSpan; fEntry = dict->getEntry(header.codeSnippetID); SkASSERT(fEntry); } @@ -349,7 +331,7 @@ SkPaintParamsKey::BlockReader SkPaintParamsKey::BlockReader::child( childOffset += header.blockSize; } - return BlockReader(dict, fBlock, fPointerSpan, childOffset); + return BlockReader(dict, fBlock, childOffset); } int32_t SkPaintParamsKey::BlockReader::codeSnippetId() const { @@ -405,15 +387,6 @@ SkSpan SkPaintParamsKey::BlockReader::colors(int fieldIndex) co fieldIndex); } -const void* SkPaintParamsKey::BlockReader::pointer(int fieldIndex) const { - SkASSERT(fEntry->fDataPayloadExpectations[fieldIndex].fType == DataPayloadType::kPointerIndex); - SkASSERT(fEntry->fDataPayloadExpectations[fieldIndex].fCount == 1); - SkSpan dataSpan = payload_subspan_for_field(this->dataPayload(), - fEntry->fDataPayloadExpectations, - fieldIndex); - return fPointerSpan[dataSpan[0]]; -} - #ifdef SK_DEBUG int SkPaintParamsKey::BlockReader::numDataPayloadFields() const { diff --git a/src/core/SkPaintParamsKey.h b/src/core/SkPaintParamsKey.h index 6109a09e47a2..345b13a65b52 100644 --- a/src/core/SkPaintParamsKey.h +++ b/src/core/SkPaintParamsKey.h @@ -37,7 +37,7 @@ struct SkShaderSnippet; // Header, consisting of 2 bytes: // 4 bytes: code-snippet ID // 1 byte: size of the block, in bytes (header, plus all data payload bytes) -// The rest of the data and pointers in the block are dependent on the individual code snippet. +// The rest of the data in the block is dependent on the individual code snippet. // If a given block has child blocks, they appear in the key right after their parent // block's header. class SkPaintParamsKey { @@ -56,8 +56,6 @@ class SkPaintParamsKey { kByte, kInt, kFloat4, - // Represents a position inside the fPointerData span. - kPointerIndex, }; // A given snippet's data payload is stored as an SkSpan of DataPayloadFields in the @@ -89,7 +87,6 @@ class SkPaintParamsKey { SkSpan bytes(int fieldIndex) const; SkSpan ints(int fieldIndex) const; SkSpan colors(int fieldIndex) const; - const void* pointer(int fieldIndex) const; const SkShaderSnippet* entry() const { return fEntry; } @@ -103,7 +100,6 @@ class SkPaintParamsKey { BlockReader(const SkShaderCodeDictionary*, SkSpan parentSpan, - SkSpan pointerSpan, int offsetInParent); int32_t codeSnippetId() const; @@ -113,7 +109,6 @@ class SkPaintParamsKey { SkSpan dataPayload() const; SkSpan fBlock; - SkSpan fPointerSpan; const SkShaderSnippet* fEntry; }; @@ -132,10 +127,6 @@ class SkPaintParamsKey { const uint8_t* data() const { return fData.data(); } int sizeInBytes() const { return SkTo(fData.size()); } - SkSpan pointerSpan() const { return fPointerData; } - const void** pointerData() const { return fPointerData.data(); } - int numPointers() const { return SkTo(fPointerData.size()); } - bool operator==(const SkPaintParamsKey& that) const; bool operator!=(const SkPaintParamsKey& that) const { return !(*this == that); } @@ -150,9 +141,7 @@ class SkPaintParamsKey { // This ctor is to be used when paintparams keys are being consecutively generated // by a key builder. The memory backing this key's span is shared between the // builder and its keys. - SkPaintParamsKey(SkSpan span, - SkSpan pointerSpan, - SkPaintParamsKeyBuilder* originatingBuilder); + SkPaintParamsKey(SkSpan span, SkPaintParamsKeyBuilder* originatingBuilder); // This ctor is used when this key isn't being created by a builder (i.e., when the key // is in the dictionary). In this case the dictionary will own the memory backing the span. @@ -162,14 +151,12 @@ class SkPaintParamsKey { const SkPaintParamsKey::BlockReader&, SkShaderInfo*); - // The memory referenced in 'fData' and 'fPointerData' is always owned by someone else. + // The memory referenced in 'fData' is always owned by someone else. // If 'fOriginatingBuilder' is null, the dictionary's SkArena owns the 'fData' memory and no // explicit freeing is required. // If 'fOriginatingBuilder' is non-null then the 'fData' memory must be explicitly locked (in // the ctor) and unlocked (in the dtor) on the 'fOriginatingBuilder' object. - // The 'fPointerData' memory is always managed external to this class. SkSpan fData; - SkSpan fPointerData; // This class should only ever access the 'lock' and 'unlock' calls on 'fOriginatingBuilder' SkPaintParamsKeyBuilder* fOriginatingBuilder; @@ -218,12 +205,6 @@ class SkPaintParamsKeyBuilder { this->add(/*numColors=*/1, &color); } - // `addPointer` is optional sidecar data. The pointer data in a PaintParamsKey is not checked at - // all when checking the equality of two keys; cached PaintParamsKey objects will not hold - // pointer data. However, pointer data will be required for actually painting pixels on the - // screen. - void addPointer(const void* ptr); - #ifdef SK_DEBUG // Check that the builder has been reset to its initial state prior to creating a new key. void checkReset(); @@ -233,7 +214,6 @@ class SkPaintParamsKeyBuilder { SkPaintParamsKey lockAsKey(); int sizeInBytes() const { return fData.count(); } - int numPointers() const { return fPointerData.count(); } bool isValid() const { return fIsValid; } @@ -245,7 +225,6 @@ class SkPaintParamsKeyBuilder { void unlock() { SkASSERT(fLocked); fData.rewind(); - fPointerData.rewind(); #ifdef SK_GRAPHITE_ENABLED fBlendInfo = {}; #endif @@ -288,13 +267,6 @@ class SkPaintParamsKeyBuilder { SkTDArray fStack; SkTDArray fData; - // The pointer data is used by some paint types, when the key data is not sufficient to - // reconstruct all the information needed to draw. (For instance, the key for a runtime effect - // contains a hash of the shader text, but to draw, we need entire compiled shader program.) - // Cached paint-param keys will discard the pointer data. When comparing paint-param keys, - // pointer data (if any) will be ignored. - SkTDArray fPointerData; - #ifdef SK_GRAPHITE_ENABLED skgpu::BlendInfo fBlendInfo; #endif diff --git a/src/core/SkShaderCodeDictionary.cpp b/src/core/SkShaderCodeDictionary.cpp index b9429f5558b2..39fee18771dd 100644 --- a/src/core/SkShaderCodeDictionary.cpp +++ b/src/core/SkShaderCodeDictionary.cpp @@ -547,7 +547,6 @@ static constexpr SkUniform kRuntimeShaderUniforms[] = { static constexpr DataPayloadField kRuntimeShaderDataPayload[] = { {"runtime effect hash", DataPayloadType::kByte, 4}, {"uniform data size (bytes)", DataPayloadType::kByte, 4}, - {"SkRuntimeEffect pointer", DataPayloadType::kPointerIndex, 1}, }; //-------------------------------------------------------------------------------------------------- diff --git a/tests/graphite/KeyTest.cpp b/tests/graphite/KeyTest.cpp index 88c1b25abdbf..3f12e58d7479 100644 --- a/tests/graphite/KeyTest.cpp +++ b/tests/graphite/KeyTest.cpp @@ -26,20 +26,6 @@ SkPaintParamsKey create_key_with_data(SkPaintParamsKeyBuilder* builder, return builder->lockAsKey(); } -SkPaintParamsKey create_key_with_ptr(SkPaintParamsKeyBuilder* builder, - int snippetID, - SkSpan dataPayload, - void* pointerData) { - SkDEBUGCODE(builder->checkReset()); - - builder->beginBlock(snippetID); - builder->addBytes(dataPayload.size(), dataPayload.data()); - builder->addPointer(pointerData); - builder->endBlock(); - - return builder->lockAsKey(); -} - SkPaintParamsKey create_key(SkPaintParamsKeyBuilder* builder, int snippetID, int size) { SkASSERT(size <= 1024); static constexpr uint8_t kDummyData[1024] = {}; @@ -149,38 +135,6 @@ DEF_GRAPHITE_TEST_FOR_CONTEXTS(KeyEqualityChecksData, reporter, context) { REPORTER_ASSERT(reporter, !(keyA != keyB)); } -DEF_GRAPHITE_TEST_FOR_CONTEXTS(KeyEqualityDoesNotCheckPointers, reporter, context) { - - SkShaderCodeDictionary* dict = context->priv().shaderCodeDictionary(); - static const int kBlockDataSize = 4; - static constexpr SkPaintParamsKey::DataPayloadField kDataFields[] = { - {"data", SkPaintParamsKey::DataPayloadType::kByte, kBlockDataSize}, - {"ptrIndex", SkPaintParamsKey::DataPayloadType::kPointerIndex, 1}, - }; - - int userSnippetID = dict->addUserDefinedSnippet("key", SkSpan(kDataFields)); - - static constexpr uint8_t kData[kBlockDataSize] = {1, 2, 3, 4}; - int arbitraryData1 = 1; - int arbitraryData2 = 2; - - SkPaintParamsKeyBuilder builderA(dict, SkBackend::kGraphite); - SkPaintParamsKeyBuilder builderB(dict, SkBackend::kGraphite); - SkPaintParamsKeyBuilder builderC(dict, SkBackend::kGraphite); - SkPaintParamsKey keyA = create_key_with_ptr(&builderA, userSnippetID, SkSpan(kData), - &arbitraryData1); - SkPaintParamsKey keyB = create_key_with_ptr(&builderB, userSnippetID, SkSpan(kData), - &arbitraryData2); - SkPaintParamsKey keyC = create_key_with_ptr(&builderC, userSnippetID, SkSpan(kData), - nullptr); - - // Verify that keyA, keyB, and keyC all match, even though the pointer data does not. - REPORTER_ASSERT(reporter, keyA == keyB); - REPORTER_ASSERT(reporter, keyB == keyC); - REPORTER_ASSERT(reporter, !(keyA != keyB)); - REPORTER_ASSERT(reporter, !(keyB != keyC)); -} - DEF_GRAPHITE_TEST_FOR_CONTEXTS(KeyBlockReaderWorks, reporter, context) { SkShaderCodeDictionary* dict = context->priv().shaderCodeDictionary(); @@ -226,44 +180,3 @@ DEF_GRAPHITE_TEST_FOR_CONTEXTS(KeyBlockReaderWorks, reporter, context) { REPORTER_ASSERT(reporter, readerBytesZ.size() == kCountZ); REPORTER_ASSERT(reporter, 0 == memcmp(readerBytesZ.data(), kDataZ, sizeof(kDataZ))); } - -DEF_GRAPHITE_TEST_FOR_CONTEXTS(KeyBlockReaderPointersWork, reporter, context) { - - SkShaderCodeDictionary* dict = context->priv().shaderCodeDictionary(); - static const int kCountX = 3; - static constexpr SkPaintParamsKey::DataPayloadField kDataFields[] = { - {"PtrIndexA", SkPaintParamsKey::DataPayloadType::kPointerIndex, 1}, - {"DataX", SkPaintParamsKey::DataPayloadType::kByte, kCountX}, - {"PtrIndexB", SkPaintParamsKey::DataPayloadType::kPointerIndex, 1}, - }; - - int userSnippetID = dict->addUserDefinedSnippet("key", kDataFields); - int arbitraryDataA = 123; - int arbitraryDataB = 456; - - static constexpr uint8_t kDataX[kCountX] = {1, 2, 3}; - SkPaintParamsKeyBuilder builder(dict, SkBackend::kGraphite); - - builder.beginBlock(userSnippetID); - builder.addPointer(&arbitraryDataA); - builder.addBytes(sizeof(kDataX), kDataX); - builder.addPointer(&arbitraryDataB); - builder.endBlock(); - - SkPaintParamsKey key = builder.lockAsKey(); - - // Verify that the block reader can extract out our data from the SkPaintParamsKey. - SkPaintParamsKey::BlockReader reader = key.reader(dict, /*headerOffset=*/0); - REPORTER_ASSERT(reporter, reader.blockSize() == (sizeof(SkPaintParamsKey::Header) + - 1 + sizeof(kDataX) + 1)); - - const void* readerPtrA = reader.pointer(0); - REPORTER_ASSERT(reporter, readerPtrA == &arbitraryDataA); - - SkSpan readerDataX = reader.bytes(1); - REPORTER_ASSERT(reporter, readerDataX.size() == kCountX); - REPORTER_ASSERT(reporter, 0 == memcmp(readerDataX.data(), kDataX, sizeof(kDataX))); - - const void* readerPtrB = reader.pointer(2); - REPORTER_ASSERT(reporter, readerPtrB == &arbitraryDataB); -}