Add type check to DepthTexture #9774

Merged
merged 3 commits into from Nov 13, 2016

Projects

None yet

3 participants

@takahirox
Contributor

Refer to #9725

@mrdoob
Owner
mrdoob commented Sep 26, 2016

Hmm... I have the feeling this code could be more readable...

@takahirox
Contributor
takahirox commented Sep 26, 2016 edited

How about this one?

if ( this.format === DepthFormat ) {

    this.type = type !== undefined ? type : UnsignedShortType;

    if ( this.type !== UnsignedShortType && this.type !== UnsignedIntType ) {

        console.warn( 'THREE.DepthTexture: DepthFormat requires UnsignedShortType or UnsignedIntType.' );

    }

} else {

    this.type = type !== undefined ? type : UnsignedInt248Type;

    if ( this.type !== UnsignedInt248Type ) {

        console.warn( 'THREE.DepthTexture: DepthStencilFormat requires UnsignedInt248Type.' );

    }

}
@mrdoob
Owner
mrdoob commented Sep 26, 2016 edited

Some clean up.

if ( this.format === DepthFormat ) {

    if ( type === undefined ) type = UnsignedShortType;

    if ( type !== UnsignedShortType && type !== UnsignedIntType ) {

        console.warn( 'THREE.DepthTexture: DepthFormat requires UnsignedShortType or UnsignedIntType.' );

    }

} else {

    if ( type === undefined ) type = UnsignedInt248Type;

    if ( type !== UnsignedInt248Type ) {

        console.warn( 'THREE.DepthTexture: DepthStencilFormat requires UnsignedInt248Type.' );

    }

}

this.type = type;

Getting there but still feels complicated.

@mrdoob
Owner
mrdoob commented Sep 26, 2016 edited
if ( format === DepthFormat ) {

    if ( type !== UnsignedShortType && type !== UnsignedIntType ) {

        console.warn( 'THREE.DepthTexture: DepthFormat requires UnsignedShortType or UnsignedIntType.' );
        type = UnsignedShortType;

    }

} else {

    if ( type !== UnsignedInt248Type ) {

        console.warn( 'THREE.DepthTexture: DepthStencilFormat requires UnsignedInt248Type.' );
        type = UnsignedInt248Type;

    }

}

this.type = type;

Better...

@mrdoob
Owner
mrdoob commented Sep 26, 2016

Actually, that last approach will force to user to have to pass the type to the constructor, otherwise they'll get warnings... So the previous approach seems the cleanest so far.

if ( format === DepthFormat ) {

    if ( type === undefined ) type = UnsignedShortType;

    if ( type !== UnsignedShortType && type !== UnsignedIntType ) {

        console.warn( 'THREE.DepthTexture: DepthFormat requires UnsignedShortType or UnsignedIntType.' );

    }

} else {

    if ( type === undefined ) type = UnsignedInt248Type;

    if ( type !== UnsignedInt248Type ) {

        console.warn( 'THREE.DepthTexture: DepthStencilFormat requires UnsignedInt248Type.' );

    }

}

this.type = type;
@mrdoob
Owner
mrdoob commented Sep 26, 2016

Actually, that approach would force the user to have to pass a type to the constructor or they would have gotten a warning.

Lets just add this logic in this file:

if ( format === undefined ) format = DepthFormat;
if ( type === undefined && format === DepthFormat ) type = UnsignedShortType;
if ( type === undefined && format === DepthStencilFormat ) type = UnsignedInt248Type;

Texture.call( this, null, mapping, wrapS, wrapT, magFilter, minFilter, format, type, anisotropy );

We can then move the other checks to WebGLTextures. That way we can fix things even if the user changes the format and/or type after initialisation.

See #9566 for reference.

@takahirox
Contributor

Agreed!

@takahirox
Contributor

I've updated.

@takahirox
Contributor

Ah I realized that I should modify some lines. I'll do that later. Please don't merge until I'll have done.

@takahirox
Contributor

Done

@mrdoob
Owner
mrdoob commented Oct 14, 2016

Cool! Looks good to me.

@mattdesl looks good?

@takahirox
Contributor

ping @mattdesl

@takahirox
Contributor

\ping @mattdesl

@takahirox
Contributor

Let me send ping again @mattdesl

@mattdesl
Contributor

Sure! 👍

@mrdoob mrdoob merged commit 62923d6 into mrdoob:dev Nov 13, 2016
@mrdoob
Owner
mrdoob commented Nov 13, 2016

Thanks!

@takahirox takahirox deleted the takahirox:AddCheckTypeToDepthTexture branch Nov 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment