Make profiler screenshots and frame recording work in the WR OS compositor configuration on Windows
Categories
(Core :: Graphics: WebRender, enhancement)
Tracking
()
Tracking | Status | |
---|---|---|
firefox74 | --- | fixed |
People
(Reporter: sotaro, Assigned: sotaro)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/x-phabricator-request
|
Details |
Created from Bug 1592031 .
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 1•5 years ago
|
||
If RenderCompositorANGLE::MaybeReadback() is re-used, it could take 30-60ms:( IDCompositionDevice::WaitForCommitCompletion() and ::PrintWindow() took very long time.
Updated•5 years ago
|
Assignee | ||
Comment 2•5 years ago
|
||
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.
Assignee | ||
Comment 3•5 years ago
|
||
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•5 years ago
|
||
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•5 years ago
|
||
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•5 years ago
|
||
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.
Assignee | ||
Comment 7•5 years ago
|
||
(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.
Assignee | ||
Comment 8•5 years ago
|
||
Updated•5 years ago
|
Assignee | ||
Comment 9•5 years ago
|
||
By Attachment 9120393 [details], it seems possible to implement [3], though it is wip patch and has some problems.
Comment 10•5 years ago
|
||
(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.
Updated•5 years ago
|
Assignee | ||
Comment 11•5 years ago
|
||
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•5 years ago
|
||
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.
Assignee | ||
Comment 13•5 years ago
|
||
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•5 years ago
|
||
Comment on attachment 9120393 [details]
Bug 1602643 - Disable WebRender compositor dinamically for async screenshot
Added feedback in the phabricator review.
Comment 15•5 years ago
|
||
Comment 16•5 years ago
|
||
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
Assignee | ||
Comment 17•5 years ago
|
||
Build failure was addressed.
https://treeherder.mozilla.org/#/jobs?repo=try&author=sikeda.birchill%40mozilla.com&selectedJob=286260351
Comment 18•5 years ago
|
||
Comment 19•5 years ago
|
||
bugherder |
Assignee | ||
Updated•3 years ago
|
Description
•