feConvolveMatrix 'order' attribute does not default to 3

RESOLVED FIXED

Status

()

Core
SVG
--
minor
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Martin, Assigned: Martin)

Tracking

({fixed1.9.1})

unspecified
fixed1.9.1
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Assignee)

Description

9 years ago
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
(Assignee)

Comment 1

9 years ago
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.
(Assignee)

Comment 2

9 years ago
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.
Attachment #362418 - Flags: review?(roc)
(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
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.
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
Attachment #362418 - Flags: review?(roc) → review+
Keywords: checkin-needed
Whiteboard: [needs landing]
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 ..." :-)
(Assignee)

Comment 7

9 years ago
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.
Attachment #362418 - Attachment is obsolete: true
Attachment #362548 - Flags: review?(roc)

Comment 8

9 years ago
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

Updated

9 years ago
Keywords: checkin-needed
Whiteboard: [needs landing]
Also, you can use pass.svg as your reference and still be fast by using filterRes to limit the filter resolution to something small.
(Assignee)

Updated

9 years ago
Attachment #362548 - Flags: review?(roc)
(Assignee)

Comment 10

9 years ago
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.
Attachment #362548 - Attachment is obsolete: true
Attachment #362694 - Flags: review?(longsonr)

Updated

9 years ago
Attachment #362694 - Flags: review?(longsonr) → review+
(Assignee)

Comment 11

9 years ago
Created attachment 362696 [details]
Extreme example of filterRes thing

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

Updated

9 years ago
Assignee: nobody → gzlist
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Whiteboard: [needs landing]

Comment 12

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

simple fix to default values, includes a patch.
Attachment #362694 - Flags: approval1.9.1?
Attachment #362694 - Flags: approval1.9.1? → approval1.9.1+
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [needs landing] → [needs 191 landing]

Comment 13

9 years ago
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
Keywords: fixed1.9.1
Whiteboard: [needs 191 landing]
You need to log in before you can comment on or make changes to this bug.