Make GrGLShaderBuilder check whether GrEffect advertised that it would require the dst color or fragment position
R=senorblanco@chromium.org, robertphillips@google.com
Author: bsalomon@google.com
Review URL: https://chromiumcodereview.appspot.com/14998007
git-svn-id: http://skia.googlecode.com/svn/trunk@9074 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/include/gpu/GrConfig.h b/include/gpu/GrConfig.h
index 94c65d8..95e518a 100644
--- a/include/gpu/GrConfig.h
+++ b/include/gpu/GrConfig.h
@@ -280,10 +280,13 @@
#define GrAlwaysAssert(COND) GR_ALWAYSASSERT(COND)
/**
- * Crash from unrecoverable condition, optionally with a message.
+ * Crash from unrecoverable condition, optionally with a message. The debug variants only
+ * crash in a debug build. The message versions print the message regardless of release vs debug.
*/
inline void GrCrash() { GrAlwaysAssert(false); }
inline void GrCrash(const char* msg) { GrPrintf(msg); GrAlwaysAssert(false); }
+inline void GrDebugCrash() { GrAssert(false); }
+inline void GrDebugCrash(const char* msg) { GrPrintf(msg); GrAssert(false); }
/**
* GR_DEBUGCODE compiles the code X in debug builds only
diff --git a/include/gpu/GrEffect.h b/include/gpu/GrEffect.h
index 4cef430..20dc5da 100644
--- a/include/gpu/GrEffect.h
+++ b/include/gpu/GrEffect.h
@@ -153,7 +153,10 @@
GrTexture* texture(int index) const { return this->textureAccess(index).getTexture(); }
/** Will this effect read the destination pixel value? */
- bool willReadDst() const { return fWillReadDst; }
+ bool willReadDstColor() const { return fWillReadDstColor; }
+
+ /** Will this effect read the fragment position? */
+ bool willReadFragmentPosition() const { return fWillReadFragmentPosition; }
int numVertexAttribs() const { return fVertexAttribTypes.count(); }
@@ -213,7 +216,7 @@
*/
void addVertexAttrib(GrSLType type);
- GrEffect() : fWillReadDst(false), fEffectRef(NULL) {}
+ GrEffect() : fWillReadDstColor(false), fWillReadFragmentPosition(false), fEffectRef(NULL) {}
/** This should be called by GrEffect subclass factories. See the comment on AutoEffectUnref for
an example factory function. */
@@ -269,7 +272,14 @@
* from its constructor. Otherwise, when its generated backend-specific effect class attempts
* to generate code that reads the destination pixel it will fail.
*/
- void setWillReadDst() { fWillReadDst = true; }
+ void setWillReadDstColor() { fWillReadDstColor = true; }
+
+ /**
+ * If the effect will generate a backend-specific effect that will read the fragment position
+ * in the FS then it must call this method from its constructor. Otherwise, the request to
+ * access the fragment position will be denied.
+ */
+ void setWillReadFragmentPosition() { fWillReadFragmentPosition = true; }
private:
bool isEqual(const GrEffect& other) const {
@@ -302,7 +312,8 @@
SkSTArray<4, const GrTextureAccess*, true> fTextureAccesses;
SkSTArray<kMaxVertexAttribs, GrSLType, true> fVertexAttribTypes;
- bool fWillReadDst;
+ bool fWillReadDstColor;
+ bool fWillReadFragmentPosition;
GrEffectRef* fEffectRef;
typedef GrRefCnt INHERITED;
diff --git a/src/core/SkXfermode.cpp b/src/core/SkXfermode.cpp
index eb03ef9..7896061 100644
--- a/src/core/SkXfermode.cpp
+++ b/src/core/SkXfermode.cpp
@@ -1358,7 +1358,7 @@
GR_DECLARE_EFFECT_TEST;
private:
- XferEffect(SkXfermode::Mode mode) : fMode(mode) { this->setWillReadDst(); }
+ XferEffect(SkXfermode::Mode mode) : fMode(mode) { this->setWillReadDstColor(); }
virtual bool onIsEqual(const GrEffect& other) const SK_OVERRIDE { return true; }
SkXfermode::Mode fMode;
diff --git a/src/effects/SkLightingImageFilter.cpp b/src/effects/SkLightingImageFilter.cpp
index bb2927b..c86bf6b 100644
--- a/src/effects/SkLightingImageFilter.cpp
+++ b/src/effects/SkLightingImageFilter.cpp
@@ -447,6 +447,7 @@
virtual ~GrGLDistantLight() {}
virtual void setData(const GrGLUniformManager&, const SkLight* light) const SK_OVERRIDE;
virtual void emitSurfaceToLight(GrGLShaderBuilder*, const char* z) SK_OVERRIDE;
+
private:
typedef GrGLLight INHERITED;
UniformHandle fDirectionUni;
@@ -459,6 +460,7 @@
virtual ~GrGLPointLight() {}
virtual void setData(const GrGLUniformManager&, const SkLight* light) const SK_OVERRIDE;
virtual void emitSurfaceToLight(GrGLShaderBuilder*, const char* z) SK_OVERRIDE;
+
private:
typedef GrGLLight INHERITED;
SkPoint3 fLocation;
@@ -473,6 +475,7 @@
virtual void setData(const GrGLUniformManager&, const SkLight* light) const SK_OVERRIDE;
virtual void emitSurfaceToLight(GrGLShaderBuilder*, const char* z) SK_OVERRIDE;
virtual void emitLightColor(GrGLShaderBuilder*, const char *surfaceToLight) SK_OVERRIDE;
+
private:
typedef GrGLLight INHERITED;
@@ -509,6 +512,8 @@
virtual bool isEqual(const SkLight& other) const {
return fColor == other.fColor;
}
+ // Called to know whether the generated GrGLLight will require access to the fragment position.
+ virtual bool requiresFragmentPosition() const = 0;
protected:
SkLight(SkColor color)
@@ -553,6 +558,8 @@
return NULL;
#endif
}
+ virtual bool requiresFragmentPosition() const SK_OVERRIDE { return false; }
+
virtual bool isEqual(const SkLight& other) const SK_OVERRIDE {
if (other.type() != kDistant_LightType) {
return false;
@@ -604,6 +611,7 @@
return NULL;
#endif
}
+ virtual bool requiresFragmentPosition() const SK_OVERRIDE { return true; }
virtual bool isEqual(const SkLight& other) const SK_OVERRIDE {
if (other.type() != kPoint_LightType) {
return false;
@@ -674,6 +682,7 @@
return NULL;
#endif
}
+ virtual bool requiresFragmentPosition() const SK_OVERRIDE { return true; }
virtual LightType type() const { return kSpot_LightType; }
const SkPoint3& location() const { return fLocation; }
const SkPoint3& target() const { return fTarget; }
@@ -1044,6 +1053,9 @@
, fLight(light)
, fSurfaceScale(surfaceScale) {
fLight->ref();
+ if (light->requiresFragmentPosition()) {
+ this->setWillReadFragmentPosition();
+ }
}
GrLightingEffect::~GrLightingEffect() {
diff --git a/src/gpu/GrAAHairLinePathRenderer.cpp b/src/gpu/GrAAHairLinePathRenderer.cpp
index a857dc4..4b10cea 100644
--- a/src/gpu/GrAAHairLinePathRenderer.cpp
+++ b/src/gpu/GrAAHairLinePathRenderer.cpp
@@ -666,6 +666,7 @@
private:
HairLineEdgeEffect() {
this->addVertexAttrib(kVec4f_GrSLType);
+ this->setWillReadFragmentPosition();
}
virtual bool onIsEqual(const GrEffect& other) const SK_OVERRIDE {
diff --git a/src/gpu/GrAARectRenderer.cpp b/src/gpu/GrAARectRenderer.cpp
index 28dae24..0b593b9 100644
--- a/src/gpu/GrAARectRenderer.cpp
+++ b/src/gpu/GrAARectRenderer.cpp
@@ -220,6 +220,7 @@
GrRectEffect() : GrEffect() {
this->addVertexAttrib(kVec4f_GrSLType);
this->addVertexAttrib(kVec2f_GrSLType);
+ this->setWillReadFragmentPosition();
}
virtual bool onIsEqual(const GrEffect&) const SK_OVERRIDE { return true; }
diff --git a/src/gpu/GrDrawState.h b/src/gpu/GrDrawState.h
index e34e4ab..91a8723 100644
--- a/src/gpu/GrDrawState.h
+++ b/src/gpu/GrDrawState.h
@@ -453,7 +453,7 @@
*/
bool willEffectReadDst() const {
for (int s = 0; s < kNumStages; ++s) {
- if (this->isStageEnabled(s) && (*this->getStage(s).getEffect())->willReadDst()) {
+ if (this->isStageEnabled(s) && (*this->getStage(s).getEffect())->willReadDstColor()) {
return true;
}
}
diff --git a/src/gpu/gl/GrGLProgramDesc.cpp b/src/gpu/gl/GrGLProgramDesc.cpp
index 1ae6aa6..a1cd85e 100644
--- a/src/gpu/gl/GrGLProgramDesc.cpp
+++ b/src/gpu/gl/GrGLProgramDesc.cpp
@@ -87,7 +87,7 @@
const GrBackendEffectFactory& factory = effect->getFactory();
GrDrawEffect drawEffect(drawState.getStage(s), requiresLocalCoordAttrib);
desc->fEffectKeys[s] = factory.glEffectKey(drawEffect, gpu->glCaps());
- if (effect->willReadDst()) {
+ if (effect->willReadDstColor()) {
readsDst = true;
}
} else {
diff --git a/src/gpu/gl/GrGLShaderBuilder.cpp b/src/gpu/gl/GrGLShaderBuilder.cpp
index c1732b2..98d7e4c 100644
--- a/src/gpu/gl/GrGLShaderBuilder.cpp
+++ b/src/gpu/gl/GrGLShaderBuilder.cpp
@@ -230,6 +230,14 @@
}
const char* GrGLShaderBuilder::dstColor() {
+ if (fCodeStage.inStageCode()) {
+ const GrEffectRef& effect = *fCodeStage.effectStage()->getEffect();
+ if (!effect->willReadDstColor()) {
+ GrDebugCrash("GrGLEffect asked for dst color but its generating GrEffect "
+ "did not request access.");
+ return "";
+ }
+ }
static const char kFBFetchColorName[] = "gl_LastFragData[0]";
GrGLCaps::FBFetchType fetchType = fCtxInfo.caps()->fbFetchType();
if (GrGLCaps::kEXT_FBFetchType == fetchType) {
@@ -241,7 +249,7 @@
} else if (fDstCopySampler.isInitialized()) {
return kDstCopyColorName;
} else {
- return NULL;
+ return "";
}
}
@@ -457,6 +465,14 @@
}
const char* GrGLShaderBuilder::fragmentPosition() {
+ if (fCodeStage.inStageCode()) {
+ const GrEffectRef& effect = *fCodeStage.effectStage()->getEffect();
+ if (!effect->willReadFragmentPosition()) {
+ GrDebugCrash("GrGLEffect asked for frag position but its generating GrEffect "
+ "did not request access.");
+ return "";
+ }
+ }
#if 1
if (fCtxInfo.caps()->fragCoordConventionsSupport()) {
if (!fSetupFragPosition) {
diff --git a/tests/GLProgramsTest.cpp b/tests/GLProgramsTest.cpp
index 95187a6..d996cd7 100644
--- a/tests/GLProgramsTest.cpp
+++ b/tests/GLProgramsTest.cpp
@@ -63,7 +63,7 @@
const GrBackendEffectFactory& factory = (*stages[s]->getEffect())->getFactory();
GrDrawEffect drawEffect(*stages[s], useLocalCoords);
fEffectKeys[s] = factory.glEffectKey(drawEffect, gpu->glCaps());
- if ((*stages[s]->getEffect())->willReadDst()) {
+ if ((*stages[s]->getEffect())->willReadDstColor()) {
dstRead = true;
}
}