1897584 - Rendering artifacts on a Codepen demo on pinch-zoom (split off from bug 1892018)

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Mayank Bansal]

Go to https://codepen.io/thomasrye/pen/VaRoYv
Pinch-zoom

AR: The bottom corners get some sort of renderign artifacts.
ER: Not so

Bisection:
Bug 1892398 - Supply local space coords to compositing quads when drawing patterns. r=gw

The quad primitive info takes a scale+offset transform which is applied to the coordinate space of the pattern. This allows the quad coorinates to be optionally specified in device space while the pattern remains in layout space.

Differential Revision: https://phabricator.services.mozilla.com/D209694

Comment 1

[Mayank Bansal]

Attachment: Partial Fix Behaviour2

Comment 2

[Mayank Bansal]

Attachment: about:support

Comment 3

[Mayank Bansal]

Attachment: Actual Regression.png

This is the actual regression from the above bisection : Without doing any pinch-zoom, the edges have this squircle thingy and the colors are all wrong.

Comment 4

[Mayank Bansal]

Attachment: Partial Fix Behaviour1.mp4

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1892398

:nical, since you are the author of the regressor, bug 1892398, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 6

[Mayank Bansal]

Partially fixed by:
Bug 1896979 - Handle matrix transforms that simplify into 2d scale + offset. r=gfx-reviewers,gw

Differential Revision: https://phabricator.services.mozilla.com/D210612

Comment 7

[Mayank Bansal]

Also occurs on https://codepen.io/alvaromontoro/pen/OJwWPOB

Comment 8

[Nicolas Silva [:nical]]

Attachment: Bug 1897584 - Fix backwards terminology in ScaleOffset and usage in prepare_quad. r=gw

Comment 9

[Nicolas Silva [:nical]]

Attachment: Bug 1897584 - Make SurfaceInfo::map_to_device_rect less confusing. r=gw

There was an undocumented assumption that the raster node is the root spatial node if raster node != surface node which was very confusing because this function can be called in cases the raster node is different. This patch cleans up the local/picture terminology, replaces the use of the root node with the surface's raster node (which are assumed to be equal) since that is more generally correct and asserts the assumption about the root node. There should be no behavior change in the patch (unless the working assumption does not hold true and the added assertion is hit).

Depends on D211830

Comment 10

[Nicolas Silva [:nical]]

Attachment: Bug 1897584 - Fix incorrect order of transformation in CompositeState::register_transform. r=gw

Depends on D211849

Comment 11

[Nicolas Silva [:nical]]

Attachment: Bug 1897584 - Fix incorrect order of transformation in TileCacheInstance::pre_update. r=gw

Depends on D211850

Comment 12

[Nicolas Silva [:nical]]

Attachment: Bug 1897584 - Fix incorrect transform order in setup_compositor_surfaces_impl. r=gw

Depends on D211851

Comment 13

[Nicolas Silva [:nical]]

Attachment: Bug 1897584 - Favor ScaleOffset::then over pre_transform. r=gw

This is purely a stylistic choice, feel free to reject if you disagree. I have a much easier time reading the math this way. This patch does not change the behavior of the code.

Depends on D211852

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bd3877bbde56
Fix backwards terminology in ScaleOffset and usage in prepare_quad. r=gw

Comment 15

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/bd3877bbde56

Comment 16

[Mayank Bansal]

This is fixed for me with the latest nightly https://hg.mozilla.org/mozilla-central/rev/e1da1893a0c56d2752be4e6eb1efa16c58723a55

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a7c3b97f32c4
Make SurfaceInfo::map_to_device_rect less confusing. r=gw
https://hg.mozilla.org/integration/autoland/rev/be09294c8814
Fix incorrect order of transformation in CompositeState::register_transform. r=gw
https://hg.mozilla.org/integration/autoland/rev/f681c82f6d5a
Fix incorrect order of transformation in TileCacheInstance::pre_update. r=gw
https://hg.mozilla.org/integration/autoland/rev/14230f12bba0
Fix incorrect transform order in setup_compositor_surfaces_impl. r=gw
https://hg.mozilla.org/integration/autoland/rev/66062779f726
Favor ScaleOffset::then over pre_transform. r=gw

Comment 19

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/a7c3b97f32c4
https://hg.mozilla.org/mozilla-central/rev/be09294c8814
https://hg.mozilla.org/mozilla-central/rev/f681c82f6d5a
https://hg.mozilla.org/mozilla-central/rev/14230f12bba0
https://hg.mozilla.org/mozilla-central/rev/66062779f726

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Is there more to do here?

Comment 21

[Nicolas Silva [:nical]]

No, let's close it.

Comment 22

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

Issue is reproducible on a 2024-05-17 Nightly build on Windows 10.
Verified as fixed on Firefox 128.0b2 and Firefox Nightly 129.0a1 on Windows 10, Ubuntu 22, macOS 14.