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

glTF 2.0 support #3

Closed
lexaknyazev opened this issue Nov 14, 2016 · 8 comments
Closed

glTF 2.0 support #3

lexaknyazev opened this issue Nov 14, 2016 · 8 comments
Labels

Comments

@lexaknyazev
Copy link
Member

Since glTF 1.1 is incompatible with 1.0, converter should support only newer version or provide a way to specify desired format.

@lexaknyazev
Copy link
Member Author

glTF 1.1 changes: KhronosGroup/glTF#605

@lasalvavida
Copy link
Contributor

lasalvavida commented Nov 18, 2016

@lexaknyazev these changes will be part of a larger scale refactoring based out of: https://github.com/lasalvavida/COLLADA2GLTF/tree/converter1.1

@lexaknyazev lexaknyazev changed the title glTF 1.1 support glTF 2.0 support Feb 3, 2017
@lexaknyazev lexaknyazev added the 2.0 label Feb 3, 2017
@lasalvavida
Copy link
Contributor

The 2.0 converter can now be found here: https://github.com/KhronosGroup/COLLADA2GLTF/tree/2.0

@pjcozzi
Copy link
Member

pjcozzi commented Apr 4, 2017

@lasalvavida please open a pull request when you are ready (with a task list of any any remaining work) and we'll ask the community to help test it. I know @lilleyse has been preparing test data.

@emackey
Copy link
Member

emackey commented May 2, 2017

Here are some things I've found with the current work-in-progress on the 2.0 branch:

  • emissiveTexture is not supported.
  • emissiveFactor defaults to all zeros in the draft spec, and will thus completely suppress any provided emissiveTexture by multiplying each texel by zero. The converter could work around this by setting emissiveFactor to (1,1,1) when any emissiveTexture is present.
  • When writing textures, the converter still outputs format, internalFormat, target, and type, which are all removed from 2.0. (Fixed in 63f4e29)
  • In one test model with two materials, it appeared the converter assigned the base color map from the first input material to both output materials, and maybe dropped the normal map from the second input material. I haven't dug further but I can produce a reduced COLLADA test file if one is needed for investigating this.
  • Multiple UV maps are not supported. This would be nice but I'm not sure how widely supported this will be. It looks like BabylonJS can handle two UV maps in its shaders.
  • Would be great to have a commandline flag to treat COLLADA's ambient texture as both occlusionTexture and metallicRoughnessTexture, using the same texture index for both. The 2.0 spec allows for reading occlusion from R and rough/metal from G/B of the same map.

@pjcozzi
Copy link
Member

pjcozzi commented May 3, 2017

Thanks for testing this out, @emackey! All good input.

Multiple UV maps are not supported. This would be nice but I'm not sure how widely supported this will be. It looks like BabylonJS can handle two UV maps in its shaders.

I would be fine with creating a new issue for this to help get this PR merged faster.

@lasalvavida
Copy link
Contributor

@emackey, I've addressed most of your feedback except for multiple UV maps. #37 has been opened to merge 2.0 into master.

@lasalvavida
Copy link
Contributor

Closed by #37.

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

No branches or pull requests

4 participants