Added distance attenuation and diffuse shading to PointLights

src/core/SkShadowShader.cpp

Index: src/core/SkShadowShader.cpp
diff --git a/src/core/SkShadowShader.cpp b/src/core/SkShadowShader.cpp
index 804258321a80f9ab0bd20b7ffa995878ab079b17..341e18562f28ee768a6963618fd6f448c91ce299 100644
--- a/src/core/SkShadowShader.cpp
+++ b/src/core/SkShadowShader.cpp
@@ -111,26 +111,35 @@ public:
         // fuse all ambient lights into a single one
         fAmbientColor.set(0.0f, 0.0f, 0.0f);
 
-        fNumDirLights = 0; // refers to directional lights.
+        fNumNonAmbLights = 0; // count of non-ambient lights
         for (int i = 0; i < lights->numLights(); ++i) {
             if (SkLights::Light::kAmbient_LightType == lights->light(i).type()) {
                 fAmbientColor += lights->light(i).color();
-            } else if (fNumDirLights < SkShadowShader::kMaxNonAmbientLights) {
-                fLightColor[fNumDirLights] = lights->light(i).color();
-                fLightDir[fNumDirLights] = lights->light(i).dir();
+            } else if (fNumNonAmbLights < SkShadowShader::kMaxNonAmbientLights) {
+                fLightColor[fNumNonAmbLights] = lights->light(i).color();
+                if (lights->light(i).type() == SkLights::Light::kPoint_LightType) {
+                    fLightDirOrPos[fNumNonAmbLights] = lights->light(i).pos();
+                    fLightIntensity[fNumNonAmbLights] = lights->light(i).intensity();
+                } else {
+                    fLightDirOrPos[fNumNonAmbLights] = lights->light(i).dir();
+                    fLightIntensity[fNumNonAmbLights] = 0.0f;
+                }
+                fIsPointLight[fNumNonAmbLights] =
+                         SkLights::Light::kPoint_LightType == lights->light(i).type();
+
                 SkImage_Base* shadowMap = ((SkImage_Base*)lights->light(i).getShadowMap());
 
                 // gets deleted when the ShadowFP is destroyed, and frees the GrTexture*
-                fTexture[fNumDirLights] = sk_sp<GrTexture>(shadowMap->asTextureRef(context,
+                fTexture[fNumNonAmbLights] = sk_sp<GrTexture>(shadowMap->asTextureRef(context,
                                                            GrTextureParams::ClampNoFilter(),
                                                            SkSourceGammaTreatment::kIgnore));
-                fDepthMapAccess[fNumDirLights].reset(fTexture[fNumDirLights].get());
-                this->addTextureAccess(&fDepthMapAccess[fNumDirLights]);
+                fDepthMapAccess[fNumNonAmbLights].reset(fTexture[fNumNonAmbLights].get());
+                this->addTextureAccess(&fDepthMapAccess[fNumNonAmbLights]);
 
-                fDepthMapHeight[fNumDirLights] = shadowMap->height();
-                fDepthMapWidth[fNumDirLights] = shadowMap->width();
+                fDepthMapHeight[fNumNonAmbLights] = shadowMap->height();
+                fDepthMapWidth[fNumNonAmbLights] = shadowMap->width();
 
-                fNumDirLights++;
+                fNumNonAmbLights++;
             }
         }
 
@@ -150,34 +159,29 @@ public:
 
             GrGLSLFragmentBuilder* fragBuilder = args.fFragBuilder;
             GrGLSLUniformHandler* uniformHandler = args.fUniformHandler;
+            const ShadowFP& shadowFP = args.fFp.cast<ShadowFP>();
 
-            // add uniforms
-            int32_t numLights = args.fFp.cast<ShadowFP>().fNumDirLights;
-            SkASSERT(numLights <= SkShadowShader::kMaxNonAmbientLights);
+            SkASSERT(shadowFP.fNumNonAmbLights <= SkShadowShader::kMaxNonAmbientLights);
 
+            // add uniforms
             const char* lightDirUniName[SkShadowShader::kMaxNonAmbientLights] = {nullptr};
             const char* lightColorUniName[SkShadowShader::kMaxNonAmbientLights] = {nullptr};
+            const char* lightIntensityUniName[SkShadowShader::kMaxNonAmbientLights] = {nullptr};
 
-            const char* depthMapWidthUniName[SkShadowShader::kMaxNonAmbientLights]
-                    = {nullptr};
-            const char* depthMapHeightUniName[SkShadowShader::kMaxNonAmbientLights]
-                    = {nullptr};
+            const char* depthMapWidthUniName[SkShadowShader::kMaxNonAmbientLights] = {nullptr};
+            const char* depthMapHeightUniName[SkShadowShader::kMaxNonAmbientLights] = {nullptr};
 
-            SkString lightDirUniNameBase("lightDir");
-            SkString lightColorUniNameBase("lightColor");
-
-            SkString depthMapWidthUniNameBase("dmapWidth");
-            SkString depthMapHeightUniNameBase("dmapHeight");
-
-            for (int i = 0; i < numLights; i++) {
-                SkString lightDirUniNameStr(lightDirUniNameBase);
+            for (int i = 0; i < shadowFP.fNumNonAmbLights; i++) {

[robertphillips, 2016/08/23 16:51:02]

Should lightDir be lightDirOrPos ?

[vjiaoblack, 2016/08/23 18:28:52]

Done.

+                SkString lightDirUniNameStr("lightDir");
                 lightDirUniNameStr.appendf("%d", i);
-                SkString lightColorUniNameStr(lightColorUniNameBase);
+                SkString lightColorUniNameStr("lightColor");
                 lightColorUniNameStr.appendf("%d", i);
+                SkString lightIntensityUniNameStr("lightIntensity");
+                lightIntensityUniNameStr.appendf("%d", i);
 
-                SkString depthMapWidthUniNameStr(depthMapWidthUniNameBase);
+                SkString depthMapWidthUniNameStr("dmapWidth");
                 depthMapWidthUniNameStr.appendf("%d", i);
-                SkString depthMapHeightUniNameStr(depthMapHeightUniNameBase);
+                SkString depthMapHeightUniNameStr("dmapHeight");
                 depthMapHeightUniNameStr.appendf("%d", i);
 
                 fLightDirUni[i] = uniformHandler->addUniform(kFragment_GrShaderFlag,
@@ -190,6 +194,12 @@ public:
                                                                kDefault_GrSLPrecision,
                                                                lightColorUniNameStr.c_str(),
                                                                &lightColorUniName[i]);
+                fLightIntensityUni[i] =
+                        uniformHandler->addUniform(kFragment_GrShaderFlag,
+                                                   kFloat_GrSLType,
+                                                   kDefault_GrSLPrecision,
+                                                   lightIntensityUniNameStr.c_str(),
+                                                   &lightIntensityUniName[i]);
 
                 fDepthMapWidthUni[i]  = uniformHandler->addUniform(kFragment_GrShaderFlag,
                                                    kInt_GrSLType,
@@ -203,7 +213,6 @@ public:
                                                    &depthMapHeightUniName[i]);
             }
 
-
             const char* widthUniName = nullptr;
             const char* heightUniName = nullptr;
 
@@ -216,7 +225,6 @@ public:
                                                     kDefault_GrSLPrecision,
                                                     "height", &heightUniName);
 
-
             SkString povDepth("povDepth");
             this->emitChild(0, nullptr, &povDepth, args);
 
@@ -225,15 +233,19 @@ public:
 
             SkString depthMaps[SkShadowShader::kMaxNonAmbientLights];
 
-            for (int i = 0; i < numLights; i++) {
+            // Multiply by 255 to transform from sampler coordinates to world
+            // coordinates (since 1 channel is 0xFF)
+            fragBuilder->codeAppendf("vec3 worldCor = vec3(vMatrixCoord_0_1_Stage0 * "
+                                                     "vec2(%s, %s), %s.b * 255);\n",
+                                     widthUniName, heightUniName, povDepth.c_str());
+
+            for (int i = 0; i < shadowFP.fNumNonAmbLights; i++) {
                 SkString povCoord("povCoord");
                 povCoord.appendf("%d", i);
 
                 // vMatrixCoord_0_1_Stage0 is the texture sampler coordinates.
                 // povDepth.b * 255 scales it to 0 - 255, bringing it to world space,
-                // and the / 400 brings it back to a sampler coordinate, 0 - 1
-                // The 400 comes from the shadowmaps GM.
-                // TODO use real shadowmaps size
+                // and the / vec2(width, height) brings it back to a sampler coordinate
                 SkString offset("offset");
                 offset.appendf("%d", i);
 
@@ -243,9 +255,23 @@ public:
                 SkString scaleOffsetVec("scaleOffsetVec");
                 scaleOffsetVec.appendf("%d", i);
 
-                fragBuilder->codeAppendf("vec2 %s = vec2(%s) * povDepth.b * 255 / 400;\n",
-                                         offset.c_str(), lightDirUniName[i]);
-
+                fragBuilder->codeAppendf("vec2 %s;", offset.c_str());
+
+                if (shadowFP.fIsPointLight[i]) {
+                    fragBuilder->codeAppendf("vec3 fragToLight%d = %s - worldCor;\n",
+                                             i, lightDirUniName[i]);
+                    fragBuilder->codeAppendf("float distsq%d = dot(fragToLight%d, fragToLight%d);"
+                                             "fragToLight%d = normalize(fragToLight%d);",
+                                             i, i, i, i, i);
+                    fragBuilder->codeAppendf("%s = -vec2(%s.x - worldCor.x, worldCor.y - %s.y)*"
+                                                    "(povDepth.b) / vec2(%s, %s);\n",
+                                             offset.c_str(), lightDirUniName[i],
+                                             lightDirUniName[i], widthUniName, heightUniName);

[jvanverth1, 2016/08/23 17:25:22]

Could you store 1/w, 1/h in these uniforms and so avoid the divide every
fragment? Also, why not combine them into a vec2?

[vjiaoblack, 2016/08/23 18:28:52]

Yes; I wanted to make a final run for efficiency after more of the code has
landed, to avoid making redundant changes in separate branches though. That
okay?

+                } else {
+                    fragBuilder->codeAppendf("%s = vec2(%s) * povDepth.b * 255 / vec2(%s, %s);\n",
+                                             offset.c_str(), lightDirUniName[i],
+                                             widthUniName, heightUniName);
+                }
                 fragBuilder->codeAppendf("vec2 %s = (vec2(%s, %s) / vec2(%s, %s));\n",
                                          scaleVec.c_str(),
                                          widthUniName, heightUniName,
@@ -254,11 +280,9 @@ public:
                 fragBuilder->codeAppendf("vec2 %s = 1 - %s;\n",
                                          scaleOffsetVec.c_str(), scaleVec.c_str());
 
-
                 fragBuilder->codeAppendf("vec2 %s = (vMatrixCoord_0_1_Stage0 + "
-                                                    "vec2(%s.x, 0 - %s.y)) "
+                                                   "vec2(%s.x, 0 - %s.y)) "
                                                    " * %s + vec2(0,1) * %s;\n",
-
                                          povCoord.c_str(), offset.c_str(), offset.c_str(),
                                          scaleVec.c_str(), scaleOffsetVec.c_str());
 
@@ -274,20 +298,29 @@ public:
 
             fragBuilder->codeAppendf("vec4 resultDiffuseColor = %s;", diffuseColor.c_str());
 
-            // Essentially,
             // diffColor * (ambientLightTot + foreachDirLight(lightColor * (N . L)))
             SkString totalLightColor("totalLightColor");
             fragBuilder->codeAppendf("vec3 %s = vec3(0);", totalLightColor.c_str());
 
-            for (int i = 0; i < numLights; i++) {
-                fragBuilder->codeAppendf("if (%s.b >= %s.b) {",
-                                         povDepth.c_str(), depthMaps[i].c_str());
-                // Note that dot(vec3(0,0,1), %s) == %s.z * %s
-                fragBuilder->codeAppendf("%s += %s.z * %s;",
-                                         totalLightColor.c_str(),
-                                         lightDirUniName[i],
-                                         lightColorUniName[i]);
-                fragBuilder->codeAppendf("}");
+            for (int i = 0; i < shadowFP.fNumNonAmbLights; i++) {
+                if (!shadowFP.fIsPointLight[i]) {
+                    fragBuilder->codeAppendf("if (%s.b >= %s.b) {",
+                                             povDepth.c_str(), depthMaps[i].c_str());
+                    // Note that dot(vec3(0,0,1), %s) == %s.z * %s
+                    fragBuilder->codeAppendf("%s += %s.z * %s;",
+                                             totalLightColor.c_str(),
+                                             lightDirUniName[i],
+                                             lightColorUniName[i]);
+                    fragBuilder->codeAppendf("}");
+                } else {
+                    // fragToLight%d.z is equal to the fragToLight dot the surface normal.
+                    fragBuilder->codeAppendf("%s += fragToLight%d.z * %s / "

[jvanverth1, 2016/08/23 17:25:22]

Clamp fragToLight.z to be >= 0. The light could have a smaller z than the
primitive so you'd end up with a negative value.

[vjiaoblack, 2016/08/23 18:28:52]

Done.

+                                                   "(1 + distsq%d / (%s * %s));",

[jvanverth1, 2016/08/23 17:25:22]

Still dividing by intensity^2 here.

[vjiaoblack, 2016/08/23 18:28:52]

I think this preserves conceptual clarity right now, and in the future will
either be an optimization or another design discussion. Is it okay to leave it?

+                                             totalLightColor.c_str(), i,
+                                             lightColorUniName[i], i,
+                                             lightIntensityUniName[i],
+                                             lightIntensityUniName[i]);
+                }
             }
 
             fragBuilder->codeAppendf("%s += %s;",
@@ -303,7 +336,12 @@ public:
         static void GenKey(const GrProcessor& proc, const GrGLSLCaps&,
                            GrProcessorKeyBuilder* b) {
             const ShadowFP& shadowFP = proc.cast<ShadowFP>();
-            b->add32(shadowFP.fNumDirLights);
+            b->add32(shadowFP.fNumNonAmbLights);
+            int isPL = 0;
+            for (int i = 0; i < SkShadowShader::kMaxNonAmbientLights; i++) {
+                isPL = isPL | ((shadowFP.fIsPointLight[i] ? 1 : 0) << i);
+            }
+            b->add32(isPL);
         }
 
     protected:
@@ -313,17 +351,27 @@ public:
             fNumDirLights = shadowFP.numLights();
 
             for (int i = 0; i < fNumDirLights; i++) {
-                const SkVector3& lightDir = shadowFP.lightDir(i);
-                if (lightDir != fLightDir[i]) {
-                    pdman.set3fv(fLightDirUni[i], 1, &lightDir.fX);
-                    fLightDir[i] = lightDir;
+                bool isPoint = shadowFP.isPointLight(i);
+                fIsPointLight[i] = isPoint;
+
+                const SkVector3& lightDirOrPos = shadowFP.lightDirOrPos(i);
+                if (lightDirOrPos != fLightDir[i]) {
+                    pdman.set3fv(fLightDirUni[i], 1, &lightDirOrPos.fX);

[robertphillips, 2016/08/23 16:51:02]

Should fLightDir be fLightDirOrPos ?

[vjiaoblack, 2016/08/23 18:28:52]

Done.

+                    fLightDir[i] = lightDirOrPos;
                 }
+
                 const SkColor3f& lightColor = shadowFP.lightColor(i);
                 if (lightColor != fLightColor[i]) {
                     pdman.set3fv(fLightColorUni[i], 1, &lightColor.fX);
                     fLightColor[i] = lightColor;
                 }
 
+                SkScalar lightIntensity = shadowFP.lightIntensity(i);
+                if (lightIntensity != fLightIntensity[i]) {
+                    pdman.set1f(fLightIntensityUni[i], lightIntensity);
+                    fLightIntensity[i] = lightIntensity;
+                }
+
                 int depthMapWidth = shadowFP.depthMapWidth(i);
                 if (depthMapWidth != fDepthMapWidth[i]) {
                     pdman.set1i(fDepthMapWidthUni[i], depthMapWidth);
@@ -355,6 +403,8 @@ public:
         }
 
     private:
+        bool fIsPointLight[SkShadowShader::kMaxNonAmbientLights];
+
         SkVector3 fLightDir[SkShadowShader::kMaxNonAmbientLights];

[jvanverth1, 2016/08/23 17:25:22]

fLightDirOrPos

[vjiaoblack, 2016/08/23 18:28:52]

Done.

         GrGLSLProgramDataManager::UniformHandle
                 fLightDirUni[SkShadowShader::kMaxNonAmbientLights];

[jvanverth1, 2016/08/23 17:25:22]

fLightDirOrPosUni

[vjiaoblack, 2016/08/23 18:28:52]

Done.

@@ -363,6 +413,10 @@ public:
         GrGLSLProgramDataManager::UniformHandle
                 fLightColorUni[SkShadowShader::kMaxNonAmbientLights];
 
+        SkScalar fLightIntensity[SkShadowShader::kMaxNonAmbientLights];
+        GrGLSLProgramDataManager::UniformHandle
+                fLightIntensityUni[SkShadowShader::kMaxNonAmbientLights];
+
         int fDepthMapWidth[SkShadowShader::kMaxNonAmbientLights];
         GrGLSLProgramDataManager::UniformHandle
                 fDepthMapWidthUni[SkShadowShader::kMaxNonAmbientLights];
@@ -391,23 +445,31 @@ public:
     void onComputeInvariantOutput(GrInvariantOutput* inout) const override {
         inout->mulByUnknownFourComponents();
     }
-    int32_t numLights() const { return fNumDirLights; }
+    int32_t numLights() const { return fNumNonAmbLights; }
     const SkColor3f& ambientColor() const { return fAmbientColor; }
-    const SkVector3& lightDir(int i) const {
-        SkASSERT(i < fNumDirLights);
-        return fLightDir[i];
+    bool isPointLight(int i) const {
+        SkASSERT(i < fNumNonAmbLights);
+        return fIsPointLight[i];
+    }
+    const SkVector3& lightDirOrPos(int i) const {
+        SkASSERT(i < fNumNonAmbLights);
+        return fLightDirOrPos[i];
     }
     const SkVector3& lightColor(int i) const {
-        SkASSERT(i < fNumDirLights);
+        SkASSERT(i < fNumNonAmbLights);
         return fLightColor[i];
     }
+    SkScalar lightIntensity(int i) const {
+        SkASSERT(i < fNumNonAmbLights);
+        return fLightIntensity[i];
+    }
 
     int depthMapWidth(int i) const {
-        SkASSERT(i < fNumDirLights);
+        SkASSERT(i < fNumNonAmbLights);
         return fDepthMapWidth[i];
     }
     int depthMapHeight(int i) const {
-        SkASSERT(i < fNumDirLights);
+        SkASSERT(i < fNumNonAmbLights);
         return fDepthMapHeight[i];
     }
     int width() const {return fWidth; }
@@ -418,7 +480,7 @@ private:
 
     bool onIsEqual(const GrFragmentProcessor& proc) const override {
         const ShadowFP& shadowFP = proc.cast<ShadowFP>();

[robertphillips, 2016/08/23 16:51:02]

\n

[vjiaoblack, 2016/08/23 18:28:52]

Done.

-        if (fAmbientColor != shadowFP.fAmbientColor || fNumDirLights != shadowFP.fNumDirLights) {
+        if (fAmbientColor != shadowFP.fAmbientColor || fNumNonAmbLights != shadowFP.fNumNonAmbLights) {
             return false;
         }
 
@@ -426,9 +488,11 @@ private:
             return false;
         }
 
-        for (int i = 0; i < fNumDirLights; i++) {
-            if (fLightDir[i] != shadowFP.fLightDir[i] ||
-                fLightColor[i] != shadowFP.fLightColor[i]) {
+        for (int i = 0; i < fNumNonAmbLights; i++) {
+            if (fLightDirOrPos[i] != shadowFP.fLightDirOrPos[i] ||
+                fLightColor[i] != shadowFP.fLightColor[i] ||
+                fLightIntensity[i] != shadowFP.fLightIntensity[i] ||
+                fIsPointLight[i] != shadowFP.fIsPointLight[i]) {
                 return false;
             }
 
@@ -441,10 +505,12 @@ private:
         return true;
     }
 
-    int              fNumDirLights;
+    int              fNumNonAmbLights;
 
-    SkVector3        fLightDir[SkShadowShader::kMaxNonAmbientLights];
+    bool             fIsPointLight[SkShadowShader::kMaxNonAmbientLights];
+    SkVector3        fLightDirOrPos[SkShadowShader::kMaxNonAmbientLights];
     SkColor3f        fLightColor[SkShadowShader::kMaxNonAmbientLights];
+    SkScalar         fLightIntensity[SkShadowShader::kMaxNonAmbientLights];
     GrTextureAccess  fDepthMapAccess[SkShadowShader::kMaxNonAmbientLights];
     sk_sp<GrTexture> fTexture[SkShadowShader::kMaxNonAmbientLights];
 
@@ -561,11 +627,19 @@ void SkShadowShaderImpl::ShadowShaderContext::shadeSpan(int x, int y,
                     accum.fX += light.color().fX * SkColorGetR(diffColor);
                     accum.fY += light.color().fY * SkColorGetG(diffColor);
                     accum.fZ += light.color().fZ * SkColorGetB(diffColor);
-                } else {
+                } else if (SkLights::Light::kDirectional_LightType == light.type()) {
                     // scaling by fZ accounts for lighting direction
-                    accum.fX += light.color().makeScale(light.dir().fZ).fX * SkColorGetR(diffColor);
-                    accum.fY += light.color().makeScale(light.dir().fZ).fY * SkColorGetG(diffColor);
-                    accum.fZ += light.color().makeScale(light.dir().fZ).fZ * SkColorGetB(diffColor);
+                    accum.fX += light.color().makeScale(light.dir().fZ).fX *
+                                                        SkColorGetR(diffColor);
+                    accum.fY += light.color().makeScale(light.dir().fZ).fY *
+                                                        SkColorGetG(diffColor);
+                    accum.fZ += light.color().makeScale(light.dir().fZ).fZ *
+                                                        SkColorGetB(diffColor);
+                } else {
+                    // TODO: do point lights for raster, currently treated like ambient
+                    accum.fX += light.color().fX * SkColorGetR(diffColor);
+                    accum.fY += light.color().fY * SkColorGetG(diffColor);
+                    accum.fZ += light.color().fZ * SkColorGetB(diffColor);
                 }
             }