Index: src/effects/SkLightingImageFilter.cpp |
diff --git a/src/effects/SkLightingImageFilter.cpp b/src/effects/SkLightingImageFilter.cpp |
index 7a74f73624bc89411c12d79c8e149417f2e50852..9bcb1d8f658f240f61f659cf16d4a78e494f3d34 100644 |
--- a/src/effects/SkLightingImageFilter.cpp |
+++ b/src/effects/SkLightingImageFilter.cpp |
@@ -46,6 +46,18 @@ void setUniformNormal3(const GrGLUniformManager& uman, UniformHandle uni, const |
} |
#endif |
+void getClampedColor(const SkPoint3 color, U8CPU& r8, U8CPU& g8, U8CPU& b8) { |
Stephen White
2013/08/21 20:49:26
Personal style, but I'm not a fan of non-const ref
sugoi
2013/08/21 21:12:09
Ok, will do.
scroggo
2013/08/21 23:25:27
That's actually in the Skia style guide, so it's m
sugoi1
2013/08/22 15:41:00
Done.
|
+ // Sending negative numbers to SkPackARGB32 would convert them to |
+ // unsigned, changing small negative integers into very large positive |
+ // unsigned integers. To avoid issues, clamp colors to the [0, 255] range. |
+ int r = SkScalarFloorToInt(color.fX); |
+ int g = SkScalarFloorToInt(color.fY); |
+ int b = SkScalarFloorToInt(color.fZ); |
+ r8 = r < 0 ? 0 : (r > 255 ? 255 : r); |
+ g8 = g < 0 ? 0 : (g > 255 ? 255 : g); |
+ b8 = b < 0 ? 0 : (b > 255 ? 255 : b); |
+} |
+ |
// Shift matrix components to the left, as we advance pixels to the right. |
inline void shiftMatrixLeft(int m[9]) { |
m[0] = m[1]; |
@@ -64,10 +76,9 @@ public: |
SkScalar colorScale = SkScalarMul(fKD, normal.dot(surfaceTolight)); |
colorScale = SkScalarClampMax(colorScale, SK_Scalar1); |
SkPoint3 color(lightColor * colorScale); |
- return SkPackARGB32(255, |
- SkScalarFloorToInt(color.fX), |
- SkScalarFloorToInt(color.fY), |
- SkScalarFloorToInt(color.fZ)); |
+ U8CPU r(0), g(0), b(0); |
+ getClampedColor(color, r, g, b); |
+ return SkPackARGB32(255, r, g, b); |
} |
private: |
SkScalar fKD; |
@@ -85,10 +96,9 @@ public: |
SkScalarPow(normal.dot(halfDir), fShininess)); |
colorScale = SkScalarClampMax(colorScale, SK_Scalar1); |
SkPoint3 color(lightColor * colorScale); |
- return SkPackARGB32(SkScalarFloorToInt(color.maxComponent()), |
- SkScalarFloorToInt(color.fX), |
- SkScalarFloorToInt(color.fY), |
- SkScalarFloorToInt(color.fZ)); |
+ U8CPU r(0), g(0), b(0); |
+ getClampedColor(color, r, g, b); |
+ return SkPackARGB32(r > g ? (r > b ? r : b) : (g > b ? g : b), r, g, b); |
Stephen White
2013/08/21 20:49:26
Could we not simply clamp the alpha after doing ma
sugoi
2013/08/21 21:12:09
Sure, I'll just add the clamping code here.
sugoi1
2013/08/22 15:41:00
Done.
|
} |
private: |
SkScalar fKS; |
@@ -661,7 +671,7 @@ public: |
: INHERITED(color), |
fLocation(location), |
fTarget(target), |
- fSpecularExponent(specularExponent) |
+ fSpecularExponent(SkScalarPin(specularExponent, kSpecularExponentMin, kSpecularExponentMax)) |
{ |
fS = target - location; |
fS.normalize(); |
@@ -745,6 +755,11 @@ protected: |
} |
private: |
+ // According to the spec, the specular term should be in the range [1, 128] : |
+ // http://www.w3.org/TR/SVG/filters.html#feSpecularLightingSpecularExponentAttribute |
+ static const SkScalar kSpecularExponentMin = SkFloatToScalar(1.0f); |
+ static const SkScalar kSpecularExponentMax = SkFloatToScalar(128.0f); |
+ |
typedef SkLight INHERITED; |
SkPoint3 fLocation; |
SkPoint3 fTarget; |