Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce frame output support. #1687

Merged
merged 1 commit into from Sep 13, 2017
Merged

Introduce frame output support. #1687

merged 1 commit into from Sep 13, 2017

Conversation

@glennw
Copy link
Member

glennw commented Sep 11, 2017

This adds a new feature to WR, that allows copying the output
of a given pipeline ID to an external texture. This is a feature
request from the webvr prototyping team.

The included example demonstrates how to use the API to copy the
output of an iframe to an external texture, which is then copied
onto the screen via custom GL commands.


This change is Reviewable

@glennw
Copy link
Member Author

glennw commented Sep 11, 2017

Copy link
Member

kvark left a comment

Looks fine by me.
My biggest concern here is the GL-only example. I think it's not strictly necessary - we could use WR's power to work with produced texture ID like it's coming from the outside. One way to write such an example would be to define 2 documents: one with a pipeline external target, and another one with an external image where this GL texture is used.

@@ -2679,17 +2746,19 @@ impl Renderer {
}

for (i, texture) in self.color_render_targets.iter().chain(self.alpha_render_targets.iter()).enumerate() {
let dimensions = texture.get_dimensions();
let src_rect = DeviceIntRect::new(DeviceIntPoint::zero(),
DeviceIntSize::new(dimensions.width as i32,

This comment has been minimized.

@kvark

kvark Sep 11, 2017

Member

nit: could probably do some casting of euclid::TypedSize2D

/// call succeeds, when WR has issued the GL commands to copy the output
/// to the texture handle.
pub trait OutputImageHandler {
fn lock(&mut self, pipeline_id: PipelineId) -> Option<(u32, DeviceIntSize)>;

This comment has been minimized.

@kvark

kvark Sep 11, 2017

Member

we might want to revisit the lock/unlock semantics here later (is that how VR team wants to work with WR?), especially since WR pretty much guarantees that unlock is going to be called right away after some blitting (contrary to external image locks, where unlocking can happen at any time later).

This comment has been minimized.

@glennw

glennw Sep 11, 2017

Author Member

The vr team have a demo working with the lock/unlock - but yes, we may want to revisit these semantics later on once we work out the exact synchronization story.

@kvark
Copy link
Member

kvark commented Sep 11, 2017

Oh, and I forgot to mention that this would be really nice to hook up via the document API: having VR content in a separate WR document that's routed into a separate target, as opposed to hacking the PipelineId handling.

@glennw
Copy link
Member Author

glennw commented Sep 11, 2017

@kvark I would like to hook up to the document API too - I think the pipeline API is fine for now (allows easy prototyping in Servo). But certainly once we make better use of the document API in Servo, we could consider switching frame output to support both documents and pipelines, or switch to documents exclusively.

@bors-servo
Copy link
Contributor

bors-servo commented Sep 12, 2017

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

@glennw glennw force-pushed the glennw:frame-output branch from cade5de to e7756f7 Sep 12, 2017
@glennw
Copy link
Member Author

glennw commented Sep 12, 2017

Rebased and fixed cast nit.

@bors-servo
Copy link
Contributor

bors-servo commented Sep 12, 2017

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

@glennw glennw force-pushed the glennw:frame-output branch from e7756f7 to 8e9a87e Sep 12, 2017
@kvark
Copy link
Member

kvark commented Sep 12, 2017

Ok, merging on the merit that this is going to be ported to document API later on (or at least considered :) )
@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented Sep 12, 2017

📌 Commit 8e9a87e has been approved by kvark

@bors-servo
Copy link
Contributor

bors-servo commented Sep 12, 2017

🔒 Merge conflict

@bors-servo
Copy link
Contributor

bors-servo commented Sep 12, 2017

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

This adds a new feature to WR, that allows copying the output
of a given pipeline ID to an external texture. This is a feature
request from the webvr prototyping team.

The included example demonstrates how to use the API to copy the
output of an iframe to an external texture, which is then copied
onto the screen via custom GL commands.
@glennw glennw force-pushed the glennw:frame-output branch from 8e9a87e to f9b5e6c Sep 12, 2017
@glennw
Copy link
Member Author

glennw commented Sep 12, 2017

Rebased.

@bors-servo r=kvark

@bors-servo
Copy link
Contributor

bors-servo commented Sep 12, 2017

📌 Commit f9b5e6c has been approved by kvark

@bors-servo
Copy link
Contributor

bors-servo commented Sep 12, 2017

Testing commit f9b5e6c with merge 65cbe63...

bors-servo added a commit that referenced this pull request Sep 12, 2017
Introduce frame output support.

This adds a new feature to WR, that allows copying the output
of a given pipeline ID to an external texture. This is a feature
request from the webvr prototyping team.

The included example demonstrates how to use the API to copy the
output of an iframe to an external texture, which is then copied
onto the screen via custom GL commands.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1687)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Sep 12, 2017

💔 Test failed - status-travis

@glennw
Copy link
Member Author

glennw commented Sep 13, 2017

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Sep 13, 2017

Testing commit f9b5e6c with merge 47d40d9...

bors-servo added a commit that referenced this pull request Sep 13, 2017
Introduce frame output support.

This adds a new feature to WR, that allows copying the output
of a given pipeline ID to an external texture. This is a feature
request from the webvr prototyping team.

The included example demonstrates how to use the API to copy the
output of an iframe to an external texture, which is then copied
onto the screen via custom GL commands.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1687)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Sep 13, 2017

☀️ Test successful - status-travis
Approved by: kvark
Pushing 47d40d9 to master...

@bors-servo bors-servo merged commit f9b5e6c into servo:master Sep 13, 2017
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
@glennw glennw deleted the glennw:frame-output branch Sep 13, 2017
@MortimerGoro MortimerGoro mentioned this pull request Sep 21, 2017
3 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.