KhronosGroup/glTF-Validator

Consider warning on redundant definitions

donmccurdy opened this issue · 4 comments

Recently we had a couple user-facing bugs in three.js (mrdoob/three.js#21559, mrdoob/three.js#21819) which came down to redundant definitions in a glTF file. Both bugs led to duplicate instances of the THREE.Texture class and redundant GPU texture allocation:

This output came from Blender:

"textures": [
  {
    "sampler": 0,
    "source": 0
  },
  {
    "sampler": 0,
    "source": 0
  }
],
"images": [
  {
    "mimeType": "image/jpeg",
    "name": "REDgrad",
    "uri": "REDgrad.jpg"
  }
],

And this output came from the Babylon.js 3DS Max Exporter:

"texCoord": 0,
"extensions": {
  "KHR_texture_transform": {
    "texCoord": 0
  }
}

Both pass the validator, and aren't "wrong", but — especially in the first example — it's difficult for our loader to do deep comparisons and figure out how many things to allocate. Equivalently, the textures could have (but didn't) referred to different but identical samplers, and we'll (still) end up allocating extra texture memory in that case.

The second case, with the texture transform, partially our fault because of limitations in our KHR_texture_transform support, but basically the extension isn't doing anything here and would ideally not be included.

tl;dr – it'd be helpful for the ecosystem if the validator could flag redundant definitions, like identical samplers and textures.

it's difficult for our loader to do deep comparisons and figure out how many things to allocate

This is interesting. Is the three.js loader deduping identical textures? I would not expect loaders to do this.

it'd be helpful for the ecosystem if the validator could flag redundant definitions, like identical samplers and textures.

+1

How should extras and extensions affect redundancy detection?

Is the three.js loader deduping identical textures?

Currently not, but we may start de-duping textures entries with the same source and sampler indices to work around this issue. Duplicate images entries would not be deduped. We do some caching at the network layer but that isn't enough prevent duplicate GPU texture allocation.

How should extras and extensions affect redundancy detection?

Ideally some type of "deep equality", but even skipping redundancy detection on anything with extensions or extras would probably catch a lot.