Skip to content
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

Allocation of implementation-specific enum values #214

Closed
kainino0x opened this issue Aug 17, 2023 · 13 comments
Closed

Allocation of implementation-specific enum values #214

kainino0x opened this issue Aug 17, 2023 · 13 comments
Labels
extensibility Adding features without breaking API changes has resolution Issue is resolved, just needs to be done needs docs Non-trivial API contract needs to be documented. Orthogonal to open vs closed; remove when doc'd.

Comments

@kainino0x
Copy link
Collaborator

kainino0x commented Aug 17, 2023

We should have a "registry" of allocated ranges to use for enums, especially WGPUSType and WGPUFeatureName
Per #212 the end of the name of the new values will be the name of the implementation.

In the webgpu.h meeting we agreed to use the top 16 bits for the implementation namespace. I think we can use this for all of the (non-bitflag) enums.

No one really cares about the actual IDs so here's an arbitrary assignment of ranges:

  • 0x0000_????: core
    Suffix: none.

    Should be in all implementations, both native and Wasm. Used for features added to WebGPU core after this header stabilizes.

  • 0x0001_????: multi-implementation C-specific extensions
    Suffix: none.

    May be native-specific or Wasm-specific, both can go in this block.

  • 0x0002_????: WebGPU Compat
    Suffix: none? TBD.

    Special: implementations that don't support compat should ignore these instead of error. This is to be used only for Compat additions, which don't affect semantics of a valid program if a non-compat implementation ignores them.

  • 0x0003_????: wgpu-native
    Suffix: _WGPU

  • 0x0004_????: Emscripten
    Suffix: _Emscripten

  • 0x0005_????: Dawn
    Suffix: _Dawn


EDIT: For bitflags (becoming uint64_t in #273):

  • The least-significant 53 bits are reserved for core - that is, bits defined by the JS API, since it can only represent 53 bits.
  • The most-significant 11 bits are for native-only and implementation-specific bits, then overflows into the most-significant end of the least-significant 53 bits if we run out. The allocation of these bits is ad hoc for now.
    • Only bits specified in this header will be stable; implementation-specific bits can overlap or change value for now. Eventually for ABI compatibility we'll probably need a registry of these.
@kainino0x kainino0x added has resolution Issue is resolved, just needs to be done needs docs Non-trivial API contract needs to be documented. Orthogonal to open vs closed; remove when doc'd. labels Aug 17, 2023
@kainino0x kainino0x added the extensibility Adding features without breaking API changes label Aug 19, 2023
rajveermalviya added a commit to rajveermalviya/wgpu-native that referenced this issue Sep 16, 2023
rajveermalviya added a commit to rajveermalviya/wgpu-native that referenced this issue Sep 19, 2023
rajveermalviya added a commit to gfx-rs/wgpu-native that referenced this issue Sep 20, 2023
* New enum values for wgpu-native specific enums.

webgpu-native/webgpu-headers#214

* update comment

Co-authored-by: Almar Klein <[email protected]>

---------

Co-authored-by: Almar Klein <[email protected]>
@rajveermalviya
Copy link
Collaborator

wgpu-native now uses the 0x0003_???? namespace

@Kangz
Copy link
Collaborator

Kangz commented Sep 22, 2023

0x0001_????: (reserved)

Shouldn't this be reserved for native-only and not upstream WebGPU (as in the JS spec) features? If we need another reserved block Dawn can move to 0x0005_???? so 0x0002_???? is free.

@kainino0x
Copy link
Collaborator Author

Good idea. We don't really need another reserved block so I'll just change what 0x0001_???? is, above.

@kainino0x
Copy link
Collaborator Author

kainino0x commented Oct 11, 2023

We need to decide where WebGPU Compat values live (for STypes in particular). They could be in the Core block, but we might also want to give them special semantics - ignore them, rather than error, if the implementation doesn't understand them. See:
https://dawn-review.googlesource.com/c/dawn/+/155780/comment/4e84af8f_54c41233/

@kainino0x
Copy link
Collaborator Author

meeting: Decided tentatively to do that: make a block for Compat so the whole block can be treated specially. If either compat isn't enabled, on the implementation doesn't implement compat at all (but knows that it exists), it should ignore chained structs in this block, rather than error on them.

This raises the question of what to do in an implementation that implements compat, if an SType is passed that's in the compat block but not recognized. This is an edge case since we don't provide ABI compatibility. But I think it should error, regardless of whether compat is enabled on the device.

I'm going to make 0x0002 Compat, and move Dawn to 0x0005 since we haven't started using it yet.

@kainino0x kainino0x pinned this issue Jan 20, 2024
@kainino0x
Copy link
Collaborator Author

For bitflags enums, which are becoming 64 bits, the first 10-11 extensions should be allocated (still ad hoc) in the top 11 bits that JS can't represent. (10 because might need to skip the MSB because if C enums don't like being u64.)

@kainino0x
Copy link
Collaborator Author

Mar 14 meeting:

  • KN: Seems safe, don’t think we could possibly ever accidentally allocate something as native-only and then it turns out to be added to the JS API (and not a big deal if we have to dup a bit anyway)
  • (brief discussion)
  • Seems fine. Otherwise keep ad hoc allocation for now

@kainino0x kainino0x removed the !discuss Needs discussion (at meeting or online) label Mar 16, 2024
@kainino0x
Copy link
Collaborator Author

Updated top post.

@kainino0x kainino0x added the !discuss Needs discussion (at meeting or online) label Jun 27, 2024
@kainino0x
Copy link
Collaborator Author

Do we need a block for multi-implementation wasm-only extensions? WGPUSType_SurfaceDescriptorFromCanvasHTMLSelector could belong in that block. (Its actual behavior in Emscripten is a bit Emscripten-specific because of how it handles looking up canvases from workers, but that doesn't mean the extension has to be in the Emscripten block.)

@kainino0x
Copy link
Collaborator Author

(Relatedly we are assuming WGPUSType_SurfaceDescriptorFromMetalLayer belongs in the "multi-implementation native-only extensions" block: https://dawn-review.googlesource.com/c/dawn/+/196174)

@kainino0x
Copy link
Collaborator Author

Jul 18 meeting:

  • Do we need a block for multi-implementation wasm-only extensions?
  • WGPUSurfaceDescriptorFromCanvasHTMLSelector provides a string to pass to querySelector() DOM API. In Emscripten it also has some special behavior.
  • CF: wgpu-rs does not use the C API so this extension would not exist. So it is Emscripten-only for now.
  • KN: SG, we can put it in the Emscripten block.
  • Tweak the definition of block 1 - instead of "multi-implementation native", "multi-implementation C-specific".

@kainino0x
Copy link
Collaborator Author

See also the Jul 18 notes for #212 (comment)

I have updated the top comment on this issue for both resolutions.

@kainino0x
Copy link
Collaborator Author

kainino0x commented Nov 9, 2024

Closing, nothing to do in this repo until we start using block 0x0001 (e.g. in #200, EDIT: and #420).

(except for add docs, so we keep the needs docs label for that.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extensibility Adding features without breaking API changes has resolution Issue is resolved, just needs to be done needs docs Non-trivial API contract needs to be documented. Orthogonal to open vs closed; remove when doc'd.
Projects
None yet
Development

No branches or pull requests

3 participants