1602643 - Make profiler screenshots and frame recording work in the WR OS compositor configuration on Windows

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement)

Description

[Sotaro Ikeda [:sotaro]]

Created from Bug 1592031 .

Comment 1

[Sotaro Ikeda [:sotaro]]

If RenderCompositorANGLE::MaybeReadback() is re-used, it could take 30-60ms:( IDCompositionDevice::WaitForCommitCompletion() and ::PrintWindow() took very long time.

Comment 2

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Add log to RenderCompositorANGLE::MaybeReadback()

Since Bug 1606685 and Bug 1604383, I re-tested how long did RenderCompositorANGLE::MaybeReadback() spend time for taking a snapshot. It spent around 30ms for each call.

Comment 3

[Sotaro Ikeda [:sotaro]]

It seems that there are 3 options to achieve async screenshot.

• [1] Use DC capture api
  • rendering result is same as rendering to display
  • screenshot take long time(30ms)
• [2] Cache tiles contents and render tiles to screenshot
  • Implementation is used only for async screenshot. It could be buggy.
• [3] Dynamically disable native compositor usage

Comment 4

[Markus Stange [:mstange]]

I agree. My guess is that 3 would be the hardest to implement but have the best performance, followed by 2. I don't think we should do 1 - it would mean that profiling with screenshots enabled would destroy animation smoothness.

Comment 5

[Markus Stange [:mstange]]

Bas, just a heads up: We're having trouble getting "overhead-free" frame recording to work in a DirectComposition world. This is somewhat understandable: With DC, Firefox is no longer responsible for computing the assembled window "scene"; this work is now done by the window manager. A buffer with the window pixel contents no longer exists. So frame recording is now asking us to compute something that we don't have, so it will fundamentally come with some overhead. Either Firefox will need to do some extra work to do the compositing itself (which is what we've been doing in the past anyway), or we need to get the window pixels from the window manager somehow. The latter has multiple problems: It's either synchronous and slow, or asynchronous with uncertain contents (could return old frames and even skip frames), and it also has to do extra work because our window might be occluded.
Do you have ideas or preferences for how to address this?

Comment 6

[Bas Schouten (:bas.schouten)]

If firefox would do the compositing itself, this could be fairly cheap from a CPU perspective (possibly using some memory bandwidth obviously). I believe DC will allow us to composite whatever DC tree we have into a texture, we could then, much as we do now, only readback those textures once we finish the recording.

Comment 7

[Sotaro Ikeda [:sotaro]]

(In reply to Bas Schouten (:bas.schouten) from comment #6)

I believe DC will allow us to composite whatever DC tree we have into a texture

:bas.schouten, how can we do it? I do not know about it.

Comment 8

[Sotaro Ikeda [:sotaro]]

Attachment: Bug 1602643 - Disable WebRender compositor dinamically for async screenshot

Comment 9

[Sotaro Ikeda [:sotaro]]

By Attachment 9120393 [details], it seems possible to implement [3], though it is wip patch and has some problems.

Comment 10

[Bas Schouten (:bas.schouten)]

(In reply to Sotaro Ikeda [:sotaro] from comment #7)

(In reply to Bas Schouten (:bas.schouten) from comment #6)

I believe DC will allow us to composite whatever DC tree we have into a texture

:bas.schouten, how can we do it? I do not know about it.

Looks like you're right, I thought they'd recently added a way to create an IDCompositionTarget for a surface. But it doesn't look like they have. There must be some better way to do this one would hope :s.

Comment 11

[Sotaro Ikeda [:sotaro]]

Comment on attachment 9120393 [details]
Bug 1602643 - Disable WebRender compositor dinamically for async screenshot

:gw, patch worked on my local windows. But I am not sure if changes to WebRender are acceptable. Can you comment to them?

Comment 12

[Glenn Watson [:gw]]

Comment on attachment 9120393 [details]
Bug 1602643 - Disable WebRender compositor dinamically for async screenshot

Added some feedback in the phab review - the API seems good, but I think it'd be great if it's possible to handle the compositor surface destruction / invalidation during the normal picture cache update code.

Comment 13

[Sotaro Ikeda [:sotaro]]

Comment on attachment 9120393 [details]
Bug 1602643 - Disable WebRender compositor dinamically for async screenshot

:gw, I updated the patch by applying your comment. Can you feedback to the patch again?

Comment 14

[Glenn Watson [:gw]]

Comment on attachment 9120393 [details]
Bug 1602643 - Disable WebRender compositor dinamically for async screenshot

Added feedback in the phabricator review.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f3b490c076d6
Disable WebRender compositor dinamically for async screenshot r=gw

Comment 16

[Alexandru Michis [:malexandru]]

Backed out changeset f3b490c076d6 for causing bustages regarding CompositorKindChanged

Backout link: https://hg.mozilla.org/integration/autoland/rev/52234324a69eb012bac0b0897a66b754f416e413

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=f3b490c076d623d6ef3bcfd297a0522d9727477c&selectedJob=286147504

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=286147504&repo=autoland&lineNumber=3510

[task 2020-01-23T14:21:23.926Z] Running rustc --edition=2018 --crate-name blob examples/blob.rs --color never --emit=dep-info,link -C debuginfo=2 --test -C metadata=a463c77227765408 -C extra-filename=-a463c77227765408 --out-dir /builds/worker/checkouts/gecko/gfx/wr/target/debug/deps -C incremental=/builds/worker/checkouts/gecko/gfx/wr/target/debug/incremental -L dependency=/builds/worker/checkouts/gecko/gfx/wr/target/debug/deps --extern app_units=/builds/worker/checkouts/gecko/gfx/wr/target/debug/deps/libapp_units-0ee706634a7b1357.rlib --extern env_logger=/builds/worker/checkouts/gecko/gfx/wr/target/debug/deps/libenv_logger-46df478ec27df316.rlib --extern euclid=/builds/worker/checkouts/gecko/gfx/wr/target/debug/deps/libeuclid-79d04d1a47320d90.rlib --extern gleam=/builds/worker/checkouts/gecko/gfx/wr/target/debug/deps/libgleam-e81edc6300e342da.rlib --extern glutin=/builds/worker/checkouts/gecko/gfx/wr/target/debug/deps/libglutin-38e8fbbbfdc71122.rlib --extern rayon=/builds/worker/checkouts/gecko/gfx/wr/target/debug/deps/librayon-a1c1e69a45745d5e.rlib --extern webrender=/builds/worker/checkouts/gecko/gfx/wr/target/debug/deps/libwebrender-e3b96d3751dca171.rlib --extern winit=/builds/worker/checkouts/gecko/gfx/wr/target/debug/deps/libwinit-45219a0a61f5c252.rlib --deny warnings -L native=/builds/worker/checkouts/gecko/gfx/wr/target/debug/build/libloading-4d2ce89d76ac4023/out -L native=/usr/lib/x86_64-linux-gnu
[task 2020-01-23T14:21:24.366Z] error[E0004]: non-exhaustive patterns: Some(CompositorKindChanged) not covered
[task 2020-01-23T14:21:24.366Z] --> tileview/src/main.rs:58:15
[task 2020-01-23T14:21:24.366Z] |
[task 2020-01-23T14:21:24.366Z] 58 | match tile.invalidation_reason {
[task 2020-01-23T14:21:24.366Z] | ^^^^^^^^^^^^^^^^^^^^^^^^ pattern Some(CompositorKindChanged) not covered
[task 2020-01-23T14:21:24.366Z] |
[task 2020-01-23T14:21:24.366Z] = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
[task 2020-01-23T14:21:24.366Z]
[task 2020-01-23T14:21:24.475Z] error: aborting due to previous error
[task 2020-01-23T14:21:24.475Z]
[task 2020-01-23T14:21:24.475Z] For more information about this error, try rustc --explain E0004.
[task 2020-01-23T14:21:24.493Z] error: could not compile tileview.
[task 2020-01-23T14:21:24.494Z]
[task 2020-01-23T14:21:24.495Z] Caused by:
[task 2020-01-23T14:21:24.496Z] process didn't exit successfully: rustc --edition=2018 --crate-name tileview tileview/src/main.rs --color never --emit=dep-info,link -C debuginfo=2 --test -C metadata=7e487246a1019d64 -C extra-filename=-7e487246a1019d64 --out-dir /builds/worker/checkouts/gecko/gfx/wr/target/debug/deps -C incremental=/builds/worker/checkouts/gecko/gfx/wr/target/debug/incremental -L dependency=/builds/worker/checkouts/gecko/gfx/wr/target/debug/deps --extern ron=/builds/worker/checkouts/gecko/gfx/wr/target/debug/deps/libron-b7ddb79eccdefed5.rlib --extern serde=/builds/worker/checkouts/gecko/gfx/wr/target/debug/deps/libserde-bc32f8f3108ce4f8.rlib --extern webrender=/builds/worker/checkouts/gecko/gfx/wr/target/debug/deps/libwebrender-e3b96d3751dca171.rlib --extern webrender_api=/builds/worker/checkouts/gecko/gfx/wr/target/debug/deps/libwebrender_api-0608348ade55c1d4.rlib --deny warnings -L native=/usr/lib/x86_64-linux-gnu (exit code: 1)
[task 2020-01-23T14:21:24.497Z] warning: build failed, waiting for other jobs to finish...
[task 2020-01-23T14:21:30.252Z] error: build failed
[fetches 2020-01-23T14:21:30.254Z] removing /builds/worker/fetches
[fetches 2020-01-23T14:21:30.448Z] finished
[taskcluster 2020-01-23 14:21:30.852Z] === Task Finished ===
[taskcluster 2020-01-23 14:21:30.853Z] Unsuccessful task run with exit code: 101 completed in 1275.717 seconds

Comment 17

[Sotaro Ikeda [:sotaro]]

Build failure was addressed.
https://treeherder.mozilla.org/#/jobs?repo=try&author=sikeda.birchill%40mozilla.com&selectedJob=286260351

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/971767c6ed2b
Disable WebRender compositor dinamically for async screenshot r=gw

Comment 19

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/971767c6ed2b