619968 - feGaussianBlur with a zero std deviation in either direction should not cause rendering to fail

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Cameron McCormack (:heycam)]

If an <feGaussianBlur> has stdDeviation="0" or stdDeviation="1 0" or stdDeviation="0 1", then the filter output should simply not blur the directions with 0 std deviation, rather than producing no output.

This causes the above URL to fail as well as http://dev.w3.org/SVG/profiles/1.1F2/test/harness/htmlObjectMiniApproved/filters-gauss-03-f.html.

Comment 1

[Robert Longson [:longsonr]]

Attachment: patch

Comment 2

[Jonathan Watt [:jwatt]]

heycam, for future reference, can you assign me to bugs you file that you know I (or someone else) has a patch for to avoid a third person duplicating work? In this case it doesn't really matter much since the patch is trivial, but something to keep in mind.

Comment 3

[Jonathan Watt [:jwatt]]

Comment on attachment 498495 [details] [diff] [review]
patch

longsonr, can you also remove the special case code at the callee?

Comment 4

[Robert Longson [:longsonr]]

Attachment: patch

Comment 5

[Jonathan Watt [:jwatt]]

Comment on attachment 498507 [details]
patch

r=jwatt

Can you also change the test to this though (assuming it passes!):

  <defs>
    <filter id="identity" filterUnits="objectBoundingBox">
       <feGaussianBlur stdDeviation="0"/>
       <feOffset dx="-5" dy="-5"/>
    </filter>
  </defs>
  <rect width="100%" height="100%" fill="lime"/>
  <rect x="5" y="5" width="50" height="50" fill="red" stroke="red" stroke-width="2"/>
  <rect x="10" y="10" width="50" height="50" fill="lime" stroke="lime" stroke-width="2" filter="url(#identity)"/>

The idea of the feOffset is to make sure the test only passes if filters are supported, and the idea behind putting the rects further up in the top left corner is to make sure the red is onscreen even on the tiniest of screens (not really relevant to how we test, but it is as we try and get other implementations to use our tests).

Comment 6

[Jonathan Watt [:jwatt]]

Oh, and I removed the x/y/width/height attributes on the <filter> since they shouldn't be necessary with the rects the size they are in comment 5.

Comment 7

[Robert Longson [:longsonr]]

(In reply to comment #5)
> Can you also change the test to this though (assuming it passes!):
> 

It doesn't pass.

Comment 8

[Robert Longson [:longsonr]]

Attachment: patch

Comment 9

[Robert Longson [:longsonr]]

I discovered there's already a reftest that checks it doesn't render so I adjusted that to check that it does.

Comment 10

[Jonathan Watt [:jwatt]]

(In reply to comment #7)
> It doesn't pass.

That seems to be because we're not calculating the filter primitive subregion correctly. Looking into that now.

Comment 11

[Cameron McCormack (:heycam)]

(In reply to comment #2)
> heycam, for future reference, can you assign me to bugs you file that you know
> I (or someone else) has a patch for to avoid a third person duplicating work?
> In this case it doesn't really matter much since the patch is trivial, but
> something to keep in mind.

Ouch, sorry about that.  I should've remembered that you'd started looking at it.

Comment 12

[Jonathan Watt [:jwatt]]

I actually showed you it working, although given the number of tests we went through you could be forgiven for forgetting! ;) Anyway, no worries, it was a small patch. :)

Pushed http://hg.mozilla.org/mozilla-central/rev/01cb674212d4

Comment 13

[Robert Longson [:longsonr]]

Not sure what you mean, the comment 5 testcase shows a red border on Windows.

Comment 14

[Robert Longson [:longsonr]]

Ah comment 12 was a reply to comment 11.