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

VideoTexture: Set format to RGBFormat by default. #15178

Merged
merged 2 commits into from
Nov 1, 2018
Merged

VideoTexture: Set format to RGBFormat by default. #15178

merged 2 commits into from
Nov 1, 2018

Conversation

mrdoob
Copy link
Owner

@mrdoob mrdoob commented Oct 31, 2018

Texture sets format to RGBAFormat if the user doesn't set it.
Would it make sense to set VideoTexture's format to RGBFormat by default instead?

@mrdoob mrdoob added this to the r99 milestone Oct 31, 2018
@mrdoob
Copy link
Owner Author

mrdoob commented Oct 31, 2018

/ping @WestLangley @Mugen87

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 31, 2018

I would say yes. All examples using VideoTexture right now set format to RGBFormat.

BTW: Would it also make sense to set minFilter to LinearFilter? Instead of this pattern:

var texture = new THREE.VideoTexture( video );
texture.minFilter = THREE.LinearFilter;
texture.format = THREE.RGBFormat;

it would be just

var texture = new THREE.VideoTexture( video );

@WestLangley
Copy link
Collaborator

@mrdoob yes.
@Mugen87 yes.

import { Texture } from './Texture.js';

function VideoTexture( video, mapping, wrapS, wrapT, magFilter, minFilter, format, type, anisotropy ) {

if ( format === undefined ) format = RGBFormat;

Copy link
Collaborator

Choose a reason for hiding this comment

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

Although we usually use this pattern:

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

this.format = format !== undefined ? format : RGBFormat;

this.minFilter = minFilter !== undefined ? minFilter : THREE.LinearFilter;
this.magFilter = magFilter !== undefined ? magFilter : THREE.LinearFilter;

this.generateMipmaps = false;

Copy link
Owner Author

Choose a reason for hiding this comment

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

Implemented. Thanks!

@mrdoob mrdoob merged commit 5989b78 into dev Nov 1, 2018
@mrdoob mrdoob deleted the videotexture branch November 1, 2018 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants