Revert of Moved ambient lights out of SkLight's light array (patchset #7 id:120001 of https://codereview.chromium.org/2287553002/ )

Reason for revert:
Made Deigo's GM miss their ambient lights

Original issue's description:
> 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

TBR=robertphillips@google.com,djsollen@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Review-Url: https://codereview.chromium.org/2291663002
This commit is contained in:
vjiaoblack 2016-08-29 08:38:04 -07:00 committed by Commit bot
parent b398ecc2a4
commit 84cddf6fa7
14 changed files with 87 additions and 87 deletions

View File

@ -52,7 +52,7 @@ public:
SkVector3::Make(SK_ScalarRoot2Over2,
0.0f,
SK_ScalarRoot2Over2)));
builder.setAmbientLightColor(SkColor3f::Make(0.2f, 0.2f, 0.2f));
builder.add(SkLights::Light::MakeAmbient(SkColor3f::Make(0.2f, 0.2f, 0.2f)));
fLights = builder.finish();
}

View File

@ -47,12 +47,12 @@ protected:
SkLights::Builder builder;
builder.add(SkLights::Light::MakeDirectional(SkColor3f::Make(1.0f, 1.0f, 1.0f),
kLightFromUpperRight));
builder.setAmbientLightColor(SkColor3f::Make(0.2f, 0.2f, 0.2f));
builder.add(SkLights::Light::MakeAmbient(SkColor3f::Make(0.2f, 0.2f, 0.2f)));
fLights = builder.finish();
// No directional lights
SkLights::Builder builderNoDir;
builderNoDir.setAmbientLightColor(SkColor3f::Make(0.2f, 0.2f, 0.2f));
builderNoDir.add(SkLights::Light::MakeAmbient(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.setAmbientLightColor(SkColor3f::Make(0.2f, 0.2f, 0.2f));
builderTwoDir.add(SkLights::Light::MakeAmbient(SkColor3f::Make(0.2f, 0.2f, 0.2f)));
fLightsTwoDir = builderTwoDir.finish();
fRect = SkRect::MakeIWH(kTexSize, kTexSize);

View File

@ -38,7 +38,7 @@ protected:
builder.add(SkLights::Light::MakeDirectional(SkColor3f::Make(1.0f, 1.0f, 1.0f),
kLightFromUpperRight));
builder.setAmbientLightColor(SkColor3f::Make(0.2f, 0.2f, 0.2f));
builder.add(SkLights::Light::MakeAmbient(SkColor3f::Make(0.2f, 0.2f, 0.2f)));
fLights = builder.finish();
// fRect is assumed to be square throughout this file

View File

@ -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.setAmbientLightColor(SkColor3f::Make(0.4f, 0.4f, 0.4f));
builder.add(SkLights::Light::MakeAmbient(SkColor3f::Make(0.4f, 0.4f, 0.4f)));
fLights = builder.finish();
fShadowParams.fShadowRadius = 4.0f;

View File

@ -22,6 +22,7 @@ public:
class Light {
public:
enum LightType {
kAmbient_LightType, // only 'fColor' is used
kDirectional_LightType,
kPoint_LightType
};
@ -42,6 +43,10 @@ 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()) {
@ -118,10 +123,10 @@ public:
sk_sp<SkImage> fShadowMap;
Light(LightType type, const SkColor3f& color,
const SkVector3& dirOrPos, SkScalar intensity = 0.0f) {
const SkVector3& dir, SkScalar intensity = 0.0f) {
fType = type;
fColor = color;
fDirOrPos = dirOrPos;
fDirOrPos = dir;
fIntensity = intensity;
}
};
@ -142,12 +147,6 @@ public:
}
}
void setAmbientLightColor(const SkColor3f& color) {
if (fLights) {
fLights->fAmbientLightColor = color;
}
}
sk_sp<SkLights> finish() {
return std::move(fLights);
}
@ -168,20 +167,13 @@ public:
return fLights[index];
}
const SkColor3f& ambientLightColor() const {
return fAmbientLightColor;
}
static sk_sp<SkLights> MakeFromBuffer(SkReadBuffer& buf);
void flatten(SkWriteBuffer& buf) const;
private:
SkLights() {
fAmbientLightColor.set(0.0f, 0.0f, 0.0f);
}
SkLights() {}
SkTArray<Light> fLights;
SkColor3f fAmbientLightColor;
typedef SkRefCnt INHERITED;
};

View File

@ -749,7 +749,7 @@ protected:
fLightDefs[i].fDirXY.fY,
fLightDefs[i].fDirZ)));
}
builder.setAmbientLightColor(SkColor3f::Make(0.4f, 0.4f, 0.4f));
builder.add(SkLights::Light::MakeAmbient(SkColor3f::Make(0.4f, 0.4f, 0.4f)));
fLights = builder.finish();
// Draw shapes

