glTF 2.0: Please remove warning for empty node.

[McNopper]

First at all, thx a lot for this. The validator did help me to remove some errors and warnings. So back to this issue:

An empty node is valid in glTF 2.0. It is even mentioned on the specification site:
https://github.com/KhronosGroup/glTF/tree/master/specification/2.0#scenes

At first, I wanted to suggest to convert this output from a warning to an optimization but I think even empty nodes do make sense, e.g. having an empty node, where in the 3D game engine a dynamic object like a weapon can be placed.

So, as it is valid, it should not cause any warnings.

"warnings": {
            "NODE_EMPTY": [
                {
                    "type": "NODE_EMPTY",
                    "path": "#/nodes/3",
                    "message": "Empty node encountered."
                }
            ]
        },

[lexaknyazev]

Warning isn't an error in validation or conformance sense.
It just means that there's something in the asset that could be a source of other issues (e.g., a node is empty because a mesh wasn't attached to it by mistake).

Here's a list of all currently recognized warnings:

• Image has non power of two dimensions
• JSON object has unknown property
• xmag or ymag of orthographic camera are zeros
• Mesh has TANGENT attribute but no NORMAL
• Points-based mesh has TANGENT data
• Node explicitly defines default matrix transform
• Node is empty (current definition of non-empty node is: has children, camera, mesh, extras, or extensions defined or is referred from skin.joints)
• Buffer or Image URI is non-relative
• Number of vertices isn't aligned with used primitive mode (e.g., 5 vertices when mode is TRIANGLES)
• Asset contains unsupported extension
• GLB container contains unknown chunk

[McNopper]

Hmmm, okay I got it but e.g. if I have a warning in C/C++, I normally fix this.

With the glTF 2.0 validator, I would expect somehow a similar behaviour. But I can't fix this empty node, as I really want it.

The feature is good, but I would not name the output - for all of the above - as warning.

[zellski]

This threw me a bit, too. When I see a "warning" from something like a validator, I really want it to be an actionable item. While I agree it's really useful to call attention to potential problem areas, I think a label like "note" might be more appropriate?

[lexaknyazev]

@McNopper @zellski
Could you please go through ISSUES.md and point out all warnings that you find questionable?

I'm going to extend Severity enum to Error, Warning, Information, Hint (to match VSCode diagnostics levels).

/cc @emackey

[zellski]

I'll try to take a pass soon.

[emackey]

Just trying to take a best guess here, but, I think:

• Error - Means a direct violation of official schema, and/or means that a full-featured, conformant glTF viewer would be reasonably expected to fail to render all or part of this asset based on this finding.
• Warning - The validator found something that isn't a guarantee of failure, but isn't good, and perhaps the validator can't reasonably predict whether this asset will render correctly or not.
• Info - The validator found something that should be optimized or removed, but will not cause any harm to the rendered asset if left intact.
• Hint - Best practices, bad smells, etc. Option to turn these off completely, or maybe this validator doesn't need to produce any of these.

Of the list you have now, I think most severity levels are fine. Here are some potential changes to consider:

• NODE_EMPTY - The subject of this issue originally, this should almost certainly be info instead of warning.

• NODE_MATRIX_DEFAULT - This should probably be info, not warning.

• UNUSED_EXTENSION_REQUIRED - This is currently error, it could perhaps come down to warning.

• ACCESSOR_INDEX_TRIANGLE_DEGENERATE - Currently warning, I personally think this should be info. This is likely the artist, not the glTF exporter, doing this.

• IMAGE_NPOT_DIMENSIONS - Currently warning. Demote to info? The spec permits these explicitly.

[emackey]

Also, I hesitate to even suggest this, but I think there are some compilers/validators out there that allow individual types/codes to be configured to custom severities. For example, "Treat all degenerate triangles as errors!" or "Don't talk to me about empty nodes", etc.

[lexaknyazev]

Mostly agree, couple comments:

• UNUSED_EXTENSION_REQUIRED - direct violation of spec: Specifying Extensions
• IMAGE_NPOT_DIMENSIONS - while spec permits these, some hardware won't render them without rescaling. I'm OK with demoting it to Info after NPoT textures glTF-Sample-Viewer#28.

There were certain concerns about indecomposable matrices and related precision issues. I'll follow up with proposal.

Runtime severity reassignment shouldn't be that difficult but we'll have to come up with robust API (and cmd-line syntax) for such things.

[zellski]

The ability to explicitly disable specific warnings can become really interesting in the course of a real world hectic development project, where you have to make pragmatic concessions like “Okay we screwed up our asset pipeline and it’s too late to fix it and there’s just going to be empty nodes in the glTF.” — but you still want all *other* validation to proceed without lessened stringency. The command line switch for controlling such a thing can surely be unapologetically designed for power users. :)

…

On Wed, Oct 11, 2017 at 15:14 Alexey Knyazev ***@***.***> wrote: Mostly agree, couple comments: - UNUSED_EXTENSION_REQUIRED - direct violation of spec: Specifying Extensions <https://github.com/KhronosGroup/glTF/tree/master/specification/2.0#specifying-extensions> - IMAGE_NPOT_DIMENSIONS - while spec permits these, some hardware won't render them without rescaling. I'm OK with demoting it to Info after KhronosGroup/glTF-Sample-Viewer#28 <KhronosGroup/glTF-Sample-Viewer#28>. There were certain concerns about indecomposable matrices and related precision issues. I'll follow up with proposal. Runtime severity reassignment shouldn't be that difficult but we'll have to come up with robust API (and cmd-line syntax) for such things. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#14 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AArlty7okYb_y7xcsu_KlxS0ZSKdk4bLks5srT3MgaJpZM4OgPH-> .

[lexaknyazev]

@zellski @emackey
Initially, I was considering only "muting" some checks, but now I think that full control is more useful.
How about this (example with js interface):

// max of 100 messages, mute empty nodes
validator.validateString(name, json, loadExternalResourceCallback, 100, {"NODE_EMPTY": -1});

// unlim messages, treat npot dimensions as errors
validator.validateString(name, json, loadExternalResourceCallback, 0, {"IMAGE_NPOT_DIMENSIONS": 0});

[zellski]

I think that's great. Would this entirely replace support through command line switches? (I think that's fine; if you're interested in this level of control, encoding your requirements in a little JS script is probably preferable to a mammoth command-line invocation.)

[lexaknyazev]

Before putting this in production, we need to close #24 and agree on existing error codes #5, so people can rely on them. I'll think more about command-line interface for this.

[lexaknyazev]

@zellski

Would this entirely replace support through command line switches?

With #28 we have both options. Cmd-line app supports YAML config with ignored/overridden issues, while JS API accepts them as arguments.

[zellski]

Fantastic work @lexaknyazev, I already use this tool daily.