load Maxar feature properties as structural metadata#1402
Conversation
j9liu
left a comment
j9liu
left a comment
There was a problem hiding this comment.
Thanks for opening this PR @timoore. Ultimately my concern is the correctness of the conversion, which is difficult to verify from reading alone. Unit tests would greatly help with that.
We should test some data in Unreal to ensure they work with metadata picking/styling.
| const CesiumGeospatial::Ellipsoid& ellipsoid, | ||
| const CesiumUtility::IntrusivePointer<CesiumGltf::Schema>& pSchema); |
There was a problem hiding this comment.
Perhaps pSchema should be optional (default nullptr) in case no schema is specified.
There was a problem hiding this comment.
I agree, but I'm not sure what the optional behavior should be. If there's not schema, should we create one based on the features that are present and still give access to the metadata?
| for (auto& [_, propRepVariant] : packedProps) { | ||
| if (auto* pPropRep = | ||
| std::get_if<StringPropertyRepresentation>(&propRepVariant)) { | ||
| pPropRep->offsets.back() = pPropRep->buffer.size(); | ||
| // Patch up any missing data | ||
| size_t lastValidOffset = 0; | ||
| for (size_t& offset : pPropRep->offsets) { | ||
| if (offset == std::numeric_limits<size_t>::max()) { | ||
| offset = lastValidOffset; | ||
| } else { | ||
| lastValidOffset = offset; | ||
| } | ||
| } | ||
| metadataSize += pPropRep->buffer.size(); | ||
| offsetsSize += pPropRep->offsets.size(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Since we're going through and compacting the buffer data anyway to get its true size, it would be great if the offset type was downcasted from size_t to a smaller unsigned int when possible.
There was a problem hiding this comment.
I agree, but I think it's barely in scope for the work I'm doing. I will look at doing that once the rest seems to working well.
Thank you 32 bit Emscripten.
Also includes a change to schema conversion from CesiumGS#1402: use 64 bit types.
This is consistent with how IntrusivePointer objects are passed to continuations, and for the moment resolves a mysterious problem in Cesium for Unreal.
The external tilesets in the Maxar GeoJSON scheme don't contain a full URL to the schema, so it needs to be parsed once and passed around.
3 participants
Load the feature data in MAXAR_content_geojson tiles as EXT_structural_metadata, along with the accompanying schema.
This depends on #1396, so either merge it first or withdraw it.
This is marked as draft awaiting some tests and verification that the schema is complete.