User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US) AppleWebKit/533.4 (KHTML, like Gecko) Chrome/5.0.375.125 Safari/533.4
Build Identifier: Mozilla/5.0 (X11; Linux i686; rv:2.0b2) Gecko/20100720 Firefox/4.0b2
I have created this SVG which renders hair on a monster using filters (feTurbulence and feDisplacementMap).
The hair appears to be offset to the bottom, right. To overcome the error I had to add another feOffset filter.
In Opera, there is no such offset required, and hence the above SVG renders wrong in Opera.
Since no offset seems intuitive, I am assuming it's a bug with Firefox rather than Opera. Webkit has only recently added support for feTurbulence and so I couldn't test it.
Steps to Reproduce:
1. Open http://lavadip.com/experiments/bakemono_svg/snap.svg with Firefox 3.5+ or 4.0 b2
The hair appears centered, but only because an offset has been subtracted.
The hair should appear centered even without the feOffset filter.
For convenience, a simpler SVG is here http://lavadip.com/experiments/bakemono_svg/snap_small.svg
Just confirmed that Inkscape renders the SVGs exactly like Opera. If they are right, then Firefox might be doing something wrong.
Hmm. I wonder whether there's a colorspace issue here or something; this looks like the sort of systematic offset that could come up if linearRGB values are interpreted as sRGB or vice versa...
(In reply to comment #2)
> Hmm. I wonder whether there's a colorspace issue here or something; this looks
> like the sort of systematic offset that could come up if linearRGB values are
> interpreted as sRGB or vice versa...
Ah, sounds like a valid reason. Thanks for looking into it.
If true, is it a problem with the browser, or incomplete color space specification in my SVG?
If the former, is there anything I can do to work-around the problem while the browser gets a fix?
My current guess is this is an issue on our end, especially since playing around with the colorspace values in the testcase didn't seem to change the behavior... but filters do their own weird colorspace stuff in the code. :(
Created attachment 462848 [details]
testcase with scale=1 [no difference vs. opera]
Confirming that Opera & Inkscape both disagree with us.
Here's a reduced-ish testcase of bugzilla's own. The red blob is offset to the lower-right on Firefox, but not on Opera / Inkscape.
It looks like the amount of offset is (in part) controlled by 'scale' on the <feDisplacementMap>. I can set 'scale' to anything without offsetting the blob in Opera/Inkscape, whereas in Firefox, the lower-right offset increases as 'scale' increases. At scale=1, we match Opera/Inkscape.
Created attachment 462849 [details]
testcase with scale=50
(In reply to comment #5)
> At scale=1, we match Opera/Inkscape.
d'oh -- of course, to verify that, I set the scale to 1 in my testcase right before clicking 'submit', and ended up submitting a testcase that doesn't show any brokenness. :)
Here's a version with scale=50 that demonstrates the bug.
Hmm. So our relevant code seems to be:
5843 double scaleOver255 = scale / 255.0;
5844 double scaleAdjustment = 0.5 - 0.5 * scale;
5850 PRInt32 sourceX = x +
5851 NSToIntFloor(scaleOver255 * displacementData[targIndex + xChannel] +
The spec here says:
P'(x,y) <- P( x + scale * (XC(x,y) - .5), y + scale * (YC(x,y) - .5))
The mapping between the two is:
* targIndex == y*width + 4*x (so the byte offset in the target image where we
want to write the data).
* displacementData[targIndex + xChannel] == XC(x,y) * 255.0
So our formula for sourceX comes out to (ignoring the rounding):
x + scale/255.0 * XC(x,y) * 255.0 + (0.5 - 0.5 * scale) =
x + scale * (XC(x,y) - 0.5) + 0.5
which seems fine (the +0.5 is to make the floor actually round-to-nearest)....
This seems invalid to me too, so I'm going to mark it as such. It can always be reopened if anything else turns up that contradicts our reasoning.
HRJ feel free to take this up with w3c or Opera and Inkscape. w3c can be contacted via http://lists.w3.org/Archives/Public/www-svg/
No, I think the bug is valid. The feDisplacementMap impl looks ok, so the issue is either in the color spaces (as in, we get them wrong) or in the feTurbulence output. If the color component mean of the feDisplacementMap is not .5, then that would give the observed effect, for example.
We get this right FWIW: http://www.w3.org/Graphics/SVG/Test/20061213/htmlObjectHarness/full-filters-turb-01-f.html it's in the linearRGB colour space.
(In reply to comment #9)
If there is a problem with feTurbulence or colour spaces then surely scale="1" would be affected and according to comment 5 that's OK.
Are you by any chance mixing up premultiplied and non-premultiplied colors? According to the spec the image data should be premultiplied and the displacement data non-premultiplied.
Yeah, the scale="1" thing confuses me too... Especially because when I try feDisplacementMap on feFloods, it seems to work fine at first glance (but could you double-check that?).
Note that from what I just see visually, our feTurbulence output looks visually different from Opera's, but so do our colors in general. So that might be a red herring.
(In reply to comment #12)
> Are you by any chance mixing up premultiplied and non-premultiplied colors?
> According to the spec the image data should be premultiplied and the
> displacement data non-premultiplied.
Created attachment 463835 [details] [diff] [review]
patch with reftest
Why do you need the aImage parameter?
aImage allows us to distinguish between IN1 and result. In this case we happen not to need it as it's IN2 rather than IN1 we're interested in. I can remove it if you like.
You mean we know it's operating on an input if aImage is non-null? That's a bit of a hack :-)
How about making the index a signed int and making -1 mean the output? Give it a proper constant name and it won't even look like a hack :-)
Created attachment 464001 [details] [diff] [review]
address review comment
Comment on attachment 464001 [details] [diff] [review]
address review comment
Simple low risk fix that just changes how the feDisplacementMap filter works. Includes a test.
The reftest that is part of the patch was not landed. That's why it failed. It's the only new file in the patch.
I made up a commit message; hope it's ok. (It would have been nice to have one in the patch.)