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

Godot fails to import glTF2 file with NaN values #21668

Closed
TheOnlyJoey opened this issue Sep 1, 2018 · 15 comments
Closed

Godot fails to import glTF2 file with NaN values #21668

TheOnlyJoey opened this issue Sep 1, 2018 · 15 comments

Comments

@TheOnlyJoey
Copy link

Godot version: 3.0.6

OS/device including version: Ubuntu 18.04 Intel UHD 620 Mesa 18.0.5

Issue description: Godot fails to import this asset exported with glb from Maya. It does import in Blender correctly using the gltf2 importer.

Steps to reproduce: Add to project, try to open file

Minimal reproduction project: DomeAssetTest.zip

@reduz
Copy link
Member

reduz commented Sep 1, 2018

Support for glb exists and was tested at some point, wonder why It is not working

@capnm
Copy link
Contributor

capnm commented Sep 1, 2018

Your project contains only corrupt gltf + bin files exported from Blender. You can check / validate your glTF files online here: https://gltf-viewer.donmccurdy.com/

@TheOnlyJoey
Copy link
Author

@capnm Incorrect, the file is exported from Maya, and can be imported in both Maya and Blender (each with its own importer) without any issues.

@capnm
Copy link
Contributor

capnm commented Sep 1, 2018

Please, as I said, test your files with https://gltf-viewer.donmccurdy.com/

DomeAssetTest/alphatree$ ls
 Beech_Leaf1_Bunch_Platanus_2.png          Beech_Leaf2_BranchBeech2.png.import  'Test Tree.gltf.import'
 Beech_Leaf1_Bunch_Platanus_2.png.import  'Test Tree.bin'                        Twisted_Oak_Bark_Bark2.png
 Beech_Leaf2_BranchBeech2.png             'Test Tree.gltf'                       Twisted_Oak_Bark_Bark2.png.import

godot -e
SCRIPT ERROR: : Expected 'true','false' or 'null', got 'NaN'.
   At: res://alphatree/Test Tree.gltf:83.

Test Tree.gltf :

   "asset" : {
        "generator" : "Khronos Blender glTF 2.0 exporter",
        "version" : "2.0"
    },

…

       {
            "bufferView" : 5,
            "componentType" : 5126,
            "count" : 4500,
            "max" : [
                NaN,
                NaN
            ],
            "min" : [
                NaN,
                NaN
            ],
            "type" : "VEC2"
        },

…

@TheOnlyJoey
Copy link
Author

Why would i test something on a three.js importer? to be honest i trust the one from Blender more (both the official Khronos one and the community one, which both import the file correctly).

@capnm
Copy link
Contributor

capnm commented Sep 1, 2018

Because I has a built-in validator. Sorry, whatever you opened was not that from your zip file.

@TheOnlyJoey
Copy link
Author

TheOnlyJoey commented Sep 1, 2018 via email

@TheOnlyJoey
Copy link
Author

Confirmed with GLTF2 developer and specification (https://github.com/KhronosGroup/glTF/tree/master/specification/2.0) NaN is a valid value for non floating point variables, Other importers (Blender and Maya as mentioned before) check on this case and either add a 0 or a null as a replacement variable.

I have been trying to find where i can catch this case in the importer, but i have a bit of trouble finding where its caused while loading the file.
My idea is to create a 'assign' function that checks for this case when assigning something from the Json to the internal structures.

@akien-mga akien-mga changed the title Godot fails to import gltf2 file with glb Godot fails to import glTF2 file with NaN values Sep 2, 2018
@capnm
Copy link
Contributor

capnm commented Sep 2, 2018

NaN is a valid value for non floating point variables

If that's true, it sounds really crazy to me. In the spec is actually stated:
» Accessors Bounds
accessor.min and accessor.max properties are arrays that contain per-component minimum and maximum values, respectively. Exporters and loaders must treat these values as having the same data type as accessor's componentType, i.e., use integers (JSON number without fractional part) for integer types and use floating-point decimals for 5126 (FLOAT). «

and the JSON spec https://json.org/ is very clear about numbers:
image

@reduz
Copy link
Member

reduz commented Sep 2, 2018

Been investigating and nan/inf/etc. are clearly not valid per JSON specification. We should probably raise this to the GLTF spec.

@reduz
Copy link
Member

reduz commented Sep 2, 2018

Additionally, even in the GLTF spec it clearly states for binary data:

Values of NaN, +Infinity, and -Infinity are not allowed.

@reduz
Copy link
Member

reduz commented Sep 2, 2018

I dont mind adding support for them in our JSON parser, but I am not sure if this would be a mistake

@capnm
Copy link
Contributor

capnm commented Sep 2, 2018

Personally, I think it would be a mistake ;-) it could only give a clearer error message.
Otherwise, you have to put workarounds in every correct JSON parser in the world:

package main

import (
	"encoding/json"
	"fmt"
)

func main() {
	var glft interface{}
	data := []byte(`{
            "bufferView" : 5,
            "componentType" : 5126,
            "count" : 4500,
            "max" : [
                NaN,
                NaN
            ],
            "min" : [
                NaN,
                NaN
            ],
            "type" : "VEC2"
        }`)
	if err := json.Unmarshal(data, &glft); err != nil {
		fmt.Println(err)
	}
}

go run main.go
invalid character 'N' looking for beginning of value

@emackey
Copy link

emackey commented Oct 18, 2018

The Khronos Blender import/export has been corrected to disallow NaN as per glTF spec and JSON spec. If Maya export still has that problem, a separate issue should be filed on that project. I think this issue can be closed, as Godot is not affected.

@akien-mga
Copy link
Member

Thanks, closing.

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

5 participants