There were three different problems with lighting filters:

1)  Texture offsets (fImageIncrement) have to be signed depending on
whether the texture is "right way up" (texture upload) or "upside down"
(render target), so the surface normals were coming out upside down.
2)  Light normals have to y-negated on upload
These two bugs were cancelling each other out in SampleApp, (where we
were testing w/textures) but not in Chrome (where we were testing
w/render targets).
3)  The extract-the-height-from-the-view-matrix hack I was using to
compare light positions vs. gl_FragCoord doesn't work in Chrome where we
compile with GR_STATIC_RECT_VB, and the view matrix contains more than
the viewport transform (to accomodate the canonical vertex buffer).
Fixed by passing the destination render target to
GrGLProgramStage::setData(), so it can flip the light positions in Y on
the CPU.

Review URL:  http://codereview.appspot.com/6343109/



git-svn-id: http://skia.googlecode.com/svn/trunk@4605 2bbb7eff-a529-9590-31e7-b0007b416f81

7 files changed, 52 insertions, 33 deletions

diff --git a/src/effects/SkLightingImageFilter.cpp b/src/effects/SkLightingImageFilter.cpp
index 8d1e8e7714..d4e2a3e259 100644
--- a/src/effects/SkLightingImageFilter.cpp
+++ b/src/effects/SkLightingImageFilter.cpp
@@ -35,6 +35,14 @@ void setUniformPoint3(const GrGLInterface* gl, GrGLint location, const SkPoint3&
     GR_GL_CALL(gl, Uniform3f(location, x, y, z));
 }
 
+void setUniformNormal3(const GrGLInterface* gl, GrGLint location, const SkPoint3& point) {
+    setUniformPoint3(gl, location, SkPoint3(point.fX, -point.fY, point.fZ));
+}
+
+void setUniformPoint3FlipY(const GrGLInterface* gl, GrGLint location, const SkPoint3& point, int height) {
+    setUniformPoint3(gl, location, SkPoint3(point.fX, height-point.fY, point.fZ));
+}
+
 // Shift matrix components to the left, as we advance pixels to the right.
 inline void shiftMatrixLeft(int m[9]) {
     m[0] = m[1];
@@ -357,7 +365,7 @@ public:
     virtual void emitSurfaceToLight(SkString* builder, const char* z) const = 0;
     virtual void emitLightColor(SkString* builder, const char *surfaceToLight) const;
     virtual void initUniforms(const GrGLInterface* gl, int programID);
-    virtual void setData(const GrGLInterface*, const SkLight* light) const;
+    virtual void setData(const GrGLInterface*, const GrRenderTarget* rt, const SkLight* light) const;
 
 private:
     typedef SkRefCnt INHERITED;
@@ -374,7 +382,7 @@ public:
     virtual ~GrGLDistantLight() {}
     virtual void setupVariables(GrGLShaderBuilder* state, int stage) SK_OVERRIDE;
     virtual void initUniforms(const GrGLInterface* gl, int programID) SK_OVERRIDE;
-    virtual void setData(const GrGLInterface* gl, const SkLight* light) const SK_OVERRIDE;
+    virtual void setData(const GrGLInterface* gl, const GrRenderTarget* rt, const SkLight* light) const SK_OVERRIDE;
     virtual void emitSurfaceToLight(SkString* builder, const char* z) const SK_OVERRIDE;
 
 private:
@@ -390,7 +398,7 @@ public:
     virtual ~GrGLPointLight() {}
     virtual void setupVariables(GrGLShaderBuilder* state, int stage);
     virtual void initUniforms(const GrGLInterface* gl, int programID);
-    virtual void setData(const GrGLInterface* gl, const SkLight* light) const SK_OVERRIDE;
+    virtual void setData(const GrGLInterface* gl, const GrRenderTarget* rt, const SkLight* light) const SK_OVERRIDE;
     virtual void emitVS(SkString* builder) const;
     virtual void emitSurfaceToLight(SkString* builder, const char* z) const SK_OVERRIDE;
 
@@ -399,7 +407,6 @@ private:
     SkPoint3 fLocation;
     const GrGLShaderVar* fLocationVar;
     int fLocationLocation;
-    const char* fHeightVaryingName;
 };
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -409,7 +416,7 @@ public:
     virtual ~GrGLSpotLight() {}
     virtual void setupVariables(GrGLShaderBuilder* state, int stage);
     virtual void initUniforms(const GrGLInterface* gl, int programID);
-    virtual void setData(const GrGLInterface* gl, const SkLight* light) const SK_OVERRIDE;
+    virtual void setData(const GrGLInterface* gl, const GrRenderTarget* rt, const SkLight* light) const SK_OVERRIDE;
     virtual void emitVS(SkString* builder) const;
     virtual void emitFuncs(SkString* builder) const;
     virtual void emitSurfaceToLight(SkString* builder, const char* z) const SK_OVERRIDE;
@@ -430,7 +437,6 @@ private:
     int                  fConeScaleLocation;
     const GrGLShaderVar* fSVar;
     int                  fSLocation;
-    const char*          fHeightVaryingName;
 };
 
 };
@@ -880,6 +886,7 @@ public:
     virtual void initUniforms(const GrGLInterface*, int programID) SK_OVERRIDE;
     virtual void setData(const GrGLInterface*, 
                          const GrCustomStage&,
+                         const GrRenderTarget*,
                          int stageNum) SK_OVERRIDE;
 
 private:
@@ -904,6 +911,7 @@ public:
     virtual void initUniforms(const GrGLInterface*, int programID) SK_OVERRIDE;
     virtual void setData(const GrGLInterface*, 
                          const GrCustomStage&,
+                         const GrRenderTarget*,
                          int stageNum) SK_OVERRIDE;
 
 private:
@@ -925,6 +933,7 @@ public:
     virtual void initUniforms(const GrGLInterface*, int programID) SK_OVERRIDE;
     virtual void setData(const GrGLInterface*, 
                          const GrCustomStage&,
+                         const GrRenderTarget*,
                          int stageNum) SK_OVERRIDE;
 
 private:
