478570 - feConvolveMatrix 'order' attribute does not default to 3

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Martin]

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]

Attachment: 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]

Attachment: 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)]

(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)]

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)]

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)]

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]

Attachment: 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 [:longsonr]]

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)]

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]

Attachment: 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]

Attachment: Extreme example of filterRes thing

On the off-chance it's not intended behaviour.

Comment 12

[Robert Longson [:longsonr]]

Comment on attachment 362694 [details] [diff] [review]
Reftest renamed as requested

simple fix to default values, includes a patch.

Comment 13

[Robert Longson [:longsonr]]

Checked in http://hg.mozilla.org/releases/mozilla-1.9.1/rev/556f55915997 and http://hg.mozilla.org/releases/mozilla-1.9.1/rev/cef52557e692