Closed
Bug 522394
Opened 15 years ago
Closed 15 years ago
SVG filter output not correct for big filter size
Categories
(Core :: SVG, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: krit, Assigned: roc)
References
Details
(Whiteboard: opacity=0 is bug 519144)
Attachments
(2 files)
(deleted),
image/svg+xml
|
Details | |
(deleted),
patch
|
jwatt
:
review+
|
Details | Diff | Splinter Review |
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"
Reporter | ||
Comment 1•15 years ago
|
||
test
Assignee | ||
Updated•15 years ago
|
Attachment #406359 -
Attachment is patch: false
Assignee | ||
Updated•15 years ago
|
Attachment #406359 -
Attachment mime type: text/plain → image/svg
Assignee | ||
Updated•15 years ago
|
Summary: SVF filter output not correct for big filter size → SVG filter output not correct for big filter size
Assignee | ||
Updated•15 years ago
|
Attachment #406359 -
Attachment mime type: image/svg → image/svg+xml
Assignee | ||
Updated•15 years ago
|
Assignee: nobody → roc
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee | ||
Comment 2•15 years ago
|
||
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!
Assignee | ||
Comment 3•15 years ago
|
||
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)
Reporter | ||
Comment 4•15 years ago
|
||
(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.
Assignee | ||
Comment 5•15 years ago
|
||
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.
Updated•15 years ago
|
Attachment #406384 -
Flags: review?(jwatt) → review+
Assignee | ||
Comment 6•15 years ago
|
||
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: 15 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•15 years ago
|
Flags: in-testsuite+
Assignee | ||
Comment 7•15 years ago
|
||
Oh, there was a bustage fix for Windows:
http://hg.mozilla.org/mozilla-central/rev/556728346e52
Comment 8•14 years ago
|
||
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.
Description
•