2064321 - Weird shadow like artifacts on the microsoft store page

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Mauro V [:cheff]]

Attachment: bug.mov

Going into any app inside the microsoft store makes firefox to start rendering weird shadow-like objects specially when interacting with elements, such as the search bar.

Comment 1

[Alice0775 White]

STR:

1. Open https://apps.microsoft.com/detail/9nksqgp7f2nh?hl=en-US&gl=JP
2. Move the mouse around randomly

Regression window:
https://hg-edge.mozilla.org/integration/autoland/pushloghtml?fromchange=91ac834f411d3bea953c9ee02e6c2ff5561b2f82&tochange=420c5f80a6ca3d16482b7be85a2457608ef4ebf3

Comment 2

[BugBot [:suhaib / :marco]]

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

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

For more information, please visit BugBot documentation.

Comment 3

[Nicolas Silva [:nical]]

Attachment: Bug 2064321 - Emit repetitions for the surface's needs, not the tile cache dirty rect. r=gw

prepare_repeatable_quad's "repeat by duplicating the primitive" path enumerated
repetitions against frame_state.current_dirty_region(), which is the picture
cache tile's dirty region: push_dirty_region only happens for
PictureCompositeMode::TileCache. Inside an intermediate surface that samples
outside of its own footprint, the repetitions falling in that surface's
inflation margin were dropped, so a blur read missing content and the wrong
result was baked into the picture cache tile with nothing left to invalidate it.

Cull against the current surface's clipping_rect instead. For a primitive drawn
straight onto a picture cache slice that is the dirty region (push_surface is
passed surface_local_dirty_rect), so the optimisation is preserved; inside a
filter surface it is surface_rects.clipped_local, the inflated region that
surface needs. This is also the rect prepare_quad_impl already uses to decide
what to draw, so the two now agree.

prepare_image_quads' tiled path had the same bug, from the same
compute_conservative_visible_rect(current_dirty_region()) call. It is worse
there: that rect culls the repetitions and, within each repetition, the
individual texture cache tiles, so even a plain non-repeating tiled image loses
the tiles that fall in the blur's margin, and those tiles are never requested
from the resource cache at all. Both callers now share
compute_surface_visible_rect, and compute_conservative_visible_rect goes away
with its last caller.

The reftests put a repeating gradient, a repeating tiled image and a plain tiled
image inside a horizontally blurred surface that straddles the x=1024 picture
cache tile boundary, and invalidate only the tiles on one side of it so that the
dirty region ends mid-surface. Each reference needs a first frame that
invalidates every tile: the harness renders the reference into the same wrench
instance right after the test, so a reference display list identical to the
test's last frame invalidates nothing and merely re-composites the test's own
tiles, reproducing whatever the test got wrong.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/300754d1d96f
https://hg.mozilla.org/integration/autoland/rev/c423e0bf521f
Emit repetitions for the surface's needs, not the tile cache dirty rect. r=gw

Comment 5

[Cosmin Sabou [:CosminS]]

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

Comment 6

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected, along with ESR.
:nical, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta and ESR approvals.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox155 and the ESR status flag(s) to wontfix.

For more information, please visit BugBot documentation.

Comment 7

[Nicolas Silva [:nical]]

Attachment: Bug 2064321 - Emit repetitions for the surface's needs, not the tile cache dirty rect.

prepare_repeatable_quad's "repeat by duplicating the primitive" path enumerated
repetitions against frame_state.current_dirty_region(), which is the picture
cache tile's dirty region: push_dirty_region only happens for
PictureCompositeMode::TileCache. Inside an intermediate surface that samples
outside of its own footprint, the repetitions falling in that surface's
inflation margin were dropped, so a blur read missing content and the wrong
result was baked into the picture cache tile with nothing left to invalidate it.

Cull against the current surface's clipping_rect instead. For a primitive drawn
straight onto a picture cache slice that is the dirty region (push_surface is
passed surface_local_dirty_rect), so the optimisation is preserved; inside a
filter surface it is surface_rects.clipped_local, the inflated region that
surface needs. This is also the rect prepare_quad_impl already uses to decide
what to draw, so the two now agree.

prepare_image_quads' tiled path had the same bug, from the same
compute_conservative_visible_rect(current_dirty_region()) call. It is worse
there: that rect culls the repetitions and, within each repetition, the
individual texture cache tiles, so even a plain non-repeating tiled image loses
the tiles that fall in the blur's margin, and those tiles are never requested
from the resource cache at all. Both callers now share
compute_surface_visible_rect, and compute_conservative_visible_rect goes away
with its last caller.

The reftests put a repeating gradient, a repeating tiled image and a plain tiled
image inside a horizontally blurred surface that straddles the x=1024 picture
cache tile boundary, and invalidate only the tiles on one side of it so that the
dirty region ends mid-surface. Each reference needs a first frame that
invalidates every tile: the harness renders the reference into the same wrench
instance right after the test, so a reference display list identical to the
test's last frame invalidates nothing and merely re-composites the test's own
tiles, reproducing whatever the test got wrong.

Original Revision: https://phabricator.services.mozilla.com/D319789

Comment 8

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined/Reason for urgency: visual glitches when repeated backgrounds are nested in a blur filter.
• Code covered by automated testing?: yes
• Fix verified in Nightly?: yes
• Needs manual QE testing?: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: Simple fix.
• String changes made/needed?: No.
• Is Android affected?: yes

Comment 9

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/9e687bf9bae4
https://hg.mozilla.org/releases/mozilla-beta/rev/7c504b615add