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

jgltf-browser preview release 4 is not support glTF 1.1? #10

Closed
cx20 opened this issue Dec 30, 2016 · 6 comments
Closed

jgltf-browser preview release 4 is not support glTF 1.1? #10

cx20 opened this issue Dec 30, 2016 · 6 comments

Comments

@cx20
Copy link

cx20 commented Dec 30, 2016

It seems that it becomes an error when reading the following sample.
https://cdn.rawgit.com/javagl/gltfTutorialModels/master/SimpleTexture/glTF/SimpleTexture.gltf

Errors (1)
  Message: The value is null when looking up key [exampleTexture]
  Context: glTF -> scenes[scene0] -> nodes[node0] -> meshes[mesh0] -> meshPrimitives[0] -> materials[textureMaterial] -> techniques[textureTechnique] -> uniform u_diffuse -> technique.parameters[diffuseParameter] -> textures[[exampleTexture]]
Warnings (1)
  Message: The value of techniqueParameters diffuseParameter is ArrayList, but should be String
  Context: glTF -> scenes[scene0] -> nodes[node0] -> meshes[mesh0] -> meshPrimitives[0] -> materials[textureMaterial] -> techniques[textureTechnique] -> uniform u_diffuse -> technique.parameters[diffuseParameter]

I want latest gltf-browser jar...

@javagl
Copy link
Owner

javagl commented Dec 30, 2016

Yes, I also noticed this during my tests yesterday, and I fixed it in a728240

This problem was only in the validator. The viewer could actually display it, but the browser does a check like

if (isValid(gltf)) display(gltf);
else showErrorMessage();

(pseudocode). I have to think about how to tackle this in general. The validator is in a very early state, and was only intended to detect the errors that would cause a crash of the browser. It might require further updates for glTF 1.1, and in any case, it will have to be extended to perform a more systematic validation, like the official https://github.com/KhronosGroup/glTF-Validator

I will try to create a new release ASAP. I'll not be able to do this before Monday, but maybe I can start extending the validator before the weekend. Maybe even add an option to ignore errors from the validator, even if this means that the viewer might crash for a certain model. The error handling for glTF in general is something that is not completely settled yet (related: KhronosGroup/glTF#695 (comment) )

@cx20
Copy link
Author

cx20 commented Dec 30, 2016

I will try to create a new release ASAP.

I am not in a hurry, I would like to wait for a new release next year :)

When reading SimpleTexture.gltf with GLBoost, the display was inverted upside down.
https://cx20.github.io/gltf-test/examples/glboost/index.html?category=tutorialModels&model=SimpleTexture&scale=1&type=glTF

I tried to confirm with JglTF whether it was correct.

@javagl
Copy link
Owner

javagl commented Dec 30, 2016

If the texture seems to be flipped vertically, then this is the same as in JglTF. For the screenshot, I rotated the camera (around the x-axis) to show the other side of the object.

I cannot say for sure whether this is "correct" or not. But think that it is "correct".

The problem of vertically flipped textures is omnipresent in computer graphics (and it's really annoying). All the APIs and file formats have different specifications and conventions, and in some cases, additional ambiguities are introduced by the programming language or the image loader library. For WebGL, there is even a dedicated setting gl.pixelStorei(gl.UNPACK_FLIP_Y_WEBGL, true); to "fix" this. But IMHO, it is not really a "fix", but just another workaround that introduces another ambiguity...
The root cause of all this may be the fact that OpenGL image coordinates (0,0) are at the upper left, whereas for a right-handed coordinate system, the point (0,0) is usually at the "lower left".


Off-topic:

I already mentioned it elsewhere, but your loader test is awesome, and really useful (particularly for me: I'm not so familiar with JavaScript, and setting up all these libraries for a basic test would take me many hours...)

But I'm surprised that so many loaders seem to have problems with loading these "mini" test models. For example, the "SimpleMaterial" (embedded) one:

  • Three.js complains about "arraybuffer is undefined" in a line that contains return arraybuffer.slice...
  • Babylon.js complains about "currentScene.nodes" being undefined. This most likely has the same reason as in cesium:
  • Cesium.js complains about "t.scenes[t.scene] is undefined". In both cases, this is most likely caused by the fact that they expect gltf.scene to be present. But in fact, it is optional. I just brought this up at The glTF.scene should be optional CesiumGS/cesium#4793
  • xeogl does not display anything, and I can't interpret the error messages...
  • GLBoost works. 🥇
  • Grimoire.js also complains about an undefined variable, but due to the minimized code, I can only guess that this might be related to the glTF.scene field as well...
  • The minimal-gltf-loader complains about a.translation being undefined. This seems to be caused by the fact that the loader assumes that a node has a translation property if it does not have a matrix property (but I didn't analyze this in detail)

It seems like someone is going to open quite a few issue reports in the next days...

@javagl
Copy link
Owner

javagl commented Jan 13, 2017

I have created a new (still: preview) release at https://github.com/javagl/JglTF/releases/tag/v0.0.0-alpha05

@javagl javagl closed this as completed Jan 13, 2017
@cx20
Copy link
Author

cx20 commented Jan 13, 2017

Thank you very much. I will use it.

@javagl javagl mentioned this issue Jan 26, 2017
12 tasks
@javagl
Copy link
Owner

javagl commented Jan 28, 2017

@cx20 The point about the texture being displayed upside down that you mentioned in #10 (comment) has been "fixed". Actually, there was nothing wrong. But it was considered to be potentially confusing in CesiumGS/cesium#4808 (comment) :

Originally, the texture coordinates had been a "unit square"

0,1---1,1
 |     |
 |     |
0,0---1,0

And now ( javagl/gltfTutorialModels@3a2dbf4 ) they are a vertically flipped unit square:

0,0---1,0
 |     |
 |     |
0,1---1,1

I have also updated the texture image to reflect this, and emphasized this in the README at https://github.com/javagl/gltfTutorialModels/tree/master/SimpleTexture .

Maybe it will help to reduce the confusion. Of course, this is not a reliable analysis, but searching GitHub for issues involving texture AND (flipped OR "upside down") at least gives a rough idea about how many hundreds of thousands of work hours have been wasted with this issue already. It's a pity.


Note: This is just an information. The models here are still in the 1.1 state, and will be updated to 2.0 and added to the official Khronos repo when the dust around 2.0 has settled.

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

No branches or pull requests

2 participants