Moved ambient lights out of SkLight's light array

include/core/SkLights.h

Index: include/core/SkLights.h
diff --git a/include/core/SkLights.h b/include/core/SkLights.h
index 329e5d387dc926b2a7ca709377c753fb576e52d6..70ee98cd0b9510a0d044685bb15bca1212717154 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
         };
@@ -30,25 +29,21 @@ public:
         Light(const Light& other)
             : fType(other.fType)
             , fColor(other.fColor)
-            , fDirection(other.fDirection)
+            , fDirOrPos(other.fDirOrPos)
             , fShadowMap(other.fShadowMap) {
         }
 
         Light(Light&& other)
             : fType(other.fType)
             , fColor(other.fColor)
-            , fDirection(other.fDirection)
+            , fDirOrPos(other.fDirOrPos)
             , 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.fDirection.normalize()) {
-                light.fDirection.set(0.0f, 0.0f, 1.0f);
+            if (!light.fDirOrPos.normalize()) {
+                light.fDirOrPos.set(0.0f, 0.0f, 1.0f);
             }
             return light;
         }
@@ -60,12 +55,12 @@ public:
         LightType type() const { return fType; }
         const SkColor3f& color() const { return fColor; }
         const SkVector3& dir() const {

[robertphillips, 2016/08/26 16:08:05]

keep as yoda-speak. The changeable variable is put on right.

-            SkASSERT(kDirectional_LightType == fType);
-            return fDirection;
+            SkASSERT(fType == kDirectional_LightType);
+            return fDirOrPos;
         }
         const SkPoint3& pos() const {

[robertphillips, 2016/08/26 16:08:05]

same here

-            SkASSERT(kPoint_LightType == fType);
-            return fDirection;
+            SkASSERT(fType == kPoint_LightType);
+            return fDirOrPos;
         }
 
         void setShadowMap(sk_sp<SkImage> shadowMap) {
@@ -83,7 +78,7 @@ public:
 
             fColor = b.fColor;
             fType = b.fType;
-            fDirection = b.fDirection;
+            fDirOrPos = b.fDirOrPos;
             fShadowMap = b.fShadowMap;
             return *this;
         }
@@ -95,7 +90,7 @@ public:
 
             return (fColor     == b.fColor) &&
                    (fType      == b.fType) &&
-                   (fDirection == b.fDirection) &&
+                   (fDirOrPos == b.fDirOrPos) &&
                    (fShadowMap == b.fShadowMap);
         }
 
@@ -104,16 +99,16 @@ public:
     private:
         LightType   fType;
         SkColor3f   fColor;           // linear (unpremul) color. Range is 0..1 in each channel.

[robertphillips, 2016/08/26 16:08:05]

line up comment

-        SkVector3   fDirection;       // For directional lights, holds the direction towards the
+        SkVector3   fDirOrPos;       // For directional lights, holds the direction towards the
                                       // light (+Z is out of the screen).
                                       // If degenerate, it will be replaced with (0, 0, 1).
                                       // For point lights, holds location of point light
         sk_sp<SkImage> fShadowMap;
 
-        Light(LightType type, const SkColor3f& color, const SkVector3& dir) {
+        Light(LightType type, const SkColor3f& color, const SkVector3& dirOrPos) {
             fType = type;
             fColor = color;
-            fDirection = dir;
+            fDirOrPos = dirOrPos;
         }
     };
 
@@ -122,15 +117,15 @@ public:
         Builder() : fLights(new SkLights) { }
 
         void add(const Light& light) {

[robertphillips, 2016/08/26 16:08:05]

keep if test in case the caller tries to add stuff after a finish call

[vjiaoblack, 2016/08/26 17:24:25]

Done.

-            if (fLights) {
-                fLights->fLights.push_back(light);
-            }
+            fLights->fNonAmbLights.push_back(light);
         }
 
         void add(Light&& light) {

[robertphillips, 2016/08/26 16:08:05]

here too

[vjiaoblack, 2016/08/26 17:24:25]

Done.

-            if (fLights) {
-                fLights->fLights.push_back(std::move(light));
-            }
+            fLights->fNonAmbLights.push_back(std::move(light));
+        }
+
+        void setAmbientLightColor(const SkColor3f& color) {
+            fLights->fAmbientLight = color;
         }
 
         sk_sp<SkLights> finish() {
@@ -142,15 +137,19 @@ public:
     };
 
     int numLights() const {
-        return fLights.count();
+        return fNonAmbLights.count();
     }
 
     const Light& light(int index) const {
-        return fLights[index];
+        return fNonAmbLights[index];
     }
 
     Light& light(int index) {
-        return fLights[index];
+        return fNonAmbLights[index];
+    }
+
+    const SkColor3f& ambientLightColor() const {
+        return fAmbientLight;
     }
 
     static sk_sp<SkLights> MakeFromBuffer(SkReadBuffer& buf);
@@ -159,7 +158,8 @@ public:
 
 private:
     SkLights() {}
-    SkTArray<Light> fLights;
+    SkTArray<Light> fNonAmbLights;

[robertphillips, 2016/08/26 16:08:05]

fAmbientLightColor

+    SkColor3f fAmbientLight;
     typedef SkRefCnt INHERITED;
 };