-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Support voxel tilesets containing glTF tiles using the EXT_primitive_voxels
extension
#12432
base: main
Are you sure you want to change the base?
Conversation
Thank you for the pull request, @jjhembd! ✅ We can confirm we have a CLA on file for you. |
// names, types, etc. from the provider. | ||
// Find the appropriate glTF attribute based on its name. | ||
// Note: glTF custom attribute names are prefixed with "_" | ||
const name = `_${names[i]}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this change to retrieve the property names from the glTF's property attribute? The attribute for a temperature
property might be named _TEMPERATURE
, which will mess with the find
function below.
@jjhembd do you mind also accounting for CesiumGS/3d-tiles#780? (still working on test data, but I'll get it to you ASAP) |
@@ -350,18 +350,20 @@ ModelUtility.supportedExtensions = { | |||
EXT_mesh_features: true, | |||
EXT_mesh_gpu_instancing: true, | |||
EXT_meshopt_compression: true, | |||
EXT_primitive_voxels: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops forgot to mention, could we add EXT_implicit_cylinder_region
and EXT_implicit_ellipsoid_region
to this list? They're not required for our implementation but they'll be included in generated cylinder / ellipsoid voxels
https://github.com/CesiumGS/glTF/tree/ext-primitive-voxels-revisions/extensions/2.0/Vendor/EXT_implicit_cylinder_region
https://github.com/CesiumGS/glTF/tree/ext-primitive-voxels-revisions/extensions/2.0/Vendor/EXT_implicit_ellipsoid_region
Description
This PR updates
VoxelPrimitive
and related classes to support tilesets using the proposed update toEXT_primitive_voxels
.Along the way, the internal APIs were cleaned up and made more consistent with
Cesium3DTileset
, to prepare for work on #12297.Key code changes include:
VoxelProvider
interface always returns aVoxelContent
, rather than an array of metadata. This affects not justCesium3DTilesVoxelProvider
, but procedural data as well--see the Sandcastles for an example.VoxelContent
can now be constructed before the data is ready, via the asynchronousVoxelContent.fromGltf
method, and updated updated viaVoxelContent.prototype.update
. This makes it more similar toCesium3DTileContent
. For synchronous construction (e.g. with procedural data), useVoxelContent.fromMetadataArray
.VoxelTraversal
was refactored to use smaller functions that are structured more likeCesium3DTilesetTraversal
. For example,loadAndUnload
was broken intoselectKeyframeNodes
(similar toCesium3DTilesetTraversal.selectTiles
) andupdateTiles
. TheupdateTiles
loop is where we callVoxelContent.update
to process glTFs.Test datasets were also updated to the new format. Note that the tilesets in
Apps/SampleData/Cesium3DTiles/Voxel/
are a copy ofSpecs/Data/Cesium3DTiles/Voxel
.Issue number and link
Resolves #12368.
Related: #12297
Testing plan
Load the three Voxel-related Sandcastles and verify all datasets load and functionalities are unchanged.
Author checklist
CONTRIBUTORS.md
CHANGES.md
with a short summary of my change