KhronosGroup / glTF

glTF – Runtime 3D Asset Delivery
Other
7.15k stars 1.14k forks source link

Clarify handling of meshes with undefined data #2169

Open lexaknyazev opened 2 years ago

lexaknyazev commented 2 years ago

There's no such requirement in the spec, so data remains undefined.

Okey, so, if I take one of these examples (provided the node and the mesh are instanced in a scene) and I try to load the model in a game engine or something.... which is the expected behavior? crash on load?, silently skip the mesh? I think this should be clarified

Originally posted by @vpenades in https://github.com/KhronosGroup/glTF-Validator/issues/189#issuecomment-1148566794

vpenades commented 2 years ago

I've wrote this one to see what happens on different viewers;

So I think the issue here is there's a conflict between what the schema says, and what can be realistically done by glTF readers.

scurest commented 2 years ago

Is it not clear?

5.1.1. accessor.bufferView

The index of the buffer view. When undefined, the accessor MUST be initialized with zeros; sparse property or extensions MAY override zeros with actual values.

vpenades commented 2 years ago

Well, my fault, I didn't dig deep enough into the docs to find that point, and from the tests I did with several engines, I'm not the only one

lexaknyazev commented 2 years ago

Also this

MUST be initialized with zeros

makes sense only for sparse accessors (as they need something to override by definition). In all other cases, it would be better to leave data undefined (for example, a zero-filled NORMAL or TANGENT accessor would be 100% invalid).

/cc @emackey @bghgary @javagl WDYT?

javagl commented 2 years ago

I think that the quoted statement of "When undefined, the accessor MUST be initialized with zeros" already answers the question, This would cause NORMAL and TANGENT to be classified as 'invalid', but... when they contain only zeros, they are invalid. It does not matter whether these zeros come from this automatic initialization, or from a buffer view that really 'physically' contains these zeros....

One could consider checking this case explicitly in the validator, just to avoid confusing messages. So instead of reporting "32131 normals of length zero", it should say "Accessor X is used for normals but does not have a buffer view, is therefore initialized with zeros, and thus, causes all these invalid normals"

Otherwise... what would 'leaving data undefined' mean, exactly, in terms of the expected behavior of implementations?

It would still have to be an accessor, with the specified number of elements and so on. It could not be an "invalid/undefined" accessor, and couldn't suddenly have a count of 0 or so. I'm really wondering what an implementation should do in a line like vec element = normalsAccessorWithoutBufferView[i].


For a moment, I thought that it might be possible to treat the problematic cases explicitly, and say:

When an accessor is used for NORMAL and does not have a bufferView, then its elements are assumed to be (0,0,1)

or so. But ... that doesn't make much sense: There are too many cases, no sensible 'default' values, and importantly: Something like this would not be consistent (i.e. compatible) with the current wording in the spec...

lexaknyazev commented 2 years ago

There's another statement in the spec that is not fully compatible with enforcing zero-filled accessor state

When neither sparse nor bufferView is defined, min and max properties MAY have any values. This is intended for use cases when binary data is supplied by external means (e.g., via extensions).

lexaknyazev commented 2 years ago

It would still have to be an accessor, with the specified number of elements and so on.

Yes, a mesh with no-data accessors would still define its vertex formats (enabling VAO creation) and its bounding box (enabling culling). An extension like Draco may provide the actual data later, not blocking regular asset's processing.

javagl commented 2 years ago

I don't see the conflict between zero-filling and the statement that min and max properties MAY have any values. It is giving a degree of freedom, and not an additional constraint. So when the accessor is zero-filled, then min/max may contain zeros (i.e. the real min/max values), or something else. Can you point out the actual contradiction here?


An aside, more generally referring to the quoted statement about min/max: I didn't have on the radar that min/max may contain 'any values' in these cases. Looking at the quoted statement itself, it can be hard for an implementation to determine whether min/max contain any useful data at all. I'm really thinking about the "pragmatic implementation level" here, in pseudocode:

vec3 min = positionsAccessor.min;
vec3 max = positionsAccessor.max;
if (viewFrustum.contains(min) && viewFrustum.contains(max)) {

    //objectIsVisible = true; // Whoops, no, that's not right!

    // It has to do this check, which may easily be overlooked:
    if (positionsAccessor.hasSparse || positionsAccessor.hasBufferView) {
        objectIsVisible = true;
    } else {
        // Dang, we don't know...
    }
}
vpenades commented 2 years ago

I think whether the min and max values are valid is irrelevant, because the underlaying issue remains:

Should an accessor with missing data be considered valid?

Right now I think that allowing an accessor to miss a BufferView, and not providing alternate data sources (like sparse or draco) is a loop hole that allows delivering "valid" gltf files that make most engines to fail on load.

So I think that if BufferView is missing, and no other data source is provided, the glTF should be considered invalid.

I understand this is a bit edgy and it makes writing tests on gltf validator more complex because it requires to add additional data to the tests... but this is an edge case my engine is covering by considering these cases as invalid gltfs... and I have to fill the unit tests with exceptions because gltfvalidator is not considering them as invalid.

lexaknyazev commented 2 years ago

Should an accessor with missing data be considered valid?

Yes, because extensions like Draco may provide its data by other means. Given that the Draco extension is defined on a mesh primitive rather than on the accessor, we have to allow no-data accessors in general because the validity of an accessor cannot depend on other objects that use it.

javagl commented 2 years ago

So I think that if BufferView is missing, and no other data source is provided, the glTF should be considered invalid.

Imagine an accessor that does not have a buffer view:

        {
            // "bufferView": 0, // NO!

            "byteOffset": 0,
            "componentType": 5123,
            "count": 12636,
            "type": "SCALAR",
...

Such an accessor could have an extension EXAMPLE_accessors_linear that declares that the value of the accessor at index i should be offset+i*stepSize:

...
            "extensions": {
                "EXAMPLE_accessors_linear": {
                    "offset": 10,
                    "stepSize": 2
                }
            }            

(These could be some sort of indices, or identifiers, or whatnot...)

The problem now is that the validator cannot say whether this accessor is 'valid' (unless it dedicatedly supports this particular extension). So there is no general way of determining whether "no other data source is provided", because the specification (intentionally!) leaves open what exactly this "other data source" could be.