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

Add a feature ID getter for picking #10022

Merged
merged 4 commits into from Jan 20, 2022
Merged

Add a feature ID getter for picking #10022

merged 4 commits into from Jan 20, 2022

Conversation

ptrgags
Copy link
Contributor

@ptrgags ptrgags commented Jan 20, 2022

This PR adds two getters:

  • Cesium3DTileFeature.featureId (before this, it was only accessible through the private pickedFeature._batchId)
  • ModelFeature.featureId (formally the private pickedFeature._featureId)

@lilleyse two questions on this one:

  1. Right now I marked these as experimental in case we have better ideas of how to expose feature ID values to the public API. Do we want this experimental flag or is this a good addition to the public API?
  2. I also realized it would be easy to expose featuresLength as well to get the total count of features in the Cesium3DTileBatchTable/ModelFeatureTable. Do we want this too?

Updated photogrammetry classification sandcastle

@ptrgags ptrgags requested a review from lilleyse January 20, 2022 16:50
@cesium-concierge
Copy link

Thanks for the pull request @ptrgags!

  • ✔️ Signed CLA found.
  • CHANGES.md was not updated.
    • If this change updates the public API in any way, please add a bullet point to CHANGES.md.

Reviewers, don't forget to make sure that:

  • Cesium Viewer works.
  • Works in 2D/CV.

@lilleyse
Copy link
Contributor

  1. I also realized it would be easy to expose featuresLength as well to get the total count of features in the Cesium3DTileBatchTable/ModelFeatureTable. Do we want this too?

My only hestitation is that featuresLength would not be well defined for feature IDs without an associated property table.

(That is, once we actually get picking working in that case. CC #9884 (comment))

I'm good with merging this PR before we make a decision on featuresLength.

@lilleyse lilleyse merged commit 9a7088e into main Jan 20, 2022
@lilleyse lilleyse deleted the feature-id-getter branch January 20, 2022 19:44
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

Successfully merging this pull request may close these issues.

None yet

3 participants