Closed Bug 1591758 Opened 5 years ago Closed 5 years ago

Full tile repaint seems broken during video playback with OS compositor

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox72 --- fixed

People

(Reporter: mstange, Assigned: gw)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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?

Summary: Full tile repaint seems broken during video playback → Full tile repaint seems broken during video playback with OS compositor

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/

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).

(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.

Priority: -- → P3

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.

Blocks: 1592016

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()

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.

Blocks: 1592509
Blocks: 1592906
Assignee: nobody → gwatson
Pushed by gwatson@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/95dfa337c1db Fix dirty rects for OS compositors without partial updates. r=nical
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: