Added PointLights to SkLights::Light

include/core/SkLights.h

Index: include/core/SkLights.h
diff --git a/include/core/SkLights.h b/include/core/SkLights.h
index 52e60c71a07a5fe3cae5f441f58e858617490251..5889227d187cdebbb1fcd8cb9f5322e90d3a8fc3 100644
--- a/include/core/SkLights.h
+++ b/include/core/SkLights.h
@@ -9,10 +9,12 @@
 #ifndef SkLights_DEFINED
 #define SkLights_DEFINED
 
-#include "SkPoint3.h"
-#include "SkRefCnt.h"
 #include "../private/SkTArray.h"

[robertphillips, 2016/08/11 14:56:18]

Do we need to include SkImage or can we just predeclare it ?

[vjiaoblack, 2016/08/11 19:18:54]

Done.

 #include "SkImage.h"
+#include "SkPoint3.h"

[robertphillips, 2016/08/11 14:56:18]

predeclare ReadBuffer and WriteBuffer and outline the code.

One isn't allowed to pull headers from src in include

[vjiaoblack, 2016/08/11 19:18:54]

Done.

+#include "../../src/core/SkReadBuffer.h"
+#include "SkRefCnt.h"
+#include "SkWriteBuffer.h"
 
 class SK_API SkLights  : public SkRefCnt {
 public:
@@ -20,7 +22,8 @@ public:
     public:
         enum LightType {
             kAmbient_LightType,       // only 'fColor' is used
-            kDirectional_LightType
+            kDirectional_LightType,
+            kPoint_LightType
         };
 
         Light(const Light& other)
@@ -37,27 +40,32 @@ public:
             , fShadowMap(std::move(other.fShadowMap)) {
         }
 
-        Light(const SkColor3f& color)
-            : fType(kAmbient_LightType)
-            , fColor(color) {
-            fDirection.set(0.0f, 0.0f, 1.0f);
+        static Light MakeAmbient(const SkColor3f& color) {
+            return Light(kAmbient_LightType, color, SkVector3::Make(0.0f, 0.0f, 1.0f));
         }
 
-        Light(const SkColor3f& color, const SkVector3& dir)
-            : fType(kDirectional_LightType)
-            , fColor(color)
-            , fDirection(dir) {
-            if (!fDirection.normalize()) {
-                fDirection.set(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);
             }
+            return light;
+        }
+
+        static Light MakePoint(const SkColor3f& color, const SkPoint3& pos) {
+            return Light(kPoint_LightType, color, pos);
         }
 
         LightType type() const { return fType; }
         const SkColor3f& color() const { return fColor; }
         const SkVector3& dir() const { 
-            SkASSERT(kAmbient_LightType != fType);
+            SkASSERT(kDirectional_LightType == fType);
             return fDirection; 
         }
+        const SkPoint3& pos() const {
+            SkASSERT(kPoint_LightType == fType);
+            return fDirection;
+        }
 
         void setShadowMap(sk_sp<SkImage> shadowMap) {
             fShadowMap = std::move(shadowMap);
@@ -84,7 +92,14 @@ public:
         SkColor3f   fColor;           // linear (unpremul) color. Range is 0..1 in each channel.

[robertphillips, 2016/08/11 14:56:18]

// For directional lights, holds the direction towards ...

[vjiaoblack, 2016/08/11 19:18:54]

Done.

         SkVector3   fDirection;       // 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) {
+            fType = type;
+            fColor = color;
+            fDirection = dir;
+        }
     };
 
     class Builder {
@@ -123,6 +138,56 @@ public:
         return fLights[index];
     }
 

[robertphillips, 2016/08/11 14:56:18]

The pattern seems to be:

static sk_sp<SkLights> MakeFromBuffer(SkReadBuffer& buffer);

[vjiaoblack, 2016/08/11 19:18:53]

Done.

+    bool unFlatten(SkReadBuffer& buf) {
+        int numLights = buf.readInt();
+        fLights.reset();
+        fLights.reserve(numLights);
+
+        for (int l = 0; l < numLights; ++l) {
+            bool isAmbient = buf.readBool();
+            bool isPoint = buf.readBool();
+
+            SkColor3f color;
+            if (!buf.readScalarArray(&color.fX, 3)) {
+                return false;
+            }
+
+            if (isAmbient) {
+                fLights[l] = Light::MakeAmbient(color);
+            } else {
+                SkVector3 dirOrPos;
+                if (!buf.readScalarArray(&dirOrPos.fX, 3)) {
+                    return false;
+                }
+                if (isPoint) {
+                    fLights[l] = Light::MakePoint(color, dirOrPos);
+                } else {
+                    fLights[l] = Light::MakeDirectional(color, dirOrPos);
+                }
+            }
+        }
+
+        return true;
+    }
+

[robertphillips, 2016/08/11 14:56:18]

Are you intentionally not serializing/deserializing the shadow maps ?

[vjiaoblack, 2016/08/11 19:18:53]

Done.

+    void flatten(SkWriteBuffer& buf) const {
+
+        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) {
+                buf.writeScalarArray(&light.dir().fX, 3);
+            }
+        }
+    }
+
 private:
     SkLights() {}