[CSS Filters][Regression] hue-rotate matrix has an incorrect entry

RESOLVED FIXED in mozilla35

Status

()

Core
Graphics
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Max Vujovic, Assigned: Max Vujovic)

Tracking

Trunk
mozilla35
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

4.89 KB, patch
Max Vujovic
: checkin+
Details | Diff | Splinter Review
(Assignee)

Description

3 years ago
I was writing this example of CSS filters:
http://codepen.io/adobe/pen/KyEpe

And I noticed that hue-rotate(0deg) changed the rendering of the image, which is incorrect.

Due to a recent refactoring, one of the hue-rotate matrix entries has the wrong equation. I'll post a patch to fix it shortly.

Our tests didn't catch this because the colors used in our tests (typically red / green) weren't affected by the incorrect entry. We need a test that touches the blue channel, too.
(Assignee)

Updated

3 years ago
Assignee: nobody → mvujovic
Blocks: 1058756, 1057180
(Assignee)

Comment 1

3 years ago
Created attachment 8488827 [details] [diff] [review]
Patch

"oneMinusLumB" should've been "lumB" in:
aOutMatrix[7] = lumB - oneMinusLumB * c - hueRotateB * s;

To match the previous code (which changed to use variables in 1058756):
aOutMatrix[7] = 0.072f - 0.072f * c - 0.283f * s;

For reference, lumB = 0.0722f.

I haven't ran this patch though the Windows try bots yet; they might require a bit of fuzziness on the color results.
Attachment #8488827 - Flags: review?(mstange)
Attachment #8488827 - Flags: review?(mstange) → review+
(Assignee)

Comment 2

3 years ago
Created attachment 8489471 [details] [diff] [review]
Patch [v2]

Added a bit of fuzziness for D2D and changed the filter in the test from an SVG hue-rotate filter to a CSS hue-rotate filter.

https://hg.mozilla.org/integration/mozilla-inbound/rev/390d478480f9
https://tbpl.mozilla.org/?tree=Try&rev=11849e0f6a68
Attachment #8488827 - Attachment is obsolete: true
Attachment #8489471 - Flags: checkin+
https://hg.mozilla.org/mozilla-central/rev/390d478480f9
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.