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

14 files changed, 87 insertions, 87 deletions

diff --git a/gm/lightingshader.cpp b/gm/lightingshader.cpp
index 4a79ebea1d..137fd64a26 100644
--- a/gm/lightingshader.cpp
+++ b/gm/lightingshader.cpp
@@ -52,7 +52,7 @@ public:
                                                      SkVector3::Make(SK_ScalarRoot2Over2,
                                                                      0.0f,
                                                                      SK_ScalarRoot2Over2)));
-        builder.add(SkLights::Light::MakeAmbient(SkColor3f::Make(0.2f, 0.2f, 0.2f)));
+        builder.setAmbientLightColor(SkColor3f::Make(0.2f, 0.2f, 0.2f));
 
         fLights = builder.finish();
     }
diff --git a/gm/lightingshader2.cpp b/gm/lightingshader2.cpp
index 5afe2558b9..886c647ff7 100644
--- a/gm/lightingshader2.cpp
+++ b/gm/lightingshader2.cpp
@@ -47,12 +47,12 @@ protected:
         SkLights::Builder builder;
         builder.add(SkLights::Light::MakeDirectional(SkColor3f::Make(1.0f, 1.0f, 1.0f),
                                                      kLightFromUpperRight));
-        builder.add(SkLights::Light::MakeAmbient(SkColor3f::Make(0.2f, 0.2f, 0.2f)));
+        builder.setAmbientLightColor(SkColor3f::Make(0.2f, 0.2f, 0.2f));
         fLights = builder.finish();
 
         // No directional lights
         SkLights::Builder builderNoDir;
-        builderNoDir.add(SkLights::Light::MakeAmbient(SkColor3f::Make(0.2f, 0.2f, 0.2f)));
+        builderNoDir.setAmbientLightColor(SkColor3f::Make(0.2f, 0.2f, 0.2f));
         fLightsNoDir = builderNoDir.finish();
 
         // Two directional lights
@@ -61,7 +61,7 @@ protected:
                                                            kLightFromUpperRight));
         builderTwoDir.add(SkLights::Light::MakeDirectional(SkColor3f::Make(0.0f, 1.0f, 1.0f),
                                                            kLightFromUpperLeft));
-        builderTwoDir.add(SkLights::Light::MakeAmbient(SkColor3f::Make(0.2f, 0.2f, 0.2f)));
+        builderTwoDir.setAmbientLightColor(SkColor3f::Make(0.2f, 0.2f, 0.2f));
         fLightsTwoDir = builderTwoDir.finish();
 
         fRect = SkRect::MakeIWH(kTexSize, kTexSize);
diff --git a/gm/lightingshaderbevel.cpp b/gm/lightingshaderbevel.cpp
index 9107cc6c94..4e95c8ecc2 100644
--- a/gm/lightingshaderbevel.cpp
+++ b/gm/lightingshaderbevel.cpp
@@ -38,7 +38,7 @@ protected:
 
         builder.add(SkLights::Light::MakeDirectional(SkColor3f::Make(1.0f, 1.0f, 1.0f),
                                                      kLightFromUpperRight));
-        builder.add(SkLights::Light::MakeAmbient(SkColor3f::Make(0.2f, 0.2f, 0.2f)));
+        builder.setAmbientLightColor(SkColor3f::Make(0.2f, 0.2f, 0.2f));
         fLights = builder.finish();
 
         // fRect is assumed to be square throughout this file
diff --git a/gm/shadowmaps.cpp b/gm/shadowmaps.cpp
index 6c9738b247..d8682ce44f 100644
--- a/gm/shadowmaps.cpp
+++ b/gm/shadowmaps.cpp
@@ -74,7 +74,7 @@ public:
                                                      SkVector3::Make(0.2f, 0.1f, 1.0f)));
         builder.add(SkLights::Light::MakeDirectional(SkColor3f::Make(0.4f, 0.3f, 0.2f),
                                                      SkVector3::Make(0.1f, 0.2f, 1.0f)));
-        builder.add(SkLights::Light::MakeAmbient(SkColor3f::Make(0.4f, 0.4f, 0.4f)));
+        builder.setAmbientLightColor(SkColor3f::Make(0.4f, 0.4f, 0.4f));
         fLights = builder.finish();
 
         fShadowParams.fShadowRadius = 4.0f;
diff --git a/include/core/SkLights.h b/include/core/SkLights.h
index d8ec87d532..1371e441d4 100644
--- a/include/core/SkLights.h
+++ b/include/core/SkLights.h
@@ -22,7 +22,6 @@ public:
     class Light {
     public:
         enum LightType {
-            kAmbient_LightType,       // only 'fColor' is used
             kDirectional_LightType,
             kPoint_LightType
         };
@@ -43,10 +42,6 @@ public:
             , fShadowMap(std::move(other.fShadowMap)) {
         }
 
-        static Light MakeAmbient(const SkColor3f& color) {
-            return Light(kAmbient_LightType, color, SkVector3::Make(0.0f, 0.0f, 1.0f));
-        }
-
         static Light MakeDirectional(const SkColor3f& color, const SkVector3& dir) {
             Light light(kDirectional_LightType, color, dir);
             if (!light.fDirOrPos.normalize()) {
@@ -123,17 +118,17 @@ public:
         sk_sp<SkImage> fShadowMap;
 
         Light(LightType type, const SkColor3f& color,
-              const SkVector3& dir, SkScalar intensity = 0.0f) {
+              const SkVector3& dirOrPos, SkScalar intensity = 0.0f) {
             fType = type;
             fColor = color;
-            fDirOrPos = dir;
+            fDirOrPos = dirOrPos;
             fIntensity = intensity;
         }
     };
 
     class Builder {
     public:
-        Builder() : fLights(new SkLights) { }
+        Builder() : fLights(new SkLights) {}
 
         void add(const Light& light) {
             if (fLights) {
@@ -147,6 +142,12 @@ public:
             }
         }
 
+        void setAmbientLightColor(const SkColor3f& color) {
+            if (fLights) {
+                fLights->fAmbientLightColor = color;
+            }
+        }
+
         sk_sp<SkLights> finish() {
             return std::move(fLights);
         }
@@ -167,13 +168,20 @@ public:
         return fLights[index];
     }
 
+    const SkColor3f& ambientLightColor() const {
+        return fAmbientLightColor;
+    }
+
     static sk_sp<SkLights> MakeFromBuffer(SkReadBuffer& buf);
 
     void flatten(SkWriteBuffer& buf) const;
 
 private:
-    SkLights() {}
+    SkLights() {
+        fAmbientLightColor.set(0.0f, 0.0f, 0.0f);
+    }
     SkTArray<Light> fLights;
+    SkColor3f fAmbientLightColor;
     typedef SkRefCnt INHERITED;
 };
 
diff --git a/samplecode/SampleBevel.cpp b/samplecode/SampleBevel.cpp
index e592cf191a..cbeb2499a5 100644
--- a/samplecode/SampleBevel.cpp
+++ b/samplecode/SampleBevel.cpp
@@ -749,7 +749,7 @@ protected:
                                                                         fLightDefs[i].fDirXY.fY,
                                                                         fLightDefs[i].fDirZ)));
         }
-        builder.add(SkLights::Light::MakeAmbient(SkColor3f::Make(0.4f, 0.4f, 0.4f)));
+        builder.setAmbientLightColor(SkColor3f::Make(0.4f, 0.4f, 0.4f));
         fLights = builder.finish();
 
         // Draw shapes
diff --git a/samplecode/SampleLighting.cpp b/samplecode/SampleLighting.cpp
index bab2b5df92..2218f31022 100755
--- a/samplecode/SampleLighting.cpp
+++ b/samplecode/SampleLighting.cpp
@@ -22,7 +22,7 @@ static sk_sp<SkLights> create_lights(SkScalar angle, SkScalar blue) {
     SkLights::Builder builder;
 
     builder.add(SkLights::Light::MakeDirectional(SkColor3f::Make(1.0f, 1.0f, blue), dir));
-    builder.add(SkLights::Light::MakeAmbient(SkColor3f::Make(0.1f, 0.1f, 0.1f)));
+    builder.setAmbientLightColor(SkColor3f::Make(0.1f, 0.1f, 0.1f));
 
     return builder.finish();
 }
diff --git a/samplecode/SampleLitAtlas.cpp b/samplecode/SampleLitAtlas.cpp
index cb49bf3872..149da98a02 100644
--- a/samplecode/SampleLitAtlas.cpp
+++ b/samplecode/SampleLitAtlas.cpp
@@ -184,7 +184,7 @@ private:
 
         builder.add(SkLights::Light::MakeDirectional(
                 SkColor3f::Make(1.0f, 1.0f, 1.0f), fLightDir));
-        builder.add(SkLights::Light::MakeAmbient(SkColor3f::Make(0.2f, 0.2f, 0.2f)));
+        builder.setAmbientLightColor(SkColor3f::Make(0.2f, 0.2f, 0.2f));
 
         fLights = builder.finish();
     }
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));
diff --git a/src/utils/SkShadowPaintFilterCanvas.cpp b/src/utils/SkShadowPaintFilterCanvas.cpp
index e4308709ed..ef29c3709d 100644
--- a/src/utils/SkShadowPaintFilterCanvas.cpp
+++ b/src/utils/SkShadowPaintFilterCanvas.cpp
@@ -50,7 +50,6 @@ bool SkShadowPaintFilterCanvas::onFilter(SkTCopyOnFirstWrite<SkPaint>* paint, Ty
 
 SkISize SkShadowPaintFilterCanvas::ComputeDepthMapSize(const SkLights::Light& light, int maxDepth,
                                                        int width, int height) {
-    SkASSERT(light.type() != SkLights::Light::kAmbient_LightType);
     if (light.type() != SkLights::Light::kDirectional_LightType) {
         return SkISize::Make(width *2 , height * 2);
     }
diff --git a/tests/SerializationTest.cpp b/tests/SerializationTest.cpp
index 11a705f8ce..ff9b342074 100644
--- a/tests/SerializationTest.cpp
+++ b/tests/SerializationTest.cpp
@@ -558,7 +558,7 @@ DEF_TEST(Serialization, reporter) {
 
         builder.add(SkLights::Light::MakeDirectional(SkColor3f::Make(1.0f, 1.0f, 1.0f),
                                                      SkVector3::Make(1.0f, 0.0f, 0.0f)));
-        builder.add(SkLights::Light::MakeAmbient(SkColor3f::Make(0.2f, 0.2f, 0.2f)));
+        builder.setAmbientLightColor(SkColor3f::Make(0.2f, 0.2f, 0.2f));
 
         sk_sp<SkLights> fLights = builder.finish();