1961724 - [HDR] Invesitigate extra GL operations while video direct rendering from external surfaces

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Martin Stránský [:stransky] (ni? me)]

There's reported extra GL operation over decoded video frames in HDR mode which we don't need. We should get rid of it to save GPU cycles.

Comment 1

[Alan Liu]

Thanks for working on this issue. Just for your reference, I found the following stacktrace may related to this GPU usage by firefox when HDR mode is enabled.
https://gist.github.com/alen6516/b48c2d0adc9481d3d38988a6b8547680

Comment 2

[Martin Stránský [:stransky] (ni? me)]

Looks like we call glFlush() at webrender::renderer::Renderer::draw_frame() which we may not need on native compositor under Linux.

Comment 3

[Martin Stránský [:stransky] (ni? me)]

#6  0x00007f0bc1f3ad2c in st_flush (st=0x7f0bbcae7000, fence=0x0, flags=0) at ../src/mesa/state_tracker/st_cb_flush.c:63
#7  0x00007f0bc1f3ae5e in st_glFlush (ctx=0x7f0bbca03000, gallium_flush_flags=0) at ../src/mesa/state_tracker/st_cb_flush.c:99
#8  0x00007f0bc216b8b2 in _mesa_flush (ctx=0x7f0bbca03000) at ../src/mesa/main/context.c:1624
#9  0x00007f0bc216b9df in _mesa_Flush () at ../src/mesa/main/context.c:1658
#10 0x00007f0bdb0b01f7 in webrender::renderer::Renderer::draw_frame ()
    at /home/alan/firefox_build/firefox_138_nightly/firefox/libxul.so
#11 0x00007f0bdb0a57b9 in webrender::renderer::Renderer::render_impl ()
    at /home/alan/firefox_build/firefox_138_nightly/firefox/libxul.so
#12 0x00007f0bdb2aba53 in webrender::renderer::Renderer::render ()
    at /home/alan/firefox_build/firefox_138_nightly/firefox/libxul.so
#13 0x00007f0bdb2ab26f in wr_renderer_render () at /home/alan/firefox_build/firefox_138_nightly/firefox/libxul.so

Comment 4

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

The component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit BugBot documentation.

Comment 5

[Martin Stránský [:stransky] (ni? me)]

The call comes from these two places:
https://searchfox.org/mozilla-central/source/gfx/wr/webrender/src/renderer/mod.rs#1596
https://searchfox.org/mozilla-central/source/gfx/wr/webrender/src/renderer/mod.rs#5049

I think we need it for case we paint to texture by GL and then put that on screen. But it can be avoided if the content is already there - for instance external video overlay from HW decoder.

Glenn, do you think it's possible to skip glFlush for that case and save some GPU power here?
Thanks.

Comment 6

[Glenn Watson [:gw]]

I think we could actually just remove that flush altogether, I'm not convinced that we need it.

Comment 7

[Martin Stránský [:stransky] (ni? me)]

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

I think we could actually just remove that flush altogether, I'm not convinced that we need it.

It's needed on Wayland if we paint by GL to framebuffer and then export as dmabuf/wl_buffer. But that can be easily moved to WaylandLayer code where framebuffer is managed. So +1 here.

Comment 8

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1961724 Remove device.gl().flush() r?gw

Comment 9

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1961724 [Linux] Flush GL rendering at NativeLayerWaylandRender::NotifySurfaceReady() as we need to have complete GL rendering for wl_buffer export r?gw

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/800a73315195
Remove device.gl().flush() r=gw
https://hg.mozilla.org/integration/autoland/rev/6be9c2492a2c
[Linux] Flush GL rendering at NativeLayerWaylandRender::NotifySurfaceReady() as we need to have complete GL rendering for wl_buffer export r=gw

Comment 11

[Mayank Bansal]

seems to have improved some SVGX subtests by 10.4%

Also improved TART tests on Windows!

Comment 12

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/800a73315195
https://hg.mozilla.org/mozilla-central/rev/6be9c2492a2c

Comment 13

[Florin Bilt]

(In reply to Sandor Molnar[:smolnar] from comment #12)

https://hg.mozilla.org/mozilla-central/rev/800a73315195
https://hg.mozilla.org/mozilla-central/rev/6be9c2492a2c

Perfherder has detected a talos performance change from push 6be9c2492a2c6fca50a3bc6601dcde5116be244e.

If you have any questions, please reach out to a performance sheriff. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
4%	tsvgx	linux1804-64-shippable-qr	e10s fission stylo webrender	221.16 -> 212.85

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a performance sheriff to do that for you.

You can run all of these tests on try with ./mach try perf --alert 45152

The following documentation link provides more information about this command.