Open Bug 578995 Opened 14 years ago Updated 2 years ago

Canvas shadows are not drawn according to spec (or other browsers)

Categories

(Core :: Graphics: Canvas2D, defect)

1.9.2 Branch
x86
Windows Server 2003
defect

Tracking

()

People

(Reporter: ayg, Unassigned)

References

Details

Attachments

(1 file)

Test case:

http://test.w3.org/html/tests/submission/PhilipTaylor/canvas/index.2d.shadow.blur.html

To see the difference clearly, right-click and Shift+View Image on the expected and actual, put the two tabs in the same window, and tab back and forth between them.  For another test, these should display identically:

data:text/html,<!doctype html> 
<style>* { margin: 0 }</style> 
<body> 
<canvas height="300" width="300"></canvas> 
<script> 
window.addEventListener('load', function () {
context = document.getElementsByTagName("canvas")[0].getContext("2d");
context.shadowBlur = 12.5;
context.shadowOffsetX = 320;
context.shadowOffsetY = 320;
context.shadowColor = 'black';
context.fillRect(-300, -300, 100, 100);
}, false);
</script> 

data:image/svg+xml,<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd"> 
<svg xmlns="http://www.w3.org/2000/svg"> 
<defs> 
	<filter id="Gaussian_Blur"><feGaussianBlur in="SourceGraphic" stdDeviation="5"/></filter> 
</defs> 
<rect x="20" y="20" width="100" height="100" style="fill:black;filter:url(#Gaussian_Blur)"/> 
</svg>

Firefox nightlies are wrong.  Chrome is also wrong, but Opera, Safari (on Windows), and IE9PP2 get all of those correct.  (Except Safari 5 and IE9PP2 don't support SVG filters, so the second test isn't relevant for them.  And Opera's crazy handling of # in data URLs means you need to use a file to test that.)

Philip Taylor and I both reviewed the relevant code in Gecko.  I noticed the line


    gfxFloat sigma = CurrentState().shadowBlur > 8 ? sqrt(CurrentState().shadowBlur) : CurrentState().shadowBlur / 2;

in content/canvas/src/nsCanvasRenderingContext2D.cpp, which doesn't match the spec behavior, and moreover makes no sense.  It means that a shadowBlur of 8 gives sigma of 4, but a shadowBlur of 8 + epsilon gives sigma of ~2.8 (*less* blurring*).  In fact, shadowBlur of 8 and 16 give identical results.  You're supposed to take the square root of 2*shadowBlur if shadowBlur is > 8:

http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#shadows

Philip noticed a further bug in gfx/thebes/gfxBlur.cpp, namely:

// Blur radius is approximately 3/2 times the box-blur size
static const gfxFloat GAUSSIAN_SCALE_FACTOR = (3 * sqrt(2 * M_PI) / 4) * (3/2);

which is obviously wrong since 3/2 == 1.

I'm attaching a trivial patch that fixes these two problems (no test cases or anything yet).  This makes Firefox look better on the test cases I gave, but it's still wrong.  The code in gfx/thebes/gfxBlur.cpp (which is only called for canvas right now AFAICT) should be matching the output of content/svg/content/src/nsSVGFilters.cpp, which by the way seems to duplicate a lot of code, but it's not.  I'll CC the people involved in bug 310682, which implemented canvas shadows to begin with.  Also CCing roc since he offered to review.
(In reply to comment #0)
> data:image/svg+xml,<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN"
> "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd"> 
> <svg xmlns="http://www.w3.org/2000/svg"> 
> <defs> 
>     <filter id="Gaussian_Blur"><feGaussianBlur in="SourceGraphic"
> stdDeviation="5"/></filter> 
> </defs> 
> <rect x="20" y="20" width="100" height="100"
> style="fill:black;filter:url(#Gaussian_Blur)"/> 
> </svg>

Who thought up the idea that linebreaks in data URLs should be eaten instead of becoming spaces, anyway?  This works better:

data:image/svg+xml,<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN"
 "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd"> 
<svg xmlns="http://www.w3.org/2000/svg">
<defs><filter id="Gaussian_Blur">
<feGaussianBlur in="SourceGraphic" stdDeviation="5"/>
</filter></defs> 
<rect x="20" y="20" width="100" height="100"
 style="fill:black;filter:url(#Gaussian_Blur)"/> 
</svg>
Attachment #457573 - Attachment is patch: true
Attachment #457573 - Attachment mime type: application/octet-stream → text/plain
If you can get identical displays using an SVG gaussian blur filter and canvas, can you create a reftest?
Is that patch ready for review?
(In reply to comment #2)
> If you can get identical displays using an SVG gaussian blur filter and canvas,
> can you create a reftest?

That's what I was thinking.  It would be cool if we could manage that.  If reftests fail when they're not exactly identical, though, then it might not be practical to be that precise.

(In reply to comment #3)
> Is that patch ready for review?

Sorry, I should have said.  The patch has at least the following problems:

1) It doesn't fully fix the problem.  I don't understand the code well enough to do that, which is why I CC'd the people responsible for fixing bug 310682.  (But it does fix two clear bugs by itself, so maybe this isn't prohibitive.)

2) It has no tests.  I don't know what sort of test would be suitable here.  If the code were further fixed up so that my SVG and canvas examples render pixel-perfect identical, then a reftest would work, but as-is that's not possible.

So if you think it's worth asking for review given that, I'll do so.
We certainly should be able to make such a reftest pass, since everything's under our control. Any volunteers? longsonr?
Is this fixed by bug 590039?
Hopefully, although we still need a reftest.
A Firefox nightly as of September 12 still fails the tests I give in the beginning of comment 0.  Do I need to wait for the next nightly?  (Actually, the SVG I give in the first comment doesn't have any blur at all in Firefox 3.6 or nightly.  I'm not sure why.)
aho in #whatwg points out that the SVG I give in comment #0 works if you save it as a file, but not as a data URL.  When I do that, I can still see a clear difference between actual and expected in the latest nightly.
Confirmed the issue remains in hand-compiled tip (53678:b836c7151fc6), although it's better than in Firefox 3.6.  Both of the changes in the patch from comment #0 were part of

http://hg.mozilla.org/mozilla-central/rev/830111e10951

from bug 590039, so the attached patch is obsolete.
OS: Linux → Windows Server 2003
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: