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

Overwriting the this.context.composer breaks XR views #101

Closed
ROBYER1 opened this issue Oct 25, 2022 · 11 comments
Closed

Overwriting the this.context.composer breaks XR views #101

ROBYER1 opened this issue Oct 25, 2022 · 11 comments
Labels
fixed → to verify but needs testing

Comments

@ROBYER1
Copy link

ROBYER1 commented Oct 25, 2022

Exporter 2.39.3-pre, Unity 2022.1.16f1

Tested on Android Chrome, Xperia 1 Using the below script as PostProcessing.ts in scripts added to a gameobject in the scene, toggling AR view doesn't show the place on surface marker, then pressing the back button in the browser to return to the previous normal 3d view shows the regular view in a squashed cube-shaped window on the bottom half of the screen.

import { Behaviour } from "@needle-tools/engine/engine-components/Component";
import { EffectComposer } from 'three/examples/jsm/postprocessing/EffectComposer.js';
import { RenderPass } from 'three/examples/jsm/postprocessing/RenderPass';
import { UnrealBloomPass } from 'three/examples/jsm/postprocessing/UnrealBloomPass.js';
import { WebGLRenderer} from 'three';
import { GUI } from "three/examples/jsm/libs/lil-gui.module.min.js";

export class PostProcessing extends Behaviour
{
    //@ts-ignore
    params?: PostProcessing.params = {
        //exposure: 1,
        bloomThreshold: 0.656,
        bloomStrength: 2.631,
        bloomRadius: 0.95
    };

    mainComposer?: EffectComposer; 
    renderer?: THREE.WebGLRenderer;

    start()//
    {
        //@ts-ignore
        const bloomPass = new UnrealBloomPass( new THREE.Vector2( window.innerWidth, window.innerHeight ), 1.5, 0.4, 0.85 );
        bloomPass.threshold = this.params.bloomThreshold;
        bloomPass.strength = this.params.bloomStrength;
        bloomPass.radius = this.params.bloomRadius;

        //@ts-ignore
        this.mainComposer = new EffectComposer(this.context.renderer);

        //@ts-ignore
        const renderPass = new RenderPass(this.scene, this.context.mainCamera);
        this.renderer = new WebGLRenderer({ antialias: true, });
        this.renderer?.setSize(this.context.domWidth, this.context.domHeight);
        this.mainComposer.addPass(renderPass);
        this.mainComposer?.addPass(bloomPass);
        this.mainComposer.setSize(this.context.domWidth, this.context.domHeight);

        this.context.composer = this.mainComposer;

        const gui = new GUI();

        gui.add( this.params, 'bloomThreshold', 0.0, 1.0 ).onChange( function ( value ) {

            bloomPass.threshold = Number( value );

        } );

        gui.add( this.params, 'bloomStrength', 0.0, 3.0 ).onChange( function ( value ) {

            bloomPass.strength = Number( value );

        } );

        gui.add( this.params, 'bloomRadius', 0.0, 1.0 ).step( 0.01 ).onChange( function ( value ) {

            bloomPass.radius = Number( value );

        } );
    }
}

c1c44870-928b-445e-9d67-6267e738343a

@marwie
Copy link
Member

marwie commented Oct 25, 2022

The composer does not support webxr: mrdoob/three.js#18846

We could probably internally switch back to normal rendering when in WebXR to fix that

@marwie marwie closed this as completed Oct 25, 2022
@marwie marwie reopened this Oct 25, 2022
@ROBYER1
Copy link
Author

ROBYER1 commented Oct 25, 2022

The composer does not support webxr: mrdoob/three.js#18846

Thanks, is there a default way to reset it when we enter/exit XR?

@marwie
Copy link
Member

marwie commented Oct 25, 2022

You can monitor this.context.isInXR. Would that help?

@ROBYER1
Copy link
Author

ROBYER1 commented Oct 25, 2022

You can monitor this.context.isInXR. Would that help?

Then I assume I would remove pass like this.mainComposer.removePass(bloomPass); based on that, thanks

@ROBYER1
Copy link
Author

ROBYER1 commented Oct 25, 2022

I have tried removing the UnrealBloomPass here when entering/exiting AR but AR is still not working, not sure if I've missed something else or found a bug?

