AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Remove pointer sidecar data in PaintParamsKey.

Browse Source 
Pointer data is obliterated when we assemble draw passes, so this
feature didn't work for its intended purpose.

Change-Id: I85331aa7b6212c55fe16081937465c2f38e5a103
Bug: skia:13428
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/552718
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This commit is contained in:
John Stiles 2022-06-24 11:12:40 -04:00 committed by SkCQ
parent b5ea90187c
commit 76f32c939f
5 changed files with 9 additions and 153 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
src/core/SkKeyHelpers.cpp
View File

@ -620,7 +620,6 @@ void RuntimeShaderBlock::BeginBlock(const SkKeyContext& keyContext,
builder->beginBlock(kCodeSnippetID);
builder->addBytes(sizeof(hash), reinterpret_cast<const uint8_t*>(&hash));
builder->addBytes(sizeof(uniformSize), reinterpret_cast<const uint8_t*>(&uniformSize));
builder->addPointer(shaderData.fEffect.get());
#endif // SK_GRAPHITE_ENABLED
break;
}

39
src/core/SkPaintParamsKey.cpp
View File

@ -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<const uint8_t> span,
SkSpan<const void*> 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<const uint8_t> parentSpan,
SkSpan<const void*> 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<const SkColor4f> 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<uint8_t>(this->dataPayload(),
fEntry->fDataPayloadExpectations,
fieldIndex);
return fPointerSpan[dataSpan[0]];
}
#ifdef SK_DEBUG
int SkPaintParamsKey::BlockReader::numDataPayloadFields() const {

34
src/core/SkPaintParamsKey.h
View File

@ -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 @@ public:
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 @@ public:
SkSpan<const uint8_t> bytes(int fieldIndex) const;
SkSpan<const int32_t> ints(int fieldIndex) const;
SkSpan<const SkColor4f> colors(int fieldIndex) const;
const void* pointer(int fieldIndex) const;
const SkShaderSnippet* entry() const { return fEntry; }
@ -103,7 +100,6 @@ public:
BlockReader(const SkShaderCodeDictionary*,
SkSpan<const uint8_t> parentSpan,
SkSpan<const void*> pointerSpan,
int offsetInParent);
int32_t codeSnippetId() const;
@ -113,7 +109,6 @@ public:
SkSpan<const uint8_t> dataPayload() const;
SkSpan<const uint8_t> fBlock;
SkSpan<const void*> fPointerSpan;
const SkShaderSnippet* fEntry;
};
@ -132,10 +127,6 @@ public:
const uint8_t* data() const { return fData.data(); }
int sizeInBytes() const { return SkTo<int>(fData.size()); }
SkSpan<const void*> pointerSpan() const { return fPointerData; }
const void** pointerData() const { return fPointerData.data(); }
int numPointers() const { return SkTo<int>(fPointerData.size()); }
bool operator==(const SkPaintParamsKey& that) const;
bool operator!=(const SkPaintParamsKey& that) const { return !(*this == that); }
@ -150,9 +141,7 @@ private:
// 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<const uint8_t> span,
SkSpan<const void*> pointerSpan,
SkPaintParamsKeyBuilder* originatingBuilder);
SkPaintParamsKey(SkSpan<const uint8_t> 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 @@ private:
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<const uint8_t> fData;
SkSpan<const void*> fPointerData;
// This class should only ever access the 'lock' and 'unlock' calls on 'fOriginatingBuilder'
SkPaintParamsKeyBuilder* fOriginatingBuilder;
@ -218,12 +205,6 @@ public:
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 @@ public:
SkPaintParamsKey lockAsKey();
int sizeInBytes() const { return fData.count(); }
int numPointers() const { return fPointerData.count(); }
bool isValid() const { return fIsValid; }
@ -245,7 +225,6 @@ public:
void unlock() {
SkASSERT(fLocked);
fData.rewind();
fPointerData.rewind();
#ifdef SK_GRAPHITE_ENABLED
fBlendInfo = {};
#endif
@ -288,13 +267,6 @@ private:
SkTDArray<StackFrame> fStack;
SkTDArray<uint8_t> 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<const void*> fPointerData;
#ifdef SK_GRAPHITE_ENABLED
skgpu::BlendInfo fBlendInfo;
#endif

1
src/core/SkShaderCodeDictionary.cpp
View File

@ -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},
};
//--------------------------------------------------------------------------------------------------

87
tests/graphite/KeyTest.cpp
View File

@ -26,20 +26,6 @@ SkPaintParamsKey create_key_with_data(SkPaintParamsKeyBuilder* builder,
return builder->lockAsKey();
}
SkPaintParamsKey create_key_with_ptr(SkPaintParamsKeyBuilder* builder,
int snippetID,
SkSpan<const uint8_t> 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<const uint8_t> 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);
}