1640712 - non-os-compositor: Damage rects currently get joined to the bounding rect

Status: NEW

(Core :: Graphics: WebRender, task, P5)

Description

[Robert Mader [:rmader]]

From https://bugzilla.mozilla.org/show_bug.cgi?id=1620076#c8:

WR currently is hardcoded to merge all damage rects into one due to other platforms.

Easy to test e.g. on https://fiddle.jshell.net/w0a9f7uh/show/

In the worst cases this can increase overdraw quite a lot.

Comment 1

[Sotaro Ikeda [:sotaro]]

Multiple dirty rects handling support was removed by Bug 1593154, since dirty rects of overlapping tiles did not work well.

Comment 2

[Robert Mader [:rmader]]

Gwen, NIing you as discussed - would be great if you had time to look into this at some point :)

Comment 3

[Lee Salzman [:lsalzman]]

For native WR compositors instead of draw compositors, this is already reported as individual dirty rects via start_compositing. Take a look at how SwCompositor uses it. I am guessing set_buffer_damage_region was only added as a hack to support legacy draw compositors, but we should really start moving away from those. Sotaro's work on a GL compositor for android will probably allow you to just use that native compositor instead of needing RenderCompositorEGL at all. Likewise, that GL compositor could then choose to make use of the information about dirty rects passed in via start_compositing.

Comment 4

[Lee Salzman [:lsalzman]]

Actually, now that I look, Sotaro's work only helps in the case of SWGL. In the non-SWGL case you'd still need to implement a real native compositor somehow, but it might make sense if we generalize that to non-SWGL platforms. Sotaro?

Comment 5

[Robert Mader [:rmader]]

For Wayland implementation of a native compositor is tracked in bug 1617498, but for X11 there's apparently no hope.

The main reason why I think it might still make sense to improve the draw compositor is that IIUC support for partial damage is already is place and only got deactivated because of limitations on Windows (overlapping damage rects). These limitations do not apply to EGL, so I hope this is a comparatively low hanging fruit - and it will probably take some time till the Wayland native compositor is in place and all users switched to Wayland. So, yeah, we should not invest too much time into this :)

Comment 6

[Lee Salzman [:lsalzman]]

(In reply to Robert Mader [:rmader] from comment #5)

For Wayland implementation of a native compositor is tracked in bug 1617498, but for X11 there's apparently no hope.

The main reason why I think it might still make sense to improve the draw compositor is that IIUC support for partial damage is already is place and only got deactivated because of limitations on Windows (overlapping damage rects). These limitations do not apply to EGL, so I hope this is a comparatively low hanging fruit - and it will probably take some time till the Wayland native compositor is in place and all users switched to Wayland. So, yeah, we should not invest too much time into this :)

Either way, reducing RenderCompositorOGL and RenderCompositorEGL into Sotaro's work, if it is an easy tweak. might be beneficial for us in other ways. I'll talk to Matt and see what he thinks.

Comment 7

[Sotaro Ikeda [:sotaro]]

I wonder if it is simpler to re-add multiple dirty rects support to WebRender that was removed by Bug 1593154.

:gw, how is it complex to re-add the add multiple dirty rects support?

Comment 8

[Sotaro Ikeda [:sotaro]]

RenderCompositorLayersSWGL/RenderCompositorD3D11SWGL does not handle partial rects present yet. See Bug 1695564.

Comment 9

[Glenn Watson [:gw]]

I don't think it's a huge amount of work (maybe a day or two of work), but I'm not sure of the priority of it at the moment. I think what we need to do is:

• Restore the functionality removed in Bug #1593154.
• Change that functionality so it's only enabled on platforms that don't have the overlapping dirty rect restriction (via renderer options or platform / extension detection).
• Add some invalidation test coverage to test_invalidation.rs
• Do some basic testing on platforms that support this mode before enabling in nightly.

It'd be interesting to know how many frames hit this case where the area of the combined dirty rect is significantly more than the area of the individual dirty rects, on real web content. But the time to implement / test that might be just as much as the work to do the implementation above.

Comment 10

[Robert Mader [:rmader]]

It'd be interesting to know how many frames hit this case where the area of the combined dirty rect is significantly more than the area of the individual dirty rects, on real web content.

On Gnome one can get a visual impression of that by running alt+f2 -> lg -> Meta.add_clutter_debug_flags(0, Clutter.DrawDebugFlag.PAINT_DAMAGE_REGION, 0) (and Meta.remove_clutter_debug_flags(0, Clutter.DrawDebugFlag.PAINT_DAMAGE_REGION, 0) to disable it again).

I personally have the impression that there are a couple of sites where it would probably help quite a bit, especially those with audio players and small gifs etc.