Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ValidFeatureIdAttributeWithLargerFeatureCount.gltf not valid #294

Closed
bertt opened this issue Jan 15, 2024 · 8 comments
Closed

ValidFeatureIdAttributeWithLargerFeatureCount.gltf not valid #294

bertt opened this issue Jan 15, 2024 · 8 comments

Comments

@bertt
Copy link

bertt commented Jan 15, 2024

I think the file ValidFeatureIdAttributeWithLargerFeatureCount.gltf (https://github.com/CesiumGS/3d-tiles-validator/blob/main/specs/data/gltfExtensions/meshFeatures/ValidFeatureIdAttributeWithLargerFeatureCount.gltf) is not valid, because the featureCount (123) does not correspond to the number of features in the attribute (4)

@javagl
Copy link
Contributor

javagl commented Jan 15, 2024

This may indeed look confusing at the first glance, referring to the specification that says

The definition also includes a featureCount value, which is the number of unique features that are identified.

But such a file is valid, because not all "IDs for 'identified features'" necessarily have to appear in a specific attribute or texture. For example, you might have feature IDs like

  • 12 = Water
  • 23 = Grass
  • 34 = Sand
  • 45 = Concrete

(meaning that the featureCount would be 4). But you then might have an accessor that models the geometry of a beach, and there, only 'Water' and 'Sand' will appear.

(I also got it wrong initially, and did a === check. But this was explicitly fixed in bbe0130#diff-077b29d7c199f4aa7053f1f1ceba31584d101cb6a0c78cc8c21ad9ec1fbbd4aaL532 , following the comment from #280 (comment) )

@bertt
Copy link
Author

bertt commented Jan 15, 2024

ah so in this case the file is valid (there is no nullFeatureId) - because this rules applies "when there is no nullFeatureId, then the number of values (4) may not be larger than the featureIdCount (123)"

@javagl
Copy link
Contributor

javagl commented Jan 15, 2024

Yes. We should consider elaborating this a bit in the specification, i.e. emphasizing that

  • the featureCount is the number of identifiable features, regardless of whether all IDs are 'used' in one specific attribute/texture
  • the featureCount does not include the nullFeatureId

For example:

  • There could be IDs like [0, 1, 2] in an attribute, but the featureCount could still be 10 (meaning that not all IDs are used)
  • There could be IDs like [0, 1, 2, 99] in an attribute, even when the featureCount is only 3 - namely when 99 is the nullFeatureId

@bertt
Copy link
Author

bertt commented Jan 15, 2024

I’m also questioning the relation with primitives: suppose there are two meshPrimitives, one with 1 triangle with FeatureId = 0 , other meshprimitive has 2 triangles with featureid = 1 and featureid = 2

What should the FeatureCount be? 3 in both primitives I suppose now? Before, I thought 1 for the first primitive and 2 for the second (because FeatureCount is property of featureid of the meshprimitive)

If it’s 3, we could/should check that the FeatureCount is the same when there are multiple primitives (even with a NullFeatureId specified somewhere??).

@javagl
Copy link
Contributor

javagl commented Jan 16, 2024

I'll probably have to cross-check my assumptions with the specification and some internal discussion.
Until then, with the disclaimer that the following is only my current understanding:

The number of distinct feature ID values that appear in an attribute or texture is somewhat "independent" of the featureCount. The only constraint that that number may not be larger than the featureCount.

Referring to your example:

There are two mesh primitives. Both of them have feature ID definitions. These definitions each refer to one attribute of the respective mesh primitive. This attribute contains the actual numeric feature ID values.

  • For one mesh primitive, there are feature ID values [ 0 ]
  • For the other mesh primitive, there are feature ID values [ 1, 2 ]

These values are just "arbitrary" identifiers. The specification does not really impose any constraints or meaning here.

So when you access the feature ID definitions with pseudocode like this

const fromPrimitive0 = gltf.meshes[0].primitives[0].extensions["EXT_mesh_features"].featureIds[0];
const fromPrimitive1 = gltf.meshes[0].primitives[1].extensions["EXT_mesh_features"].featureIds[0];

then these are independent objects, and they do not need to have the same featureCount. The featureCount could be 10 for the first one, and 999 for the second one.

The featureCount only tells the consumer: "There may be up to featureCount different feature ID values in there".


If it’s 3, we could/should check that the FeatureCount is the same when there are multiple primitives

It would not be valid to assume this.

But... not everything that is "theoretically possible" will necessarily make sense in practice. For example, you could have a glTF asset where

  • primitive 0 of mesh 0 has 4 feature ID definitions
  • primitive 1 of mesh 0 has 2 feature ID definitions
  • primitive 0 of mesh 1 has 5 feature ID definitions
  • primitive 1 of mesh 1 has 7 feature ID definitions

and all of these feature ID definitions have a different featureCount. At this point, it would only be a bunch of numbers, and it could be hard to convey a "meaning".

In practice, one can probably say that...

  • Usually, each mesh primitive will have the same number of feature ID definitions
    • (and in many cases, it will only be one feature ID for each primitive)
  • Usually, the feature ID definitions of mesh primitives at the same index will have the same featureCount

The idea here would be to have something like (pseudocode):

meshes: [
  primitives: [ {
    // First primitive with two feature ID definitions:
    featureIds: [ {
      featureCount: 5,
     },  {
       featureCount: 10,
     }
  }, {
    // Second primitive with two feature ID definitions:
    featureIds: [ {
      featureCount: 5,
     },  {
       featureCount: 10,
     }
  }
}

This could, for example, be IDs for buildings:

  • The ID definitions with featureCount: 5 refer to a component of the building ("door", "window", "roof", ...).
  • The ID definitions with featureCount: 10 refer to materials ("stone", "glass", "plastic", ...).

There may be a glTF asset where none of the primitives really contains all of the 10 materials. But the one who created this glTF asset did set the featureCount: 10 because there might be a primitive that involves 10 materials.

@bertt
Copy link
Author

bertt commented Jan 16, 2024

ok, thanks for the explanation, but must say the definition featureCount = number of distinct features in the asset (without nullFeatureId) is much more simple/clear/exact to me...

@javagl
Copy link
Contributor

javagl commented Jan 16, 2024

Yeah... I cannot point my finger at "THE" exact reason for why the featureCount was introduced in this exact form (beyond the message in the change log). I'm about 80% sure that it is related to ~"the picking functionality in CesiumJS". Roughly: The Picking system of CesiumJS might need information about ~"how many different 'things' it has to expect there".

On one hand, this would be a somewhat weak justification: The quirks of the picking system of one client should not affect a generic specification on this level. On the other hand, it's not totally unreasonable to have that information. Otherwise, you'll load a glTF and obtain the feature ID values, and receive the values [12, 34, 56789], but don't know the "domain" of the feature ID values. Having the information: "These are 3 values, and in total there may be featureCount=10 values" does not harm (even though it does not really tell you explicitly which other IDs might be there, and most clients might not need that information...)

From that perspective, the main functionality of the extension is: "These are different things when they have different IDs". Everything beyond that has to be managed on another level. For example, the exact meaning of the IDs - i.e. "What is the 'feature' that is identified with the value 56789?"

This was actually an explicit request from the early user/customer feedback/review pass, and one of the reasons for why EXT_mesh_features was promoted into its own extension: People wanted to assign "some (arbitrary) value" to these elements, and leave the interpretation of this value to the application layer. This is mentioned in the last section of the specification, with this image:

feature-id-lookup


EDIT: I just opened CesiumGS/3d-tiles#756 to better keep track of this. (Not sure whether it will be considered to be "important", but maybe it can be tackled together with other, smaller issues in some cleanup pass at some point)

@javagl
Copy link
Contributor

javagl commented Feb 27, 2024

I hope it's OK to close this in favor of CesiumGS/3d-tiles#756

@javagl javagl closed this as completed Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants