Closed Bug 522394 Opened 10 years ago Closed 10 years ago

SVG filter output not correct for big filter size

Categories

(Core :: SVG, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: krit, Assigned: roc)

References

(Depends on 1 open bug)

Details

(Whiteboard: opacity=0 is bug 519144)

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; de-de) AppleWebKit/531.2+ (KHTML, like Gecko) Safari/531.2+
Build Identifier: FF 3.5.3

Not sure if it is a problem of filterRegion calculation, feFlood or both. But as a test result I see a pink rect, with the size of the window. The result scales with the window. You'll see no scrollbars.

Reproducible: Always



Expected Results:  
red sqaure with: x="10" y="10" width="100" height="100"
test
Attachment #406359 - Attachment mime type: text/plain → image/svg
Summary: SVF filter output not correct for big filter size → SVG filter output not correct for big filter size
Attachment #406359 - Attachment mime type: image/svg → image/svg+xml
Assignee: nobody → roc
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Actually the main part of this testcase is INVALID IMHO.

Here's what we do in this testcase:
1) Because the filter region is huge, we set the temporary surface to a maximum size of 16384x16384
2) When the bounding-box of the <rect> object is mapped into the coordinates of the temporary surface, it comes to (0.0016384,0.00016384)-(0.0180224,0.00180224)
3) The spec doesn't say how to handle non-pixel-aligned filter primitive regions. We round them out to the nearest pixel boundary.
4) So in this case, the top-left pixel of the temporary surface is filled with red.
5) We draw that back to the screen, and that one pixel covers the entire screen.

I honestly think that's all OK, although I'm willing to listen to arguments that we should do something else. In practice, don't use crazy filter region sizes!
Attached patch fix assertionSplinter Review
We can remove the assertion. This patch makes nsSVGUtils::ConvertToSurfaceSize more robust, right now if you pass in a large value it can be converted to a negative number by the PRInt32() cast and that results in the filter not rendering at all (it's treated as "in error"). Unfortunately we still don't pass a reftest because scaling fails due to the extremely large scale factor, but oh well. At least I have some crashtests to detect crashes or asserts.
Attachment #406384 - Flags: review?(jwatt)
(In reply to comment #2)
> I honestly think that's all OK, although I'm willing to listen to arguments
> that we should do something else. In practice, don't use crazy filter region
> sizes!

Yes, definitly. But there are users that create filters with this size. Maybe just to test the behavior of browsers :-P
But my understanding is that the spec are very clear at this point. Even if the filterRect is huge, the feFlood should have the size and position of the filtered object (in this case). I haven't found a hint on how to handle big sized filterRegions or big sized filterRegion in combination with big sized filter primitives. It's up to the implementer how to deal with it.

> Actually the main part of this testcase is INVALID IMHO.
Why? It's correct according to the spec. Even if it is difficult to handle.
I think the spec allows us to use whatever default filterRes we choose.

It does not say how to handle filter primitive regions that are not pixel-aligned with the pixels of the filter temporary surface(s).

So, I think choosing the filterRes we use and aligning the filter primitive region the way we do is permitted, even if it's unexpected.
Attachment #406384 - Flags: review?(jwatt) → review+
http://hg.mozilla.org/mozilla-central/rev/52c525d3a384

I'm closing this until/unless someone convinces me that there's a better thing to do here.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Flags: in-testsuite+
Blocks: 477929
Depends on: 601064
Depends on: 631720
No longer depends on: 601064
This bug was incorrectly cited in the checkin comment for bug 519144.
Whiteboard: opacity=0 is bug 519144
You need to log in before you can comment on or make changes to this bug.