1591758 - Full tile repaint seems broken during video playback with OS compositor

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P3)

Description

[Markus Stange [:mstange]]

When applying the patch queue leading up to D50726, turning on gfx.webrender.compositor, and going to https://twitter.com/firefox , there is broken rendering during page interactions (scrolling + hovering) as videos on the page play. It looks like outdated content is flashing on the screen, which is what would happen if the old content wasn't completely replaced with new content. When a video is drawn into a tile, are we making sure we also draw all the other content in the tile?

Comment 1

[Markus Stange [:mstange]]

Yeah, it seems like even paused videos cause the surfaces they touch to be bound on every frame (despite nothing in the surface visually changing), but then only the video part of the surface gets painted to. Here's another page where this is easier to observe: https://www.cbsnews.com/news/christopher-columbus-letters-stolen-from-vatican-libraries-around-the-world-60-minutes-2019-10-20/

Comment 2

[Glenn Watson [:gw]]

I suspect this is probably related to https://searchfox.org/mozilla-central/source/gfx/wr/webrender/src/render_backend.rs#1457.

That code tries to skip the main WR paint/composite loop when video is the only changing element, as a workaround / optimization for power.

If it is related to that, we can probably disable that optimization when the native compositor is enabled (dirty rects + future work to place the videos into OS surfaces directly should reduce the usefulness of that optimization).

Comment 3

[Sotaro Ikeda [:sotaro]]

(In reply to Glenn Watson [:gw] from comment #2)

I suspect this is probably related to https://searchfox.org/mozilla-central/source/gfx/wr/webrender/src/render_backend.rs#1457.

That code tries to skip the main WR paint/composite loop when video is the only changing element, as a workaround / optimization for power.

The above code seems not directly related to the problem. Current code trigger frame build if there is a valid image update. It skips frame build and WR rendering if image of the image update is not used for WR rendering. I modified the code to always trigger frame build, but the problem still exist.

During video/WebGL rendering, if it it is an only update, display list is not sent to WR. Instead, only image is updated by ResourceUpdate::UpdateImage(). It seemed not handled well by WR yet. When is_cacheable() was changed as to always return true, I did not saw the flickering. But I saw video frame update only during mouse move.

Comment 4

[Sotaro Ikeda [:sotaro]]

It seems that we need to update whole Tile if the Tile has a dirty region and rendering target is compositor surface. Current os compositor implementation does not handle partial update, WR renders only to dirty region and whole compositor surface is flipped if there is an update.

Comment 5

[Sotaro Ikeda [:sotaro]]

Dirty rect of non cacheable primitive is added in Tile::add_prim_dependency(). Dirty rect of cachable primitive is added at TileNode::update_dirty_rects()

• https://searchfox.org/mozilla-central/source/gfx/wr/webrender/src/picture.rs#646
• https://searchfox.org/mozilla-central/source/gfx/wr/webrender/src/picture.rs#4632

Comment 6

[Markus Stange [:mstange]]

Makes sense! So the bug is that add_prim_dependency for non-cacheable primitives can create partial update rects even when max_update_rects is zero. Currently, max_update_rects == 0 is only respected in the sense that we don't split tiles into smaller nodes, so that the node-based computation of the tile's dirty rect doesn't create a partial dirty rect. But that's not the only source of dirty_rect modifications.

Comment 7

[Glenn Watson [:gw]]

Attachment: Bug 1591758 - Fix dirty rects for OS compositors without partial updates.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/95dfa337c1db
Fix dirty rects for OS compositors without partial updates. r=nical

Comment 9

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/95dfa337c1db