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

Model EXT_mesh_features: handle feature IDs without a property table #9884

Open
ptrgags opened this issue Oct 21, 2021 · 6 comments
Open

Model EXT_mesh_features: handle feature IDs without a property table #9884

ptrgags opened this issue Oct 21, 2021 · 6 comments

Comments

@ptrgags
Copy link
Contributor

ptrgags commented Oct 21, 2021

in #9872 I noticed that supporting feature IDs without property tables (a new detail in EXT_mesh_features is a little more involved than expected:

  • missing property tables should be created as empty tables
  • need to create a list of ModelFeatureTable or content feature tables across the whole model
  • Need to make sure the new generated indices are stored on the attributes

#9872 was getting large, and also I want to make sure I have recent changes from #9873 so just leaving a note for this now. #9880 is a higher priority right now.

@sanjeetsuhag
Copy link
Contributor

Currently, #9873 has functionality that enables B3dmLoader to do this for B3DMs with Batch IDs but no Batch Table. That should be replaced by this functionality in GltfLoader

@ptrgags
Copy link
Contributor Author

ptrgags commented Jan 11, 2022

One subtlety I'm noticing: for picking, the model has featureIdAttributeIndex -- does that correspond to FEATURE_ID_n or is that supposed to be the index within the featureIds array (which can also include implicit feature ID attributes and feature ID textures)? That's something that we should sort out

@ptrgags
Copy link
Contributor Author

ptrgags commented Jan 13, 2022

Some more specific notes about what needs to be updated (part of a larger set of notes I'll post in another issue)

parseFeatureMetadata

  • If extension.featureIds.length > extension.propertyTables.length, create empty property tables to make up the difference

GltfLoader

  • In loadPrimitiveMetadata, check if propertyTablesArray[i] exists before pushing.
  • Similarly for loadInstanceMetadata

@ptrgags
Copy link
Contributor Author

ptrgags commented Jan 13, 2022

opened #10007 for the larger details about handling multiple sets of feature IDs. This issue should be addressed first.

@ptrgags
Copy link
Contributor Author

ptrgags commented Jan 14, 2022

Today I hit a major snag: with the current picking system, it's unclear how to handle the batch texture since the total range of values is unknown (no propertyTable.count), potentially large and sparse (think unique ids into an external database, not necessarily contiguous).

Talking with @lilleyse, for right now we'll skip picking (though I do still plan to add the value to custom shaders). See also #9852 (comment)

@j9liu j9liu changed the title ModelExperimental EXT_mesh_features: handle feature IDs without a property table Model EXT_mesh_features: handle feature IDs without a property table Sep 1, 2022
@lilleyse
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants