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

Issues with representing some JS interfaces as C structs #279

Closed
kainino0x opened this issue Mar 6, 2024 · 3 comments
Closed

Issues with representing some JS interfaces as C structs #279

kainino0x opened this issue Mar 6, 2024 · 3 comments

Comments

@kainino0x
Copy link
Collaborator

A few JS (WebIDL) interfaces are currently represented by C structs, namely:
WGPUAdapterInfo, WGPUCompilationInfo/Message, WGPUSupportedLimits.

This doesn't match quite directly for a few reasons:

  • attributes on an interface are actually getters. In Wasm bindings, we would have to call all of them.
    • It's possible that browsers would want to pay attention to which attributes are actually accessed (e.g. on GPUAdapterInfo and GPUSupportedLimits for fingerprint monitoring).
    • It's also theoretically possible that those getters could do something else (like mutate state or throw an exception) but that's very unlikely.
  • interfaces could gain methods.

This issue is to consider whether this is a problem worth fixing. Off the top of my head it doesn't make sense for WGPUCompilationInfo/Message, but it might for the other two.

cc #266, #272, #260

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

Mar 7 meeting:

@kainino0x
Copy link
Collaborator Author

Mar 14 meeting:

  • KN: I asked and it sounds like measurability / finer granularity of fingerprintable bits is still important.
  • We can always give BackendType, AdapterType without exposing any bits.
  • Design alternatives for AdapterInfo:
    • Separate getters for each field (vendor, architecture, device, description)
    • Similar but a single function, GetAdapterInfoString(enum)
    • GetAdapterInfo but with a bit for each field
      • Only need request bits for the ones in the JS API.
      • Can probably assume that if the app asks, they want Vendor, so only provide request bits for the other three?
      • KN: Propose in native we ignore the bits and provide all information regardless. Only require the bits in Wasm.
      • Overall pretty compatible
    • — out of time —
    • AE: A core API (getters / GetAdapterInfoString) AND a native-only GetAdapterInfo(struct chain)
    • KN: Non-extensible structs and one getter for each struct
      • Core (vendor/architecture/device/description)
      • IDs (vendorID/deviceID)
      • The only other instance of WGPUChainedStructOut is WGPUSurfaceCapabilities (at least in the header right now, unsure about open issues). Could arguably get rid of the ChainedStructOut concept entirely?
  • Still have problems with GPUSupportedLimits.

@kainino0x
Copy link
Collaborator Author

Mar 28 meeting:

  • CF: It seems extremely silly to do this. Rust bindings would just undo this work.
  • KN: Some languages have getters which map better.
  • KN: Fingerprinting measurement probably doesn’t matter on any program using the C API through Wasm. Browser probably has to assume that if the application does any actual work (draw/dispatch) then it can deduce all of the fingerprint bits from the limits and adapter info anyway (limits by testing validation, adapter info by checking results). So I'm thinking there's no point in making it more granular in this API.
  • We can add something in the future if we need it.
  • Close issue.

@kainino0x kainino0x closed this as not planned Won't fix, can't repro, duplicate, stale Mar 28, 2024
@kainino0x kainino0x removed the !discuss Needs discussion (at meeting or online) label Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant