From: bsalomon Date: Thu, 17 Jul 2014 14:55:11 +0000 (-0700) Subject: Makes GrGLProgramDesc's key store the lengths as well as offsets of the effect keys. X-Git-Tag: accepted/tizen/5.0/unified/20181102.025319~6698 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=929f29a5c1bffa7f7c1b5a376351d0762b8ac561;p=platform%2Fupstream%2FlibSkiaSharp.git Makes GrGLProgramDesc's key store the lengths as well as offsets of the effect keys. Makes it possible to use GrBackendEffectFactories other than GrTBEF by moving meta-key generation out of GrTBEF. Cleans up docs around GrBackendEffectFactory. Committed: https://skia.googlesource.com/skia/+/c0ea398aff8254e31152cbb94c9ab6150428e252 R=robertphillips@google.com, jvanverth@google.com Author: bsalomon@google.com Review URL: https://codereview.chromium.org/379113004 --- diff --git a/include/gpu/GrBackendEffectFactory.h b/include/gpu/GrBackendEffectFactory.h index 32f14f2..ef9e436 100644 --- a/include/gpu/GrBackendEffectFactory.h +++ b/include/gpu/GrBackendEffectFactory.h @@ -14,22 +14,12 @@ #include "SkTypes.h" #include "SkTArray.h" -/** Given a GrEffect of a particular type, creates the corresponding graphics-backend-specific - effect object. Also tracks equivalence of shaders generated via a key. Each factory instance - is assigned a generation ID at construction. The ID of the return of GrEffect::getFactory() - is used as a type identifier. Thus a GrEffect subclass must return a singleton from - getFactory(). GrEffect subclasses should use the derived class GrTBackendEffectFactory that is - templated on the GrEffect subclass as their factory object. It requires that the GrEffect - subclass has a nested class (or typedef) GLEffect which is its GL implementation and a subclass - of GrGLEffect. - */ - class GrGLEffect; class GrGLCaps; class GrDrawEffect; /** - * Used by effects to build their keys. It incorpates each per-effect key into a larger shader key. + * Used by effects to build their keys. It incorporates each per-effect key into a larger shader key. */ class GrEffectKeyBuilder { public: @@ -42,6 +32,14 @@ public: fData->push_back_n(4, reinterpret_cast(&v)); } + /** Inserts count uint32_ts into the key. The returned pointer is only valid until the next + add*() call. */ + uint32_t* SK_WARN_UNUSED_RESULT add32n(int count) { + SkASSERT(count > 0); + fCount += count; + return reinterpret_cast(fData->push_back_n(4 * count)); + } + size_t size() const { return sizeof(uint32_t) * fCount; } private: @@ -49,43 +47,74 @@ private: int fCount; // number of uint32_ts added to fData by the effect. }; +/** + * Given a GrEffect of a particular type, creates the corresponding graphics-backend-specific + * effect object. It also tracks equivalence of shaders generated via a key. The factory for an + * effect is accessed via GrEffect::getFactory(). Each factory instance is assigned an ID at + * construction. The ID of GrEffect::getFactory() is used as a type identifier. Thus, a GrEffect + * subclass must always return the same object from getFactory() and that factory object must be + * unique to the GrEffect subclass (and unique from any further derived subclasses). + * + * Rather than subclassing this class themselves, it is recommended that GrEffect authors use + * the templated subclass GrTBackendEffectFactory by writing their getFactory() method as: + * + * const GrBackendEffectFactory& MyEffect::getFactory() const { + * return GrTBackendEffectFactory::getInstance(); + * } + * + * Using GrTBackendEffectFactory places a few constraints on the effect. See that class's comments. + */ class GrBackendEffectFactory : SkNoncopyable { public: typedef uint32_t EffectKey; - virtual bool getGLEffectKey(const GrDrawEffect&, const GrGLCaps&, GrEffectKeyBuilder*) const = 0; + /** + * Generates an effect's key. The key is based on the aspects of the GrEffect object's + * configuration that affect GLSL code generation. Two GrEffect instances that would cause + * this->createGLInstance()->emitCode() to produce different code must produce different keys. + */ + virtual void getGLEffectKey(const GrDrawEffect&, const GrGLCaps&, GrEffectKeyBuilder*) const = 0; + + /** + * Creates a GrGLEffect instance that is used both to generate code for the GrEffect in a GLSL + * program and to manage updating uniforms for the program when it is used. + */ virtual GrGLEffect* createGLInstance(const GrDrawEffect&) const = 0; - bool operator ==(const GrBackendEffectFactory& b) const { - return fEffectClassID == b.fEffectClassID; - } - bool operator !=(const GrBackendEffectFactory& b) const { - return !(*this == b); - } - + /** + * Produces a human-reable name for the effect. + */ virtual const char* name() const = 0; + /** + * A unique value for every instance of this factory. It is automatically incorporated into the + * effect's key. This allows keys generated by getGLEffectKey() to only be unique within a + * GrEffect subclass and not necessarily across subclasses. + */ + uint32_t effectClassID() const { return fEffectClassID; } + protected: + GrBackendEffectFactory() : fEffectClassID(GenID()) {} + virtual ~GrBackendEffectFactory() {} + +private: enum { kIllegalEffectClassID = 0, }; - GrBackendEffectFactory() { - fEffectClassID = kIllegalEffectClassID; - } - virtual ~GrBackendEffectFactory() {} - - static int32_t GenID() { + static uint32_t GenID() { // fCurrEffectClassID has been initialized to kIllegalEffectClassID. The // atomic inc returns the old value not the incremented value. So we add // 1 to the returned value. - int32_t id = sk_atomic_inc(&fCurrEffectClassID) + 1; + uint32_t id = static_cast(sk_atomic_inc(&fCurrEffectClassID)) + 1; + if (!id) { + SkFAIL("This should never wrap as it should only be called once for each GrEffect " + "subclass."); + } return id; } - int32_t fEffectClassID; - -private: + const uint32_t fEffectClassID; static int32_t fCurrEffectClassID; }; diff --git a/include/gpu/GrTBackendEffectFactory.h b/include/gpu/GrTBackendEffectFactory.h index cd4c0a4..251e8c7 100644 --- a/include/gpu/GrTBackendEffectFactory.h +++ b/include/gpu/GrTBackendEffectFactory.h @@ -13,7 +13,26 @@ #include "gl/GrGLProgramEffects.h" /** - * Implements GrBackendEffectFactory for a GrEffect subclass as a singleton. + * Implements GrBackendEffectFactory for a GrEffect subclass as a singleton. This can be used by + * most GrEffect subclasses to implement the GrEffect::getFactory() method: + * + * const GrBackendEffectFactory& MyEffect::getFactory() const { + * return GrTBackendEffectFactory::getInstance(); + * } + * + * Using this class requires that the GrEffect subclass always produces the same GrGLEffect + * subclass. Additionally, It adds the following requirements to the GrEffect and GrGLEffect + * subclasses: + * + * 1. The GrGLEffect used by GrEffect subclass MyEffect must be named or typedef'ed to + * MyEffect::GLEffect. + * 2. MyEffect::GLEffect must have a static function: + * EffectKey GenKey(const GrDrawEffect, const GrGLCaps&) + * which generates a key that maps 1 to 1 with code variations emitted by + * MyEffect::GLEffect::emitCode(). + * 3. MyEffect must have a static function: + * const char* Name() + * which returns a human-readable name for the effect. */ template class GrTBackendEffectFactory : public GrBackendEffectFactory { @@ -21,35 +40,17 @@ class GrTBackendEffectFactory : public GrBackendEffectFactory { public: typedef typename EffectClass::GLEffect GLEffect; - /** Returns a human-readable name that is accessible via GrEffect or - GrGLEffect and is consistent between the two of them. - */ + /** Returns a human-readable name for the effect. Implemented using GLEffect::Name as described + * in this class's comment. */ virtual const char* name() const SK_OVERRIDE { return EffectClass::Name(); } - /** Generates an effect's key. This enables caching of generated shaders. Part of the - id identifies the GrEffect subclass. The remainder is based on the aspects of the - GrEffect object's configuration that affect GLSL code generation. If this fails - then program generation should be aborted. Failure occurs if the effect uses more - transforms, attributes, or textures than the key has space for. */ - virtual bool getGLEffectKey(const GrDrawEffect& drawEffect, + + /** Implemented using GLEffect::GenKey as described in this class's comment. */ + virtual void getGLEffectKey(const GrDrawEffect& drawEffect, const GrGLCaps& caps, GrEffectKeyBuilder* b) const SK_OVERRIDE { - SkASSERT(kIllegalEffectClassID != fEffectClassID); EffectKey effectKey = GLEffect::GenKey(drawEffect, caps); - EffectKey textureKey = GrGLProgramEffects::GenTextureKey(drawEffect, caps); - EffectKey transformKey = GrGLProgramEffects::GenTransformKey(drawEffect); - EffectKey attribKey = GrGLProgramEffects::GenAttribKey(drawEffect); - static const uint32_t kMetaKeyInvalidMask = ~((uint32_t) SK_MaxU16); - if ((textureKey | transformKey | attribKey | fEffectClassID) & kMetaKeyInvalidMask) { - return false; - } - - // effectKey must be first because it is what will be returned by - // GrGLProgramDesc::EffectKeyProvider and passed to the GrGLEffect as its key. b->add32(effectKey); - b->add32(textureKey << 16 | transformKey); - b->add32(fEffectClassID << 16 | attribKey); - return true; } /** Returns a new instance of the appropriate *GL* implementation class @@ -59,8 +60,7 @@ public: return SkNEW_ARGS(GLEffect, (*this, drawEffect)); } - /** This class is a singleton. This function returns the single instance. - */ + /** This class is a singleton. This function returns the single instance. */ static const GrBackendEffectFactory& getInstance() { static SkAlignedSTStorage<1, GrTBackendEffectFactory> gInstanceMem; static const GrTBackendEffectFactory* gInstance; @@ -72,9 +72,7 @@ public: } protected: - GrTBackendEffectFactory() { - fEffectClassID = GenID(); - } + GrTBackendEffectFactory() {} }; #endif diff --git a/src/gpu/gl/GrGLProgramDesc.cpp b/src/gpu/gl/GrGLProgramDesc.cpp index 2c260cd..27a1fab 100644 --- a/src/gpu/gl/GrGLProgramDesc.cpp +++ b/src/gpu/gl/GrGLProgramDesc.cpp @@ -14,13 +14,14 @@ #include "SkChecksum.h" -static inline bool get_key_and_update_stats(const GrEffectStage& stage, - const GrGLCaps& caps, - bool useExplicitLocalCoords, - GrEffectKeyBuilder* b, - bool* setTrueIfReadsDst, - bool* setTrueIfReadsPos, - bool* setTrueIfHasVertexCode) { +bool GrGLProgramDesc::GetEffectKeyAndUpdateStats(const GrEffectStage& stage, + const GrGLCaps& caps, + bool useExplicitLocalCoords, + GrEffectKeyBuilder* b, + uint16_t* effectKeySize, + bool* setTrueIfReadsDst, + bool* setTrueIfReadsPos, + bool* setTrueIfHasVertexCode) { const GrBackendEffectFactory& factory = stage.getEffect()->getFactory(); GrDrawEffect drawEffect(stage, useExplicitLocalCoords); if (stage.getEffect()->willReadDstColor()) { @@ -32,7 +33,17 @@ static inline bool get_key_and_update_stats(const GrEffectStage& stage, if (stage.getEffect()->hasVertexCode()) { *setTrueIfHasVertexCode = true; } - return factory.getGLEffectKey(drawEffect, caps, b); + factory.getGLEffectKey(drawEffect, caps, b); + size_t size = b->size(); + if (size > SK_MaxU16) { + *effectKeySize = 0; // suppresses a warning. + return false; + } + *effectKeySize = SkToU16(size); + if (!GrGLProgramEffects::GenEffectMetaKey(drawEffect, caps, b)) { + return false; + } + return true; } bool GrGLProgramDesc::Build(const GrDrawState& drawState, @@ -105,43 +116,54 @@ bool GrGLProgramDesc::Build(const GrDrawState& drawState, if (!skipCoverage) { numStages += drawState.numCoverageStages() - firstEffectiveCoverageStage; } - GR_STATIC_ASSERT(0 == kEffectKeyLengthsOffset % sizeof(uint32_t)); + GR_STATIC_ASSERT(0 == kEffectKeyOffsetsAndLengthOffset % sizeof(uint32_t)); // Make room for everything up to and including the array of offsets to effect keys. desc->fKey.reset(); - desc->fKey.push_back_n(kEffectKeyLengthsOffset + sizeof(uint32_t) * numStages); + desc->fKey.push_back_n(kEffectKeyOffsetsAndLengthOffset + 2 * sizeof(uint16_t) * numStages); - size_t offset = desc->fKey.count(); - int offsetIndex = 0; + int offsetAndSizeIndex = 0; bool effectKeySuccess = true; if (!skipColor) { for (int s = firstEffectiveColorStage; s < drawState.numColorStages(); ++s) { - uint32_t* offsetLocation = reinterpret_cast(desc->fKey.begin() + - kEffectKeyLengthsOffset + - offsetIndex * sizeof(uint32_t)); - *offsetLocation = offset; - ++offsetIndex; + uint16_t* offsetAndSize = + reinterpret_cast(desc->fKey.begin() + kEffectKeyOffsetsAndLengthOffset + + offsetAndSizeIndex * 2 * sizeof(uint16_t)); GrEffectKeyBuilder b(&desc->fKey); - effectKeySuccess |= get_key_and_update_stats(drawState.getColorStage(s), gpu->glCaps(), - requiresLocalCoordAttrib, &b, &readsDst, - &readFragPosition, &hasVertexCode); - offset += b.size(); + uint16_t effectKeySize; + uint32_t effectOffset = desc->fKey.count(); + effectKeySuccess |= GetEffectKeyAndUpdateStats( + drawState.getColorStage(s), gpu->glCaps(), + requiresLocalCoordAttrib, &b, + &effectKeySize, &readsDst, + &readFragPosition, &hasVertexCode); + effectKeySuccess |= (effectOffset <= SK_MaxU16); + + offsetAndSize[0] = SkToU16(effectOffset); + offsetAndSize[1] = effectKeySize; + ++offsetAndSizeIndex; } } if (!skipCoverage) { for (int s = firstEffectiveCoverageStage; s < drawState.numCoverageStages(); ++s) { - uint32_t* offsetLocation = reinterpret_cast(desc->fKey.begin() + - kEffectKeyLengthsOffset + - offsetIndex * sizeof(uint32_t)); - *offsetLocation = offset; - ++offsetIndex; + uint16_t* offsetAndSize = + reinterpret_cast(desc->fKey.begin() + kEffectKeyOffsetsAndLengthOffset + + offsetAndSizeIndex * 2 * sizeof(uint16_t)); + GrEffectKeyBuilder b(&desc->fKey); - effectKeySuccess |= get_key_and_update_stats(drawState.getCoverageStage(s), - gpu->glCaps(), requiresLocalCoordAttrib, - &b, &readsDst, &readFragPosition, - &hasVertexCode); - offset += b.size(); + uint16_t effectKeySize; + uint32_t effectOffset = desc->fKey.count(); + effectKeySuccess |= GetEffectKeyAndUpdateStats( + drawState.getCoverageStage(s), gpu->glCaps(), + requiresLocalCoordAttrib, &b, + &effectKeySize, &readsDst, + &readFragPosition, &hasVertexCode); + effectKeySuccess |= (effectOffset <= SK_MaxU16); + + offsetAndSize[0] = SkToU16(effectOffset); + offsetAndSize[1] = effectKeySize; + ++offsetAndSizeIndex; } } if (!effectKeySuccess) { diff --git a/src/gpu/gl/GrGLProgramDesc.h b/src/gpu/gl/GrGLProgramDesc.h index d7652f4..c8aae19 100644 --- a/src/gpu/gl/GrGLProgramDesc.h +++ b/src/gpu/gl/GrGLProgramDesc.h @@ -172,14 +172,20 @@ private: // 1. uint32_t for total key length. // 2. uint32_t for a checksum. // 3. Header struct defined above. - // 4. uint32_t offsets to beginning of every effects' key (see 5). + // 4. An array of offsets to effect keys and their sizes (see 5). uint16_t for each + // offset and size. // 5. per-effect keys. Each effect's key is a variable length array of uint32_t. enum { + // Part 1. kLengthOffset = 0, + // Part 2. kChecksumOffset = kLengthOffset + sizeof(uint32_t), + // Part 3. kHeaderOffset = kChecksumOffset + sizeof(uint32_t), kHeaderSize = SkAlign4(sizeof(KeyHeader)), - kEffectKeyLengthsOffset = kHeaderOffset + kHeaderSize, + // Part 4. + // This is the offset in the overall key to the array of per-effect offset,length pairs. + kEffectKeyOffsetsAndLengthOffset = kHeaderOffset + kHeaderSize, }; template T* atOffset() { @@ -194,6 +200,16 @@ private: KeyHeader* header() { return this->atOffset(); } + // Shared code between setRandom() and Build(). + static bool GetEffectKeyAndUpdateStats(const GrEffectStage& stage, + const GrGLCaps& caps, + bool useExplicitLocalCoords, + GrEffectKeyBuilder* b, + uint16_t* effectKeySize, + bool* setTrueIfReadsDst, + bool* setTrueIfReadsPos, + bool* setTrueIfHasVertexCode); + void finalize(); const KeyHeader& getHeader() const { return *this->atOffset(); } @@ -212,9 +228,11 @@ private: } EffectKey get(int index) const { - const uint32_t* offsets = reinterpret_cast(fDesc->fKey.begin() + - kEffectKeyLengthsOffset); - uint32_t offset = offsets[fBaseIndex + index]; + const uint16_t* offsets = reinterpret_cast( + fDesc->fKey.begin() + kEffectKeyOffsetsAndLengthOffset); + // We store two uint16_ts per effect, one for the offset to the effect's key and one for + // its length. Here we just need the offset. + uint16_t offset = offsets[2 * (fBaseIndex + index)]; return *reinterpret_cast(fDesc->fKey.begin() + offset); } private: @@ -225,7 +243,7 @@ private: enum { kMaxPreallocEffects = 8, kIntsPerEffect = 4, // This is an overestimate of the average effect key size. - kPreAllocSize = kEffectKeyLengthsOffset + + kPreAllocSize = kEffectKeyOffsetsAndLengthOffset + kMaxPreallocEffects * sizeof(uint32_t) * kIntsPerEffect, }; diff --git a/src/gpu/gl/GrGLProgramEffects.cpp b/src/gpu/gl/GrGLProgramEffects.cpp index 65d14fd..9936aa5 100644 --- a/src/gpu/gl/GrGLProgramEffects.cpp +++ b/src/gpu/gl/GrGLProgramEffects.cpp @@ -114,6 +114,27 @@ SkMatrix get_transform_matrix(const GrDrawEffect& drawEffect, int transformIdx) //////////////////////////////////////////////////////////////////////////////// +bool GrGLProgramEffects::GenEffectMetaKey(const GrDrawEffect& drawEffect, const GrGLCaps& caps, + GrEffectKeyBuilder* b) { + + EffectKey textureKey = GrGLProgramEffects::GenTextureKey(drawEffect, caps); + EffectKey transformKey = GrGLProgramEffects::GenTransformKey(drawEffect); + EffectKey attribKey = GrGLProgramEffects::GenAttribKey(drawEffect); + uint32_t classID = drawEffect.effect()->getFactory().effectClassID(); + + // Currently we allow 16 bits for each of the above portions of the meta-key. Fail if they + // don't fit. + static const uint32_t kMetaKeyInvalidMask = ~((uint32_t) SK_MaxU16); + if ((textureKey | transformKey | attribKey | classID) & kMetaKeyInvalidMask) { + return false; + } + + uint32_t* key = b->add32n(2); + key[0] = (textureKey << 16 | transformKey); + key[1] = (classID << 16 | attribKey); + return true; +} + EffectKey GrGLProgramEffects::GenAttribKey(const GrDrawEffect& drawEffect) { EffectKey key = 0; int numAttributes = drawEffect.getVertexAttribIndexCount(); diff --git a/src/gpu/gl/GrGLProgramEffects.h b/src/gpu/gl/GrGLProgramEffects.h index 5a2fefd..c4d8843 100644 --- a/src/gpu/gl/GrGLProgramEffects.h +++ b/src/gpu/gl/GrGLProgramEffects.h @@ -9,9 +9,9 @@ #define GrGLProgramEffects_DEFINED #include "GrBackendEffectFactory.h" +#include "GrGLUniformManager.h" #include "GrTexture.h" #include "GrTextureAccess.h" -#include "GrGLUniformManager.h" class GrEffect; class GrEffectStage; @@ -31,11 +31,14 @@ public: typedef GrGLUniformManager::UniformHandle UniformHandle; /** - * These methods generate different portions of an effect's final key. + * This class emits some of the code inserted into the shaders for an effect. The code it + * creates may be dependent on properties of the effect that the effect itself doesn't use + * in its key (e.g. the pixel format of textures used). So this class inserts a meta-key for + * every effect using this function. It is also responsible for inserting the effect's class ID + * which must be different for every GrEffect subclass. It can fail if an effect uses too many + * textures, attributes, etc for the space allotted in the meta-key. */ - static EffectKey GenAttribKey(const GrDrawEffect&); - static EffectKey GenTransformKey(const GrDrawEffect&); - static EffectKey GenTextureKey(const GrDrawEffect&, const GrGLCaps&); + static bool GenEffectMetaKey(const GrDrawEffect&, const GrGLCaps&, GrEffectKeyBuilder*); virtual ~GrGLProgramEffects(); @@ -98,6 +101,13 @@ public: typedef SkTArray TextureSamplerArray; protected: + /** + * Helpers for GenEffectMetaKey. + */ + static EffectKey GenAttribKey(const GrDrawEffect&); + static EffectKey GenTransformKey(const GrDrawEffect&); + static EffectKey GenTextureKey(const GrDrawEffect&, const GrGLCaps&); + GrGLProgramEffects(int reserveCount) : fGLEffects(reserveCount) , fSamplers(reserveCount) { diff --git a/tests/GLProgramsTest.cpp b/tests/GLProgramsTest.cpp index dd0f80f..2264385 100644 --- a/tests/GLProgramsTest.cpp +++ b/tests/GLProgramsTest.cpp @@ -35,42 +35,33 @@ bool GrGLProgramDesc::setRandom(SkRandom* random, int numStages = numColorStages + numCoverageStages; fKey.reset(); - GR_STATIC_ASSERT(0 == kEffectKeyLengthsOffset % sizeof(uint32_t)); + GR_STATIC_ASSERT(0 == kEffectKeyOffsetsAndLengthOffset % sizeof(uint32_t)); // Make room for everything up to and including the array of offsets to effect keys. - fKey.push_back_n(kEffectKeyLengthsOffset + sizeof(uint32_t) * numStages); - - size_t offset = fKey.count(); - int offsetIndex = 0; + fKey.push_back_n(kEffectKeyOffsetsAndLengthOffset + 2 * sizeof(uint16_t) * numStages); bool dstRead = false; bool fragPos = false; bool vertexCode = false; for (int s = 0; s < numStages; ++s) { - uint32_t* offsetLocation = reinterpret_cast(fKey.begin() + - kEffectKeyLengthsOffset + - offsetIndex * sizeof(uint32_t)); - *offsetLocation = offset; - ++offsetIndex; - - const GrBackendEffectFactory& factory = stages[s]->getEffect()->getFactory(); + uint16_t* offsetAndSize = reinterpret_cast(fKey.begin() + + kEffectKeyOffsetsAndLengthOffset + + s * 2 * sizeof(uint16_t)); + uint32_t effectKeyOffset = fKey.count(); + if (effectKeyOffset > SK_MaxU16) { + fKey.reset(); + return false; + } GrDrawEffect drawEffect(*stages[s], useLocalCoords); GrEffectKeyBuilder b(&fKey); - if (!factory.getGLEffectKey(drawEffect, gpu->glCaps(), &b)) { + uint16_t effectKeySize; + if (!GetEffectKeyAndUpdateStats(*stages[s], gpu->glCaps(), useLocalCoords, &b, + &effectKeySize, &dstRead, &fragPos, &vertexCode)) { fKey.reset(); return false; } - if (stages[s]->getEffect()->willReadDstColor()) { - dstRead = true; - } - if (stages[s]->getEffect()->willReadFragmentPosition()) { - fragPos = true; - } - if (stages[s]->getEffect()->hasVertexCode()) { - vertexCode = true; - } - - offset += b.size(); + offsetAndSize[0] = effectKeyOffset; + offsetAndSize[1] = effectKeySize; } KeyHeader* header = this->header();