-
Notifications
You must be signed in to change notification settings - Fork 1.2k
BENTLEY_primitive_restart #2478
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?
Conversation
@@ -0,0 +1,152 @@ | |||
<!-- | |||
Copyright 2015-2021 The Khronos Group Inc. |
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.
Please update this line to say 2025 and the actual copyright holder.
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.
@pjcozzi any guidance on copyright and license here? The extension template includes this placeholder, but only one vendor extension actually includes it.
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.
@rudybear can you confirm if extensions should have a copyright comment? If so, is it copyright Khronos?
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.
It's a part of REUSE compliance. All files must have such comments (if some don't we should fix that).
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.
A vendor-prefixed extension should be owned by the vendor. Khronos usually owns extensions with KHR
and EXT
prefixes.
extensions/2.0/Vendor/BENTLEY_primitive_restart/schema/primitiveGroup.schema.json
Outdated
Show resolved
Hide resolved
...sions/2.0/Vendor/BENTLEY_primitive_restart/schema/mesh.BENTLEY_primitive_restart.schema.json
Show resolved
Hide resolved
extensions/2.0/Vendor/BENTLEY_primitive_restart/schema/primitiveGroup.schema.json
Outdated
Show resolved
Hide resolved
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.
Disclosure: I'm an independent contributor, but also a freelancer with a connection to Bentley. I do not speak on behalf of either side here. I also have not yet reviewed this extension proposal on a technical level (only causally watching the comments and discussions).
Only about the naming:
Whether the prefix is BENTEY
or EXT
is up do decide for the person who proposes the extension.
(Although some of the extension development process is about to be refined, the section at https://github.com/KhronosGroup/glTF/tree/main/extensions#naming will likely still be kept in a similar form. One addition here might be that the EXT
prefix suggests some broad, general applicability or that the extension is not bound to any form of ~"vendor-specific functionality" - but that's very vague...)
The naming section also says
Names SHOULD be structured as
<PREFIX>_<scope>_<feature>
, where scope is an existing glTF concept (e.g. mesh, texture, image)
It is not entirely clear whether that only refers to top-level elements (somehow related to the point of gltfProperty
vs. glTFChildOfRootProperty
above), and whether any "sub-element" should be part of the name.
One could make a case to always include the "top-level" element name. Be it at least for the case that someone defines an extension that goes into an animation.sampler
, and we want to make sure that it is called EXT_animation_sampler_example
, and not just EXT_sampler_example
, which would cause an ambiguity with the top-level sampler
.
So I'd be leaning towards _mesh_primitive_restart
here.
If there's no objection, I will push a change to rename the extension to |
Should we just name it KHR_ directly? We have already ratified some EXT_ extensions and wasn't able to rename to KHR_ due to compatibility. Would we ever consider ratifying this extension? |
We'd have to get an agreement within 3DF WG first and commit to providing Sample Assets at least. |
@lexaknyazev @bghgary any update on this? Or any additional feedback for @pmconne? |
This extension permits the prohibition against maximal index values in index buffers to be selectively relaxed for groups of primitives, while providing a trivial fallback for implementations that don't support primitive restart. Discussed in #1142.