SVG rendering: huge loss of precision in scaling of objects from <defs> when SVG display-list painting is enabled

NEW
Unassigned

Status

()

6 years ago
5 months ago

People

(Reporter: mikhail.ryazanov, Unassigned)

Tracking

({regression, testcase})

17 Branch
x86
Windows XP
regression, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

(Reporter)

Description

6 years ago
Created attachment 777572 [details]
test.svg

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:22.0) Gecko/20100101 Firefox/22.0 (Beta/Release)
Build ID: 20130618035212

Steps to reproduce:

Open an SVG file that uses transformation with large scaling parameters applied to objects defined in <defs>.
See the attached SVG file "test.svg" for an example.


Actual results:

The objects drawn with <use ...> are significantly displaced from the expected positions, apparently due to some intermediate rounding errors.
See the attached results of rendering "firefox.png" (results with and without "shape-rendering='geometricPrecision'" have no visible differences).


Expected results:

The objects drawn with <use ...> must behave in the same way as objects drawn directly.
See the attached example of correct rendering "correct.png" (produced using Chromium; Opera produces visually identical results).
(Reporter)

Comment 1

6 years ago
Might be related (or not) to Bug 836768.
(Reporter)

Comment 2

6 years ago
Created attachment 777574 [details]
firefox.png
(Reporter)

Comment 3

6 years ago
Created attachment 777576 [details]
correct.png
(Reporter)

Updated

6 years ago
Attachment #777574 - Attachment description: Incorrect rendering in Firefox → firefox.png
(Reporter)

Updated

6 years ago
Attachment #777576 - Attachment description: Correct rendering (in Chromium) → correct.png

Updated

6 years ago
Component: Untriaged → SVG
Product: Firefox → Core

Comment 4

6 years ago
Another issue with SVG display list.
svg.display-lists.painting.enabled=false fixes the issue.

Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=588424024294&tochange=89dcadd42ec4
Blocks: 776054
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression, testcase
Summary: SVG rendering: huge loss of precision in scaling of objects from <defs> → SVG rendering: huge loss of precision in scaling of objects from <defs> when SVG display-list painting is enabled
Version: 22 Branch → 17 Branch

Comment 5

5 months ago
Created attachment 9010318 [details]
Reduced rectangle testcase (top of the rectangle should be at 5px, not 0)

In this testcase we're rendering
<svg>
  <defs>
    <rect id='t' x='1' y='0' width='0.1' height='0.1' />
  </defs>
  <g transform='scale(500)'>
    <use xlink:href='#t' y='0.01' fill='red' stroke='none'/>
  </g>
</svg>

The y='0.01' in the <use> is accounted for during display list building by adding an offset to the <use>'s child frame[1] - the rectangle in this case. The offset is tracked in app pixels using the <use>'s mFrame position, which in this case is (0,0): in app pixels, 0.01 is 60 * 0.01 = 0.6, but in the <use> frame's mRect calculation it uses floor instead of round[2], so we get 0.

When we go to paint, that offset is converted back into user space units by dividing by 60, and then gets incorporated as a translation into the overall matrix to the rectangle's geometry.  In this case the other contributor is the scale(500) on the <g> (which is outside the <use>), so note that _whatever_ the translation contribution from the <use> had been, it would have arrived here in app units, so ints, then divided by 60 to get user units, and then multiplied by 500.  So if the <use> translation in app units had been (x, y) (integers), it would have turned into a final user space translation contribution of (500 * x / 60, 500 * y / 60) = (8.333 * x, 8.333 * y).  In other words, the <use> x and y coordinates, no matter what they are, can only change the position of the <use>'s referenced children in multiples of 8.333 pixels in this case.

Overall, the effect of the x and y attributes on the <use> is a translation in pixels of

(500 * floor(x * 60) / 60, 500 * floor(y * 60) / 60) = (8.333 * floor(x * 60), 8.333 * floor(y * 60))

[1] The <use> offset contribution is added to its child here:
https://dxr.mozilla.org/mozilla-central/rev/85b4d2bf888afa32b67638602a3338d0a6935ff9/layout/painting/nsDisplayList.h#1187

The builder's mCurrentOffsetToReferenceFrame eventually makes its way into the child rectangle's nsDisplaySVGGeometry here:
https://dxr.mozilla.org/mozilla-central/rev/85b4d2bf888afa32b67638602a3338d0a6935ff9/layout/painting/nsDisplayList.cpp#3201

[2] https://dxr.mozilla.org/mozilla-central/rev/85b4d2bf888afa32b67638602a3338d0a6935ff9/layout/svg/nsSVGUseFrame.cpp#111

Comment 6

5 months ago
Created attachment 9010319 [details]
The previous testcase with the y attribute on the <use> animated

Here's the same testcase with the y attribute on the <use> element animated from 0 to 1 (the size of the attribute doesn't matter here, aside from making sure it stays in our viewport: y values from 100 to 101 would show the same behavior) - note the jerky motion as it jumps ~8 pixels at a time.

Comment 7

5 months ago
Does anyone think it's worth changing the mRect computation rounding for the <use> from a floor (see link [2] in comment 5) to a round (since it seems we use round in most other places for app units)?  That will slightly change some of the positioning here for some values of y, but it won't fix the overall issue.

Updated

5 months ago
Duplicate of this bug: 1491076

Comment 9

5 months ago
It will be great if all rounding to coordinates is made accurate to at least 8 decimal places (maybe 10-12 for numeric stability). This will allow the direct presentation of GIS data for scientific applications. (See here: https://en.wikipedia.org/wiki/Decimal_degrees). Very useful!

Comment 10

5 months ago
(In reply to Tom Klein from comment #7)
> Does anyone think it's worth changing the mRect computation rounding for the
> <use> from a floor (see link [2] in comment 5) to a round (since it seems we
> use round in most other places for app units)?  That will slightly change
> some of the positioning here for some values of y, but it won't fix the
> overall issue.

(What was I thinking? Since it's for mRect it should round out.  I think I was thinking that for the purposes of <use>, which is not itself directly rendered, rounding might be more accurate than floor to be used as an offset (which I suspect is the only use for mRect on <use>?).  But now that I think about it again I'm not so comfortable with changing it.)
You need to log in before you can comment on or make changes to this bug.