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 sets a default metalness of 0.5 #29664

Open
schwyzl opened this issue Oct 15, 2024 · 4 comments · May be fixed by #29692
Open

GLTFExporter sets a default metalness of 0.5 #29664

schwyzl opened this issue Oct 15, 2024 · 4 comments · May be fixed by #29692
Labels

Comments

@schwyzl
Copy link
Contributor

schwyzl commented Oct 15, 2024

Description

If an object material is not a MeshStandardMaterial the GLTFExporter sets a default metallicFactor of 0.5.

		if ( material.isMeshStandardMaterial ) {

			materialDef.pbrMetallicRoughness.metallicFactor = material.metalness;
			materialDef.pbrMetallicRoughness.roughnessFactor = material.roughness;

		} else {

			materialDef.pbrMetallicRoughness.metallicFactor = 0.5;
			materialDef.pbrMetallicRoughness.roughnessFactor = 0.5;

		}

materialDef.pbrMetallicRoughness.metallicFactor = 0.5;

This should probably not happen. There is no such thing as semi-metalness in the real world so Three.js should probably choose 1 or 0 in this case. Seeing as this is a generic object exporter I would assume not adding any metalness is the more desirable behaviour.

@Mugen87 Mugen87 added the Addons label Oct 15, 2024
@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 15, 2024

How about metallicFactor = 0; and roughnessFactor = 1;? That would correspond to the default values of MeshStandardMaterial.

@schwyzl
Copy link
Contributor Author

schwyzl commented Oct 15, 2024

Yep, sounds like a good idea to match the standard material defaults.

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 15, 2024

Would you like to file a PR with the suggested change?

It seems these values exist since the beginning of the exporter and they were never discussed (see #11917).

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 18, 2024

In the meanwhile, I've filed a PR with the suggested defaults.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants