Fix lighting image filters
1. Removes clamping of spotlight's specular exponent, which is not the
specified behavior of feSpotLight in SVG.
(note: we never clamped a specular lighting effect's
specular exponent/shininess parameter, although SVG 1.1 does clamp
that; that is the client's responsibility).
2. Fixes a bug in the GPU implementation of scale factor for spot lights.
3. Saturate computed lighting color after multiplying with color scale,
instead of just saturating the color scale (allows high intensity
lights to saturate to white which is more reasonable approximation of
an HDR effect stored in an LDR color).
Note: fixes 1 and 2 were originally addressed in https://bugs.chromium.org/p/chromium/issues/detail?id=472849
but the change was reverted for layout failure reasons and it was never
relanded after rebasing.
Note2: most of the layout test rebaselines necessary for chrome are
minor and related to fix 2. The exception is svg/dynamic-updates/SVGFESpecularLightingElement-dom-kernelUnitLength-attr.html
which is the result of fix 3. It took a little digging, but I believe
that fix 3 actually makes that generated SVG more correct (it's original
expected image was "wrong").
In that test, it has a specular constant of 4 (which is a multiplier)
and a shininess of 1 (which makes it practically a "diffuse" specular).
So basically we are multiplying the greenYellow color by 4 for most of
the image that has a normal pointing out of the screen. Firefox renders
a similarly yellow oversaturated appearance instead of clamping to the
base greenYellow.
Reading the feSpecularLighting spec, there is no saturation that is
specified where it had been performed before this change. Instead, all
that is mentioned is that the results of a given filter have to be pinned
to the color channel range (e.g. the last step).
Bug: skia:11007, skia:11057, skia:11153
Change-Id: I82e4a6f1742fecea59816fda75eb931c2a51d3e1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355496
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
Reviewed-by: Tyler Denniston <tdenniston@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This commit is contained in:
Michael Ludwig
Skia Commit-Bot
parent
2c73fc43a3
commit
f577427888
@ -10,6 +10,15 @@ Milestone 90
|
||||
* Deprecate (and ignore) SkAndroidCodec::ExifOrientation
|
||||
https://review.skia.org/344763
|
||||
|
||||
* Fix several minor issues in lighting image filters:
|
||||
- The spotlight falloff exponent is no longer clamped to [1, 128]. SVG 1.1 requires the specular
|
||||
lighting effect's exponent (shininess) to be clamped; not the spotlight's falloff. Any such
|
||||
parameter clamping is the client's responisibility, which makes Skia's lighting effect easily
|
||||
adaptable to SVG 1.1 (clamp exponent) or SVG 2 (no clamp).
|
||||
- Fix spotlight incorrectly scaling light within the cone angle.
|
||||
- Move saturation of RGBA to after multiplying lighting intensity with the lighting color, which
|
||||
improves rendering when diffuse and specular constants are greater than 1.
|
||||
https://review.skia.org/355496
|
||||
|
||||
Milestone 89
|
||||
------------
|
||||
|
||||
@ -21,7 +21,7 @@
|
||||
#include "tools/ToolUtils.h"
|
||||
#include "tools/timer/TimeUtils.h"
|
||||
|
||||
#define WIDTH 330
|
||||
#define WIDTH 660
|
||||
#define HEIGHT 660
|
||||
|
||||
namespace skiagm {
|
||||
@ -76,7 +76,10 @@ protected:
|
||||
SkPoint3 spotLocation = SkPoint3::Make(spotTarget.fX + 70.7214f * cosAzimuth,
|
||||
spotTarget.fY + 70.7214f * sinAzimuth,
|
||||
spotTarget.fZ + SkIntToScalar(20));
|
||||
SkScalar spotExponent = SK_Scalar1;
|
||||
SkScalar spotExponent1 = SK_Scalar1;
|
||||
SkScalar spotExponent10 = SkIntToScalar(10);
|
||||
SkScalar cutoffAngleSmall = SkIntToScalar(15);
|
||||
SkScalar cutoffAngleNone = SkIntToScalar(180);
|
||||
|
||||
SkPoint3 pointLocation = SkPoint3::Make(spotTarget.fX + 50 * cosAzimuth,
|
||||
spotTarget.fY + 50 * sinAzimuth,
|
||||
@ -86,12 +89,12 @@ protected:
|
||||
SkPoint3 distantDirection = SkPoint3::Make(cosAzimuth * SkScalarCos(elevationRad),
|
||||
sinAzimuth * SkScalarCos(elevationRad),
|
||||
SkScalarSin(elevationRad));
|
||||
SkScalar cutoffAngle = SkIntToScalar(15);
|
||||
SkScalar kd = SkIntToScalar(2);
|
||||
SkScalar ks = SkIntToScalar(1);
|
||||
SkScalar shininess = SkIntToScalar(8);
|
||||
SkScalar surfaceScale = SkIntToScalar(1);
|
||||
SkColor white(0xFFFFFFFF);
|
||||
SkScalar surfaceScaleSmall = 0.1f;
|
||||
SkColor greenYellow = SkColorSetARGB(255, 173, 255, 47);
|
||||
SkPaint paint;
|
||||
|
||||
SkIRect cropRect = SkIRect::MakeXYWH(20, 10, 60, 65);
|
||||
@ -102,34 +105,70 @@ protected:
|
||||
for (int i = 0; i < 3; i++) {
|
||||
const SkIRect* cr = (i == 1) ? &cropRect : (i == 2) ? &fullSizeCropRect : nullptr;
|
||||
sk_sp<SkImageFilter> input = (i == 2) ? noopCropped : nullptr;
|
||||
// Basic point, distant and spot lights with diffuse lighting
|
||||
paint.setImageFilter(SkImageFilters::PointLitDiffuse(
|
||||
pointLocation, white, surfaceScale, kd, input, cr));
|
||||
pointLocation, SK_ColorWHITE, surfaceScale, kd, input, cr));
|
||||
drawClippedBitmap(canvas, paint, 0, y);
|
||||
|
||||
paint.setImageFilter(SkImageFilters::DistantLitDiffuse(
|
||||
distantDirection, white, surfaceScale, kd, input, cr));
|
||||
distantDirection, SK_ColorWHITE, surfaceScale, kd, input, cr));
|
||||
drawClippedBitmap(canvas, paint, 110, y);
|
||||
|
||||
paint.setImageFilter(SkImageFilters::SpotLitDiffuse(
|
||||
spotLocation, spotTarget, spotExponent, cutoffAngle, white, surfaceScale, kd,
|
||||
input, cr));
|
||||
spotLocation, spotTarget, spotExponent1, cutoffAngleSmall, SK_ColorWHITE,
|
||||
surfaceScale, kd, input, cr));
|
||||
drawClippedBitmap(canvas, paint, 220, y);
|
||||
|
||||
// Spot light with no angle cutoff
|
||||
paint.setImageFilter(SkImageFilters::SpotLitDiffuse(
|
||||
spotLocation, spotTarget, spotExponent10, cutoffAngleNone, SK_ColorWHITE,
|
||||
surfaceScale, kd, input, cr));
|
||||
drawClippedBitmap(canvas, paint, 330, y);
|
||||
|
||||
// Spot light with falloff exponent
|
||||
paint.setImageFilter(SkImageFilters::SpotLitDiffuse(
|
||||
spotLocation, spotTarget, spotExponent1, cutoffAngleNone, SK_ColorWHITE,
|
||||
surfaceScaleSmall, kd, input, cr));
|
||||
drawClippedBitmap(canvas, paint, 440, y);
|
||||
|
||||
// Large constant to show oversaturation
|
||||
paint.setImageFilter(SkImageFilters::DistantLitDiffuse(
|
||||
distantDirection, greenYellow, surfaceScale, 4.f * kd, input, cr));
|
||||
drawClippedBitmap(canvas, paint, 550, y);
|
||||
|
||||
y += 110;
|
||||
|
||||
// Basic point, distant and spot lights with specular lighting
|
||||
paint.setImageFilter(SkImageFilters::PointLitSpecular(
|
||||
pointLocation, white, surfaceScale, ks, shininess, input, cr));
|
||||
pointLocation, SK_ColorWHITE, surfaceScale, ks, shininess, input, cr));
|
||||
drawClippedBitmap(canvas, paint, 0, y);
|
||||
|
||||
paint.setImageFilter(SkImageFilters::DistantLitSpecular(
|
||||
distantDirection, white, surfaceScale, ks, shininess, input, cr));
|
||||
distantDirection, SK_ColorWHITE, surfaceScale, ks, shininess, input, cr));
|
||||
drawClippedBitmap(canvas, paint, 110, y);
|
||||
|
||||
paint.setImageFilter(SkImageFilters::SpotLitSpecular(
|
||||
spotLocation, spotTarget, spotExponent, cutoffAngle, white, surfaceScale, ks,
|
||||
shininess, input, cr));
|
||||
spotLocation, spotTarget, spotExponent1, cutoffAngleSmall, SK_ColorWHITE,
|
||||
surfaceScale, ks, shininess, input, cr));
|
||||
drawClippedBitmap(canvas, paint, 220, y);
|
||||
|
||||
// Spot light with no angle cutoff
|
||||
paint.setImageFilter(SkImageFilters::SpotLitSpecular(
|
||||
spotLocation, spotTarget, spotExponent10, cutoffAngleNone, SK_ColorWHITE,
|
||||
surfaceScale, ks, shininess, input, cr));
|
||||
drawClippedBitmap(canvas, paint, 330, y);
|
||||
|
||||
// Spot light with falloff exponent
|
||||
paint.setImageFilter(SkImageFilters::SpotLitSpecular(
|
||||
spotLocation, spotTarget, spotExponent1, cutoffAngleNone, SK_ColorWHITE,
|
||||
surfaceScaleSmall, ks, shininess, input, cr));
|
||||
drawClippedBitmap(canvas, paint, 440, y);
|
||||
|
||||
// Large constant to show oversaturation
|
||||
paint.setImageFilter(SkImageFilters::DistantLitSpecular(
|
||||
distantDirection, greenYellow, surfaceScale, 4.f * ks, shininess, input, cr));
|
||||
drawClippedBitmap(canvas, paint, 550, y);
|
||||
|
||||
y += 110;
|
||||
}
|
||||
}
|
||||
|
||||
@ -163,7 +163,6 @@ public:
|
||||
SkPMColor light(const SkPoint3& normal, const SkPoint3& surfaceTolight,
|
||||
const SkPoint3& lightColor) const override {
|
||||
SkScalar colorScale = fKD * normal.dot(surfaceTolight);
|
||||
colorScale = SkTPin(colorScale, 0.0f, SK_Scalar1);
|
||||
SkPoint3 color = lightColor.makeScale(colorScale);
|
||||
return SkPackARGB32(255,
|
||||
SkTPin(SkScalarRoundToInt(color.fX), 0, 255),
|
||||
@ -188,7 +187,6 @@ public:
|
||||
halfDir.fZ += SK_Scalar1; // eye position is always (0, 0, 1)
|
||||
fast_normalize(&halfDir);
|
||||
SkScalar colorScale = fKS * SkScalarPow(normal.dot(halfDir), fShininess);
|
||||
colorScale = SkTPin(colorScale, 0.0f, SK_Scalar1);
|
||||
SkPoint3 color = lightColor.makeScale(colorScale);
|
||||
return SkPackARGB32(SkTPin(SkScalarRoundToInt(max_component(color)), 0, 255),
|
||||
SkTPin(SkScalarRoundToInt(color.fX), 0, 255),
|
||||
@ -975,7 +973,7 @@ public:
|
||||
: INHERITED(color),
|
||||
fLocation(location),
|
||||
fTarget(target),
|
||||
fSpecularExponent(SkTPin(specularExponent, kSpecularExponentMin, kSpecularExponentMax))
|
||||
fSpecularExponent(specularExponent)
|
||||
{
|
||||
fS = target - location;
|
||||
fast_normalize(&fS);
|
||||
@ -1102,9 +1100,6 @@ protected:
|
||||
}
|
||||
|
||||
private:
|
||||
static const SkScalar kSpecularExponentMin;
|
||||
static const SkScalar kSpecularExponentMax;
|
||||
|
||||
SkPoint3 fLocation;
|
||||
SkPoint3 fTarget;
|
||||
SkScalar fSpecularExponent;
|
||||
@ -1116,11 +1111,6 @@ private:
|
||||
using INHERITED = SkImageFilterLight;
|
||||
};
|
||||
|
||||
// According to the spec, the specular term should be in the range [1, 128] :
|
||||
// http://www.w3.org/TR/SVG/filters.html#feSpecularLightingSpecularExponentAttribute
|
||||
const SkScalar SkSpotLight::kSpecularExponentMin = 1.0f;
|
||||
const SkScalar SkSpotLight::kSpecularExponentMax = 128.0f;
|
||||
|
||||
///////////////////////////////////////////////////////////////////////////////
|
||||
|
||||
void SkImageFilterLight::flattenLight(SkWriteBuffer& buffer) const {
|
||||
@ -1859,7 +1849,7 @@ void GrGLDiffuseLightingEffect::emitLightFunc(const GrFragmentProcessor* owner,
|
||||
};
|
||||
SkString lightBody;
|
||||
lightBody.appendf("half colorScale = %s * dot(normal, surfaceToLight);", kd);
|
||||
lightBody.appendf("return half4(lightColor * saturate(colorScale), 1.0);");
|
||||
lightBody.appendf("return half4(saturate(lightColor * colorScale), 1.0);");
|
||||
*funcName = fragBuilder->getMangledFunctionName("light");
|
||||
fragBuilder->emitFunction(kHalf4_GrSLType,
|
||||
funcName->c_str(),
|
||||
@ -1966,7 +1956,7 @@ void GrGLSpecularLightingEffect::emitLightFunc(const GrFragmentProcessor* owner,
|
||||
lightBody.appendf("half3 halfDir = half3(normalize(surfaceToLight + half3(0, 0, 1)));");
|
||||
lightBody.appendf("half colorScale = half(%s * pow(dot(normal, halfDir), %s));",
|
||||
ks, shininess);
|
||||
lightBody.appendf("half3 color = lightColor * saturate(colorScale);");
|
||||
lightBody.appendf("half3 color = saturate(lightColor * colorScale);");
|
||||
lightBody.appendf("return half4(color, max(max(color.r, color.g), color.b));");
|
||||
*funcName = fragBuilder->getMangledFunctionName("light");
|
||||
fragBuilder->emitFunction(kHalf4_GrSLType,
|
||||
@ -2108,7 +2098,7 @@ void GrGLSpotLight::emitLightColor(const GrFragmentProcessor* owner,
|
||||
lightColorBody.appendf("return %s * scale * (cosAngle - %s) * %s;",
|
||||
color, cosOuter, coneScale);
|
||||
lightColorBody.appendf("}");
|
||||
lightColorBody.appendf("return %s;", color);
|
||||
lightColorBody.appendf("return %s * scale;", color);
|
||||
fLightColorFunc = fragBuilder->getMangledFunctionName("lightColor");
|
||||
fragBuilder->emitFunction(kHalf3_GrSLType,
|
||||
fLightColorFunc.c_str(),
|
||||
|
||||
Loading…
Reference in New Issue
Block a user