- Notifications
You must be signed in to change notification settings - Fork 3.7k
Create reference implementation for BENTLEY_materials_point_style #13093
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
base: main
Are you sure you want to change the base?
Create reference implementation for BENTLEY_materials_point_style #13093
Conversation
| Thank you for the pull request, @markschlosseratbentley! ✅ We can confirm we have a CLA on file for you. |
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.
@ggetz The glTF test file had to be duplicated to be seen by both:
- Unit tests (
Specs/Data/Models/glTF-2.0/StyledPoints/points-r5-g8-b14-y10.gltf). - Sandcastle (
Apps/SampleData/models/StyledPoints/points-r5-g8-b14-y10.gltf).
Is there a better way to do this without duplication?
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.
Pull request overview
This PR implements support for the proposed BENTLEY_materials_point_style glTF extension, enabling point primitives in glTF files to specify and respect a diameter property in CesiumJS.
Key changes:
- Added glTF loader support to parse and validate the
BENTLEY_materials_point_styleextension - Implemented shader modifications to render points with specified diameters as circles
- Created a Sandcastle example demonstrating the new functionality
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/sandcastle/gallery/styled-gltf-points/sandcastle.yaml | Metadata for new Sandcastle example |
| packages/sandcastle/gallery/styled-gltf-points/main.js | Sandcastle example loading styled point glTF model |
| packages/sandcastle/gallery/styled-gltf-points/index.html | HTML wrapper for the Sandcastle example |
| packages/engine/Specs/Scene/GltfLoaderSpec.js | Unit tests for extension loading and validation |
| packages/engine/Source/Shaders/Model/ModelVS.glsl | Vertex shader support for point diameter |
| packages/engine/Source/Shaders/Model/ModelFS.glsl | Fragment shader to render points as circles |
| packages/engine/Source/Scene/ModelComponents.js | Added pointDiameter property to Material component |
| packages/engine/Source/Scene/Model/MaterialPipelineStage.js | Pipeline stage to configure point diameter uniform |
| packages/engine/Source/Scene/GltfLoader.js | Extension parsing and validation logic |
| Specs/Data/Models/glTF-2.0/StyledPoints/points-r5-g8-b14-y10.gltf | Test glTF file with styled points |
| CHANGES.md | Changelog entry for the new feature |
| Apps/SampleData/models/StyledPoints/points-r5-g8-b14-y10.gltf | Sample glTF file for Sandcastle example |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lilleyse left a comment
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.
Looks good. I left some comments in CesiumGS/glTF#91, but those may or may not affect this PR.
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.
This demo is pretty simple and could be confused with point cloud declarative styling.
Would it make sense to move this to the Development folder until we have a more interesting dataset to show?
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.
That make sense to me; thank you.
I believe I have just done this. What I did:
- I moved the sample to
packages/sandcastle/gallery/styled-gltf-points-dev/. - Inside
sandcastle.yaml, I addeddevelopment: trueand renamed the sample toStyled glTF Points - Dev. I also removed theShowcaseslabel and addedDevelopment.
Locally, I tested this by setting PROD=1 before building and hosting the gallery locally. I did not see this example (or any dev-tagged examples) showing up.
Feel free to resolve this conversation if this looks good to you!
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.
Ah, I'm not familiar with the new workflow. Maybe @ggetz can confirm.
Description
This PR adds support for the proposed
BENTLEY_materials_point_styleglTF extension. The proposed specification can be found here.This PR allows CesiumJS to process and apply the above extension when loading glTF files. This means point primitives will be able to have a
diameterproperty specified and respected in CesiumJS when loaded via glTF.To test these changes locally:
Styled glTF Pointsexample.Here is a screenshot of test file
points-r5-g8-b14-y10.gltfbeing rendered in CesiumJSThis change further enables CAD-style workflows in CesiumJS.
Issue number and link
Fixes #12891
Testing plan
This has been tested by:
Author checklist
CONTRIBUTORS.mdCHANGES.mdwith a short summary of my change