View File

@ -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.setAmbientLightColor(SkColor3f::Make(0.1f, 0.1f, 0.1f));
builder.add(SkLights::Light::MakeAmbient(SkColor3f::Make(0.1f, 0.1f, 0.1f)));
return builder.finish();
}

View File

@ -184,7 +184,7 @@ private:
builder.add(SkLights::Light::MakeDirectional(
SkColor3f::Make(1.0f, 1.0f, 1.0f), fLightDir));
builder.setAmbientLightColor(SkColor3f::Make(0.2f, 0.2f, 0.2f));
builder.add(SkLights::Light::MakeAmbient(SkColor3f::Make(0.2f, 0.2f, 0.2f)));
fLights = builder.finish();
}

View File

@ -3231,7 +3231,8 @@ 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 (fLights->light(i).getShadowMap() != nullptr) {
if (SkLights::Light::kAmbient_LightType == fLights->light(i).type() ||
fLights->light(i).getShadowMap() != nullptr) {
continue;
}

View File

@ -122,9 +122,11 @@ public:
LightingFP(sk_sp<GrFragmentProcessor> normalFP, sk_sp<SkLights> lights) {
// fuse all ambient lights into a single one
fAmbientColor = lights->ambientLightColor();
fAmbientColor.set(0.0f, 0.0f, 0.0f);
for (int i = 0; i < lights->numLights(); ++i) {
if (SkLights::Light::kDirectional_LightType == lights->light(i).type()) {
if (SkLights::Light::kAmbient_LightType == lights->light(i).type()) {
fAmbientColor += lights->light(i).color();
} else 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 {

View File

@ -10,18 +10,11 @@
#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;
@ -29,10 +22,17 @@ 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();
}
sk_sp<SkImage> depthMap;
bool hasShadowMap = buf.readBool();
@ -43,8 +43,6 @@ sk_sp<SkLights> SkLights::MakeFromBuffer(SkReadBuffer& buf) {
}
if (isPoint) {
SkScalar intensity = 0.0f;
intensity = buf.readScalar();
Light light = Light::MakePoint(color, dirOrPos, intensity);
light.setShadowMap(depthMap);
builder.add(light);
@ -54,29 +52,35 @@ sk_sp<SkLights> SkLights::MakeFromBuffer(SkReadBuffer& buf) {
builder.add(light);
}
}
}
return builder.finish();
}
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());
}
if (isPoint) {
buf.writeScalar(light.intensity());
}
}
}

View File

@ -111,21 +111,22 @@ public:
const SkShadowParams& params,
GrContext* context) {
fAmbientColor = lights->ambientLightColor();
// fuse all ambient lights into a single one
fAmbientColor.set(0.0f, 0.0f, 0.0f);
fNumNonAmbLights = 0; // count of non-ambient lights
for (int i = 0; i < lights->numLights(); ++i) {
if (fNumNonAmbLights < SkShadowShader::kMaxNonAmbientLights) {
if (SkLights::Light::kAmbient_LightType == lights->light(i).type()) {
fAmbientColor += lights->light(i).color();
} else if (fNumNonAmbLights < SkShadowShader::kMaxNonAmbientLights) {
fLightColor[fNumNonAmbLights] = lights->light(i).color();
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()) {
if (lights->light(i).type() == SkLights::Light::kPoint_LightType) {
fLightDirOrPos[fNumNonAmbLights] = lights->light(i).pos();
fLightIntensity[fNumNonAmbLights] = lights->light(i).intensity();
} else {
fLightDirOrPos[fNumNonAmbLights] = lights->light(i).dir();
fLightIntensity[fNumNonAmbLights] = 0.0f;
}
fIsPointLight[fNumNonAmbLights] =
SkLights::Light::kPoint_LightType == lights->light(i).type();
@ -696,15 +697,14 @@ 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::kDirectional_LightType == light.type()) {
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()) {
// scaling by fZ accounts for lighting direction
accum.fX += light.color().makeScale(light.dir().fZ).fX *
SkColorGetR(diffColor);
@ -713,11 +713,11 @@ void SkShadowShaderImpl::ShadowShaderContext::shadeSpan(int x, int y,
accum.fZ += light.color().makeScale(light.dir().fZ).fZ *
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));

View File

@ -50,6 +50,7 @@ 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);
}

View File

@ -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.setAmbientLightColor(SkColor3f::Make(0.2f, 0.2f, 0.2f));
builder.add(SkLights::Light::MakeAmbient(SkColor3f::Make(0.2f, 0.2f, 0.2f)));
sk_sp<SkLights> fLights = builder.finish();