Bug 69245

Summary: feColorMatrix saturation is not limited to range 0..1 anymore
Product: WebKit Reporter: Dirk Schulze <krit>
Component: SVGAssignee: Dirk Schulze <krit>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, dino, webkit.review.bot, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://dahlström.net/svg/filters/colormatrix/svgimagefiltering.xhtml
Bug Depends on:    
Bug Blocks: 68469, 26389    
Attachments:
Description Flags
Patch none

Dirk Schulze
Reported 2011-10-03 03:20:26 PDT
Oper doesn't limit the range of values for saturation from 0 to 1. The limitation was also removed from the new filter effects 1.0 specification.
Attachments
Patch (77.14 KB, patch)
2011-10-03 04:02 PDT, Dirk Schulze 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Dirk Schulze
Comment 1 2011-10-03 04:02:15 PDT
Created attachment 109464 [details] Patch
Nikolas Zimmermann
Comment 2 2011-10-03 04:11:59 PDT
Comment on attachment 109464 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109464&action=review Patch looks good, r=me. > LayoutTests/svg/filters/feColorMatrix-values.svg:66 > + <text y="340" x="210" style="filter:url(#satrange)">Saturate must </text> > + <text y="370" x="210" style="filter:url(#satrange)">have one value.</text> I don't see this text in the png. Can you remove it and make it a comment?
Dirk Schulze
Comment 3 2011-10-03 04:23:24 PDT
(In reply to comment #2) > (From update of attachment 109464 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=109464&action=review > > Patch looks good, r=me. > > > LayoutTests/svg/filters/feColorMatrix-values.svg:66 > > + <text y="340" x="210" style="filter:url(#satrange)">Saturate must </text> > > + <text y="370" x="210" style="filter:url(#satrange)">have one value.</text> > > I don't see this text in the png. Can you remove it and make it a comment? Thats the sense of this test. You should just see the text in the png, if the test fails. Wasn't my idea, but I like it :)
Nikolas Zimmermann
Comment 4 2011-10-03 04:27:29 PDT
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 109464 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=109464&action=review > > > > Patch looks good, r=me. > > > > > LayoutTests/svg/filters/feColorMatrix-values.svg:66 > > > + <text y="340" x="210" style="filter:url(#satrange)">Saturate must </text> > > > + <text y="370" x="210" style="filter:url(#satrange)">have one value.</text> > > > > I don't see this text in the png. Can you remove it and make it a comment? > > Thats the sense of this test. You should just see the text in the png, if the test fails. Wasn't my idea, but I like it :) Aww, I understand, okay just go ahead then :-)
WebKit Review Bot
Comment 5 2011-10-03 04:42:57 PDT
Comment on attachment 109464 [details] Patch Attachment 109464 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9935104 New failing tests: svg/filters/feColorMatrix-saturate.svg svg/filters/feColorMatrix-values.svg
Dirk Schulze
Comment 6 2011-10-03 07:26:24 PDT
Comment on attachment 109464 [details] Patch Clearing flags on attachment: 109464 Committed r96498: <http://trac.webkit.org/changeset/96498>
Dirk Schulze
Comment 7 2011-10-03 07:26:34 PDT
All reviewed patches have been landed. Closing bug.
Dirk Schulze
Comment 8 2011-10-03 09:01:38 PDT
Committed r96504: <http://trac.webkit.org/changeset/96504>
Note You need to log in before you can comment on or make changes to this bug.