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

Support for glTF 2.0 #10806

Closed
11 of 12 tasks
donmccurdy opened this issue Feb 14, 2017 · 28 comments
Closed
11 of 12 tasks

Support for glTF 2.0 #10806

donmccurdy opened this issue Feb 14, 2017 · 28 comments

Comments

@donmccurdy
Copy link
Collaborator

donmccurdy commented Feb 14, 2017

Core changes:

  • Create GLTF2Loader.
  • Add PBR materials.
    • Metallic-Roughness.
    • Specular-Glossiness.
  • Add morph targets.
  • Add image.bufferView so that textures/geometry/keyframes can be stored in one binary blob if desired.
  • Replace top-level glTF object properties that are currently accessed by property name, e.g., the accessors object, with arrays that are accessed by index.
  • Nodes allow only 1 node.mesh, not node.meshes array.

Extension changes:

  • KHR_binary_glTF extension is moved into core spec.
  • KHR_common_materials extension no longer includes lights.
  • KHR_lights extension.
  • KHR_technique_webgl for GLSL. GLSL is no longer part of core spec.

I'm probably missing some things, there is a more exhaustive list: Spec changes from 1.0 to 2.0. This will settle over the coming weeks.

/cc @pjcozzi @takahirox

@donmccurdy
Copy link
Collaborator Author

There may be some breaking changes here... @mrdoob @takahirox is it best to keep work on a branch until we're at least able to support features from v1.0 when loading files of v2.0 format?

@mrdoob
Copy link
Owner

mrdoob commented Feb 14, 2017

Considering that there aren't that many files out there yet... Would make sense to do a new GLTF2Loader that only supports 2.0 and keep the code clean?

@takahirox
Copy link
Collaborator

takahirox commented Feb 15, 2017

gltf.asset.version is a required property in glTF.

https://github.com/KhronosGroup/glTF/tree/master/specification/1.0#asset

Branching some logics depending on the version would be another option
if we can keep the code clean?

UPDATE:
gltf.asset is not a required property so there'd be files not including gltf.asset.version.
https://github.com/KhronosGroup/glTF/tree/master/specification/1.0#gltfasset

UPDATE2:
gltf.asset is a required property in 2.0.
https://github.com/KhronosGroup/glTF/tree/2.0/specification/2.0#gltfasset-white_check_mark
So if gltf.asset doesn't exist in .gltf file, we can detect it's 1.0.

@donmccurdy
Copy link
Collaborator Author

Well the good news is, GLTFLoader already supports most of the existing 2.0 sample models. The only exception I see is the glTF-MaterialsCommon versions, which will need changes, and I don't have any samples of those models yet.

Since a lot of the 2.0 changes involve logic going from extension A to extension B, or to/from the core spec, branching within one loader sounds a little unpleasant. If we split into GLTFLoader and GLTF2Loader, how long would we keep the first around? If we're OK with removing that after a few releases (say, r86?) then I'd vote to make the split. If we think we want to actively maintain both for a while, then let's try to keep them in one loader.

Or possible middle ground, we could drop support for 1.0 extensions (lights in common_materials, binary_gltf extension) in favor of their 2.0 counterparts, but try to keep compatibility with core 1.0 for now.

@mrdoob
Copy link
Owner

mrdoob commented Feb 15, 2017

I would vote for forking to GLTF2Loader, and encourage people to export to 2.0. Eventually removing the first version.

@donmccurdy
Copy link
Collaborator Author

donmccurdy commented Feb 15, 2017

Sounds reasonable to me.

@takahirox The key thing is probably converters. Once collada2gltf and obj2gltf provide glTF2.0, use of 1.0 will taper off, and that's when we can consider dropping the first version. It would be great to use gltf.asset.version to trigger a warning like For glTF <2.0, use THREE.GLTFLoader r85.

@takahirox
Copy link
Collaborator

OK, that sounds good to me.

Just what I concerned was if 1.0 keeps being used or not.
If converters will deprecate 1.0 support, we don't need to take care of 1.0.

Just in case, I'm asking @pjcozzi about their opinion.
https://twitter.com/superhoge/status/831719215727992834

@takahirox
Copy link
Collaborator

Anyways, let's start with making GLTF2Loader.
We can choose merging or replacing later.

BTW I received the answer from @pjcozzi .
https://twitter.com/pjcozzi/status/832394717119782912

we have no plans to deprecate 1.0, but we are encouraging exporters, converters, and runtimes to upgrade to 2.0.
Cesium's approach (still in progress) is to convert 1.0 to 2.0 on the fly using gltf-pipeline,
https://github.com/AnalyticalGraphicsInc/gltf-pipeline

@donmccurdy
Copy link
Collaborator Author

donmccurdy commented Feb 17, 2017

Yay! I will start a clean copy now unless you've started someplace. :)

EDIT: #10816

@donmccurdy
Copy link
Collaborator Author

@takahirox FYI I'm working on the .GLB changes now (and attempting to keep the checklist above up to date).

@takahirox
Copy link
Collaborator

FYI I'm working on Specular-Glossiness.
image

@takahirox
Copy link
Collaborator

Done Specular-Glossiness.
Maybe I'll work on KHR_technique_webgl extension next...

@cx20
Copy link
Contributor

