Multi-doc support #1509

kvark · 2017-07-22T03:50:01Z

A namespace can now contain multiple documents. Operations on a document are done ~~via a separate object DocumentApi, which created from ResourceApi that manages resources~~ by providing DocumentId parameter to RenderApi calls.

This PR is a step towards #1357, also relates to #1370.

This change is

glennw · 2017-07-24T02:22:07Z

Reviewed 21 of 21 files at r1.
Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

webrender/src/lib.rs, line 16 at r1 (raw file):

//! The main entry point to WebRender is the `webrender::renderer::Renderer`.
//!
//! By calling `Renderer::new(...)` you get a `Renderer`, as well as a `DocumentApiSender`.

It's not a DocumentApiSender you get from that?

webrender/src/lib.rs, line 19 at r1 (raw file):

//! Your `Renderer` is responsible to render the previously processed frames onto the screen.
//!
//! By calling `yourDocumentApiSenderInstance.create_api()`, you'll get a `DocumentApi` instance,

Same as above.

webrender/src/render_backend.rs, line 123 at r1 (raw file):

    frame_config: FrameBuilderConfig,
    documents: HashMap<DocumentId, Document>,

Let's use Fnv hasher here.

wrench/src/json_frame_writer.rs, line 73 at r1 (raw file):

    }

    pub fn begin_write_display_list(&mut self,

I wonder if we should just remove the json from writer completely? I don't think anyone uses it...

Comments from Reviewable

glennw · 2017-07-24T02:22:30Z

Looks great! Just a few minor comments / changes.

glennw · 2017-07-24T02:23:03Z

Also, it'd be good to have the corresponding Servo patch ready to go when we land this so we can get a WR update landed straight after.

nical · 2017-07-24T15:06:01Z

The separation between the resource and document apis looks nice but it will make the task of bundling image updates and display lists in the same transactions difficult.

nical · 2017-07-24T16:48:34Z

I suggest not rushing to update servo and gecko, because if I manage to be convincing enough during tomorrow's meeting, we'll do something different (not totally different but with some substantial API changes).

kvark · 2017-07-24T19:18:46Z

Servo integration is not trivial, WIP, but other notes are addressed now.

Review status: 12 of 21 files reviewed at latest revision, 4 unresolved discussions.

webrender/src/lib.rs, line 16 at r1 (raw file):

Previously, glennw (Glenn Watson) wrote…

It's not a DocumentApiSender you get from that?

fixed

webrender/src/lib.rs, line 19 at r1 (raw file):

Previously, glennw (Glenn Watson) wrote…

Same as above.

fixed

webrender/src/render_backend.rs, line 123 at r1 (raw file):

Previously, glennw (Glenn Watson) wrote…

Let's use Fnv hasher here.

fixed

wrench/src/json_frame_writer.rs, line 73 at r1 (raw file):

Previously, glennw (Glenn Watson) wrote…

I wonder if we should just remove the json from writer completely? I don't think anyone uses it...

agreed, but better left for a dedicated PR

Comments from Reviewable

jrmuizel · 2017-07-24T19:23:14Z

Previously, glennw (Glenn Watson) wrote…

I wonder if we should just remove the json from writer completely? I don't think anyone uses it...

It's been used for getting readable dumps of binary recordings. I think we could probably simplify it to just providing that functionality.

bors-servo · 2017-07-25T01:23:02Z

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

bors-servo · 2017-07-26T04:00:15Z

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

bors-servo · 2017-07-26T17:02:35Z

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

kvark · 2017-07-26T18:59:06Z

@glennw servo update is in kvark/servo@a6e54c9

glennw · 2017-07-27T01:43:50Z

Reviewed 21 of 21 files at r2.
Review status: all files reviewed at latest revision, 4 unresolved discussions.

Comments from Reviewable

glennw

Looks good! I'll leave for you to merge when ready, but r=me.

bors-servo · 2017-07-27T02:16:18Z

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

kvark · 2017-07-27T15:24:25Z

Rebasing this is a constant pain...
@bors-servo r=glennw

bors-servo · 2017-07-27T15:24:25Z

📌 Commit f6d81d9 has been approved by glennw

bors-servo · 2017-07-27T15:24:28Z

⌛ Testing commit f6d81d9 with merge 97af583...

Multi-doc support A namespace can now contain multiple documents. Operations on a document are done ~~via a separate object `DocumentApi`, which created from `ResourceApi` that manages resources~~ by providing `DocumentId` parameter to `RenderApi` calls. This PR is a step towards #1357, also relates to #1370.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1509)

bors-servo · 2017-07-27T16:20:18Z

☀️ Test successful - status-travis
Approved by: glennw
Pushing 97af583 to master...

WR multi-document update  The PR updates WR version to support multiple documents (servo/webrender#1509) but doesn't take advantage of this new feature yet. It also makes Servo to use `DevicePixel` from WR instead of rolling out another one. ---  - [ ] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable).  - [ ] There are tests for these changes OR - [x] These changes do not require tests because _____ no extra logic    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17892)

@MortimerGoro

WebGL encapsulation Refactors our WebGL usage and fixes a regression from #1509 also related to #1353 (should make it easier) r? @MortimerGoro cc @glennw  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1530)

WR multi-document update  The PR updates WR version to support multiple documents (servo/webrender#1509) but doesn't take advantage of this new feature yet. It also makes Servo to use `DevicePixel` from WR instead of rolling out another one. ---  - [ ] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable).  - [ ] There are tests for these changes OR - [x] These changes do not require tests because _____ no extra logic    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17892)

The PR updates WR version to support multiple documents (servo/webrender#1509) but doesn't take advantage of this new feature yet. It also makes Servo to use `DevicePixel` from WR instead of rolling out another one. ---  - [ ] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable).  - [ ] There are tests for these changes OR - [x] These changes do not require tests because _____ no extra logic   Source-Repo: https://github.com/servo/servo Source-Revision: 12a49dc0be8a8acd12440dd7191b349ca17de7c8 --HG-- extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear extra : subtree_revision : f036e78668713987f000f99714f86e588107baa1

The PR updates WR version to support multiple documents (servo/webrender#1509) but doesn't take advantage of this new feature yet. It also makes Servo to use `DevicePixel` from WR instead of rolling out another one. ---  - [ ] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable).  - [ ] There are tests for these changes OR - [x] These changes do not require tests because _____ no extra logic   Source-Repo: https://github.com/servo/servo Source-Revision: 12a49dc0be8a8acd12440dd7191b349ca17de7c8

As a result of the multi-document changes in servo#1509, it is necessary for Gecko to stop just inventing its own namespace ids and instead use ones that match what WR is using internally. Therefore it is necessary for the RenderApi to expose the namespace id.

Expose the namespace id from RenderApi As a result of the multi-document changes in #1509, it is necessary for Gecko to stop just inventing its own namespace ids and instead use ones that match what WR is using internally. Therefore it is necessary for the RenderApi to expose the namespace id.

sotaroikeda · 2017-08-14T03:42:44Z

@kvark, can you explain about actual benefit of supporting Multi-doc support? Is it still in the middle of implementation? Gecko creates GL context for each window, but webrender Renderer instance still owns single GL context, therefore each window needs to have different Renderer instance. Then current gecko just use one document on each Renderer and each window creates each webrender Renderer by Bug 1385003 Thanks!

kvark · 2017-08-14T14:24:58Z

@sotaroikeda the idea is that WR can have multiple Renderer instances sharing the same backend and receiving frames only from the corresponding documents.

sotaroikeda · 2017-08-25T03:48:24Z

@nical, do you have any ideas to use Multi-doc support in gecko?

nical · 2017-08-25T08:35:28Z

@nical, do you have any ideas to use Multi-doc support in gecko?

I haven't looked into the details since the multi-doc stuff landed, but I think that it'd be good to complete the implementation so that documents can use their own renderer. That way we can look into grouping some windows like tooltips into using a single render backend thread, and share fonts, glyph cache, etc.

Is it still in the middle of implementation?

Yes, as far as I can tell (it doesn't let you have a renderer per document yet for example).

kvark · 2017-08-25T14:49:49Z

Basically,

webrender/webrender/src/renderer.rs

Line 1455 in 2ac2da0

//TODO: associate `document_id` with target window

@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)

The PR updates WR version to support multiple documents (servo/webrender#1509) but doesn't take advantage of this new feature yet. It also makes Servo to use `DevicePixel` from WR instead of rolling out another one. ---  - [ ] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable).  - [ ] There are tests for these changes OR - [x] These changes do not require tests because _____ no extra logic   Source-Repo: https://github.com/servo/servo Source-Revision: 12a49dc0be8a8acd12440dd7191b349ca17de7c8 UltraBlame original commit: c424ad1c5f9441f43f3f792d853170f3fd8216e9

kvark force-pushed the document branch 2 times, most recently from 03419a0 to b89b0e5 Compare July 24, 2017 14:37

kvark force-pushed the document branch from b89b0e5 to f416ffb Compare July 24, 2017 15:28

kvark force-pushed the document branch 2 times, most recently from 9b09cc8 to 1969102 Compare July 24, 2017 19:07

kvark force-pushed the document branch from 1969102 to d7d5f35 Compare July 26, 2017 01:29

kvark force-pushed the document branch 2 times, most recently from 3d0bde5 to 562f521 Compare July 26, 2017 16:59

kvark force-pushed the document branch from 562f521 to c719e87 Compare July 26, 2017 18:14

glennw self-requested a review July 27, 2017 01:44

glennw approved these changes Jul 27, 2017

View reviewed changes

Multiple documents support

f6d81d9

kvark force-pushed the document branch from c719e87 to f6d81d9 Compare July 27, 2017 14:31

bors-servo merged commit f6d81d9 into servo:master Jul 27, 2017

kvark deleted the document branch July 27, 2017 17:00

kvark mentioned this pull request Jul 27, 2017

WR multi-document update servo/servo#17892

Merged

5 tasks

kvark mentioned this pull request Jul 28, 2017

WebGL encapsulation #1530

Merged

staktrace mentioned this pull request Aug 4, 2017

Expose the namespace id from RenderApi #1553

Merged

kvark mentioned this pull request Nov 16, 2017

DocumentAPI support on the Renderer side #2050

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multi-doc support #1509

Multi-doc support #1509

kvark commented Jul 22, 2017 •

edited

glennw commented Jul 24, 2017

glennw commented Jul 24, 2017

glennw commented Jul 24, 2017

nical commented Jul 24, 2017

nical commented Jul 24, 2017

kvark commented Jul 24, 2017

jrmuizel commented Jul 24, 2017

bors-servo commented Jul 25, 2017

bors-servo commented Jul 26, 2017

bors-servo commented Jul 26, 2017

kvark commented Jul 26, 2017 •

edited

glennw commented Jul 27, 2017

glennw left a comment

bors-servo commented Jul 27, 2017

kvark commented Jul 27, 2017

bors-servo commented Jul 27, 2017

bors-servo commented Jul 27, 2017

bors-servo commented Jul 27, 2017

sotaroikeda commented Aug 14, 2017

kvark commented Aug 14, 2017

sotaroikeda commented Aug 25, 2017

nical commented Aug 25, 2017

kvark commented Aug 25, 2017

Multi-doc support #1509

Multi-doc support #1509

Conversation

kvark commented Jul 22, 2017 • edited

glennw commented Jul 24, 2017

glennw commented Jul 24, 2017

glennw commented Jul 24, 2017

nical commented Jul 24, 2017

nical commented Jul 24, 2017

kvark commented Jul 24, 2017

jrmuizel commented Jul 24, 2017

bors-servo commented Jul 25, 2017

bors-servo commented Jul 26, 2017

bors-servo commented Jul 26, 2017

kvark commented Jul 26, 2017 • edited

glennw commented Jul 27, 2017

glennw left a comment

Choose a reason for hiding this comment

bors-servo commented Jul 27, 2017

kvark commented Jul 27, 2017

bors-servo commented Jul 27, 2017

bors-servo commented Jul 27, 2017

bors-servo commented Jul 27, 2017

sotaroikeda commented Aug 14, 2017

kvark commented Aug 14, 2017

sotaroikeda commented Aug 25, 2017

nical commented Aug 25, 2017

kvark commented Aug 25, 2017

kvark commented Jul 22, 2017 •

edited

kvark commented Jul 26, 2017 •

edited