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

GLTFExporter: Normal Texture handedness #14723

Open
2 of 12 tasks
jeyashankher opened this issue Aug 16, 2018 · 7 comments
Open
2 of 12 tasks

GLTFExporter: Normal Texture handedness #14723

jeyashankher opened this issue Aug 16, 2018 · 7 comments

Comments

@jeyashankher
Copy link

GLTF Loader: Fix for handedness in Normal Texture:
#11825

KhronosGroup/glTF#952

https://github.com/mrdoob/three.js/blob/master/examples/js/loaders/GLTFLoader.js#L2204
material.normalScale.y = - material.normalScale.y;

Given that the loader needs to adjust this, should the exporter be adjusting this as well?

Please do correct me if I'm wrong, but the current code does not seem to account for this.

		if ( material.normalMap ) {

			gltfMaterial.normalTexture = {

				index: processTexture( material.normalMap )

			};

			if ( material.normalScale.x !== - 1 ) {

				if ( material.normalScale.x !== material.normalScale.y ) {

					console.warn( 'THREE.GLTFExporter: Normal scale components are different, ignoring Y and exporting X.' );

				}

				gltfMaterial.normalTexture.scale = material.normalScale.x;

			}

		}
Three.js version
  • [x ] Dev
  • r95
  • ...
Browser
  • All of them
  • Chrome
  • Firefox
  • Internet Explorer
OS
  • All of them
  • Windows
  • macOS
  • Linux
  • Android
  • iOS
Hardware Requirements (graphics card, VR Device, ...)
@WestLangley
Copy link
Collaborator

Related: #13784 (comment) and the ensuing thread.

@jeyashankher
Copy link
Author

@WestLangley Thanks for the reference. Missed that.

Some investigation from my side if it helps:

I tried flipping UVs in processMesh() of the exporter and turning off the flipY condition in processImage(), and it looks like the output is fine in GLTFLoader.

There is still an issue with the UVs though. The UV Map seems to be translated now. I imported the gltf into blender using https://github.com/ksons/gltf-blender-importer

Original UV map of a cube.
screen shot 2018-08-17 at 10 01 28 am

UV Map with the current exporter (without UV flipping).
screen shot 2018-08-17 at 10 02 28 am

UV Map with UV flipping.
screen shot 2018-08-17 at 10 02 09 am

@mrdoob
Copy link
Owner

mrdoob commented Aug 18, 2018

/cc @fernandojsg @takahirox

@jeyashankher
Copy link
Author

Looks like the translation was due to a bug in the importer. It now looks ok.

Does look like flipping UVs in processMesh works ok.

@WestLangley
Copy link
Collaborator

/ping @donmccurdy

@donmccurdy
Copy link
Collaborator

Can you share an example where the current behavior is not setting up the normal map correctly, and your proposed fix? We cannot flip the UVs blindly, because we don't know the UV coordinate system. A few thoughts:

  1. We could take texture.flipY as an indication of UV coordinate system, but this will be more complex e.g. if there are multiple textures involved.
  2. The texture transform extension (KHR_texture_transform) is another option, but brings its own complexities.
  3. glTF accepts only a scalar normal scale, so the approach used by GLTFLoader cannot be extended to GLTFExporter.

@jeyashankher
Copy link
Author

@donmccurdy

Apologise for the delay in responding. The example I had where this wasn't working is a proprietary asset and I cannot share it. Trying to construct an example but am not being very successful (not a 3D modelling person).

In our project, we are interested in export only and the mesh is transient and not used further. So, it seemed ok to just modify the geometry.

For threejs exporter, I don't have an elegant suggestion.

If we have to flip UVs, the right place to do it is where we create the BufferView. One option is to special case the processBufferView code for UVs and write out a flipped UV. Similarly, processImage should not flip the texture.

If we choose to not flip UVs, the normals needs to be transformed to match gltf coordinate system, I guess. Not sure how to do that.

@donmccurdy donmccurdy changed the title GLTF Exporter: Normal Texture handedness GLTFExporter: Normal Texture handedness Nov 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants