Revert of Makes GrGLProgramDesc's key store the lengths as well as offsets of the effect keys. (https://codereview.chromium.org/379113004/)
Reason for revert:
Most likely candidate for Valgrind failures:
[21:10:08.755668] ==3036== Use of uninitialised value of size 8
[21:10:08.755753] ==3036== at 0x734AB2: GrGpuGL::ProgramCache::getProgram(GrGLProgramDesc const&, GrEffectStage const**, GrEffectStage const**) (GrGpuGL_program.cpp:107)
[21:10:08.755788] ==3036== by 0x734ED2: GrGpuGL::flushGraphicsState(GrGpu::DrawType, GrDeviceCoordTexture const*) (GrGpuGL_program.cpp:253)
[21:10:08.755811] ==3036== by 0x6E81C2: GrGpu::setupClipAndFlushState(GrGpu::DrawType, GrDeviceCoordTexture const*, GrDrawState::AutoRestoreEffects*, SkRect const*) (GrGpu.cpp:350)
[21:10:08.755837] ==3036== by 0x6E9BE8: GrGpu::onDraw(GrDrawTarget::DrawInfo const&) (GrGpu.cpp:390)
[21:10:08.755858] ==3036== by 0x6EEECE: GrInOrderDrawBuffer::flush() (GrDrawTarget.h:506)
[21:10:08.755879] ==3036== by 0x6D0EB4: GrContext::flush(int) (GrContext.cpp:1327)
[21:10:08.755900] ==3036== by 0x6D3F8F: GrContext::writeTexturePixels(GrTexture*, int, int, int, int, GrPixelConfig, void const*, unsigned long, unsigned int) (GrContext.cpp:1349)
[21:10:08.755922] ==3036== by 0x6D39D7: GrContext::writeRenderTargetPixels(GrRenderTarget*, int, int, int, int, GrPixelConfig, void const*, unsigned long, unsigned int) (GrContext.cpp:1632)
[21:10:08.755949] ==3036== by 0x6FFDF3: GrRenderTarget::writePixels(int, int, int, int, GrPixelConfig, void const*, unsigned long, unsigned int) (GrRenderTarget.cpp:45)
[21:10:08.755978] ==3036== by 0x735563: SkGpuDevice::onWritePixels(SkImageInfo const&, void const*, unsigned long, int, int) (SkGpuDevice.cpp:280)
[21:10:08.756003] ==3036== by 0x57A048: SkBaseDevice::writePixels(SkImageInfo const&, void const*, unsigned long, int, int) (SkDevice.cpp:106)
[21:10:08.756025] ==3036== by 0x56D0AE: SkCanvas::writePixels(SkImageInfo const&, void const*, unsigned long, int, int) (SkCanvas.cpp:700)
[21:10:08.756050] ==3036== by 0x56D156: SkCanvas::writePixels(SkBitmap const&, int, int) (SkCanvas.cpp:652)
[21:10:08.756077] ==3036== by 0x5109B6: test_WritePixels(skiatest::Reporter*, GrContextFactory*) (WritePixelsTest.cpp:464)
[21:10:08.756099] ==3036== by 0x51114C: skiatest::WritePixelsClass::onRun(skiatest::Reporter*) (WritePixelsTest.cpp:361)
[21:10:08.756122] ==3036== by 0x406BE8: skiatest::Test::run() (Test.cpp:107)
[21:10:08.756145] ==3036== by 0x4064C2: SkTestRunnable::run() (skia_test.cpp:109)
[21:10:08.756167] ==3036== by 0x405D1A: tool_main(int, char**) (skia_test.cpp:221)
[21:10:08.756189] ==3036== by 0x405F75: main (skia_test.cpp:239)
[21:10:08.756211] ==3036== Uninitialised value was created by a stack allocation
[21:10:08.756233] ==3036== at 0x734CC8: GrGpuGL::flushGraphicsState(GrGpu::DrawType, GrDeviceCoordTexture const*) (GrGpuGL_program.cpp:213)
Original issue's description:
> 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, bsalomon@google.com
TBR=bsalomon@google.com, jvanverth@google.com, robertphillips@google.com
NOTREECHECKS=true
NOTRY=true
Author: mtklein@google.com
Review URL: https://codereview.chromium.org/394213002
diff --git a/include/gpu/GrBackendEffectFactory.h b/include/gpu/GrBackendEffectFactory.h
index ef9e436..32f14f2 100644
--- a/include/gpu/GrBackendEffectFactory.h
+++ b/include/gpu/GrBackendEffectFactory.h
@@ -14,12 +14,22 @@
#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 incorporates each per-effect key into a larger shader key.
+ * Used by effects to build their keys. It incorpates each per-effect key into a larger shader key.
*/
class GrEffectKeyBuilder {
public:
@@ -32,14 +42,6 @@
fData->push_back_n(4, reinterpret_cast<uint8_t*>(&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<uint32_t*>(fData->push_back_n(4 * count));
- }
-
size_t size() const { return sizeof(uint32_t) * fCount; }
private:
@@ -47,74 +49,43 @@
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<MyEffect>::getInstance();
- * }
- *
- * Using GrTBackendEffectFactory places a few constraints on the effect. See that class's comments.
- */
class GrBackendEffectFactory : SkNoncopyable {
public:
typedef uint32_t EffectKey;
- /**
- * 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 bool getGLEffectKey(const GrDrawEffect&, const GrGLCaps&, GrEffectKeyBuilder*) const = 0;
virtual GrGLEffect* createGLInstance(const GrDrawEffect&) const = 0;
- /**
- * Produces a human-reable name for the effect.
- */
+ bool operator ==(const GrBackendEffectFactory& b) const {
+ return fEffectClassID == b.fEffectClassID;
+ }
+ bool operator !=(const GrBackendEffectFactory& b) const {
+ return !(*this == b);
+ }
+
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,
};
- static uint32_t GenID() {
+ GrBackendEffectFactory() {
+ fEffectClassID = kIllegalEffectClassID;
+ }
+ virtual ~GrBackendEffectFactory() {}
+
+ static int32_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.
- uint32_t id = static_cast<uint32_t>(sk_atomic_inc(&fCurrEffectClassID)) + 1;
- if (!id) {
- SkFAIL("This should never wrap as it should only be called once for each GrEffect "
- "subclass.");
- }
+ int32_t id = sk_atomic_inc(&fCurrEffectClassID) + 1;
return id;
}
- const uint32_t fEffectClassID;
+ int32_t fEffectClassID;
+
+private:
static int32_t fCurrEffectClassID;
};
diff --git a/include/gpu/GrTBackendEffectFactory.h b/include/gpu/GrTBackendEffectFactory.h
index 251e8c7..cd4c0a4 100644
--- a/include/gpu/GrTBackendEffectFactory.h
+++ b/include/gpu/GrTBackendEffectFactory.h
@@ -13,26 +13,7 @@
#include "gl/GrGLProgramEffects.h"
/**
- * 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<MyEffect>::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.
+ * Implements GrBackendEffectFactory for a GrEffect subclass as a singleton.
*/
template <typename EffectClass>
class GrTBackendEffectFactory : public GrBackendEffectFactory {
@@ -40,17 +21,35 @@
public:
typedef typename EffectClass::GLEffect GLEffect;
- /** Returns a human-readable name for the effect. Implemented using GLEffect::Name as described
- * in this class's comment. */
+ /** Returns a human-readable name that is accessible via GrEffect or
+ GrGLEffect and is consistent between the two of them.
+ */
virtual const char* name() const SK_OVERRIDE { return EffectClass::Name(); }
-
- /** Implemented using GLEffect::GenKey as described in this class's comment. */
- virtual void getGLEffectKey(const GrDrawEffect& drawEffect,
+ /** 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,
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
@@ -60,7 +59,8 @@
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,7 +72,9 @@
}
protected:
- GrTBackendEffectFactory() {}
+ GrTBackendEffectFactory() {
+ fEffectClassID = GenID();
+ }
};
#endif
diff --git a/src/gpu/gl/GrGLProgramDesc.cpp b/src/gpu/gl/GrGLProgramDesc.cpp
index 1807b84..2c260cd 100644
--- a/src/gpu/gl/GrGLProgramDesc.cpp
+++ b/src/gpu/gl/GrGLProgramDesc.cpp
@@ -14,14 +14,13 @@
#include "SkChecksum.h"
-bool GrGLProgramDesc::GetEffectKeyAndUpdateStats(const GrEffectStage& stage,
- const GrGLCaps& caps,
- bool useExplicitLocalCoords,
- GrEffectKeyBuilder* b,
- uint16_t* effectKeySize,
- bool* setTrueIfReadsDst,
- bool* setTrueIfReadsPos,
- bool* setTrueIfHasVertexCode) {
+static inline bool get_key_and_update_stats(const GrEffectStage& stage,
+ const GrGLCaps& caps,
+ bool useExplicitLocalCoords,
+ GrEffectKeyBuilder* b,
+ bool* setTrueIfReadsDst,
+ bool* setTrueIfReadsPos,
+ bool* setTrueIfHasVertexCode) {
const GrBackendEffectFactory& factory = stage.getEffect()->getFactory();
GrDrawEffect drawEffect(stage, useExplicitLocalCoords);
if (stage.getEffect()->willReadDstColor()) {
@@ -33,17 +32,7 @@
if (stage.getEffect()->hasVertexCode()) {
*setTrueIfHasVertexCode = true;
}
- 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;
+ return factory.getGLEffectKey(drawEffect, caps, b);
}
bool GrGLProgramDesc::Build(const GrDrawState& drawState,
@@ -116,54 +105,43 @@
if (!skipCoverage) {
numStages += drawState.numCoverageStages() - firstEffectiveCoverageStage;
}
- GR_STATIC_ASSERT(0 == kEffectKeyOffsetsAndLengthOffset % sizeof(uint32_t));
+ GR_STATIC_ASSERT(0 == kEffectKeyLengthsOffset % 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(kEffectKeyOffsetsAndLengthOffset + 2 * sizeof(uint32_t) * numStages);
+ desc->fKey.push_back_n(kEffectKeyLengthsOffset + sizeof(uint32_t) * numStages);
- int offsetAndSizeIndex = 0;
+ size_t offset = desc->fKey.count();
+ int offsetIndex = 0;
bool effectKeySuccess = true;
if (!skipColor) {
for (int s = firstEffectiveColorStage; s < drawState.numColorStages(); ++s) {
- uint16_t* offsetAndSize =
- reinterpret_cast<uint16_t*>(desc->fKey.begin() + kEffectKeyOffsetsAndLengthOffset +
- offsetAndSizeIndex * 2 * sizeof(uint16_t));
+ uint32_t* offsetLocation = reinterpret_cast<uint32_t*>(desc->fKey.begin() +
+ kEffectKeyLengthsOffset +
+ offsetIndex * sizeof(uint32_t));
+ *offsetLocation = offset;
+ ++offsetIndex;
GrEffectKeyBuilder b(&desc->fKey);
- 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;
+ effectKeySuccess |= get_key_and_update_stats(drawState.getColorStage(s), gpu->glCaps(),
+ requiresLocalCoordAttrib, &b, &readsDst,
+ &readFragPosition, &hasVertexCode);
+ offset += b.size();
}
}
if (!skipCoverage) {
for (int s = firstEffectiveCoverageStage; s < drawState.numCoverageStages(); ++s) {
- uint16_t* offsetAndSize =
- reinterpret_cast<uint16_t*>(desc->fKey.begin() + kEffectKeyOffsetsAndLengthOffset +
- offsetAndSizeIndex * 2 * sizeof(uint16_t));
-
+ uint32_t* offsetLocation = reinterpret_cast<uint32_t*>(desc->fKey.begin() +
+ kEffectKeyLengthsOffset +
+ offsetIndex * sizeof(uint32_t));
+ *offsetLocation = offset;
+ ++offsetIndex;
GrEffectKeyBuilder b(&desc->fKey);
- 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;
+ effectKeySuccess |= get_key_and_update_stats(drawState.getCoverageStage(s),
+ gpu->glCaps(), requiresLocalCoordAttrib,
+ &b, &readsDst, &readFragPosition,
+ &hasVertexCode);
+ offset += b.size();
}
}
if (!effectKeySuccess) {
diff --git a/src/gpu/gl/GrGLProgramDesc.h b/src/gpu/gl/GrGLProgramDesc.h
index c8aae19..d7652f4 100644
--- a/src/gpu/gl/GrGLProgramDesc.h
+++ b/src/gpu/gl/GrGLProgramDesc.h
@@ -172,20 +172,14 @@
// 1. uint32_t for total key length.
// 2. uint32_t for a checksum.
// 3. Header struct defined above.
- // 4. An array of offsets to effect keys and their sizes (see 5). uint16_t for each
- // offset and size.
+ // 4. uint32_t offsets to beginning of every effects' key (see 5).
// 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)),
- // Part 4.
- // This is the offset in the overall key to the array of per-effect offset,length pairs.
- kEffectKeyOffsetsAndLengthOffset = kHeaderOffset + kHeaderSize,
+ kEffectKeyLengthsOffset = kHeaderOffset + kHeaderSize,
};
template<typename T, size_t OFFSET> T* atOffset() {
@@ -200,16 +194,6 @@
KeyHeader* header() { return this->atOffset<KeyHeader, kHeaderOffset>(); }
- // 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<KeyHeader, kHeaderOffset>(); }
@@ -228,11 +212,9 @@
}
EffectKey get(int index) const {
- const uint16_t* offsets = reinterpret_cast<const uint16_t*>(
- 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)];
+ const uint32_t* offsets = reinterpret_cast<const uint32_t*>(fDesc->fKey.begin() +
+ kEffectKeyLengthsOffset);
+ uint32_t offset = offsets[fBaseIndex + index];
return *reinterpret_cast<const EffectKey*>(fDesc->fKey.begin() + offset);
}
private:
@@ -243,7 +225,7 @@
enum {
kMaxPreallocEffects = 8,
kIntsPerEffect = 4, // This is an overestimate of the average effect key size.
- kPreAllocSize = kEffectKeyOffsetsAndLengthOffset +
+ kPreAllocSize = kEffectKeyLengthsOffset +
kMaxPreallocEffects * sizeof(uint32_t) * kIntsPerEffect,
};
diff --git a/src/gpu/gl/GrGLProgramEffects.cpp b/src/gpu/gl/GrGLProgramEffects.cpp
index 9936aa5..65d14fd 100644
--- a/src/gpu/gl/GrGLProgramEffects.cpp
+++ b/src/gpu/gl/GrGLProgramEffects.cpp
@@ -114,27 +114,6 @@
////////////////////////////////////////////////////////////////////////////////
-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 c4d8843..5a2fefd 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,14 +31,11 @@
typedef GrGLUniformManager::UniformHandle UniformHandle;
/**
- * 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.
+ * These methods generate different portions of an effect's final key.
*/
- static bool GenEffectMetaKey(const GrDrawEffect&, const GrGLCaps&, GrEffectKeyBuilder*);
+ static EffectKey GenAttribKey(const GrDrawEffect&);
+ static EffectKey GenTransformKey(const GrDrawEffect&);
+ static EffectKey GenTextureKey(const GrDrawEffect&, const GrGLCaps&);
virtual ~GrGLProgramEffects();
@@ -101,13 +98,6 @@
typedef SkTArray<TextureSampler> 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 2264385..dd0f80f 100644
--- a/tests/GLProgramsTest.cpp
+++ b/tests/GLProgramsTest.cpp
@@ -35,33 +35,42 @@
int numStages = numColorStages + numCoverageStages;
fKey.reset();
- GR_STATIC_ASSERT(0 == kEffectKeyOffsetsAndLengthOffset % sizeof(uint32_t));
+ GR_STATIC_ASSERT(0 == kEffectKeyLengthsOffset % sizeof(uint32_t));
// Make room for everything up to and including the array of offsets to effect keys.
- fKey.push_back_n(kEffectKeyOffsetsAndLengthOffset + 2 * sizeof(uint16_t) * numStages);
+ fKey.push_back_n(kEffectKeyLengthsOffset + sizeof(uint32_t) * numStages);
+
+ size_t offset = fKey.count();
+ int offsetIndex = 0;
bool dstRead = false;
bool fragPos = false;
bool vertexCode = false;
for (int s = 0; s < numStages; ++s) {
- uint16_t* offsetAndSize = reinterpret_cast<uint16_t*>(fKey.begin() +
- kEffectKeyOffsetsAndLengthOffset +
- s * 2 * sizeof(uint16_t));
- uint32_t effectKeyOffset = fKey.count();
- if (effectKeyOffset > SK_MaxU16) {
- fKey.reset();
- return false;
- }
+ uint32_t* offsetLocation = reinterpret_cast<uint32_t*>(fKey.begin() +
+ kEffectKeyLengthsOffset +
+ offsetIndex * sizeof(uint32_t));
+ *offsetLocation = offset;
+ ++offsetIndex;
+
+ const GrBackendEffectFactory& factory = stages[s]->getEffect()->getFactory();
GrDrawEffect drawEffect(*stages[s], useLocalCoords);
GrEffectKeyBuilder b(&fKey);
- uint16_t effectKeySize;
- if (!GetEffectKeyAndUpdateStats(*stages[s], gpu->glCaps(), useLocalCoords, &b,
- &effectKeySize, &dstRead, &fragPos, &vertexCode)) {
+ if (!factory.getGLEffectKey(drawEffect, gpu->glCaps(), &b)) {
fKey.reset();
return false;
}
- offsetAndSize[0] = effectKeyOffset;
- offsetAndSize[1] = effectKeySize;
+ if (stages[s]->getEffect()->willReadDstColor()) {
+ dstRead = true;
+ }
+ if (stages[s]->getEffect()->willReadFragmentPosition()) {
+ fragPos = true;
+ }
+ if (stages[s]->getEffect()->hasVertexCode()) {
+ vertexCode = true;
+ }
+
+ offset += b.size();
}
KeyHeader* header = this->header();