Moved ambient lights out of SkLight's light array
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2287553002
Committed: https://skia.googlesource.com/skia/+/8f98f0aa2d3f7571a890b916c7c4b5ee831e9686
Review-Url: https://codereview.chromium.org/2287553002
diff --git a/gm/lightingshader.cpp b/gm/lightingshader.cpp
index 4a79ebe..137fd64 100644
--- a/gm/lightingshader.cpp
+++ b/gm/lightingshader.cpp
@@ -52,7 +52,7 @@
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 5afe255..886c647 100644
--- a/gm/lightingshader2.cpp
+++ b/gm/lightingshader2.cpp
@@ -47,12 +47,12 @@
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 @@
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 9107cc6..4e95c8e 100644
--- a/gm/lightingshaderbevel.cpp
+++ b/gm/lightingshaderbevel.cpp
@@ -38,7 +38,7 @@
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 6c9738b..d8682ce 100644
--- a/gm/shadowmaps.cpp
+++ b/gm/shadowmaps.cpp
@@ -74,7 +74,7 @@
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 d8ec87d..1371e44 100644
--- a/include/core/SkLights.h
+++ b/include/core/SkLights.h
@@ -22,7 +22,6 @@
class Light {
public:
enum LightType {
- kAmbient_LightType, // only 'fColor' is used
kDirectional_LightType,
kPoint_LightType
};
@@ -43,10 +42,6 @@
, 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 @@
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 @@
}
}
+ void setAmbientLightColor(const SkColor3f& color) {
+ if (fLights) {
+ fLights->fAmbientLightColor = color;
+ }
+ }
+
sk_sp<SkLights> finish() {
return std::move(fLights);
}
@@ -167,13 +168,20 @@
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 e592cf1..cbeb249 100644
--- a/samplecode/SampleBevel.cpp
+++ b/samplecode/SampleBevel.cpp
@@ -749,7 +749,7 @@
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 bab2b5d..2218f31 100755
--- a/samplecode/SampleLighting.cpp
+++ b/samplecode/SampleLighting.cpp
@@ -22,7 +22,7 @@
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 cb49bf3..149da98 100644
--- a/samplecode/SampleLitAtlas.cpp
+++ b/samplecode/SampleLitAtlas.cpp
@@ -184,7 +184,7 @@
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 fc18975..c4ce75c 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -3231,8 +3231,7 @@
// 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 8fde8bf..9030a19 100644
--- a/src/core/SkLightingShader.cpp
+++ b/src/core/SkLightingShader.cpp
@@ -122,11 +122,9 @@
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 {
@@ -386,6 +384,12 @@
}
SkColor3f accum = SkColor3f::Make(0.0f, 0.0f, 0.0f);
+
+ // Adding ambient light
+ accum.fX += lightShader.fLights->ambientLightColor().fX * SkColorGetR(diffColor);
+ accum.fY += lightShader.fLights->ambientLightColor().fY * SkColorGetG(diffColor);
+ accum.fZ += lightShader.fLights->ambientLightColor().fZ * SkColorGetB(diffColor);
+
// This is all done in linear unpremul color space (each component 0..255.0f though)
for (int l = 0; l < lightShader.fLights->numLights(); ++l) {
const SkLights::Light& light = lightShader.fLights->light(l);
diff --git a/src/core/SkLights.cpp b/src/core/SkLights.cpp
index 7d8f9ed..962997f 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 @@
return nullptr;
}
- if (isAmbient) {
- builder.add(Light::MakeAmbient(color));
- } else {
- SkVector3 dirOrPos;
- if (!buf.readScalarArray(&dirOrPos.fX, 3)) {
+ 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;
}
+ }
+
+ if (isPoint) {
SkScalar intensity = 0.0f;
- if (isPoint) {
- intensity = buf.readScalar();
- }
-
- 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);
- }
+ 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 @@
}
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 49f1b01..b1f49b8 100644
--- a/src/core/SkShadowShader.cpp
+++ b/src/core/SkShadowShader.cpp
@@ -111,24 +111,23 @@
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 @@
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 e430870..ef29c37 100644
--- a/src/utils/SkShadowPaintFilterCanvas.cpp
+++ b/src/utils/SkShadowPaintFilterCanvas.cpp
@@ -50,7 +50,6 @@
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 11a705f..ff9b342 100644
--- a/tests/SerializationTest.cpp
+++ b/tests/SerializationTest.cpp
@@ -558,7 +558,7 @@
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();