Revert "Remove GrProgramDesc::KeyHeader structure"

This reverts commit 4bcd58afbf.

Reason for revert: Possible issue with copying/assignment

Original change's description:
> Remove GrProgramDesc::KeyHeader structure
>
> Instead, just fold all of this information into the key, like everything
> else. The only value that was accessed elsewhere is the initial key
> length. That doesn't need to be part of the key, so store it separately
> in the GrProgramDesc.
>
> Removing this special case logic is just the first step in revising how
> we assemble keys.
>
> Bug: skia:11372
> Change-Id: I52eb76812045e1906797cb37e809cfd0b3332ef0
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376797
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>

TBR=bsalomon@google.com,robertphillips@google.com,brianosman@google.com

Change-Id: I58b39a173abc45cd4932fd758fa005d1f727a0d9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11372
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376848
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This commit is contained in:
Brian Osman 2021-02-26 21:23:26 +00:00 committed by Skia Commit-Bot
parent 4bcd58afbf
commit deed7f65ff
2 changed files with 69 additions and 22 deletions

View File

@ -175,7 +175,11 @@ bool GrProgramDesc::Build(GrProgramDesc* desc,
// bindings in use or other descriptor field settings) it should be set // bindings in use or other descriptor field settings) it should be set
// to a canonical value to avoid duplicate programs with different keys. // to a canonical value to avoid duplicate programs with different keys.
static_assert(0 == kProcessorKeysOffset % sizeof(uint32_t));
// Make room for everything up to the effect keys.
desc->key().reset(); desc->key().reset();
desc->key().push_back_n(kProcessorKeysOffset);
GrProcessorKeyBuilder b(&desc->key()); GrProcessorKeyBuilder b(&desc->key());
const GrPrimitiveProcessor& primitiveProcessor = programInfo.primProc(); const GrPrimitiveProcessor& primitiveProcessor = programInfo.primProc();
@ -219,29 +223,36 @@ bool GrProgramDesc::Build(GrProgramDesc* desc,
b.add32(renderTarget->getSamplePatternKey()); b.add32(renderTarget->getSamplePatternKey());
} }
// Add "header" metadata // --------DO NOT MOVE HEADER ABOVE THIS LINE--------------------------------------------------
uint32_t header = 0; // Because header is a pointer into the dynamic array, we can't push any new data into the key
SkDEBUGCODE(uint32_t header_bits = 0); // below here.
auto add_bits = [&](uint32_t nbits, uint32_t val) { KeyHeader* header = desc->atOffset<KeyHeader, kHeaderOffset>();
SkASSERT(val < (1 << nbits));
SkASSERT((header_bits += nbits) <= 32); // make sure any padding in the header is zeroed.
header = (header << nbits) | val; memset(header, 0, kHeaderSize);
}; header->fWriteSwizzle = pipeline.writeSwizzle().asKey();
add_bits(16, pipeline.writeSwizzle().asKey()); header->fColorFragmentProcessorCnt = numColorFPs;
add_bits( 1, numColorFPs); header->fCoverageFragmentProcessorCnt = numCoverageFPs;
add_bits( 2, numCoverageFPs); SkASSERT(header->fColorFragmentProcessorCnt == numColorFPs);
SkASSERT(header->fCoverageFragmentProcessorCnt == numCoverageFPs);
// If we knew the shader won't depend on origin, we could skip this (and use the same program // If we knew the shader won't depend on origin, we could skip this (and use the same program
// for both origins). Instrumenting all fragment processors would be difficult and error prone. // for both origins). Instrumenting all fragment processors would be difficult and error prone.
add_bits( 2, GrGLSLFragmentShaderBuilder::KeyForSurfaceOrigin(programInfo.origin())); header->fSurfaceOriginKey =
add_bits( 1, static_cast<uint32_t>(programInfo.requestedFeatures())); GrGLSLFragmentShaderBuilder::KeyForSurfaceOrigin(programInfo.origin());
add_bits( 1, pipeline.snapVerticesToPixelCenters()); header->fProcessorFeatures = (uint8_t)programInfo.requestedFeatures();
// Ensure enough bits.
SkASSERT(header->fProcessorFeatures == (int) programInfo.requestedFeatures());
header->fSnapVerticesToPixelCenters = pipeline.snapVerticesToPixelCenters();
// The base descriptor only stores whether or not the primitiveType is kPoints. Backend- // The base descriptor only stores whether or not the primitiveType is kPoints. Backend-
// specific versions (e.g., Vulkan) require more detail // specific versions (e.g., Vulkan) require more detail
add_bits( 1, (programInfo.primitiveType() == GrPrimitiveType::kPoints)); header->fHasPointSize = (programInfo.primitiveType() == GrPrimitiveType::kPoints);
b.add32(header); header->fInitialKeyLength = desc->keyLength();
// Fail if the initial key length won't fit in 27 bits.
desc->fInitialKeyLength = desc->keyLength(); if (header->fInitialKeyLength != desc->keyLength()) {
desc->key().reset();
return false;
}
return true; return true;
} }