@@ -1066,13 +1075,15 @@ GrGLProgramStage::StageKey GrGLLightingEffect::GenKey(
 
 void GrGLLightingEffect::setData(const GrGLInterface* gl,
                                  const GrCustomStage& data,
+                                 const GrRenderTarget* rt,
                                  int stageNum) {
     const GrLightingEffect& effect =
         static_cast<const GrLightingEffect&>(data);
-    GrTexture& texture = *data.texture(0);
-    GR_GL_CALL(gl, Uniform2f(fImageIncrementLocation, 1.0f / texture.width(), 1.0f / texture.height()));
+    GrGLTexture* texture = static_cast<GrGLTexture*>(data.texture(0));
+    float ySign = texture->orientation() == GrGLTexture::kTopDown_Orientation ? -1.0f : 1.0f;
+    GR_GL_CALL(gl, Uniform2f(fImageIncrementLocation, 1.0f / texture->width(), ySign / texture->height()));
     GR_GL_CALL(gl, Uniform1f(fSurfaceScaleLocation, effect.surfaceScale()));
-    fLight->setData(gl, effect.light());
+    fLight->setData(gl, rt, effect.light());
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -1110,8 +1121,9 @@ void GrGLDiffuseLightingEffect::emitLightFunc(SkString* funcs) {
 
 void GrGLDiffuseLightingEffect::setData(const GrGLInterface* gl,
                                         const GrCustomStage& data,
+                                        const GrRenderTarget* rt,
                                         int stageNum) {
-    INHERITED::setData(gl, data, stageNum);
+    INHERITED::setData(gl, data, rt, stageNum);
     const GrDiffuseLightingEffect& effect =
         static_cast<const GrDiffuseLightingEffect&>(data);
     GR_GL_CALL(gl, Uniform1f(fKDLocation, effect.kd()));
@@ -1179,8 +1191,9 @@ void GrGLSpecularLightingEffect::emitLightFunc(SkString* funcs) {
 
 void GrGLSpecularLightingEffect::setData(const GrGLInterface* gl,
                                          const GrCustomStage& data,
+                                         const GrRenderTarget* rt,
                                          int stageNum) {
-    INHERITED::setData(gl, data, stageNum);
+    INHERITED::setData(gl, data, rt, stageNum);
     const GrSpecularLightingEffect& effect =
         static_cast<const GrSpecularLightingEffect&>(data);
     GR_GL_CALL(gl, Uniform1f(fKSLocation, effect.ks()));
@@ -1204,7 +1217,7 @@ void GrGLLight::initUniforms(const GrGLInterface* gl, int programID) {
         GetUniformLocation(programID, fColorVar->getName().c_str()));
 }
 
-void GrGLLight::setData(const GrGLInterface* gl, const SkLight* light) const {
+void GrGLLight::setData(const GrGLInterface* gl, const GrRenderTarget* rt, const SkLight* light) const {
     setUniformPoint3(gl, fColorVarLocation, light->color() * SkScalarInvert(SkIntToScalar(255)));
 }
 
@@ -1227,11 +1240,11 @@ void GrGLDistantLight::initUniforms(const GrGLInterface* gl, int programID) {
         GetUniformLocation(programID, fDirectionVar->getName().c_str()));
 }
 
-void GrGLDistantLight::setData(const GrGLInterface* gl, const SkLight* light) const {
-    INHERITED::setData(gl, light);
+void GrGLDistantLight::setData(const GrGLInterface* gl, const GrRenderTarget* rt, const SkLight* light) const {
+    INHERITED::setData(gl, rt, light);
     SkASSERT(light->type() == SkLight::kDistant_LightType);
     const SkDistantLight* distantLight = static_cast<const SkDistantLight*>(light);
-    setUniformPoint3(gl, fDirectionLocation, distantLight->direction());
+    setUniformNormal3(gl, fDirectionLocation, distantLight->direction());
 }
 
 void GrGLDistantLight::emitSurfaceToLight(SkString* builder,
@@ -1246,7 +1259,6 @@ void GrGLPointLight::setupVariables(GrGLShaderBuilder* state, int stage) {
     fLocationVar = &state->addUniform(
         GrGLShaderBuilder::kFragment_ShaderType, kVec3f_GrSLType,
         "uLightLocation", stage);
-    state->addVarying(kFloat_GrSLType, "Height", stage, &fHeightVaryingName);
 }
 
 void GrGLPointLight::initUniforms(const GrGLInterface* gl, int programID) {
@@ -1255,23 +1267,21 @@ void GrGLPointLight::initUniforms(const GrGLInterface* gl, int programID) {
         GetUniformLocation(programID, fLocationVar->getName().c_str()));
 }
 
-void GrGLPointLight::setData(const GrGLInterface* gl, const SkLight* light) const {
-    INHERITED::setData(gl, light);
+void GrGLPointLight::setData(const GrGLInterface* gl, const GrRenderTarget* rt, const SkLight* light) const {
+    INHERITED::setData(gl, rt, light);
     SkASSERT(light->type() == SkLight::kPoint_LightType);
     const SkPointLight* pointLight = static_cast<const SkPointLight*>(light);
-    setUniformPoint3(gl, fLocationLocation, pointLight->location());
+    setUniformPoint3FlipY(gl, fLocationLocation, pointLight->location(), rt->height());
 }
 
 void GrGLPointLight::emitVS(SkString* builder) const {
-    // Compute viewport height from the Y scale of the matrix.
-    builder->appendf("\t\t%s = -2.0 / uViewM[1][1];\n", fHeightVaryingName);
 }
 
 void GrGLPointLight::emitSurfaceToLight(SkString* builder,
-                                      const char* z) const {
+                                        const char* z) const {
+    const char *lName = fLocationVar->getName().c_str();
     builder->appendf(
-        "normalize(%s - vec3(gl_FragCoord.x, %s - gl_FragCoord.y, %s))",
-        fLocationVar->getName().c_str(), fHeightVaryingName, z);
+        "normalize(%s - vec3(gl_FragCoord.xy, %s))", lName, z);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -1296,7 +1306,6 @@ void GrGLSpotLight::setupVariables(GrGLShaderBuilder* state, int stage) {
     fSVar = &state->addUniform(
         GrGLShaderBuilder::kFragment_ShaderType,
         kVec3f_GrSLType, "uS", stage);
-    state->addVarying(kFloat_GrSLType, "Height", stage, &fHeightVaryingName);
 }
 
 void GrGLSpotLight::initUniforms(const GrGLInterface* gl, int programID) {
@@ -1315,21 +1324,19 @@ void GrGLSpotLight::initUniforms(const GrGLInterface* gl, int programID) {
         GetUniformLocation(programID, fSVar->getName().c_str()));
 }
 
-void GrGLSpotLight::setData(const GrGLInterface* gl, const SkLight* light) const {
-    INHERITED::setData(gl, light);
+void GrGLSpotLight::setData(const GrGLInterface* gl, const GrRenderTarget* rt, const SkLight* light) const {
+    INHERITED::setData(gl, rt, light);
     SkASSERT(light->type() == SkLight::kSpot_LightType);
     const SkSpotLight* spotLight = static_cast<const SkSpotLight *>(light);
-    setUniformPoint3(gl, fLocationLocation, spotLight->location());
+    setUniformPoint3FlipY(gl, fLocationLocation, spotLight->location(), rt->height());
     GR_GL_CALL(gl, Uniform1f(fExponentLocation, spotLight->specularExponent()));
     GR_GL_CALL(gl, Uniform1f(fCosInnerConeAngleLocation, spotLight->cosInnerConeAngle()));
     GR_GL_CALL(gl, Uniform1f(fCosOuterConeAngleLocation, spotLight->cosOuterConeAngle()));
     GR_GL_CALL(gl, Uniform1f(fConeScaleLocation, spotLight->coneScale()));
-    setUniformPoint3(gl, fSLocation, spotLight->s());
+    setUniformNormal3(gl, fSLocation, spotLight->s());
 }
 
 void GrGLSpotLight::emitVS(SkString* builder) const {
-    // Compute viewport height from the Y scale of the matrix.
-    builder->appendf("\t\t%s = -2.0 / uViewM[1][1];\n", fHeightVaryingName);
 }
 
 void GrGLSpotLight::emitFuncs(SkString* builder) const {
@@ -1347,7 +1354,8 @@ void GrGLSpotLight::emitFuncs(SkString* builder) const {
 }
 
 void GrGLSpotLight::emitSurfaceToLight(SkString* builder, const char* z) const {
-    builder->appendf("normalize(%s - vec3(gl_FragCoord.x, %s - gl_FragCoord.y, %s))", fLocationVar->getName().c_str(), fHeightVaryingName, z);
+    builder->appendf(
+        "normalize(%s - vec3(gl_FragCoord.xy, %s))", fLocationVar->getName().c_str(), z);
 }
 
 void GrGLSpotLight::emitLightColor(SkString* builder, const char *surfaceToLight) const {
diff --git a/src/gpu/effects/GrConvolutionEffect.cpp b/src/gpu/effects/GrConvolutionEffect.cpp
index 4970d6270d..33f61b6ab6 100644
--- a/src/gpu/effects/GrConvolutionEffect.cpp
+++ b/src/gpu/effects/GrConvolutionEffect.cpp
@@ -29,6 +29,7 @@ public:
 
     virtual void setData(const GrGLInterface*,
                          const GrCustomStage&,
+                         const GrRenderTarget*,
                          int stageNum) SK_OVERRIDE;
 
     static inline StageKey GenKey(const GrCustomStage&);
@@ -118,6 +119,7 @@ void GrGLConvolutionEffect::initUniforms(const GrGLInterface* gl,
 
 void GrGLConvolutionEffect::setData(const GrGLInterface* gl,
                                     const GrCustomStage& data,
+                                    const GrRenderTarget*,
                                     int stageNum) {
     const GrConvolutionEffect& conv =
         static_cast<const GrConvolutionEffect&>(data);
diff --git a/src/gpu/effects/GrGradientEffects.cpp b/src/gpu/effects/GrGradientEffects.cpp
index d4719b3a24..3125657af2 100644
--- a/src/gpu/effects/GrGradientEffects.cpp
+++ b/src/gpu/effects/GrGradientEffects.cpp
@@ -88,6 +88,7 @@ public:
     virtual void initUniforms(const GrGLInterface*, int programID) SK_OVERRIDE;
     virtual void setData(const GrGLInterface*,
                          const GrCustomStage&,
+                         const GrRenderTarget*,
                          int stageNum) SK_OVERRIDE;
 
     static StageKey GenKey(const GrCustomStage& s) {
@@ -258,6 +259,7 @@ void GrGLRadial2Gradient::initUniforms(const GrGLInterface* gl, int programID) {
 
 void GrGLRadial2Gradient::setData(const GrGLInterface* gl,
                                   const GrCustomStage& baseData,
+                                  const GrRenderTarget*,
                                   int stageNum) {
     const GrRadial2Gradient& data =
         static_cast<const GrRadial2Gradient&>(baseData);
@@ -344,6 +346,7 @@ public:
     virtual void initUniforms(const GrGLInterface*, int programID) SK_OVERRIDE;
     virtual void setData(const GrGLInterface*,
                          const GrCustomStage&,
+                         const GrRenderTarget*,
                          int stageNum) SK_OVERRIDE;
 
     static StageKey GenKey(const GrCustomStage& s) {
@@ -573,6 +576,7 @@ void GrGLConical2Gradient::initUniforms(const GrGLInterface* gl, int programID)
 
 void GrGLConical2Gradient::setData(const GrGLInterface* gl,
                                    const GrCustomStage& baseData,
+                                   const GrRenderTarget*,
                                    int stageNum) {
     const GrConical2Gradient& data =
         static_cast<const GrConical2Gradient&>(baseData);
diff --git a/src/gpu/effects/GrMorphologyEffect.cpp b/src/gpu/effects/GrMorphologyEffect.cpp
index 3ea2cbd125..180b5eb3a6 100644
--- a/src/gpu/effects/GrMorphologyEffect.cpp
+++ b/src/gpu/effects/GrMorphologyEffect.cpp
@@ -32,6 +32,7 @@ public:
     virtual void initUniforms(const GrGLInterface*, int programID) SK_OVERRIDE;
     virtual void setData(const GrGLInterface*, 
                          const GrCustomStage&,
+                         const GrRenderTarget*,
                          int stageNum) SK_OVERRIDE;
 
 private:
@@ -124,6 +125,7 @@ GrGLProgramStage::StageKey GrGLMorphologyEffect::GenKey(
 
 void GrGLMorphologyEffect ::setData(const GrGLInterface* gl,
                                     const GrCustomStage& data,
+                                    const GrRenderTarget*,
                                     int stageNum) {
     const Gr1DKernelEffect& kern =
         static_cast<const Gr1DKernelEffect&>(data);
diff --git a/src/gpu/gl/GrGLProgramStage.cpp b/src/gpu/gl/GrGLProgramStage.cpp
index b5e4f4068c..c1d4942015 100644
--- a/src/gpu/gl/GrGLProgramStage.cpp
+++ b/src/gpu/gl/GrGLProgramStage.cpp
@@ -27,6 +27,7 @@ void GrGLProgramStage::initUniforms(const GrGLInterface*, int progID) {
 
 void GrGLProgramStage::setData(const GrGLInterface*,
                                const GrCustomStage&,
+                               const GrRenderTarget*,
                                int stageNum) {
 }
 
diff --git a/src/gpu/gl/GrGLProgramStage.h b/src/gpu/gl/GrGLProgramStage.h
index 9cf2c1aa97..d5c79b4f65 100644
--- a/src/gpu/gl/GrGLProgramStage.h
+++ b/src/gpu/gl/GrGLProgramStage.h
@@ -83,6 +83,7 @@ public:
         created in emit*(). */
     virtual void setData(const GrGLInterface* gl,
                          const GrCustomStage& stage,
+                         const GrRenderTarget* renderTarget,
                          int stageNum);
 
     const char* name() const { return fFactory.name(); }
diff --git a/src/gpu/gl/GrGpuGL_program.cpp b/src/gpu/gl/GrGpuGL_program.cpp
index a7187d548c..af9bb054a4 100644
--- a/src/gpu/gl/GrGpuGL_program.cpp
+++ b/src/gpu/gl/GrGpuGL_program.cpp
@@ -480,7 +480,8 @@ bool GrGpuGL::flushGraphicsState(DrawType type) {
                             this->getDrawState().getTexture(s));
                     fProgramData->fCustomStage[s]->setData(
                         this->glInterface(),
-                        *sampler.getCustomStage(), s);
+                        *sampler.getCustomStage(),
+                        drawState.getRenderTarget(), s);
                 }
             }
         }