Closed
Bug 826114
Opened 11 years ago
Closed 10 years ago
Support Filter Effects specification feColorMatrix "saturate" type range
Categories
(Core :: SVG, enhancement)
Tracking
()
RESOLVED
FIXED
mozilla29
People
(Reporter: solaveritasteliberum, Assigned: longsonr)
Details
Attachments
(1 file)
2.27 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
When using SVG feColorMatrix filters on HTML elements as described here (https://developer.mozilla.org/en-US/docs/CSS/filter), the "saturate" type only accepts values between 0 and 1. If a value outside of that range is entered, the entire element isn't rendered. See the example here: http://jsfiddle.net/dominic_p/wSbaR/. According to the W3C spec (https://dvcs.w3.org/hg/FXTF/raw-file/tip/filters/index.html#feColorMatrixTypeAttribute) values outside of this range should over or under saturate the element. I hope this helps.
Updated•11 years ago
|
Component: Style System (CSS) → SVG
Assignee | ||
Comment 1•11 years ago
|
||
The current SVG specification says 0-1 is the range: http://www.w3.org/TR/SVG/filters.html#feColorMatrixElement The referenced document is an Editor's Draft of the proposed new version. We'll likely implement this at some point.
Assignee | ||
Updated•11 years ago
|
Summary: Limited Support for SVG feColorMatrix "saturate" type → Support Filter Effects specification feColorMatrix "saturate" type range
@Robert, that's a good point. But, isn't the whole concept of SVG filters being applied in CSS an un-finalized spec at the moment? My thinking is: if Firefox is going to support an experimental feature, we should try to keep to the specs for that feature if possible. Does that make sense?
Assignee | ||
Comment 3•11 years ago
|
||
I'm not sure what you mean, we don't support CSS filters either.
![]() |
||
Comment 4•11 years ago
|
||
> isn't the whole concept of SVG filters being applied in CSS an un-finalized spec at the
> moment?
Applying SVG filters via the "filter" CSS property is part of SVG 1.1.
Assignee | ||
Comment 5•11 years ago
|
||
So we support the first case of https://dvcs.w3.org/hg/FXTF/raw-file/tip/filters/index.html#FilterFunction because that's also in SVG 1.1 but not the others which are new.
Ok, that makes sense. I never understood why only the first case of the filter functions was supported, but that makes perfect sense now. If you ask me, support for the rest of the filter functions listed in the draft would be awesome, but that's probably a separate issue. :)
Assignee | ||
Updated•11 years ago
|
Severity: normal → enhancement
Comment 7•10 years ago
|
||
I vote also for this change or enhancement, im currently having trouble because of this issue, as using a value of 2 or 3 in chrome creates strong saturation, but fails totally to render in firefox because it doesnt accept values over 1. I can see that in firefox, 0 means no color=bw, and 1 means 100% saturation, basically the normal color. But then how are we supposed to oversaturate or increase the color saturation of an image in firefox using this filter? we simply cant, we can only maintain the color or decrease its saturation, but there is no way to increase the saturation with this filter. Until there is a solution, if you know another way to increase the color saturation by using a different svg filter please let us know, thanks very much
Comment 8•10 years ago
|
||
As https://dvcs.w3.org/hg/FXTF/raw-file/tip/filters/index.html#feColorMatrixValuesAttribute says values can be outside the range [0,1], we should allow this. It should be a relatively straightforward fix in SVGFEColorMatrix.cpp: just remove the early exit if the saturation value is outside the range [0,1]. We already clamp the resulting colour values at the end of the function.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 9•10 years ago
|
||
Any news about this? Many of us find it rather absurd that we cannot allow users to increase the saturation applying SVG filters because Firefox is not supporting it; Consulting any designer will confirm the importance of allowing the increase of saturation to make colors more vivid, hopefully you can activate this soon, thank you
Comment 10•10 years ago
|
||
This is the part that seems to require the change: File SVGFEColorMatrixElement.cpp : case SVG_FECOLORMATRIX_TYPE_SATURATE: if (values.Length() != 1) return NS_ERROR_FAILURE; s = values[0]; if (s > 1 || s < 0) return NS_ERROR_FAILURE; Seems the change would be: if (s < 0) return NS_ERROR_FAILURE;
Assignee | ||
Comment 11•10 years ago
|
||
If you follow the instructions here you should be able to attach a correctly formatted patch: https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F We need a testcase too that's normally 2 files but can be only one if you can create something that displays entirely as lime with the fix and not-lime without. Otherwise if you could create 2 files one which displays the same always (the reference) and one which displays the same as the reference if this issue is fixed and differently if it isn't would be OK. If you need any additional help feel free to ask.
Comment 12•10 years ago
|
||
Dear Robert, thank you very much, thing is i have no experience with patches, Git, mozilla firefox code, c++ etc, i wish i could do what you are writing about, i would have done it yesterday, but i really can't, all i'm able to do is hopefully somebody else who is an experienced developer and interested in this fix can do it; Just to express it better, i go to the link you provide, and a phrase like this "Here is a very easy way to get this setup working using mq. First, you need to edit your ~/.hgrc file. You only need to do this once. Your hgrc file should have the following contents at least:" sounds like chinese to me, no idea what any of that means :) mq, hgrc, etc, etc, really im not the right person to do that, very unfortunately (i wish i could :) )
Assignee | ||
Comment 13•10 years ago
|
||
Creating the reference and the testcase and attaching them would require no coding experience. How about tackling that and I'll do the rest.
Comment 14•10 years ago
|
||
Of course, i'm happy to help, where can i find instructions for creating reference and test case? are they here also ? https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F , if not send me instructions and i will be happy to do it
Assignee | ||
Comment 15•10 years ago
|
||
Create 2 SVG files. One (the testcase) which displays correctly only if this issue is fixed. The other (the reference) displays what the reference would show if the bug is fixed but does that whether or not the bug is fixed. So the reference must work around the bug in some way. The most common way is to create a testcase that displays completely lime if the bug is fixed (see pass.svg) in which case we don't need a reference as we can reuse the pass.svg reference file. There are hundreds of examples here: http://mxr.mozilla.org/mozilla-central/find?text=&string=layout%2Freftests%2Fsvg
Comment 16•10 years ago
|
||
DEar Robert, thank you again for your support with this case, I have created the testcase, these are the files 1) The reference as you suggested is the full lime color at http://mxr.mozilla.org/mozilla-central/source/layout/reftests/svg/filters/pass.svg?force=1 2) The testcase is here: http://volandino.com/tests/firefox/testcasefirefoxsaturate.html A value of 11 is being applied over SVG saturation filter to the image Running the testcase in google chrome generates the same Lime color than the reference Running the testcase in firefox generates nothing because values of saturation over 1 just output nothing In the link above the code contains a normal png image, but i also attach here the code of the testcase with base64 image integrated: <!doctype html> <html> <head> <meta charset="utf-8"> <title>TestCase OverSaturate, should obtain Lime color as in reference</title> <svg width="0%" height="0%"> <defs> <filter id="SaturateHigh"> <feColorMatrix in="SourceGraphic" type="saturate" values="11" result="A"/> </filter> </defs> </svg> <style>.base{filter:url(#SaturateHigh);-webkit-filter:url(#SaturateHigh)}</style> </head> <body> <div class="base"><img src="data:image/jpeg;base64,/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDAAUDBAQEAwUEBAQFBQUGBwwIBwcHBw8LCwkMEQ8SEhEPERETFhwXExQaFRERGCEYGh0dHx8fExciJCIeJBweHx7/2wBDAQUFBQcGBw4ICA4eFBEUHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh7/wgARCACTASwDASIAAhEBAxEB/8QAFgABAQEAAAAAAAAAAAAAAAAAAAEF/8QAFwEBAQEBAAAAAAAAAAAAAAAAAAEEBv/aAAwDAQACEAMQAAABzhk4IAAAEBQAAAAAAAAAQFBAUAAAAAAAAAAAAAAABYQFAAAAABAUAEBQAAQFAAAAAABAUAAAAAAAAAAEBQAAAAAAAAAQFAAAAAAAAAAAAAAAAAAAAABAUAAAEBQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAQFAAAAAAAAAAAAABAUBQgAQFAAAABLABQAAAAARQ//8QAFBABAAAAAAAAAAAAAAAAAAAAkP/aAAgBAQABBQI0v//EABQRAQAAAAAAAAAAAAAAAAAAAHD/2gAIAQMBAT8BPv/EABQRAQAAAAAAAAAAAAAAAAAAAHD/2gAIAQIBAT8BPv/EABQQAQAAAAAAAAAAAAAAAAAAAJD/2gAIAQEABj8CNL//xAAUEAEAAAAAAAAAAAAAAAAAAACQ/9oACAEBAAE/ITS//9oADAMBAAIAAwAAABD7777/AO+++++++++/+/8Avvvvvvvvvvvvvvvvr/vvvvvv/vv/AL77/wC++++++/8Avvvvvvvvvvv/AL777777777/AO+++++++++++++++++++++/++++/wDvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv/vvvvvvvvvvvvvv/AL6L7/777779/wC+++++/j//xAAUEQEAAAAAAAAAAAAAAAAAAABw/9oACAEDAQE/ED7/xAAUEQEAAAAAAAAAAAAAAAAAAABw/9oACAECAQE/ED7/xAAcEAEAAwACAwAAAAAAAAAAAAABABAgMYBBUFH/2gAIAQEAAT8Q6zPQp974yRjhyV9yww8whRbX/9k=" alt=""/></div> </body> </html>
Assignee | ||
Comment 17•10 years ago
|
||
Assignee: nobody → longsonr
Attachment #8362211 -
Flags: review?(matt.woodrow)
Comment 18•10 years ago
|
||
Comment on attachment 8362211 [details] [diff] [review] patch Switching the review to Markus, since he worked on this.
Attachment #8362211 -
Flags: review?(matt.woodrow) → review?(mstange)
Comment 19•10 years ago
|
||
thank you very much, sorry for asking as i'm new to these processes, what is next? is this likely to be fixed or we don't really know until it is reviewed and approved? just wondering, thank you again very much
Updated•10 years ago
|
Attachment #8362211 -
Flags: review?(mstange) → review+
Assignee | ||
Comment 20•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=8dc06ae80f1f
Assignee | ||
Comment 21•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/01a501a19293
Flags: in-testsuite+
Assignee | ||
Updated•10 years ago
|
OS: Windows 7 → All
Hardware: x86_64 → All
Comment 22•10 years ago
|
||
question in response to previous comment (OS: Windows 7 → All), my app is running on Firefox Linux 64bits, Centos 6, Linux 64bit, will the patch apply as well to linux, in my case linux is the most critical OS i need to cover (windows as well, but linux even more because my live server is linux)
Comment 23•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/01a501a19293
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Assignee | ||
Comment 24•10 years ago
|
||
javismiles: Given this landed more than 24 hours ago, it will have made it to nightly builds by now http://nightly.mozilla.org/ should you wish to check them out.
Comment 25•10 years ago
|
||
thank you lots Robert, i just copied you also in this other bug in case you can give me a hand to make a patch also there, its for an upper font size limit, https://bugzilla.mozilla.org/show_bug.cgi?id=928449 , thank you again for all your support
Comment 26•10 years ago
|
||
Robert, this patch , the saturation one, does it apply only to windows or also to Linux? cause i need to apply it to Linux, thanks lots
Assignee | ||
Comment 27•10 years ago
|
||
If you visit the link in comment 25 you can find out yourself ;-)
Assignee | ||
Comment 28•10 years ago
|
||
Comment 24 I mean.
Comment 29•10 years ago
|
||
got you thank u Robert, so i see there is also linux there thats great, could you also tell me whats the filename where this patch was applied in the source of Xulrunner? i can see that the patch seems to be in a/gfx/src/FilterSupport.cpp 1.2 +++ b/gfx/src/FilterSupport.cpp 1.3 @@ -318,17 +318,17 @@ ComputeColorMatrix(uint32_t aColorMatrix etc but i dont find a gfx/src/FilterSupport.cpp in the Xulrunner source code, reason i ask is because i have rebuilt xulrunner with a solution also to the font size issue and i may also try to do this change myself in the source of xulrunner, but i need to identify the file where the patch is applied thanks lots
Assignee | ||
Comment 30•10 years ago
|
||
It's not in Xulrunner at all.
Comment 31•10 years ago
|
||
that file may not be, but the code has to be, reason is, i am currently using SlimerJS + Xulrunner, and i am applying SVG filters, and i am using Xulrunner, and i can apply saturate (<1) and all the other filters, so obviously the code for the SVG saturate filter is in Xulrunner, so i need to find what is the file in Xulrunner where similar comparison (<1) is being done
Assignee | ||
Comment 32•10 years ago
|
||
I'm afraid I don't know.
Comment 33•10 years ago
|
||
no probs will search for it, thank you again for your help and support, thanks lots Robert
You need to log in
before you can comment on or make changes to this bug.
Description
•