Skip to content
This repository has been archived by the owner on Dec 22, 2023. It is now read-only.

Some 2.0 sample models report their version as 1.1 #53

Closed
pjcozzi opened this issue May 2, 2017 · 13 comments
Closed

Some 2.0 sample models report their version as 1.1 #53

pjcozzi opened this issue May 2, 2017 · 13 comments
Labels

Comments

@pjcozzi
Copy link
Member

pjcozzi commented May 2, 2017

For example: https://github.com/KhronosGroup/glTF-Sample-Models/blob/master/2.0/Triangle/glTF/Triangle.gltf#L65

CC @javagl

@pjcozzi pjcozzi added the bug label May 2, 2017
@peteroupc
Copy link

peteroupc commented May 2, 2017

On a similar note, some glTF 1.0 models report a version 1.0.1, for example, the Avocado model, where as far as I know, a third number in the version string is not specified (as opposed to major and minor version).

@cx20
Copy link
Contributor

cx20 commented May 2, 2017

Several version problems seem to be solved in the following PR.
#45

Model Name Current PR
Triangle Without Indices 1.1 2.0
Triangle 1.1 2.0
Animated Triangle 1.1 2.0
Simple Material 1.1 2.0
Simple Meshes 1.1 2.0
Advanced Material 1.1 1.1
Simple Opacity 1.1 1.1
Simple Texture 1.1 2.0.0
Cameras 1.1 2.0
Simple Skin 1.1 1.1

I think that 2.0.0 of Simple Texture is incorrect.

@javagl
Copy link
Contributor

javagl commented May 3, 2017

@pjcozzi As cx20 mentioned, the #45 update fixes most of them. The ones that still carry a 1.1 or 2.0.0 after this pull request are still to be updated.

(Maybe I have to swallow my pride of being able to render these test models with my own implementation, and just update them according to the spec. Right now, I cannot foresee how long it will take until I have my own PBR implementation working. It's challenging, for sure.)

@pjcozzi
Copy link
Member Author

pjcozzi commented May 3, 2017

@javagl there are drag-and-drop glTF 2.0 loaders for three.js and BabylonJS that you could test updated models with. If you are up for it, I think it would help the ecosystem to have the sample models updated even if all our renderers aren't updated yet.

@javagl
Copy link
Contributor

javagl commented May 3, 2017

The point is: When a model does not work as expected, I don't know who to blame: The model or the renderer. But with two renderers, we have a committee ;-) so I'll tackle this ASAP

@emackey
Copy link
Member

emackey commented May 3, 2017

there are drag-and-drop glTF 2.0 loaders for three.js and BabylonJS

Does someone have the links to these two drag-and-drop loaders handy?

@bghgary
Copy link
Contributor

bghgary commented May 3, 2017

https://gltf-viewer.donmccurdy.com/
https://sandbox.babylonjs.com/

@cx20
Copy link
Contributor

cx20 commented May 20, 2017

@pjcozzi The latest PR #45 is as follows.

Model Name Current PR
Triangle Without Indices 1.1 2.0
Triangle 1.1 2.0
Animated Triangle 1.1 2.0
Simple Material 1.1 removed
Simple Meshes 1.1 2.0
Advanced Material 1.1 removed
Simple Opacity 1.1 removed
Simple Texture 1.1 removed
Cameras 1.1 2.0
Simple Skin 1.1 removed

Since there is confusion if there is a model of glTF 1.1 in the 2.0 hierarchy, I think that it is good to merge PR once.

CC @javagl

@pjcozzi
Copy link
Member Author

pjcozzi commented May 20, 2017

Thanks @cx20! Is it OK to close this now?

@peteroupc
Copy link

There's still one thing I want to clear up: " some glTF 1.0 models report a version 1.0.1, for example, the Avocado model, where as far as I know, a third number in the version string is not specified (as opposed to major and minor version)."

@cx20
Copy link
Contributor

cx20 commented May 21, 2017

@pjcozzi I think that Issue can be closed. All versions of the glTF 2.0 model are "2.0" now.
@peteroupc I think that it is better to make another Issue that some of the models of glTF 1.0 are "1.0.1".

@pjcozzi
Copy link
Member Author

pjcozzi commented May 22, 2017

@peteroupc those "1.0.1" models should report "1.0." I submitted #72. A fix would be welcome if you are able to contribute.

@pjcozzi pjcozzi closed this as completed May 22, 2017
@pjcozzi
Copy link
Member Author

pjcozzi commented May 22, 2017

Ah, I see #71.

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

No branches or pull requests

6 participants