import { Behaviour } from "@needle-tools/engine/engine-components/Component";
import { EffectComposer } from 'three/examples/jsm/postprocessing/EffectComposer.js';
import { RenderPass } from 'three/examples/jsm/postprocessing/RenderPass';
import { UnrealBloomPass } from 'three/examples/jsm/postprocessing/UnrealBloomPass.js';
import { WebGLRenderer} from 'three';
import { GUI } from "three/examples/jsm/libs/lil-gui.module.min.js";

export class PostProcessing extends Behaviour
{
    //@ts-ignore
    params?: PostProcessing.params = {
        //exposure: 1,
        bloomThreshold: 0.656,
        bloomStrength: 2.631,
        bloomRadius: 0.95
    };

    mainComposer?: EffectComposer; 
    renderer?: THREE.WebGLRenderer;
    bloomPass?: UnrealBloomPass;
    isXr?: boolean = false;

    start()//
    {
         //@ts-ignore
        const localBloomPass = new UnrealBloomPass( new THREE.Vector2( window.innerWidth, window.innerHeight ), 1.5, 0.4, 0.85 );

        //@ts-ignore
        this.bloomPass = localBloomPass;
        localBloomPass.threshold = this.params.bloomThreshold;
        localBloomPass.strength = this.params.bloomStrength;
        localBloomPass.radius = this.params.bloomRadius;

        //@ts-ignore
        this.mainComposer = new EffectComposer(this.context.renderer);

        //@ts-ignore
        const renderPass = new RenderPass(this.scene, this.context.mainCamera);
        this.renderer = new WebGLRenderer({ antialias: true, });
        this.renderer?.setSize(this.context.domWidth, this.context.domHeight);
        this.mainComposer.addPass(renderPass);
        this.mainComposer.addPass(localBloomPass);
        this.mainComposer.setSize(this.context.domWidth, this.context.domHeight);
        //this.mainComposer.removePass(bloomPass);
        this.context.composer = this.mainComposer;

        const gui = new GUI();

        gui.add( this.params, 'bloomThreshold', 0.0, 1.0 ).onChange( function ( value ) {

            localBloomPass.threshold = Number( value );

        } );

        gui.add( this.params, 'bloomStrength', 0.0, 3.0 ).onChange( function ( value ) {

            localBloomPass.strength = Number( value );

        } );

        gui.add( this.params, 'bloomRadius', 0.0, 1.0 ).step( 0.01 ).onChange( function ( value ) {

            localBloomPass.radius = Number( value );

        } );
    }

    update(){
        if(this.context.isInXR == true && this.isXr == false)
        {
            console.log("we in XR");
            this.mainComposer.removePass(this.mainComposer.passes[1]);
            console.log(this.mainComposer.passes);
            this.isXr = true;
        }
        else if(this.context.isInXR == false && this.isXr == true)
        {
            console.log("we out XR");
            if(this.mainComposer.passes.length == 1)
            {
                this.mainComposer.addPass(this.bloomPass);
                console.log(this.mainComposer.passes);
            }
            this.isXr = false;
        }
    }
}

@marwie
Copy link
Member

marwie commented Oct 25, 2022

You would need to remove the composer from the context I think

@marwie
Copy link
Member

marwie commented Oct 25, 2022

I'll add a check to the render call to not use the composer when we're in XR

image

@ROBYER1
Copy link
Author

ROBYER1 commented Oct 25, 2022

That's perfect, is good for peace of mind for future changes to the composer if I add more post processing

@hybridherbst
Copy link
Contributor

We'll have to revisit this at some point for actually using the composer in XR; there's some three samples that actually do that as far as I'm aware. Current stand-alone VR headsets are probably all too weak for in-VR render passes though, so more of a future problem.

@marwie
Copy link
Member

marwie commented Nov 2, 2022

@ROBYER1 could you verify that the latest version works without having to manually remove the composer?

@marwie marwie added the fixed → to verify but needs testing label Nov 2, 2022
@ROBYER1 ROBYER1 closed this as completed Nov 2, 2022
@ROBYER1
Copy link
Author

ROBYER1 commented Nov 2, 2022

@ROBYER1 could you verify that the latest version works without having to manually remove the composer?

Confirmed this is now fixed

@ROBYER1 ROBYER1 reopened this Nov 2, 2022
@ROBYER1 ROBYER1 closed this as completed Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed → to verify but needs testing
Projects
None yet
Development

No branches or pull requests

3 participants