Store property attributes in FCesiumPrimitiveMetadata
#1694
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
j9liu
commented
Jun 11, 2025
| @@ -26,7 +26,7 @@ void FCesiumPropertyTablePropertySpec::Define() { | |||
| UCesiumPropertyTablePropertyBlueprintLibrary:: | |||
| GetPropertyTablePropertyStatus(property), | |||
| ECesiumPropertyTablePropertyStatus::ErrorInvalidProperty); | |||
| TestEqual<int64>( | |||
| TestEqual<int64_t>( | |||
There was a problem hiding this comment.
This diff is large, but I basically swept for int64 -> int64_t, since I remember int64 having a dubious definition on some platforms.
Additionally, I switched out static_cast<int64_t>() for int64_t() in accordance with the cesium-native style guide:
Use constructor notation for very safe integer conversions (where the range is checked) rather than
static_cast.
There was a problem hiding this comment.
Cross-link to the related bug we had awhile back:
#1468 (comment)
6 tasks
kring
reviewed
Jul 30, 2025
|
Thanks @kring for the review! This is ready for another look. |
|
Thanks @j9liu! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR is in support for #1685. This generalizes access of voxel property attributes regardless of accessor structure (because that's all handled by
AccessorViewunder the hood). It also enable would enable non-uint8/floatvariables to be passed to the data textures.Summary:
FCesiumPropertyAttributeandFCesiumPropertyAttributePropertyBlueprintLibraryfunctions to retrieve values from those propertiesFCesiumPropertyAttributetoFCesiumPrimitiveMetadata. See this comment for justification on this approach.FCesiumPrimitiveMetadataBlueprintLibrary::GetPropertyAttributeIndices. This wasn't doing anything helpful to begin with 😓GetInteger64forFCesiumPropertyTextureProperty. Although int64's aren't directly supported, this enablesuint32properties to be losslessly retrieved.Issue number or link
Partially addresses #1689, but not fully.
Author checklist
CHANGES.mdwith a short summary of my change (for user-facing changes).Testing plan
This is difficult because there's no good way to implement property attribute picking (how would you pick a single vertex from a model?), and I didn't want to increase the scope of this to property attribute styling. I hope the unit tests can inspire enough confidence :')