DocumentAPI support on the Renderer side #2050

kvark · 2017-11-16T20:05:41Z

This is a second batch of changes related to Document API, following #1509.

The idea is to allow Gecko to have separate documents for the chrome UI, page content, and bottom status bar, as opposed to using different pipelines in the same document. This change would allow minimal scene rebuilds per frame when UI is affected.

WIP TODO:

strict ordering API: 532bff5
Servo patch and test runs: kvark/servo@0263fa8
Firefox patch and try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f939fbdab0231e6328061c12903db0c45847dd18
- ~~box-shadow/boxshadow-skiprect.html~~ (from #1943)
- ~~css-filter-chains/moz-element.html~~ (same as in #2053)
- ~~transform/animate-layer-scale-inherit-1.html~~ (from #2043)
example
FrameBuilder and RenderedDocument refactor
https://tools.taskcluster.net/groups/DEvThfCaQWmt5Hp806GDLg/tasks/VLCZI7hVTQCc1xrDR3PgOQ/details#
review comments

cc @glennw @mstange @jrmuizel

This change is

bors-servo · 2017-11-16T21:30:58Z

☔ The latest upstream changes (presumably #2043) made this pull request unmergeable. Please resolve the merge conflicts.

kvark · 2017-11-18T02:34:05Z

The performance aspect here is difficult to get right, since it relies on a frame semantics change. Old logic used to clear everything if either requested, or the frame size is less than the viewport. The new logic is:

main framebuffer color is cleared if the background color is provided in the options
each rendered document unconditionally clears the depth to 1.0 for its affected area
each document can also clear the affected color area, if the clear color is provided for it

Thus, we may end up clearing twice with the new logic. I expect the WR client to be careful about the clear colors and don't specify both the framebuffer and document/frame clear color unless necessary.

Aside from this aspect, the PR should be ready to review/merge now.

kvark · 2017-11-18T02:39:00Z

Actually, something wonky is going on at the start of the new document example. Investigating...

kvark · 2017-11-18T19:25:26Z

Ok, it turned out to be this: cc41e04#diff-f5062b694b9fe53fc1757ed483d577d9R2788

Some squashing is expected, but otherwise should be ready to review now.

kvark · 2017-11-19T03:53:10Z

Fresh gecko try push with all the new code: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9be1f766b547f71074a3e41a9176c2c3584c5241

glennw · 2017-11-19T23:27:11Z

Reviewed 3 of 12 files at r1, 18 of 22 files at r2, 4 of 5 files at r3, 1 of 1 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

glennw

Looks good! Squash if you want and r=me.

glennw · 2017-11-19T23:41:36Z

Also - before merging, it'd be good to have patches available for Servo and Gecko to deal with the API changes, to simplify the update process.

nical

Great stuff!

nical · 2017-11-20T13:42:31Z

webrender_api/src/api.rs

@@ -884,8 +886,9 @@ pub struct DynamicProperties {

 pub trait RenderNotifier: Send {
    fn clone(&self) -> Box<RenderNotifier>;
-    fn new_frame_ready(&self);
-    fn new_scroll_frame_ready(&self, composite_needed: bool);
+    fn wakeup(&self);


Nittiest of all nits: I'd prefer wake_up

nical · 2017-11-20T13:42:31Z

webrender_api/src/api.rs

-    fn new_frame_ready(&self);
-    fn new_scroll_frame_ready(&self, composite_needed: bool);
+    fn wakeup(&self);
+    fn new_document_ready(&self, DocumentId);


While you are modifying this interface, maybe it'd be a good time to merge new_document_ready and new_scroll_document_ready into a single one that has enough parameters (id, did_scroll, composite_needed) to avoid contortions like in #1963 when the scrolled and non-scrolled paths are merged.

kvark · 2017-11-20T16:20:44Z

Thanks for the reviews! Concerns are addressed now.
Latest try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1b91c4648c732ad90544488cc992caf0564ebaa9
Gecko changes: everything under gfx/webrender_bindings in https://hg.mozilla.org/try/rev/1b91c4648c732ad90544488cc992caf0564ebaa9
Servo changes: kvark/servo@ffbc3a7

bors-servo · 2017-11-20T17:55:07Z

☔ The latest upstream changes (presumably #2052) made this pull request unmergeable. Please resolve the merge conflicts.


        Multi-doc support on the Renderer side

glennw · 2017-11-20T19:04:19Z

@kvark Thanks for the update patches! Want to rebase this again and we'll get it merged before it bitrots again? :)


        Using the actual target rectangle by the renderer.

Naming refactor of RenderedDocument DocumentLayer index for rendering order


        Document API example

Refactor FrameBuilder usage on the RenderBackend side to avoid None Simplified RenderedDocument Fix for building an empty frame Refactored RenderNotifier according to review comments Added an optimization to avoid clearing the whole screen if one of the documents does it

kvark · 2017-11-20T20:05:00Z

Rebased, added an optimization to avoid redundant clears (in a simple case of 1 document), and pushed another try push - https://treeherder.mozilla.org/#/jobs?repo=try&revision=376c737901e3a27405808e5a828126e60bd91cc5

glennw · 2017-11-20T20:47:42Z

@kvark Looks like there are some failures in that try push, but they seem the same (or similar) to ones we've had with recent PRs. I'm not sure if those are relevant or not in this case?

kvark · 2017-11-20T22:51:53Z

@glennw those 10 are expected, same as with your #2052: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e71afa7f78443212f702971f0a4eb05e27bf7b80
Edit: unless I screwed up something there too :)

glennw · 2017-11-20T22:54:51Z

@bors-servo r+ 🚀 💥

bors-servo · 2017-11-20T22:54:52Z

📌 Commit 07ebcdf has been approved by glennw

bors-servo · 2017-11-21T00:32:06Z

⌛ Testing commit 07ebcdf with merge 0864d35...


        Auto merge of #2050 - kvark:document, r=glennw

@glennw

DocumentAPI support on the Renderer side This is a second batch of changes related to Document API, following #1509. The idea is to allow Gecko to have separate documents for the chrome UI, page content, and bottom status bar, as opposed to using different pipelines in the same document. This change would allow minimal scene rebuilds per frame when UI is affected. WIP TODO: - [x] strict ordering API: 532bff5 - [x] Servo patch and test runs: kvark/servo@0263fa8 - [x] Firefox patch and try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f939fbdab0231e6328061c12903db0c45847dd18 - ~~`box-shadow/boxshadow-skiprect.html`~~ (from #1943) - ~~`css-filter-chains/moz-element.html`~~ (same as in #2053) - ~~`transform/animate-layer-scale-inherit-1.html`~~ (from #2043) - [x] example - [x] `FrameBuilder` and `RenderedDocument` refactor https://tools.taskcluster.net/groups/DEvThfCaQWmt5Hp806GDLg/tasks/VLCZI7hVTQCc1xrDR3PgOQ/details# - [x] review comments cc @glennw @mstange @jrmuizel  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2050)

bors-servo · 2017-11-21T02:27:31Z

☀️ Test successful - status-appveyor, status-travis
Approved by: glennw
Pushing 0864d35 to master...


        Auto merge of #2098 - kvark:rt-pool, r=glennw

@glennw

Flexible render target pool This is a follow-up to #2050, also a more generic solution in place of the fix in #2039. With multiple documents, we can't rely on stack-based render target re-use, since each document would expect a different stack and will force the wasteful re-allocation of render targets. With this PR, we'll first do a pass to re-use all perfectly matching targets for all documents. Then when each document is rendered, it will grab the remaining targets and re-initialize some parts of them, if needed. This scheme should guarantee efficient re-use of render target space for multiple documents, which are supposed to be used in Gecko for chrome, status bar, main page, and more. Note: target selection doesn't take the depth surface into account at the moment, so these may sometimes re-allocate inefficiently. This is something to keep an eye on and potentially follow-up. r? @glennw  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2098)

kvark force-pushed the kvark:document branch from 606a92a to 532bff5 Nov 17, 2017

kvark changed the title ~~[WIP] DocumentAPI support on the Renderer side~~ DocumentAPI support on the Renderer side Nov 18, 2017

kvark force-pushed the kvark:document branch 2 times, most recently from 31d276f to e45c6db Nov 18, 2017

kvark force-pushed the kvark:document branch from bb4c463 to 88126ab Nov 19, 2017

glennw self-requested a review Nov 19, 2017

glennw reviewed Nov 19, 2017

View changes

nical reviewed Nov 20, 2017

View changes

kvark force-pushed the kvark:document branch from 88126ab to ecb4283 Nov 20, 2017

Multi-doc support on the Renderer side

39b9065

Using the actual target rectangle by the renderer.

01bf522

Naming refactor of RenderedDocument DocumentLayer index for rendering order

kvark force-pushed the kvark:document branch from ecb4283 to b19a91f Nov 20, 2017

kvark force-pushed the kvark:document branch from b19a91f to 07ebcdf Nov 20, 2017

bors-servo mentioned this pull request Nov 21, 2017

support font subpixel AA with content transforms #2058

Merged

kvark deleted the kvark:document branch Nov 21, 2017

kvark mentioned this pull request Nov 24, 2017

Flexible render target pool #2098

Merged

servo / webrender

DocumentAPI support on the Renderer side #2050

DocumentAPI support on the Renderer side #2050

kvark commented Nov 16, 2017 •

edited

bors-servo commented Nov 16, 2017

kvark commented Nov 18, 2017

kvark commented Nov 18, 2017

kvark commented Nov 18, 2017

kvark commented Nov 19, 2017

glennw commented Nov 19, 2017

glennw left a comment

glennw commented Nov 19, 2017

nical left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

kvark commented Nov 20, 2017

bors-servo commented Nov 20, 2017

glennw commented Nov 20, 2017

kvark commented Nov 20, 2017

glennw commented Nov 20, 2017

kvark commented Nov 20, 2017 •

edited

glennw commented Nov 20, 2017

bors-servo commented Nov 20, 2017

bors-servo commented Nov 21, 2017

bors-servo commented Nov 21, 2017

servo / webrender

Join GitHub today

DocumentAPI support on the Renderer side #2050

DocumentAPI support on the Renderer side #2050

Conversation

kvark commented Nov 16, 2017 • edited

bors-servo commented Nov 16, 2017

kvark commented Nov 18, 2017

kvark commented Nov 18, 2017

kvark commented Nov 18, 2017

kvark commented Nov 19, 2017

glennw commented Nov 19, 2017

glennw left a comment

glennw commented Nov 19, 2017

nical left a comment

This comment has been minimized.

nical Nov 20, 2017

This comment has been minimized.

nical Nov 20, 2017

This comment has been minimized.

kvark Nov 20, 2017

kvark commented Nov 20, 2017

bors-servo commented Nov 20, 2017

glennw commented Nov 20, 2017

kvark commented Nov 20, 2017

glennw commented Nov 20, 2017

kvark commented Nov 20, 2017 • edited

glennw commented Nov 20, 2017

bors-servo commented Nov 20, 2017

bors-servo commented Nov 21, 2017

bors-servo commented Nov 21, 2017

kvark commented Nov 16, 2017 •

edited

kvark commented Nov 20, 2017 •

edited