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

Examples: Clean up #11886

Merged
merged 2 commits into from
Aug 4, 2017
Merged

Examples: Clean up #11886

merged 2 commits into from
Aug 4, 2017

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Aug 4, 2017

This PR cleans up misc_controls_deviceorientation.html and explains the following line of code in all relevant examples.

geometry.scale( - 1, 1, 1 );

I was frequently asked why this operation is performed so i've added a comment in the code.

@mrdoob mrdoob merged commit dcfc3a3 into mrdoob:dev Aug 4, 2017
@mrdoob
Copy link
Owner

mrdoob commented Aug 4, 2017

Thanks!

@WestLangley
Copy link
Collaborator

Sorry, I am not in favor of this at all...

three.js does not support scale( - 1, 1, 1 ); Such a pattern leaves the normals and winding order in an inconsistent state.

Consequently, I would not add further credence to that pattern by adding a comment justifying it.

The only reason it works in these examples, is because the material is MeshBasicMaterial and that material does not respond to lights.

I was frequently asked why this operation is performed so i've added a comment in the code.

The proper answer is to say "don't do it".

The way to handle this is to use this pattern:

var geometry = new THREE.SphereBufferGeometry( 500, 60, 40 );

var texture = new THREE.TextureLoader().load( 'textures/2294472375_24a3b8ef46_o.jpg' );
texture.wrapS = THREE.RepeatWrapping;
texture.offset.set( 0, 0 ); // optional
texture.repeat.set( - 1, 1 );

var material = new THREE.MeshBasicMaterial( {
	map: texture,
	side: THREE.BackSide
} );

I'm not sure how to handle video textures in this case, though. That example appears to be distorted anyway, so something is not right...

@mrdoob
Copy link
Owner

mrdoob commented Aug 4, 2017

👍

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Aug 5, 2017

@WestLangley Yes, i know the drawbacks of this approach. I'm actually familiar with your posts at stackoverflow 😉 e.g. https://stackoverflow.com/a/16840273

Although geometry.scale( - 1, 1, 1 ); is not a consistent solution, users quickly get the desired result if they want to develop something like a 3D image gallery: https://www.youtube.com/watch?v=jT2mR9WzJ7Y&feature=youtu.be&t=1160

But yeah, the mentioned code snippet is of course the cleaner approach.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Aug 5, 2017

BTW: @WestLangley What's the problem with video textures and your code snippet?

@WestLangley
Copy link
Collaborator

WestLangley commented Aug 5, 2017

Right. Users copy these example. That is why the quality of the examples is so important.

Regarding the video example. As-implemented, it is distorted.

screen shot 2017-08-05 at 9 05 17 am

When trying repeat.set( - 1, 1 ), the video rendered scrambled for me.

@WestLangley
Copy link
Collaborator

Video distortion fixed in #11994

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Aug 20, 2017

Cool! 👍

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.

3 participants