cx20 commented Apr 29, 2017

I am testing glTF 2.0 sample models and Three.js glTF 2.0 Loader.
https://github.com/cx20/gltf-test/tree/2.0#gltf-20-pbr-models

However, it seems that some PBR models are not displayed properly.
https://cdn.rawgit.com/cx20/gltf-test/b6900f6f4d755a71691aaef5a19279da7472b69f/examples/threejs/index.html?category=tutorialModels&model=Corset&scale=1&type=glTF
image

I think that it was caused by updating to the latest specification about 10 days ago.
https://github.com/KhronosGroup/glTF-Sample-Models/tree/master/2.0/Corset

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 29, 2017

@cx20 Do you mind, if you open a new issue for this? 😊

@cx20
Copy link
Contributor

cx20 commented Apr 29, 2017

@Mugen87 Thank you for the advice. I opened a new issue: #11260

@takahirox
Copy link
Collaborator

glTF 2.0 has been released!
https://www.khronos.org/news/press/khronos-releases-gltf-2.0-specification

Time to replace GLTFLoader with GLTF2Loader?

@donmccurdy
Copy link
Collaborator Author

Time to replace GLTFLoader with GLTF2Loader?

+1 from me. The only change to GLTFLoader since r85 was the TextDecoder PR.

@densetsughem
Copy link

Hi. Would like to ask if what is the meaning of this error: THREE.Object3D.add: object not an instance of THREE.Object3D. undefined.
I got this error when I tried to load a .gltf file

@donmccurdy
Copy link
Collaborator Author

Hi @densetsughem — Could you open a new issue, and include: (1) the version of three.js you are using, and (2) the model itself? One of us will be able to help you then. :)

@mrdoob
Copy link
Owner

mrdoob commented Jun 7, 2017

What do you guys think of renaming GLTF2Loader.js to GLTFLoader.js and adding a THREE.GLTF2Loader = THREE.GLTFLoader; at the end of the file? Does the loader already show an error if the file is not 2+?

@donmccurdy
Copy link
Collaborator Author

What do you guys think of renaming GLTF2Loader.js to GLTFLoader.js

+1 from me.

Currently GLTF2Loader still attempts to load glTF1.0 files. Right now that mostly works, but when the KHR_materials_common and KHR_technique_webgl extensions are updated to glTF2.0 (those parts of the spec are not final yet) it will not be backward-compatible. So, throwing an error if the version is not 2+ would probably be best.

My only hesitation on timing is that the COLLADA2GLTF converter hasn't been updated to glTF2.0 yet (but its 2.0 branch should be fairly close). Meanwhile the glTF Blender exporter does support most glTF2.0 features.

@mlimper
Copy link

mlimper commented Jun 17, 2017

Hey folks,

thanks for the great work on glTF support for Three.JS!

I have a question about the texture orientation.
The glTF 2.0 spec says the following:
First image pixel (UV coordinates origin) corresponds to the upper left corner of the image. Implementation Note: OpenGL-based implementations must flip Y axis to achieve correct texture sampling.

However, I figured out that the Three.JS glTF 2.0 loader, at least the one used by this example, does not perform this flip. Instead, it assumes that the texture images are already flipped.

We are running into this issue with our exporter, and it seems others experienced it as well:
Kupoman/blendergltf#59

They already ended up flipping the UVs - but I would interpret the glTF 2.0 spec in such a way that the implementation of the renderer, so, in this case, Three.js, should flip the texture (or UVs) if it's necessary, not the exporter.

What do you think?

Thanks in advance.

@mlimper
Copy link

mlimper commented Jun 17, 2017

P.S.: Also asked the glTF folks for clarification as well:
KhronosGroup/glTF#1021

@donmccurdy
Copy link
Collaborator Author

donmccurdy commented Jun 18, 2017

A recent correction to the spec; and yes our loader and sample models are not updated yet.

the implementation of the renderer, so, in this case, Three.js, should flip the texture (or UVs) if it's necessary, not the exporter.

We can't detect at runtime whether it's necessary, I assume? So we'll add code in THREE.GLTF2Loader to always perform the flip, once we have updated sample models.

@mlimper
Copy link

mlimper commented Jun 18, 2017

So we'll add code in THREE.GLTF2Loader to always perform the flip, once we have updated sample models.

AFAIK, that's what most OpenGL-based applications out there do as well, pretty much standard, so this should cause less trouble with most models, as it's considered the default orientation of texture images for most (all?) exporters

@donmccurdy
Copy link
Collaborator Author

With the exception of the flipY on textures and the KHR_technique_webgl extension (which isn't ready for implementation yet), I think we've checked off all of the glTF2.0 tasks on the original post. I'll close this issue, and we can open new ones as bugs arise.

For future readers, note that the KHR_lights and KHR_common_materials are still in an experimental state (although they should match what is exported from glTF-Blender-Exporter).

Thank you to @takahirox for major portions of the implementation, as well as to @cx20 and everyone who has helped us find issues!

@qxx861305133
Copy link

Error: THREE.GLTFLoader: Unsupported asset. glTF versions >=2.0 are supported.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 3, 2023

@qxx861305133 You load a glTF asset with an unsupported version. Please ask at the forum for further guidance.

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

8 participants