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

Fix EffectComposer for vr mode #14274

Closed
wants to merge 1 commit into from
Closed

Conversation

dmarcos
Copy link
Contributor

@dmarcos dmarcos commented Jun 11, 2018

With the new VR API (without VRControls / VREffect) the frame is rendered and submitted within the renderer. The current EffectComposer API is not suitable for VR use cases. This PR relies on the onAfterRender callback enabling postprocessing passes to access the frame before is submitted to the headset. I admit this is a bit of a contortion. I put myself at the disposal of the THREE gods to incorporate any API changes you think are appropriate.

@dmarcos dmarcos force-pushed the postprocessingVR branch 2 times, most recently from fbb6c12 to f746b2d Compare June 11, 2018 21:01
@dmarcos dmarcos force-pushed the postprocessingVR branch from f746b2d to 7be36bf Compare June 11, 2018 21:04

context.stencilFunc( context.NOTEQUAL, 1, 0xffffffff );
pass.scene.onAfterRender = function () {
Copy link
Contributor

@pailhead pailhead Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe not create this function here? not sure though if it's easy

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah it actually seems extremely complicated, this is a tight loop though, not sure how much of a concern this should be?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, good point. The function will be allocated once per frame. I'll factor it out.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As is it looks like it would be instantiated this.passes.length times? I thought about it but it looked kinda complicated to carry the i over.

@dmarcos
Copy link
Contributor Author

dmarcos commented Jun 14, 2018

@fernandojsg What do you think?

@nbriz
Copy link

nbriz commented Jun 15, 2018

i would LOVE to see this update! there's another PR on the subject here: #11376

@dmarcos
Copy link
Contributor Author

dmarcos commented Jun 15, 2018

@nbriz Thanks. I didn't see the other PR. This one makes use of the onAfterRender callback. Either way works for me.

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 18, 2019

Closing in favor of #15840.

@Mugen87 Mugen87 closed this Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants