Moved ambient lights out of SkLight's light array

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2287553002

Review-Url: https://codereview.chromium.org/2287553002

4 files changed, 60 insertions, 67 deletions

diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index fc1897526c..c4ce75c6c0 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -3231,8 +3231,7 @@ void SkCanvas::onDrawShadowedPicture(const SkPicture* picture,
         // skip over ambient lights; they don't cast shadows
         // lights that have shadow maps do not need updating (because lights are immutable)
 
-        if (SkLights::Light::kAmbient_LightType == fLights->light(i).type() ||
-            fLights->light(i).getShadowMap() != nullptr) {
+        if (fLights->light(i).getShadowMap() != nullptr) {
             continue;
         }
 
diff --git a/src/core/SkLightingShader.cpp b/src/core/SkLightingShader.cpp
index 8fde8bfd37..b788b3c645 100644
--- a/src/core/SkLightingShader.cpp
+++ b/src/core/SkLightingShader.cpp
@@ -122,11 +122,9 @@ public:
     LightingFP(sk_sp<GrFragmentProcessor> normalFP, sk_sp<SkLights> lights) {
 
         // fuse all ambient lights into a single one
-        fAmbientColor.set(0.0f, 0.0f, 0.0f);
+        fAmbientColor = lights->ambientLightColor();
         for (int i = 0; i < lights->numLights(); ++i) {
-            if (SkLights::Light::kAmbient_LightType == lights->light(i).type()) {
-                fAmbientColor += lights->light(i).color();
-            } else if (SkLights::Light::kDirectional_LightType == lights->light(i).type()) {
+            if (SkLights::Light::kDirectional_LightType == lights->light(i).type()) {
                 fDirectionalLights.push_back(lights->light(i));
                 // TODO get the handle to the shadow map if there is one
             } else {
diff --git a/src/core/SkLights.cpp b/src/core/SkLights.cpp
index 7d8f9ed7a5..962997fc31 100644
--- a/src/core/SkLights.cpp
+++ b/src/core/SkLights.cpp
@@ -10,11 +10,18 @@
 #include "SkReadBuffer.h"
 
 sk_sp<SkLights> SkLights::MakeFromBuffer(SkReadBuffer& buf) {
+    Builder builder;
+
+    SkColor3f ambColor;
+    if (!buf.readScalarArray(&ambColor.fX, 3)) {
+        return nullptr;
+    }
+
+    builder.setAmbientLightColor(ambColor);
+
     int numLights = buf.readInt();
 
-    Builder builder;
     for (int l = 0; l < numLights; ++l) {
-        bool isAmbient = buf.readBool();
         bool isPoint = buf.readBool();
 
         SkColor3f color;
@@ -22,35 +29,29 @@ sk_sp<SkLights> SkLights::MakeFromBuffer(SkReadBuffer& buf) {
             return nullptr;
         }
 
-        if (isAmbient) {
-            builder.add(Light::MakeAmbient(color));
-        } else {
-            SkVector3 dirOrPos;
-            if (!buf.readScalarArray(&dirOrPos.fX, 3)) {
-                return nullptr;
-            }
-            SkScalar intensity = 0.0f;
-            if (isPoint) {
-                intensity = buf.readScalar();
-            }
+        SkVector3 dirOrPos;
+        if (!buf.readScalarArray(&dirOrPos.fX, 3)) {
+            return nullptr;
+        }
 
-            sk_sp<SkImage> depthMap;
-            bool hasShadowMap = buf.readBool();
-            if (hasShadowMap) {
-                if (!(depthMap = buf.readImage())) {
-                    return nullptr;
-                }
+        sk_sp<SkImage> depthMap;
+        bool hasShadowMap = buf.readBool();
+        if (hasShadowMap) {
+            if (!(depthMap = buf.readImage())) {
+                return nullptr;
             }
+        }
 
-            if (isPoint) {
-                Light light = Light::MakePoint(color, dirOrPos, intensity);
-                light.setShadowMap(depthMap);
-                builder.add(light);
-            } else {
-                Light light = Light::MakeDirectional(color, dirOrPos);
-                light.setShadowMap(depthMap);
-                builder.add(light);
-            }
+        if (isPoint) {
+            SkScalar intensity = 0.0f;
+            intensity = buf.readScalar();
+            Light light = Light::MakePoint(color, dirOrPos, intensity);
+            light.setShadowMap(depthMap);
+            builder.add(light);
+        } else {
+            Light light = Light::MakeDirectional(color, dirOrPos);
+            light.setShadowMap(depthMap);
+            builder.add(light);
         }
     }
 
@@ -58,29 +59,24 @@ sk_sp<SkLights> SkLights::MakeFromBuffer(SkReadBuffer& buf) {
 }
 
 void SkLights::flatten(SkWriteBuffer& buf) const {
+    buf.writeScalarArray(&this->ambientLightColor().fX, 3);
 
     buf.writeInt(this->numLights());
     for (int l = 0; l < this->numLights(); ++l) {
         const Light& light = this->light(l);
 
-        bool isAmbient = Light::kAmbient_LightType == light.type();
         bool isPoint = Light::kPoint_LightType == light.type();
 
-        buf.writeBool(isAmbient);
         buf.writeBool(isPoint);
         buf.writeScalarArray(&light.color().fX, 3);
-        if (!isAmbient) {
-            if (isPoint) {
-                buf.writeScalarArray(&light.pos().fX, 3);
-                buf.writeScalar(light.intensity());
-            } else {
-                buf.writeScalarArray(&light.dir().fX, 3);
-            }
-            bool hasShadowMap = light.getShadowMap() != nullptr;
-            buf.writeBool(hasShadowMap);
-            if (hasShadowMap) {
-                buf.writeImage(light.getShadowMap());
-            }
+        buf.writeScalarArray(&light.dir().fX, 3);
+        bool hasShadowMap = light.getShadowMap() != nullptr;
+        buf.writeBool(hasShadowMap);
+        if (hasShadowMap) {
+            buf.writeImage(light.getShadowMap());
+        }
+        if (isPoint) {
+            buf.writeScalar(light.intensity());
         }
     }
 }
diff --git a/src/core/SkShadowShader.cpp b/src/core/SkShadowShader.cpp
index 49f1b01b32..b1f49b851d 100644
--- a/src/core/SkShadowShader.cpp
+++ b/src/core/SkShadowShader.cpp
@@ -111,24 +111,23 @@ public:
              const SkShadowParams& params,
              GrContext* context) {
 
-        // fuse all ambient lights into a single one
-        fAmbientColor.set(0.0f, 0.0f, 0.0f);
+        fAmbientColor = lights->ambientLightColor();
 
         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 (fNumNonAmbLights < SkShadowShader::kMaxNonAmbientLights) {
+            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 {
+
+                if (SkLights::Light::kDirectional_LightType == lights->light(i).type()) {
                     fLightDirOrPos[fNumNonAmbLights] = lights->light(i).dir();
                     fLightIntensity[fNumNonAmbLights] = 0.0f;
+                } else if (SkLights::Light::kPoint_LightType == lights->light(i).type()) {
+                    fLightDirOrPos[fNumNonAmbLights] = lights->light(i).pos();
+                    fLightIntensity[fNumNonAmbLights] = lights->light(i).intensity();
                 }
+
                 fIsPointLight[fNumNonAmbLights] =
-                         SkLights::Light::kPoint_LightType == lights->light(i).type();
+                        SkLights::Light::kPoint_LightType == lights->light(i).type();
 
                 SkImage_Base* shadowMap = ((SkImage_Base*)lights->light(i).getShadowMap());
 
@@ -697,27 +696,28 @@ void SkShadowShaderImpl::ShadowShaderContext::shadeSpan(int x, int y,
 
             SkColor3f accum = SkColor3f::Make(0.0f, 0.0f, 0.0f);
             // This is all done in linear unpremul color space (each component 0..255.0f though)
+
+            accum.fX += lightShader.fLights->ambientLightColor().fX * SkColorGetR(diffColor);
+            accum.fY += lightShader.fLights->ambientLightColor().fY * SkColorGetG(diffColor);
+            accum.fZ += lightShader.fLights->ambientLightColor().fZ * SkColorGetB(diffColor);
+
             for (int l = 0; l < lightShader.fLights->numLights(); ++l) {
                 const SkLights::Light& light = lightShader.fLights->light(l);
 
-                if (SkLights::Light::kAmbient_LightType == light.type()) {
-                    accum.fX += light.color().fX * SkColorGetR(diffColor);
-                    accum.fY += light.color().fY * SkColorGetG(diffColor);
-                    accum.fZ += light.color().fZ * SkColorGetB(diffColor);
-                } else if (SkLights::Light::kDirectional_LightType == light.type()) {
+                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);
+                                SkColorGetR(diffColor);
                     accum.fY += light.color().makeScale(light.dir().fZ).fY *
-                                                        SkColorGetG(diffColor);
+                                SkColorGetG(diffColor);
                     accum.fZ += light.color().makeScale(light.dir().fZ).fZ *
-                                                        SkColorGetB(diffColor);
+                                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);
                 }
+
             }
 
             result[i] = convert(accum, SkColorGetA(diffColor));