Closed Bug 1602643 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Graphics: WebRender, enhancement)

Unspecified
Windows 10
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla74
Tracking Status
firefox74 --- fixed

People

(Reporter: sotaro, Assigned: sotaro)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Created from Bug 1592031 .

OS: Unspecified → Windows 10
Assignee: nobody → sotaro.ikeda.g
Blocks: 1592509, 1592031

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

No longer blocks: 1592031

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.

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

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.

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?

Flags: needinfo?(bas)

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.

Flags: needinfo?(bas)

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

Flags: needinfo?(bas)
Attachment #9120393 - Attachment description: Bug 1602643 - Disable WebRender compositor dinamically for async screenshot → Bug 1602643 - (WIP) Disable WebRender compositor dinamically for async screenshot

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

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

Flags: needinfo?(bas)
Depends on: 1609598
Attachment #9120393 - Attachment description: Bug 1602643 - (WIP) Disable WebRender compositor dinamically for async screenshot → Bug 1602643 - Disable WebRender compositor dinamically for async screenshot

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?

Attachment #9120393 - Flags: feedback?(gwatson)

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.

Attachment #9120393 - Flags: feedback?(gwatson)

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?

Attachment #9120393 - Flags: feedback?(gwatson)

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

Added feedback in the phabricator review.

Attachment #9120393 - Flags: feedback?(gwatson)
Pushed by sikeda.birchill@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f3b490c076d6 Disable WebRender compositor dinamically for async screenshot r=gw

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

Flags: needinfo?(sotaro.ikeda.g)
Pushed by sikeda.birchill@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/971767c6ed2b Disable WebRender compositor dinamically for async screenshot r=gw
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla74
Regressions: 1627117
Regressions: 1624817
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: