Bug 106674

Summary:

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

Product:

WebKit

Reporter:

Mike Sierra <letmespellitoutforyou>

Component:

CSS

Assignee:

Dirk Schulze <krit>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

buildbot, cmarcelo, dglazkov, dino, eric.lemoine, eric, krit, macpherson, menard, ojan.autocc, rniwa, simon.fraser, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

URL:

https://github.com/mike-sierra/webplatform/blob/master/bugs/brightness.html

Bug Depends on:

Bug Blocks:

106808

Attachments:

Description	Flags
patch for bots	none
Patch	none
Patch	none
Patch	dino: review+

Mike Sierra

Reported 2013-01-11 09:50:32 PST

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.

Attachments
patch for bots (3.45 KB, patch) 2013-01-13 09:40 PST, Dirk Schulze	no flags	Details Formatted Diff Diff
Patch (14.42 KB, patch) 2013-01-13 12:41 PST, Dirk Schulze	no flags	Details Formatted Diff Diff
Patch (18.45 KB, patch) 2013-01-13 14:07 PST, Dirk Schulze	no flags	Details Formatted Diff Diff
Patch (18.53 KB, patch) 2013-01-13 17:01 PST, Dirk Schulze	dino: review+	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Mike Sierra

Comment 1 2013-01-11 11:00:15 PST

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

Dirk Schulze

Comment 2 2013-01-11 15:34:55 PST

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

Dirk Schulze

Comment 3 2013-01-13 08:26:18 PST

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

Comment 4 2013-01-13 09:40:00 PST

Created attachment 182486 [details] patch for bots

Simon Fraser (smfr)

Comment 5 2013-01-13 10:41:14 PST

Comment on attachment 182486 [details] patch for bots Why no test?

WebKit Review Bot

Comment 6 2013-01-13 11:47:25 PST

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 7 2013-01-13 12:09:15 PST

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

Comment 8 2013-01-13 12:27:20 PST

(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

Comment 9 2013-01-13 12:41:13 PST

Created attachment 182488 [details] Patch Waiting for Chromium bot to know which pixel results need updates.

Build Bot

Comment 10 2013-01-13 13:08:17 PST

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 11 2013-01-13 13:49:31 PST

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

Comment 12 2013-01-13 14:07:57 PST

Created attachment 182492 [details] Patch

WebKit Review Bot

Comment 13 2013-01-13 15:17:25 PST

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 14 2013-01-13 15:52:58 PST

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

Comment 15 2013-01-13 17:01:35 PST

Created attachment 182495 [details] Patch Chrome has a separate expected file that needs an update.

Dean Jackson

Comment 16 2013-01-14 10:42:17 PST

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

Comment 17 2013-01-15 12:03:53 PST

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

Note You need to log in before you can comment on or make changes to this bug.