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: Removed webgl_loader_gltf_extensions #22276

Merged
merged 1 commit into from
Aug 5, 2021
Merged

Examples: Removed webgl_loader_gltf_extensions #22276

merged 1 commit into from
Aug 5, 2021

Conversation

mrdoob
Copy link
Owner

@mrdoob mrdoob commented Aug 5, 2021

Description

I think, at this point, these models make gltf look bad 🙈

@mrdoob mrdoob added this to the r132 milestone Aug 5, 2021
@mrdoob mrdoob merged commit bb1244d into dev Aug 5, 2021
@mrdoob mrdoob deleted the gltf branch August 5, 2021 10:26
@Mugen87
Copy link
Collaborator

Mugen87 commented Aug 5, 2021

I think it's also better to have examples for showcasing just a single extension rather than webgl_loader_gltf_extensions.

@donmccurdy
Copy link
Collaborator

Plenty of other glTF examples without these, agreed! I think this example had become too long/complex to be useful anyway.

@WestLangley
Copy link
Collaborator

I also do not think it is a good idea to have use cases (i.e., extensions) hidden behind GUI menu items, because CI will not test it.

@takahirox
Copy link
Collaborator

takahirox commented Aug 5, 2021

Yeah, webgl_loader_gltf_extensions design was bad for CI test.

Now, some glTF extensions (specular-glossiness, unlit, and lights?) are not in our examples (let me know if I'm wrong). it would be good to somehow test them in test directory instead? (I don't think they are not looking so interesting, that we may not need to make showcasing examples for them.)

@mrdoob
Copy link
Owner Author

mrdoob commented Aug 5, 2021

I've heard they're trying to deprecate specular-glossiness.

@takahirox
Copy link
Collaborator

takahirox commented Aug 5, 2021

Yeah, glTF has specular extension now. So off topic, I'm personally thinking of moving specular-glossiness extension support to my https://github.com/takahirox/three-gltf-extensions repo.

@donmccurdy
Copy link
Collaborator

Now, some glTF extensions (specular-glossiness, unlit, and lights?) are not in our examples

I don't think it is necessary that examples exist for every glTF extension. Creating unit tests for them would be ideal.

The GLB used for https://threejs.org/examples/?q=unreal#webgl_postprocessing_unreal_bloom could probably be switched to unlit though (gltf-transform unlit in.glb out.glb)

I'm personally thinking of moving specular-glossiness extension support to my https://github.com/takahirox/three-gltf-extensions repo.

Let's start an issue about this, there may be a few options and considerations here.

@mrdoob
Copy link
Owner Author

mrdoob commented Aug 24, 2021

The GLB used for https://threejs.org/examples/?q=unreal#webgl_postprocessing_unreal_bloom could probably be switched to unlit though (gltf-transform unlit in.glb out.glb)

That sounds good to me 👍

@donmccurdy
Copy link
Collaborator

Hm I tested this out, but it turns out the PrimaryIonDrive.glb model has on emissive effect that we'd lose by switching to unlit, probably better to leave that as-is after all.

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.

5 participants