Commit a78ade27 authored by fmalita@google.com's avatar fmalita@google.com

Switch back to the original brightness filter implementation.

As requested by Dirk Schulze, revert Chromium's brightness filter to the old spec-compliant
implementation (RGB multiplier instead of color space translation).

Dirk also updated WK/Safari to use the same brightness matrix.

http://trac.webkit.org/changeset/139770
https://bugs.webkit.org/show_bug.cgi?id=106674


BUG=


Review URL: https://chromiumcodereview.appspot.com/12033075

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@181940 0039d316-1c4b-4281-b951-d872f2087c98
parent 46d80b97
...@@ -23,15 +23,19 @@ namespace { ...@@ -23,15 +23,19 @@ namespace {
void getBrightnessMatrix(float amount, SkScalar matrix[20]) void getBrightnessMatrix(float amount, SkScalar matrix[20])
{ {
// Spec implementation
// (http://dvcs.w3.org/hg/FXTF/raw-file/tip/filters/index.html#brightnessEquivalent)
// <feFunc[R|G|B] type="linear" slope="[amount]">
memset(matrix, 0, 20 * sizeof(SkScalar));
matrix[0] = matrix[6] = matrix[12] = amount;
matrix[18] = 1;
}
void getSaturatingBrightnessMatrix(float amount, SkScalar matrix[20])
{
// Legacy implementation used by internal clients.
// <feFunc[R|G|B] type="linear" intercept="[amount]"/>
memset(matrix, 0, 20 * sizeof(SkScalar)); memset(matrix, 0, 20 * sizeof(SkScalar));
// Old implementation, a la the draft spec, a straight-up scale,
// representing <feFunc[R|G|B] type="linear" slope="[amount]">
// (See http://dvcs.w3.org/hg/FXTF/raw-file/tip/filters/index.html#brightnessEquivalent)
// matrix[0] = matrix[6] = matrix[12] = amount;
// matrix[18] = 1;
// New implementation, a translation in color space, representing
// <feFunc[R|G|B] type="linear" intercept="[amount]"/>
// (See https://www.w3.org/Bugs/Public/show_bug.cgi?id=15647)
matrix[0] = matrix[6] = matrix[12] = matrix[18] = 1; matrix[0] = matrix[6] = matrix[12] = matrix[18] = 1;
matrix[4] = matrix[9] = matrix[14] = amount * 255; matrix[4] = matrix[9] = matrix[14] = amount * 255;
} }
...@@ -198,6 +202,10 @@ bool getColorMatrix(const WebKit::WebFilterOperation& op, SkScalar matrix[20]) ...@@ -198,6 +202,10 @@ bool getColorMatrix(const WebKit::WebFilterOperation& op, SkScalar matrix[20])
getBrightnessMatrix(op.amount(), matrix); getBrightnessMatrix(op.amount(), matrix);
return true; return true;
} }
case WebKit::WebFilterOperation::FilterTypeSaturatingBrightness: {
getSaturatingBrightnessMatrix(op.amount(), matrix);
return true;
}
case WebKit::WebFilterOperation::FilterTypeContrast: { case WebKit::WebFilterOperation::FilterTypeContrast: {
getContrastMatrix(op.amount(), matrix); getContrastMatrix(op.amount(), matrix);
return true; return true;
...@@ -352,6 +360,7 @@ WebKit::WebFilterOperations RenderSurfaceFilters::optimize(const WebKit::WebFilt ...@@ -352,6 +360,7 @@ WebKit::WebFilterOperations RenderSurfaceFilters::optimize(const WebKit::WebFilt
newList.append(op); newList.append(op);
break; break;
case WebKit::WebFilterOperation::FilterTypeBrightness: case WebKit::WebFilterOperation::FilterTypeBrightness:
case WebKit::WebFilterOperation::FilterTypeSaturatingBrightness:
case WebKit::WebFilterOperation::FilterTypeContrast: case WebKit::WebFilterOperation::FilterTypeContrast:
case WebKit::WebFilterOperation::FilterTypeGrayscale: case WebKit::WebFilterOperation::FilterTypeGrayscale:
case WebKit::WebFilterOperation::FilterTypeSepia: case WebKit::WebFilterOperation::FilterTypeSepia:
...@@ -360,7 +369,6 @@ WebKit::WebFilterOperations RenderSurfaceFilters::optimize(const WebKit::WebFilt ...@@ -360,7 +369,6 @@ WebKit::WebFilterOperations RenderSurfaceFilters::optimize(const WebKit::WebFilt
case WebKit::WebFilterOperation::FilterTypeInvert: case WebKit::WebFilterOperation::FilterTypeInvert:
case WebKit::WebFilterOperation::FilterTypeOpacity: case WebKit::WebFilterOperation::FilterTypeOpacity:
case WebKit::WebFilterOperation::FilterTypeColorMatrix: case WebKit::WebFilterOperation::FilterTypeColorMatrix:
default: // FIXME: temporary place holder to prevent build failures
break; break;
} }
} }
...@@ -427,6 +435,7 @@ SkBitmap RenderSurfaceFilters::apply(const WebKit::WebFilterOperations& filters, ...@@ -427,6 +435,7 @@ SkBitmap RenderSurfaceFilters::apply(const WebKit::WebFilterOperations& filters,
break; break;
} }
case WebKit::WebFilterOperation::FilterTypeBrightness: case WebKit::WebFilterOperation::FilterTypeBrightness:
case WebKit::WebFilterOperation::FilterTypeSaturatingBrightness:
case WebKit::WebFilterOperation::FilterTypeContrast: case WebKit::WebFilterOperation::FilterTypeContrast:
case WebKit::WebFilterOperation::FilterTypeGrayscale: case WebKit::WebFilterOperation::FilterTypeGrayscale:
case WebKit::WebFilterOperation::FilterTypeSepia: case WebKit::WebFilterOperation::FilterTypeSepia:
...@@ -434,7 +443,6 @@ SkBitmap RenderSurfaceFilters::apply(const WebKit::WebFilterOperations& filters, ...@@ -434,7 +443,6 @@ SkBitmap RenderSurfaceFilters::apply(const WebKit::WebFilterOperations& filters,
case WebKit::WebFilterOperation::FilterTypeHueRotate: case WebKit::WebFilterOperation::FilterTypeHueRotate:
case WebKit::WebFilterOperation::FilterTypeInvert: case WebKit::WebFilterOperation::FilterTypeInvert:
case WebKit::WebFilterOperation::FilterTypeOpacity: case WebKit::WebFilterOperation::FilterTypeOpacity:
default: // FIXME: temporary place holder to prevent build failures
NOTREACHED(); NOTREACHED();
break; break;
} }
......
...@@ -55,9 +55,17 @@ TEST(RenderSurfaceFiltersTest, testColorMatrixFiltersCombined) ...@@ -55,9 +55,17 @@ TEST(RenderSurfaceFiltersTest, testColorMatrixFiltersCombined)
EXPECT_TRUE(isCombined(WebFilterOperation::createOpacityFilter(0.5))); EXPECT_TRUE(isCombined(WebFilterOperation::createOpacityFilter(0.5)));
EXPECT_TRUE(isCombined(WebFilterOperation::createOpacityFilter(1))); EXPECT_TRUE(isCombined(WebFilterOperation::createOpacityFilter(1)));
// Several filters should never combine: brightness(amount > 0), blur, drop-shadow. // Brightness combines when amount <= 1
EXPECT_FALSE(isCombined(WebFilterOperation::createBrightnessFilter(0.5))); EXPECT_TRUE(isCombined(WebFilterOperation::createBrightnessFilter(0.5)));
EXPECT_FALSE(isCombined(WebFilterOperation::createBrightnessFilter(1))); EXPECT_TRUE(isCombined(WebFilterOperation::createBrightnessFilter(1)));
EXPECT_FALSE(isCombined(WebFilterOperation::createBrightnessFilter(1.5)));
// SaturatingBrightness combines only when amount == 0
EXPECT_TRUE(isCombined(WebFilterOperation::createSaturatingBrightnessFilter(0)));
EXPECT_FALSE(isCombined(WebFilterOperation::createSaturatingBrightnessFilter(0.5)));
EXPECT_FALSE(isCombined(WebFilterOperation::createSaturatingBrightnessFilter(1)));
// Several filters should never combine: blur, drop-shadow.
EXPECT_FALSE(isCombined(WebFilterOperation::createBlurFilter(3))); EXPECT_FALSE(isCombined(WebFilterOperation::createBlurFilter(3)));
EXPECT_FALSE(isCombined(WebFilterOperation::createDropShadowFilter(WebPoint(2, 2), 3, 0xffffffff))); EXPECT_FALSE(isCombined(WebFilterOperation::createDropShadowFilter(WebPoint(2, 2), 3, 0xffffffff)));
...@@ -102,8 +110,8 @@ TEST(RenderSurfaceFiltersTest, testColorMatrixFiltersCombined) ...@@ -102,8 +110,8 @@ TEST(RenderSurfaceFiltersTest, testColorMatrixFiltersCombined)
TEST(RenderSurfaceFiltersTest, testOptimize) TEST(RenderSurfaceFiltersTest, testOptimize)
{ {
WebFilterOperation combines(WebFilterOperation::createBrightnessFilter(0)); WebFilterOperation combines(WebFilterOperation::createBrightnessFilter(1));
WebFilterOperation doesntCombine(WebFilterOperation::createBrightnessFilter(1)); WebFilterOperation doesntCombine(WebFilterOperation::createBrightnessFilter(1.5));
WebFilterOperations filters; WebFilterOperations filters;
WebFilterOperations optimized = RenderSurfaceFilters::optimize(filters); WebFilterOperations optimized = RenderSurfaceFilters::optimize(filters);
......
...@@ -22,6 +22,7 @@ void ParamTraits<WebKit::WebFilterOperation>::Write( ...@@ -22,6 +22,7 @@ void ParamTraits<WebKit::WebFilterOperation>::Write(
case WebKit::WebFilterOperation::FilterTypeHueRotate: case WebKit::WebFilterOperation::FilterTypeHueRotate:
case WebKit::WebFilterOperation::FilterTypeInvert: case WebKit::WebFilterOperation::FilterTypeInvert:
case WebKit::WebFilterOperation::FilterTypeBrightness: case WebKit::WebFilterOperation::FilterTypeBrightness:
case WebKit::WebFilterOperation::FilterTypeSaturatingBrightness:
case WebKit::WebFilterOperation::FilterTypeContrast: case WebKit::WebFilterOperation::FilterTypeContrast:
case WebKit::WebFilterOperation::FilterTypeOpacity: case WebKit::WebFilterOperation::FilterTypeOpacity:
case WebKit::WebFilterOperation::FilterTypeBlur: case WebKit::WebFilterOperation::FilterTypeBlur:
...@@ -40,11 +41,6 @@ void ParamTraits<WebKit::WebFilterOperation>::Write( ...@@ -40,11 +41,6 @@ void ParamTraits<WebKit::WebFilterOperation>::Write(
WriteParam(m, p.zoomRect()); WriteParam(m, p.zoomRect());
WriteParam(m, p.amount()); WriteParam(m, p.amount());
break; break;
default:
// FIXME: temporary place holder to prevent build failures
// (pending a new FilterType).
NOTREACHED();
break;
} }
} }
...@@ -69,6 +65,7 @@ bool ParamTraits<WebKit::WebFilterOperation>::Read( ...@@ -69,6 +65,7 @@ bool ParamTraits<WebKit::WebFilterOperation>::Read(
case WebKit::WebFilterOperation::FilterTypeHueRotate: case WebKit::WebFilterOperation::FilterTypeHueRotate:
case WebKit::WebFilterOperation::FilterTypeInvert: case WebKit::WebFilterOperation::FilterTypeInvert:
case WebKit::WebFilterOperation::FilterTypeBrightness: case WebKit::WebFilterOperation::FilterTypeBrightness:
case WebKit::WebFilterOperation::FilterTypeSaturatingBrightness:
case WebKit::WebFilterOperation::FilterTypeContrast: case WebKit::WebFilterOperation::FilterTypeContrast:
case WebKit::WebFilterOperation::FilterTypeOpacity: case WebKit::WebFilterOperation::FilterTypeOpacity:
case WebKit::WebFilterOperation::FilterTypeBlur: case WebKit::WebFilterOperation::FilterTypeBlur:
...@@ -107,11 +104,6 @@ bool ParamTraits<WebKit::WebFilterOperation>::Read( ...@@ -107,11 +104,6 @@ bool ParamTraits<WebKit::WebFilterOperation>::Read(
success = true; success = true;
} }
break; break;
default:
// FIXME: temporary place holder to prevent build failures
// (pending a new FilterType).
NOTREACHED();
break;
} }
return success; return success;
} }
...@@ -129,6 +121,7 @@ void ParamTraits<WebKit::WebFilterOperation>::Log( ...@@ -129,6 +121,7 @@ void ParamTraits<WebKit::WebFilterOperation>::Log(
case WebKit::WebFilterOperation::FilterTypeHueRotate: case WebKit::WebFilterOperation::FilterTypeHueRotate:
case WebKit::WebFilterOperation::FilterTypeInvert: case WebKit::WebFilterOperation::FilterTypeInvert:
case WebKit::WebFilterOperation::FilterTypeBrightness: case WebKit::WebFilterOperation::FilterTypeBrightness:
case WebKit::WebFilterOperation::FilterTypeSaturatingBrightness:
case WebKit::WebFilterOperation::FilterTypeContrast: case WebKit::WebFilterOperation::FilterTypeContrast:
case WebKit::WebFilterOperation::FilterTypeOpacity: case WebKit::WebFilterOperation::FilterTypeOpacity:
case WebKit::WebFilterOperation::FilterTypeBlur: case WebKit::WebFilterOperation::FilterTypeBlur:
...@@ -153,11 +146,6 @@ void ParamTraits<WebKit::WebFilterOperation>::Log( ...@@ -153,11 +146,6 @@ void ParamTraits<WebKit::WebFilterOperation>::Log(
l->append(", "); l->append(", ");
LogParam(p.amount(), l); LogParam(p.amount(), l);
break; break;
default:
// FIXME: temporary place holder to prevent build failures
// (pending a new FilterType).
NOTREACHED();
break;
} }
l->append(")"); l->append(")");
} }
......
...@@ -326,7 +326,7 @@ void Layer::SetLayerFilters() { ...@@ -326,7 +326,7 @@ void Layer::SetLayerFilters() {
// cause further color matrix filters to be applied separately. In this order, // cause further color matrix filters to be applied separately. In this order,
// they all can be combined in a single pass. // they all can be combined in a single pass.
if (layer_brightness_) { if (layer_brightness_) {
filters.append(WebKit::WebFilterOperation::createBrightnessFilter( filters.append(WebKit::WebFilterOperation::createSaturatingBrightnessFilter(
layer_brightness_)); layer_brightness_));
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment