Last Comment Bug 478570 - feConvolveMatrix 'order' attribute does not default to 3
: feConvolveMatrix 'order' attribute does not default to 3
Status: RESOLVED FIXED
: fixed1.9.1
Product: Core
Classification: Components
Component: SVG (show other bugs)
: unspecified
: All All
: -- minor (vote)
: ---
Assigned To: Martin
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-02-14 11:23 PST by Martin
Modified: 2009-02-21 10:34 PST (History)
4 users (show)
roc: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Testcase (332 bytes, image/svg+xml)
2009-02-14 11:27 PST, Martin
no flags Details
Not sure if this is the *right* way of defaulting it (666 bytes, patch)
2009-02-14 11:36 PST, Martin
roc: review+
Details | Diff | Splinter Review
Same patch with reftest added (2.48 KB, patch)
2009-02-16 01:18 PST, Martin
no flags Details | Diff | Splinter Review
Reftest renamed as requested (2.50 KB, patch)
2009-02-17 02:17 PST, Martin
longsonr: review+
roc: approval1.9.1+
Details | Diff | Splinter Review
Extreme example of filterRes thing (294 bytes, image/svg+xml)
2009-02-17 02:48 PST, Martin
no flags Details

Description Martin 2009-02-14 11:23:22 PST
User-Agent:       Opera/9.63 (Windows NT 5.1; U; en-GB) Presto/2.1.1
Build Identifier: 

An SVG filter with a feConvolveMatrix element lacking an "order" attribute produces no output.

By the spec:
<http://www.w3.org/TR/SVG/filters.html#feConvolveMatrixElementOrderAttribute>
"If the attribute is not specified, the effect is as if a value of "3" were specified."
...just noticed the DTD has it as required, and searching for more info turned up exactly this question (which was never answered?) from a year ago:
<http://lists.w3.org/Archives/Public/www-svg/2008Feb/0010.html>

Reproducible: Always
Comment 1 Martin 2009-02-14 11:27:59 PST
Created attachment 362417 [details]
Testcase

Passes in Batik 1.7 and Opera 9.6 (bugzilla didn't need to record their UA string really...), fails in trunk Firefox.
Comment 2 Martin 2009-02-14 11:36:15 PST
Created attachment 362418 [details] [diff] [review]
Not sure if this is the *right* way of defaulting it

Tested before I noticed the contradiction in the spec, roc can decide what to do with it as he's months ahead of me.

I'd be just as happy with a patch that surfaced the filter error to the console so I had some idea of *why* it wasn't doing anything.
Comment 3 Cameron McCormack (:heycam) (away Sep 28) 2009-02-14 13:00:15 PST
(In reply to comment #2)
> Tested before I noticed the contradiction in the spec, roc can decide what to
> do with it as he's months ahead of me.

It seems to have been overlooked.  I've raised an issue so that it will be discussed:

  http://www.w3.org/Graphics/SVG/WG/track/issues/2225
Comment 4 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-02-15 13:29:43 PST
Oops, I just posted another message to www-svg about it. Thanks Cameron.

Martin, I think your patch looks good. If the WG decides the default should be 3, we'll take it.
Comment 5 Cameron McCormack (:heycam) (away Sep 28) 2009-02-15 17:24:33 PST
Seems like we did already create an erratum for this issue; that mail just wasn't replied to.

http://dev.w3.org/SVG/profiles/1.1F2/errata/errata.xml#clarify-feconvolvematrix-order-attribute
Comment 6 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-02-15 17:32:55 PST
Thanks Cameron.

Martin, this can land as-is, but for bonus points, you could create an SVG reftest (see layout/reftests/svg/filters) that ensures a filter without the 'order' attribute renders the same as the filter with the 'order' attribute. You could also change the commit comment to include the bug number and "r=roc", and you don't need to say "Probably the wrong way ..." :-)
Comment 7 Martin 2009-02-16 01:18:40 PST
Created attachment 362548 [details] [diff] [review]
Same patch with reftest added

I wasn't certain it'd really be that easy when taking non-square matrices into account as well, and hadn't read enough of the code to find out, so felt a caveat was needed. :)

Went with your suggestion of comparing same-but-for-order-attribute filters. Had just added the testcase above originally, but didn't like being the slowest reftest in the dir. When fiddling filter size, added ref rather than use generic one.
Comment 8 Robert Longson 2009-02-16 07:20:49 PST
Please make the test file names include the filter element (feConvolvematrix) and a number. So convolve-order-default-ref.svg should be feConvolveMatrix-order-01-ref.svg and convolve-order-default.svg would be feConvolveMatrix-order-01.svg
Comment 9 Robert O'Callahan (:roc) (email my personal email if necessary) 2009-02-16 12:36:02 PST
Also, you can use pass.svg as your reference and still be fast by using filterRes to limit the filter resolution to something small.
Comment 10 Martin 2009-02-17 02:17:35 PST
Created attachment 362694 [details] [diff] [review]
Reftest renamed as requested

Didn't seem to fit the naming scheme described at the top of the reftest.list so had emulated the newer tests in the bottom half.

The filterRes idea was nice, but blurs at the edges. Could rely on the default 20% filter padding and the resolution being high enough to hide, but seemed dodgy.
Comment 11 Martin 2009-02-17 02:48:08 PST
Created attachment 362696 [details]
Extreme example of filterRes thing

On the off-chance it's not intended behaviour.
Comment 12 Robert Longson 2009-02-19 02:01:21 PST
Comment on attachment 362694 [details] [diff] [review]
Reftest renamed as requested

simple fix to default values, includes a patch.

Note You need to log in before you can comment on or make changes to this bug.