View File

@ -65,7 +65,7 @@ public:
return !(*this == other); return !(*this == other);
} }
uint32_t initialKeyLength() const { return fInitialKeyLength; } uint32_t initialKeyLength() const { return this->header().fInitialKeyLength; }
protected: protected:
friend class GrDawnCaps; friend class GrDawnCaps;
@ -100,11 +100,48 @@ protected:
return true; return true;
} }
// TODO: this should be removed and converted to just data added to the key
struct KeyHeader {
// Set to uniquely identify any swizzling of the shader's output color(s).
uint16_t fWriteSwizzle;
uint8_t fColorFragmentProcessorCnt; // Can be packed into 4 bits if required.
uint8_t fCoverageFragmentProcessorCnt;
// Set to uniquely identify the rt's origin, or 0 if the shader does not require this info.
uint32_t fSurfaceOriginKey : 2;
uint32_t fProcessorFeatures : 1;
uint32_t fSnapVerticesToPixelCenters : 1;
uint32_t fHasPointSize : 1;
// This is the key size (in bytes) after core key construction. It doesn't include any
// portions added by the platform-specific backends.
uint32_t fInitialKeyLength : 27;
};
static_assert(sizeof(KeyHeader) == 8);
const KeyHeader& header() const { return *this->atOffset<KeyHeader, kHeaderOffset>(); }
template<typename T, size_t OFFSET> T* atOffset() {
return reinterpret_cast<T*>(reinterpret_cast<intptr_t>(fKey.begin()) + OFFSET);
}
template<typename T, size_t OFFSET> const T* atOffset() const {
return reinterpret_cast<const T*>(reinterpret_cast<intptr_t>(fKey.begin()) + OFFSET);
}
// The key, stored in fKey, is composed of two parts:
// 1. Header struct defined above.
// 2. A Backend specific payload which includes the per-processor keys.
enum KeyOffsets {
kHeaderOffset = 0,
kHeaderSize = SkAlign4(sizeof(KeyHeader)),
// This is the offset into the backend-specific part of the key, which includes
// per-processor keys.
kProcessorKeysOffset = kHeaderOffset + kHeaderSize,
};
enum { enum {
kHeaderSize = 4, // "header" in ::Build
kMaxPreallocProcessors = 8, kMaxPreallocProcessors = 8,
kIntsPerProcessor = 4, // This is an overestimate of the average effect key size. kIntsPerProcessor = 4, // This is an overestimate of the average effect key size.
kPreAllocSize = kHeaderSize + kPreAllocSize = kHeaderOffset + kHeaderSize +
kMaxPreallocProcessors * sizeof(uint32_t) * kIntsPerProcessor, kMaxPreallocProcessors * sizeof(uint32_t) * kIntsPerProcessor,
}; };
@ -112,7 +149,6 @@ protected:
private: private:
SkSTArray<kPreAllocSize, uint8_t, true> fKey; SkSTArray<kPreAllocSize, uint8_t, true> fKey;
uint32_t fInitialKeyLength = 0;
}; };
#endif #endif