CesiumGS/3d-tiles

Attribute names must have an underscore prefix

javagl opened this issue · 5 comments

The Vertex Attribute section says

Names of feature ID attribute semantics follow the naming convention FEATURE_ID_n where n must start with 0 and continue with consecutive positive integers: FEATURE_ID_0, FEATURE_ID_1, etc.

The glTF specification says

Application-specific attribute semantics MUST start with an underscore, e.g., _TEMPERATURE .

The following is a (trivial, complete, embedded, standalone) glTF asset with a quad that defines feature ID attributes

{
  "extensionsUsed" : [ "EXT_mesh_features" ],
  "accessors" : [ {
    "bufferView" : 0,
    "byteOffset" : 0,
    "componentType" : 5123,
    "count" : 6,
    "type" : "SCALAR",
    "max" : [ 3 ],
    "min" : [ 0 ]
  }, {
    "bufferView" : 1,
    "byteOffset" : 0,
    "componentType" : 5126,
    "count" : 4,
    "type" : "VEC3",
    "max" : [ 1.0, 1.0, 0.0 ],
    "min" : [ 0.0, 0.0, 0.0 ]
  }, {
    "bufferView" : 1,
    "byteOffset" : 48,
    "componentType" : 5126,
    "count" : 4,
    "type" : "VEC3",
    "max" : [ 0.0, 0.0, 1.0 ],
    "min" : [ 0.0, 0.0, 1.0 ]
  }, {
    "bufferView" : 1,
    "byteOffset" : 96,
    "componentType" : 5121,
    "count" : 4,
    "type" : "SCALAR",
    "max" : [ 45 ],
    "min" : [ 12 ]
  } ],
  "asset" : {
    "generator" : "JglTF from https://github.com/javagl/JglTF",
    "version" : "2.0"
  },
  "buffers" : [ {
    "uri" : "data:application/gltf-buffer;base64,AAABAAIAAQADAAIAAAAAAAAAAAAAAAAAAACAPwAAAAAAAAAAAAAAAAAAgD8AAAAAAACAPwAAgD8AAAAAAAAAAAAAAAAAAIA/AAAAAAAAAAAAAIA/AAAAAAAAAAAAAIA/AAAAAAAAAAAAAIA/DAAAAAAAAAAAAAAAFwAAAAAAAAAAAAAAIgAAAAAAAAAAAAAALQAAAAAAAAAAAAAA",
    "byteLength" : 156
  } ],
  "bufferViews" : [ {
    "buffer" : 0,
    "byteOffset" : 0,
    "byteLength" : 12,
    "target" : 34963
  }, {
    "buffer" : 0,
    "byteOffset" : 12,
    "byteLength" : 144,
    "byteStride" : 12,
    "target" : 34962
  } ],
  "materials" : [ {
    "pbrMetallicRoughness" : {
      "baseColorFactor" : [ 0.5, 1.0, 0.5, 1.0 ],
      "metallicFactor" : 0.0,
      "roughnessFactor" : 1.0
    },
    "alphaMode" : "OPAQUE",
    "doubleSided" : false
  } ],
  "meshes" : [ {
    "primitives" : [ {
      "extensions" : {
        "EXT_mesh_features" : {
          "featureIds" : [ {
            "attribute" : 0
          }, {
            "offset" : 5,
            "repeat" : 2
          } ]
        }
      },
      "attributes" : {
        "POSITION" : 1,
        "NORMAL" : 2,
        "FEATURE_ID_0" : 3
      },
      "indices" : 0,
      "material" : 0,
      "mode" : 4
    } ]
  } ],
  "nodes" : [ {
    "mesh" : 0
  } ],
  "scene" : 0,
  "scenes" : [ {
    "nodes" : [ 0 ]
  } ]
}

The validator reports one error:

                "code": "MESH_PRIMITIVE_INVALID_ATTRIBUTE",
                "message": "Invalid attribute name.",
                "severity": 0,
                "pointer": "/meshes/0/primitives/0/attributes/FEATURE_ID_0"

Adding the _ underscore prefix to the attribute name fixes this.

In my opinion a semantic formally defined by an extension should not be considered an application-specific semantic — if the validator gets support for KHR_mesh_features the warning above will disappear.

It's not a warning, but an error. I think a warning could be ignored for now, but an error might, in the worst case, cause viewers to just bail out saying "I'm not even gonna try and render an invalid asset" (for a good reason - who knows which crashes that might cause...)

Do you know, from the tip of your head, any extensions that already define attribute names without the underscore? (Otherwise, I might have a look at the existing extensions, and also on how the validator handles this - i.e. whether the validator actually allows for this case if the extension is known by the validator).

Beyond that, it may boil down to the exact meaning of "application-specific", but from a purely technical viewpoint, defining something that causes a validation error does not seem to be a good idea...

Historically we've used the phrase "application-specific" to separate the purpose of Extras from the purpose of Extensions – I think that also applies here. After all, why require userland attributes to have a _* prefix, if no new un-prefixed attributes can be added?

The two examples I can think of are EXT_mesh_gpu_instancing (which is somewhat different, since it has an instancing-specific attributes array) and the proposal in KhronosGroup/glTF#1235, which relies on _* vertex attributes, but allows the user to choose arbitrary names after the underscore.

Neither is the same as our case, so perhaps this is a question for Khronos.

After all, why require userland attributes to have a _* prefix, if no new un-prefixed attributes can be added?

The seemingly obvious (but admittedly, rather shallow) answer could be that (originally) there should have been the option to add attributes to the core spec, without the risk of name clashes with existing extension attributes.

More abstractly: The _ could be considered as a "namespace", meaning "this is not part of the core spec". But even more abstractly, one could raise the question of what happens when two extensions both define attributes with ambiguous names like ID or VALUE or so. In order to really prevent clashes there, a more elaborate concept of "namespaces" might be required.

I'm a bit on the fence here. On the one hand, I'd say "Let's just add that _, it does not harm, and makes our assets pass validation". On the other hand, I think that the question about namespacing and disambiguation of attribute names for extensions is important - so I just opened KhronosGroup/glTF#2111 asking about this in general, and the necessity for the undercore for extensions in particular.

We're now using underscore-prefixed attributes in the latest version of EXT_mesh_features: CesiumGS/glTF#51