-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Officially document + prove support for KHR_mesh_primitive_restart #13634
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?
Changes from all commits
44e5be8
61be2bc
b8e0220
cac5b8f
283dc26
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| # PrimitiveRestart | ||
|
|
||
| Sample models demonstrating the [`KHR_mesh_primitive_restart`](https://github.com/KhronosGroup/glTF/pull/2569) glTF extension. Each model contains a single mesh primitive batching multiple topological primitives (line strips, line loops, triangle strips, or triangle fans) separated by inline restart index values. | ||
|
|
||
| ## License Information | ||
|
|
||
| Created by Don McCurdy as test data for the `KHR_mesh_primitive_restart` extension proposal ([KhronosGroup/glTF#2569](https://github.com/KhronosGroup/glTF/pull/2569)). | ||
|
|
||
| Licensed under [CC BY 4.0](https://creativecommons.org/licenses/by/4.0/). See also the `KHR_mesh_primitive_restart sample models` entry in [LICENSE.md](../../../../LICENSE.md). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| { | ||
| "asset": { | ||
| "version": "2.0" | ||
| }, | ||
| "extensionsUsed": [ | ||
| "KHR_mesh_primitive_restart", | ||
| "BENTLEY_materials_line_style" | ||
| ], | ||
| "extensionsRequired": [ | ||
| "KHR_mesh_primitive_restart" | ||
| ], | ||
| "accessors": [ | ||
| { | ||
| "name": "Positions", | ||
| "bufferView": 0, | ||
| "byteOffset": 0, | ||
| "componentType": 5126, | ||
| "count": 7, | ||
| "type": "VEC3", | ||
| "min": [0.0, 0.0, 0.0], | ||
| "max": [2.0, 5.0, 0.0] | ||
| }, | ||
| { | ||
| "name": "Cumulative distances", | ||
| "bufferView": 1, | ||
| "byteOffset": 0, | ||
| "componentType": 5126, | ||
| "count": 7, | ||
| "type": "SCALAR" | ||
| }, | ||
| { | ||
| "name": "Indices with inline restart values", | ||
| "bufferView": 2, | ||
| "byteOffset": 0, | ||
| "componentType": 5123, | ||
| "count": 9, | ||
| "type": "SCALAR" | ||
| } | ||
| ], | ||
| "bufferViews": [ | ||
| { | ||
| "buffer": 0, | ||
| "byteOffset": 0, | ||
| "byteLength": 84, | ||
| "byteStride": 12, | ||
| "target": 34962 | ||
| }, | ||
| { | ||
| "buffer": 0, | ||
| "byteOffset": 84, | ||
| "byteLength": 28, | ||
| "target": 34962 | ||
| }, | ||
| { | ||
| "buffer": 0, | ||
| "byteOffset": 112, | ||
| "byteLength": 18, | ||
| "target": 34963 | ||
| } | ||
| ], | ||
| "buffers": [ | ||
| { | ||
| "byteLength": 130, | ||
| "uri": "data:application/octet-stream;base64,AAAAAAAAAAAAAAAAAACAPwAAgD8AAAAAAAAAQAAAAAAAAAAAAAAAAAAAAEAAAAAAAACAPwAAQEAAAAAAAAAAAAAAgEAAAAAAAACAPwAAoEAAAAAAAAAAAPMEtT/zBDVAAAAAAPMEtT8AAAAA8wS1PwAAAQACAP//AwAEAP//BQAGAA==" | ||
| } | ||
| ], | ||
| "materials": [ | ||
| { | ||
| "pbrMetallicRoughness": { | ||
| "baseColorFactor": [1.0, 0.0, 0.0, 1.0] | ||
| }, | ||
| "extensions": { | ||
| "BENTLEY_materials_line_style": { | ||
| "width": 3, | ||
| "pattern": 61680 | ||
| } | ||
| } | ||
| } | ||
| ], | ||
| "meshes": [ | ||
| { | ||
| "primitives": [ | ||
| { | ||
| "attributes": { | ||
| "POSITION": 0, | ||
| "BENTLEY_materials_line_style:CUMULATIVE_DISTANCE": 1 | ||
| }, | ||
| "indices": 2, | ||
| "mode": 3, | ||
| "material": 0 | ||
| } | ||
| ] | ||
| } | ||
| ], | ||
| "nodes": [ | ||
| { | ||
| "mesh": 0 | ||
| } | ||
| ], | ||
| "scenes": [ | ||
| { | ||
| "nodes": [0] | ||
| } | ||
| ], | ||
| "scene": 0 | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| <style> | ||
| @import url(../templates/bucket.css); | ||
| </style> | ||
| <div id="cesiumContainer" class="fullSize"></div> | ||
| <div id="loadingOverlay"><h1>Loading...</h1></div> | ||
| <div id="toolbar"></div> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| import * as Cesium from "cesium"; | ||
| import Sandcastle from "Sandcastle"; | ||
|
|
||
| // KHR_mesh_primitive_restart allows many line strips, line loops, triangle | ||
| // strips, or triangle fans to be batched into a single glTF primitive. The | ||
| // primitive's indices accessor may contain inline "restart" values (the | ||
| // maximum value for the accessor's component type (0xFF / 0xFFFF / | ||
| // 0xFFFFFFFF) to begin a new primitive. | ||
| // | ||
| // Requires WebGL 2 (PRIMITIVE_RESTART_FIXED_INDEX). Use the context dropdown | ||
| // to force a WebGL 1 context, where results vary by browser/GPU backend: | ||
| // nothing, corrupted geometry, or even correct rendering. | ||
| // | ||
| // The sample models below are from the KHR_mesh_primitive_restart proposal: | ||
| // https://github.com/KhronosGroup/glTF/pull/2569 | ||
| // Each contains a single mesh primitive with multiple topologies separated | ||
| // by inline restart values. | ||
|
|
||
| let viewer; | ||
|
|
||
| function createViewer(requestWebgl1) { | ||
| if (Cesium.defined(viewer)) { | ||
| viewer.destroy(); | ||
| } | ||
| viewer = new Cesium.Viewer("cesiumContainer", { | ||
| contextOptions: { requestWebgl1: requestWebgl1 }, | ||
| }); | ||
| } | ||
|
|
||
| createViewer(false); | ||
|
|
||
| const models = { | ||
| "Hilbert Curve (LINE_STRIP, 32-bit indices)": { | ||
| url: "../../SampleData/models/PrimitiveRestart/PrimitiveRestartLineStrip.glb", | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I touched base with @donmccurdy on the inclusion of these test assets in this dev Cesium Sandcastle and he seemed to think it was fine to include them. However, we were not sure how Cesium-included assets are licensed in general. Anyone have any insights? Context: these test assets were created by @donmccurdy and were provided as test data for this Khronos spec PR: KhronosGroup/glTF#2569
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know the answer, and am not a lawyer, but would think that unless there is specific license information for a model, the "root" license (Apache 2) would apply. The main question might therefore be: Under which license would Don like to publish them? (The point is: I think that there could be some And now for something completely different: I don't see a difference between the WebGL 1 and 2 versions in terms of primitive restart. (There is a difference, in terms of (anti)aliasing, so I think that they are indeed different contexts, but the geometry looks the same for me...)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @javagl Interesting, thanks for the webgl1 + 2 observation. I tested on Firefox on macOS. What browser/OS are you using?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'm comfortable contributing these under any license conventional for non-code assets in CesiumJS. I might suggest CC BY if there's no prior convention. If there are no objections, I can just add that license to these files as XMP data (
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @markschlosseratbentley This was on Windows 11 with some AMD Radeon (Laptop) card (on Chrome, if that might matter). Of course, it's nothing that affects this PR. It might even be an AMD driver bug 🤪 and things unexpectedly working is a nice "problem" to have. @donmccurdy No strong preference (details are up to you anyhow) but... 1. for that license information to even be accessible, one needs a library/loader/viewer that supports the extension, and 2. one has to explicitly and dedicatedly look for it, exactly there (which isn't self-evident - I cannt remember having seen license infos embedded in XMP).
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Small addendum: I haven't looked at the details of the models, but for the other models, there is the expected difference (i.e. the rendering is "broken" on WebGL 1). Only the "hilbert curve" is working. (Are the other ones using non-32-bit indices by any chance? That might be a path to an explanation...)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe the Hilbert curve is the only UNSIGNED_INT model and others are 16-bit, which indeed could explain things. After some research, on ANGLE/D3D11 (working theory):
Based on this, WebGL 1 behavior is implementation-defined / undefined: macOS Firefox/Chrome render nothing, but Safari renders correctly (Metal probably behaves similar to D3D11 in this regard...). I will update this PR's docs to clarify webgl1 behavior is "implementation-defined / undefined."
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's quite some archeology there 🙂 Given that WebGL 1 support in CesiumJS is ... maybe not completely dropped, but quickly phasing out, and everything that is not running on Windows 3.1 and Mosaic supports WebGL 2 anyhow, it's just a matter of setting the expectations right for people with really old setups.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's also https://github.com/KhronosGroup/glTF-Sample-Assets/blob/main/LICENSES/CC-BY-4.0.txt
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @donmccurdy and @javagl. Based on this licensing discussion, I additionally followed what I found to be an existing repo convention: added a CC-BY 4.0 entry to the root I also placed a @donmccurdy I credited you in both places, so LMK if you have any issues with this. |
||
| heading: 0.0, | ||
| pitch: -45.0, | ||
| }, | ||
| "Line Strips": { | ||
| url: "../../SampleData/models/PrimitiveRestart/primitive-restart-line-strip.glb", | ||
| heading: -90.0, | ||
| pitch: 0.0, | ||
| }, | ||
| "Line Loops": { | ||
| url: "../../SampleData/models/PrimitiveRestart/primitive-restart-line-loop.glb", | ||
| heading: -90.0, | ||
| pitch: 0.0, | ||
| }, | ||
| "Triangle Strips": { | ||
| url: "../../SampleData/models/PrimitiveRestart/primitive-restart-triangle-strip.glb", | ||
| heading: -90.0, | ||
| pitch: 0.0, | ||
| }, | ||
| "Triangle Fans": { | ||
| url: "../../SampleData/models/PrimitiveRestart/primitive-restart-triangle-fan.glb", | ||
| heading: -90.0, | ||
| pitch: 0.0, | ||
| }, | ||
| }; | ||
|
|
||
| const origin = Cesium.Cartesian3.fromDegrees(-75.152408, 39.946975, 50.0); | ||
| const modelMatrix = Cesium.Transforms.headingPitchRollToFixedFrame( | ||
| origin, | ||
| new Cesium.HeadingPitchRoll(0.0, 0.0, 0.0), | ||
| ); | ||
|
|
||
| let model; | ||
| let currentEntry; | ||
|
|
||
| async function loadModel(entry) { | ||
| currentEntry = entry; | ||
| if (Cesium.defined(model)) { | ||
| viewer.scene.primitives.remove(model); | ||
| model = undefined; | ||
| } | ||
|
|
||
| try { | ||
| model = viewer.scene.primitives.add( | ||
| await Cesium.Model.fromGltfAsync({ | ||
| url: entry.url, | ||
| modelMatrix: modelMatrix, | ||
| }), | ||
| ); | ||
|
|
||
| model.readyEvent.addEventListener(() => { | ||
| const camera = viewer.camera; | ||
| const center = model.boundingSphere.center; | ||
| const r = 3.0 * Math.max(model.boundingSphere.radius, 1.0); | ||
| camera.lookAt( | ||
| center, | ||
| new Cesium.HeadingPitchRange( | ||
| Cesium.Math.toRadians(entry.heading), | ||
| Cesium.Math.toRadians(entry.pitch), | ||
| r, | ||
| ), | ||
| ); | ||
| camera.lookAtTransform(Cesium.Matrix4.IDENTITY); | ||
| }); | ||
| } catch (error) { | ||
| window.alert(`Error loading model: ${error}`); | ||
| } | ||
| } | ||
|
|
||
| Sandcastle.addToolbarMenu( | ||
| Object.entries(models).map(([text, entry]) => ({ | ||
| text: text, | ||
| onselect: () => loadModel(entry), | ||
| })), | ||
| ); | ||
|
|
||
| Sandcastle.addToolbarMenu([ | ||
| { | ||
| text: "WebGL 2 (restart supported)", | ||
| onselect: () => { | ||
| createViewer(false); | ||
| loadModel(currentEntry); | ||
| }, | ||
| }, | ||
| { | ||
| text: "WebGL 1 (restart unsupported; behavior varies)", | ||
| onselect: () => { | ||
| createViewer(true); | ||
| loadModel(currentEntry); | ||
| }, | ||
| }, | ||
| ]); | ||
|
|
||
| await loadModel(Object.values(models)[0]); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| title: KHR_mesh_primitive_restart - Dev | ||
| description: Load the KHR_mesh_primitive_restart sample models from the Khronos extension proposal (KhronosGroup/glTF#2569). Each model batches multiple line strips, line loops, triangle strips, or triangle fans into a single glTF primitive using inline restart index values. | ||
| labels: | ||
| - Development | ||
| development: true |

Uh oh!
There was an error while loading. Please reload this page.
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.
Since this PR does not include any functional changes to CesiumJS, I did not see a need to include an entry in
CHANGES.md, leaving it just to inline documentation here. However, I am open to adding aCHANGES.mdentry if someone thinks it's necessary and/or desirable. @ggetz?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.
I agree it isn't strictly necessary – but I'd be tempted to include it, as many CesiumJS users probably aren't aware of the extension otherwise and might benefit from knowing that it exists and CesiumJS supports it.
Something like:
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.
Thanks for the feedback; I think this makes sense. Added.