Fix noncomformant behavior by not requiring the source field to be present in textures.

[pcwalton]

It's perfectly legal for source to not be present. Typically, this will happen when an extension such as KHR_texture_basisu provides the actual source.

The glTF spec § 5.29 states unambiguously that source is an optional field 1. The validator backs this up (see the blank_texture.gltf test case, which validates). Furthermore, the KHR_texture_basisu extension provides the following example in "Using without a fallback" 2:

"textures": [
{
    "extensions": {
	"KHR_texture_basisu": {
	    "source": 0
	}
    }
}
],

This doesn't contain a source field. Parsing this today with the gltf crate causes a panic in serde.

I ran into this with the gltf-transform 3 tool, which follows the standard when compressing textures and therefore produces valid glTF files that the gltf crate panics when attempting to load.

[alteous]

Thanks for the PR, this one is troubling...

The crate doesn't support the KHR_texture_basisu extension explicitly, so in a sense it's correct to reject the file. If it did load as an Option type then the application would have to use the arbitrary extension API to avoid the 'undefined behaviour' case:

When undefined, an extension or other mechanism SHOULD supply an alternate texture source, otherwise behavior is undefined.

This is a similar situation to #400 where the crate fails because an extension changes the way the glTF file is interpreted. Forcing the application to check potentially many different extensions for the 'real' texture data source is undesirable at best.

Changing the type to Option at this point would also break semver and I've promised not to do that until the next major release—see #409 and #385.

I think the least intrusive way of dealing with this would be to reject any glTF files containing required extensions that the library does not support explicitly. This should prevent situations like this and #400 from occurring whilst leaving optional extensions available via. the arbitrary extension API.

Parsing this today with the gltf crate causes a panic in serde.

Nitpicking: it doesn't panic, it produces an error, but I understood what you meant.

cargo run --example gltf-display blank-texture.gltf
     Running `target/debug/examples/gltf-display blank-texture.gltf`
thread 'main' panicked at examples/display/main.rs:16:20:
runtime error: Deserialize(Error("missing field `source`", line: 5, column: 18))
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

[teddemunnik]

The crate doesn't support the KHR_texture_basisu extension explicitly, so in a sense it's correct to reject the file

There has been a PR open for a while implementing such support. (#392) and another one for webp support (#365).

This is a similar situation to #400 where the crate fails because an extension changes the way the glTF file is interpreted.

The extension KHR_texture_basisu doesn't change the base spec. texture.source is not a required field. It is recommended that a fallback mechanism is available when it's left out, but again that's not a hard requirement in the spec.

I completely understand and appreciate the concern about breaking semver though. Is there a development branch for the next semver breaking release that changes like this could be added to?

Perhaps it could be good to in the future make users of the library specify the different extensions they support on the gltf-json level, so that if they don't specify an extension files which mark it as required may fail to load. The higher level gltf API may already specify the extensions it supports such as texture formats and then hide this from the user.

[alteous]

Is there a development branch for the next semver breaking release that changes like this could be added to?

The closest branch at the moment would be https://github.com/alteous/gltf/tree/v2-with-extension-macro. I intend to make a PR for this soon.

#418 will report a validation error where required extensions are not handled by the wrapper crate. It makes the texture source optional too. This should work for #400 too.

[alteous]

I've amended #418 to use a sentinel index value rather than Option. This way should work for everyone: it won't break builds, it'll solve the required extension problem, and the new feature allows power users to handle the empty texture case.

[alteous]

This should be covered by #418 now. Please reopen otherwise. 😄