Bug 106674 – [CSS Filters] brightness() function doesn't work as specified

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Mike Sierra]

Load source file & referenced image, then apply various brightness() values from the popup list. The spec says brightness(1) keeps the image as is & brightness(0) yields all black.  Actual behavior is brightness(0) keeps as is & brightness(1) yields all white.  Brightness values above 1 are ignored as invalid, but ordinarily they should lighten pixels from their original values.

[Mike Sierra]

Additional behavior: brightness(-1) produces all black.

[Dirk Schulze]

*** Bug 105570 has been marked as a duplicate of this bug. ***

[Dirk Schulze]

The implementation sets the slope value to the default and modifies the intercept value, while it should be the other way around. WebKit is not following the specification here, but the specification is right.

[Dirk Schulze]

Created attachment 182486 [details]
patch for bots

[Simon Fraser (smfr)]

Comment on attachment 182486 [details]
patch for bots

Why no test?

[WebKit Review Bot]

Comment on attachment 182486 [details]
patch for bots

Attachment 182486 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15868158

New failing tests:
css3/filters/effect-combined.html
css3/filters/effect-brightness-clamping.html
css3/filters/multiple-filters-invalidation.html
css3/filters/effect-brightness.html
css3/filters/filter-property-parsing-invalid.html
css3/filters/null-effect-check.html

[Build Bot]

Comment on attachment 182486 [details]
patch for bots

Attachment 182486 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15853197

New failing tests:
css3/filters/filter-property-parsing-invalid.html

[Dirk Schulze]

(In reply to comment #5)
> (From update of attachment 182486 [details])
> Why no test?

It is not for review. I had a compromised test folder and wanted to check on the bots. Looks like we have a lot of test covering brightness and I just need to adapt the test files. I'll upload a test for review. I confirmed with Photoshop that we would do the right thing with following the spec (which we don't right now).

[Dirk Schulze]

Created attachment 182488 [details]
Patch

Waiting for Chromium bot to know which pixel results need updates.

[Build Bot]

Comment on attachment 182488 [details]
Patch

Attachment 182488 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15874159

New failing tests:
css3/filters/filter-property-parsing-invalid.html
css3/filters/filter-property-parsing.html

[WebKit Review Bot]

Comment on attachment 182488 [details]
Patch

Attachment 182488 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15838203

New failing tests:
css3/filters/effect-brightness-hw.html
css3/filters/effect-combined.html
css3/filters/effect-brightness-clamping.html
css3/filters/multiple-filters-invalidation.html
css3/filters/effect-combined-hw.html
css3/filters/effect-brightness.html
css3/filters/filter-property-parsing-invalid.html
css3/filters/effect-brightness-clamping-hw.html
css3/filters/filter-property-parsing.html

[Dirk Schulze]

Created attachment 182492 [details]
Patch

[WebKit Review Bot]

Comment on attachment 182492 [details]
Patch

Attachment 182492 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15841248

New failing tests:
css3/filters/filter-property-parsing.html

[WebKit Review Bot]

Comment on attachment 182492 [details]
Patch

Attachment 182492 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15838233

New failing tests:
css3/filters/filter-property-parsing.html

[Dirk Schulze]

Created attachment 182495 [details]
Patch

Chrome has a separate expected file that needs an update.

[Dean Jackson]

Comment on attachment 182495 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182495&action=review

I guess we should still have a test for values below 0 since we accept them.

> LayoutTests/css3/filters/script-tests/filter-property-parsing.js:331
> +testFilterRule("Parameter less then -100%",

Typo "than"

> LayoutTests/css3/filters/script-tests/filter-property-parsing.js:336
> +testFilterRule("Parameter more then 100%",

Ditto.

[Dirk Schulze]

Committed r139770: <http://trac.webkit.org/changeset/139770>