Closed Bug 619968 Opened 9 years ago Closed 9 years ago

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

Categories

(Core :: SVG, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: heycam, Assigned: longsonr)

References

()

Details

Attachments

(1 file, 2 obsolete files)

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.
Assignee: nobody → longsonr
Attached patch patch (obsolete) — Splinter Review
Attachment #498495 - Flags: review?(roc)
Attachment #498495 - Flags: approval2.0?
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.
OS: Mac OS X → All
Hardware: x86 → All
Comment on attachment 498495 [details] [diff] [review]
patch

longsonr, can you also remove the special case code at the callee?
Attachment #498495 - Flags: review?(roc) → review?(jwatt)
Attached file patch (obsolete) —
Attachment #498495 - Attachment is obsolete: true
Attachment #498507 - Flags: review?(jwatt)
Attachment #498507 - Flags: approval2.0?
Attachment #498495 - Flags: review?(jwatt)
Attachment #498495 - Flags: approval2.0?
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).
Attachment #498507 - Flags: review?(jwatt) → review+
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.
(In reply to comment #5)
> Can you also change the test to this though (assuming it passes!):
> 

It doesn't pass.
Attached patch patchSplinter Review
Attachment #498507 - Attachment is obsolete: true
Attachment #498511 - Flags: approval2.0?
Attachment #498507 - Flags: approval2.0?
I discovered there's already a reftest that checks it doesn't render so I adjusted that to check that it does.
(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.
(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.
Keywords: checkin-needed
Whiteboard: [needs landing]
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
Status: NEW → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing]
Not sure what you mean, the comment 5 testcase shows a red border on Windows.
Ah comment 12 was a reply to comment 11.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.