1894106 - Seams between tiles flicker during zooming on Android

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Markus Stange [:mstange]]

Attachment: Screen recording

Steps to reproduce:

1. Go to https://loglog.games/blog/leaving-rust-gamedev/ in Firefox for Android.
2. Slowly pinch-zoom in and out.

Expected results:
Nothing should flicker.

Actual results:
White lines appear intermittently, probably at tile boundaries.

Comment 1

[Jamie Nicol [:jnicol] (PTO until 17/6)]

Seems to be caused by low-quality pinch zoom

Comment 2

[Jamie Nicol [:jnicol] (PTO until 17/6)]

I managed to capture a frame with a seam in renderdoc, and the results are interesting.

The seam is a vertical line at x=662

The gl_Position.x outputs from the vertex shader for the left column's tiles are x=-1.00 and x=0.225. And for the right column's tiles they are x=0.22593 and x=1.00. Translating those to pixels on the 1080-pixel-wide screen gives the left column's right edge as x=661.5, and the right column's left edge as x=662. Presumably this difference is where the seam comes from.

But what I don't understand is where this difference comes from, given the shader inputs:

|                 | Left column tiles                               | Right column tiles                              |
|-----------------+-------------------------------------------------+-------------------------------------------------|
| aLocalRect      | left: 0.00, right: 1079.33984                   | left: 1079.33984, right: 2158.67969             |
| aDeviceClipRect | left: 0.00, right: 662.0                        | left: 662.0, right: 1080.0                      |
| aTransform      | scale(0.6138, 0.6138) translate(-1.00, -104.00) | scale(0.6138, 0.6138) translate(-1.00, -104.00) |
| gl_Position     | left: -1.00, right: 0.225                       | left: 0.22593, right: 1.00                      |

ie the input data's right side of the left column exactly matches the left side of the right column. And aTransform is identical. So why is the gl_Position output different?

Comment 3

[Jamie Nicol [:jnicol] (PTO until 17/6)]

Oh, I think it might be the calmping to aDeviceClipRect that is causing it!

Comment 4

[Jamie Nicol [:jnicol] (PTO until 17/6)]

Yup, aDeviceClipRect is the source of some of our woes.

It gets calculated here. Importantly, get_device_rect() rounds() the value. Meanwhile the local rect is passed to the shader, and the shader transforms the local rect into device space without rounding, then clips against the clip rect.

So say you have a tile_a { y0: 0, y1: 5 } and tile_b { y:0 5, y1: 10 }, with a local-to-device scale of 2. The aDeviceClip rect values for these would be { y0: 0, y1: 2 }, and { y0: 2, y1: 5 }, respectively. The non-rounded world rects as calculated in the shader would be { y0: 0.0, y1: 2.5 } and { y0: 2.5, y1: 5 }, which get clipped as { y0: 0.0, y1: 2.0 } and { y0: 2.5, y1: 5.0 }. The half pixel gap between these can cause seams. Avoiding rounding fixes a lot of the seams, but we still get some.

Another cause is the implementation of ScaleOffset::map_rect(). Say we have 2 adjacent input rects, eg where rect1.max.y == rect2.min.y. We would expect this condition to still hold true after mapping both rects with the same ScaleOffset. However, this is not the case due to floating point innacuracies, and the implementation scaling the min point and the size, rather than the min and max points.

Again, fixing that solves some of the seams. But there are still some seams present, and when captured renderdoc is still showing a discrepancy between the clip rect and the world rect. However, I cannot for the life of me see why when the above issues are gone.

In any case, it seems we can resolve this issue by both replacing the local-space rect and transform inputs to the shader with a device-space rect, and additionally fix the ScaleOffset::map_rect implementation

Comment 5

[Jamie Nicol [:jnicol] (PTO until 17/6)]

As we're late in the cycle, I'm going to disable low-quality pinch zoom in bug 1894902 and hopefully uplift it

Comment 6

[Jamie Nicol [:jnicol] (PTO until 17/6)]

(In reply to Jamie Nicol [:jnicol] from comment #4)

Again, fixing that solves some of the seams. But there are still some seams present, and when captured renderdoc is still showing a discrepancy between the clip rect and the world rect. However, I cannot for the life of me see why when the above issues are gone.

In any case, it seems we can resolve this issue by both replacing the local-space rect and transform inputs to the shader with a device-space rect, and additionally fix the ScaleOffset::map_rect implementation

Speaking with Glenn and Ashley, we think it's plausible that the device rect calculated on the CPU could differ than that calculated on the GPU, even when using the same calculation with the same inputs. (Which is the case after changing map_rect() to operate on the min and max points rather than the origin and size). In which case I believe the only option here is to only perform the transformation on the CPU, and have the composite shader exclusively deal in device space.

In any case, even after doing so there are still some seams! This time, it's because the even local rects of adjacent tiles have a gap between them, so of course the corresponding device rects do! Again this is due to floating point innacuracies, and because we construct the local rect from an origin and size rather than two points, here. Changing that to use two points fixes the remaining seams, as far as I can tell.

So to summarize, in order to fix the seams we must:

• Transform to device space on CPU and have composite shader only use device space
• Fix ScaleOffset::(un)map_rect to operate on points instead of origin and size
• Make Tile::local_rect constructed from points instead of origin and size

Comment 7

[Jamie Nicol [:jnicol] (PTO until 17/6)]

Attachment: Bug 1894106 - Supply tile rect to composite shader in device space. r?#gfx-reviewers

Currently the composite shader takes as input the tile rect in local
space, a local-to-device, transform, and the clip rect in device
space. The shader will then transform the local rect in to device
space and then clip it.

On the CPU side, that clip rect was calculated by transforming the
same tile rect in to device space using the same transform. (And
intersecting with additional rects too). However, performing the same
floating point calculations on the CPU and GPU can produce slightly
different results. Discrepancies between the device rect as calculated
by the CPU and GPU were causing the tiles to occasionally be clipped
fractionally smaller than intended, which was resulting in visible
seams whilst zooming a page.

To fix this, we supply the tile rect in device space to the shader,
meaning the transformation is only ever calculated on the CPU. As the
transform could contain a negative scale to indicate a flip, we
replace it with a "flip" input for each axis.

Comment 8

[Jamie Nicol [:jnicol] (PTO until 17/6)]

Attachment: Bug 1894106 - Construct picture tile local rects as min and max points instead of origin and size. r?#gfx-reviewers

At certain scales, floating point innacuracies were causing adjacent
tiles to have miniscule gaps between them. This could result in
visible seams whilst zooming. By constructing the rects with start and
end points we ensure that is not the case.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/14477417af2b
Supply tile rect to composite shader in device space. r=gfx-reviewers,gw
https://hg.mozilla.org/integration/autoland/rev/f66a0d99c2ad
Construct picture tile local rects as min and max points instead of origin and size. r=gfx-reviewers,gw

Comment 10

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/14477417af2b
https://hg.mozilla.org/mozilla-central/rev/f66a0d99c2ad