From 202420e0147b02f963de761a903b2c430dfa0cec Mon Sep 17 00:00:00 2001 From: Brian Osman Date: Fri, 26 Feb 2021 14:29:39 -0500 Subject: [PATCH] Reland "Remove GrProgramDesc::KeyHeader structure" This is a reland of 4bcd58afbf1a1993e453231c575b47a0ae1ec24d 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 > Commit-Queue: Brian Osman Bug: skia:11372 Change-Id: I2cdb49aee3537e54dad9af1f9b47cf1aed1aca21 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376849 Reviewed-by: Brian Salomon Commit-Queue: Brian Osman --- src/gpu/GrProgramDesc.cpp | 47 +++++++++++++++------------------------ src/gpu/GrProgramDesc.h | 47 +++++---------------------------------- 2 files changed, 24 insertions(+), 70 deletions(-) diff --git a/src/gpu/GrProgramDesc.cpp b/src/gpu/GrProgramDesc.cpp index c8b266faa1..ebb3a7f631 100644 --- a/src/gpu/GrProgramDesc.cpp +++ b/src/gpu/GrProgramDesc.cpp @@ -175,11 +175,7 @@ bool GrProgramDesc::Build(GrProgramDesc* desc, // bindings in use or other descriptor field settings) it should be set // 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().push_back_n(kProcessorKeysOffset); - GrProcessorKeyBuilder b(&desc->key()); const GrPrimitiveProcessor& primitiveProcessor = programInfo.primProc(); @@ -223,36 +219,29 @@ bool GrProgramDesc::Build(GrProgramDesc* desc, b.add32(renderTarget->getSamplePatternKey()); } - // --------DO NOT MOVE HEADER ABOVE THIS LINE-------------------------------------------------- - // Because header is a pointer into the dynamic array, we can't push any new data into the key - // below here. - KeyHeader* header = desc->atOffset(); - - // make sure any padding in the header is zeroed. - memset(header, 0, kHeaderSize); - header->fWriteSwizzle = pipeline.writeSwizzle().asKey(); - header->fColorFragmentProcessorCnt = numColorFPs; - header->fCoverageFragmentProcessorCnt = numCoverageFPs; - SkASSERT(header->fColorFragmentProcessorCnt == numColorFPs); - SkASSERT(header->fCoverageFragmentProcessorCnt == numCoverageFPs); + // Add "header" metadata + uint32_t header = 0; + SkDEBUGCODE(uint32_t header_bits = 0); + auto add_bits = [&](uint32_t nbits, uint32_t val) { + SkASSERT(val < (1 << nbits)); + SkASSERT((header_bits += nbits) <= 32); + header = (header << nbits) | val; + }; + add_bits(16, pipeline.writeSwizzle().asKey()); + add_bits( 1, numColorFPs); + add_bits( 2, numCoverageFPs); // 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. - header->fSurfaceOriginKey = - GrGLSLFragmentShaderBuilder::KeyForSurfaceOrigin(programInfo.origin()); - header->fProcessorFeatures = (uint8_t)programInfo.requestedFeatures(); - // Ensure enough bits. - SkASSERT(header->fProcessorFeatures == (int) programInfo.requestedFeatures()); - header->fSnapVerticesToPixelCenters = pipeline.snapVerticesToPixelCenters(); + add_bits( 2, GrGLSLFragmentShaderBuilder::KeyForSurfaceOrigin(programInfo.origin())); + add_bits( 1, static_cast(programInfo.requestedFeatures())); + add_bits( 1, pipeline.snapVerticesToPixelCenters()); // The base descriptor only stores whether or not the primitiveType is kPoints. Backend- // specific versions (e.g., Vulkan) require more detail - header->fHasPointSize = (programInfo.primitiveType() == GrPrimitiveType::kPoints); + add_bits( 1, (programInfo.primitiveType() == GrPrimitiveType::kPoints)); - header->fInitialKeyLength = desc->keyLength(); - // Fail if the initial key length won't fit in 27 bits. - if (header->fInitialKeyLength != desc->keyLength()) { - desc->key().reset(); - return false; - } + b.add32(header); + + desc->fInitialKeyLength = desc->keyLength(); return true; } diff --git a/src/gpu/GrProgramDesc.h b/src/gpu/GrProgramDesc.h index 011edfdb6a..7eaf880f6f 100644 --- a/src/gpu/GrProgramDesc.h +++ b/src/gpu/GrProgramDesc.h @@ -22,7 +22,7 @@ class GrShaderCaps; */ class GrProgramDesc { public: - GrProgramDesc(const GrProgramDesc& other) : fKey(other.fKey) {} // for SkLRUCache + GrProgramDesc(const GrProgramDesc& other) = default; bool isValid() const { return !fKey.empty(); } @@ -41,6 +41,7 @@ public: uint32_t keyLength = other.keyLength(); fKey.reset(SkToInt(keyLength)); memcpy(fKey.begin(), other.fKey.begin(), keyLength); + fInitialKeyLength = other.fInitialKeyLength; return *this; } @@ -65,7 +66,7 @@ public: return !(*this == other); } - uint32_t initialKeyLength() const { return this->header().fInitialKeyLength; } + uint32_t initialKeyLength() const { return fInitialKeyLength; } protected: friend class GrDawnCaps; @@ -100,48 +101,11 @@ protected: 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(); } - - template T* atOffset() { - return reinterpret_cast(reinterpret_cast(fKey.begin()) + OFFSET); - } - - template const T* atOffset() const { - return reinterpret_cast(reinterpret_cast(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 { + kHeaderSize = 4, // "header" in ::Build kMaxPreallocProcessors = 8, kIntsPerProcessor = 4, // This is an overestimate of the average effect key size. - kPreAllocSize = kHeaderOffset + kHeaderSize + + kPreAllocSize = kHeaderSize + kMaxPreallocProcessors * sizeof(uint32_t) * kIntsPerProcessor, }; @@ -149,6 +113,7 @@ protected: private: SkSTArray fKey; + uint32_t fInitialKeyLength = 0; }; #endif