Passing extensible structures to application callbacks, backward-compatibly

[kainino0x]

Raised in #266: we want a backward-compatible way of passing extended data to application-defined callbacks. This comes up in two places:

• GetCompilationInfo: the callback receives WGPUCompilationInfo which is extensible, and also contains WGPUCompilationMessage objects which are also extensible
• RequestAdapterInfo: the callback receives WGPUAdapterInfo which is not yet extensible, but needs to be

If a webgpu.h implementation suddenly starts passing new extension structs to the application, it runs the risk of the application mishandling them.

For RequestAdapterInfo we decided that the WGPUAdapterInfo should be passed as an argument to RequestAdapterInfo so the application can set up the extension chain. Then we need a wgpuAdapterInfoFreeMembers (which replaces wgpuAdapterPropertiesFreeMembers proposed in #143 but which is becoming obsolete with #266).

[kainino0x]

Unfortunately WGPUCompilationInfo as written isn't trivial to switch: WGPUCompilationMessage structs are extensible, but can't be allocated ahead of time by the application (it doesn't know how many there will be).

• As originally proposed in Compilation info #87 (review) we could just make WGPUCompilationMessage non-extensible and return extensible data in a parallel array (in a new array in a WGPUCompilationInfo extension)
• Or we could let applications specify which chains they want, in an extension of WGPUCompilationInfo, and then we'll return those. (And guarantee we'll provide the exact list of chained structs requested, in a predictable order.)

[kainino0x]

Feb 29 meeting:

• (Offshoot of #266 but more generally - includes GetCompilationInfo too)
• KN: (explanation of problem) CompilationInfo is extensible but also holds a
• AE: RequestAdapterInfo/GetAdapterInfo are now more like MapAsync where it
• …
• RM: We need a pattern to request additional info anyway
• KN: Think this is the only one that currently needs it
• LK: A parallel array with the same indices in the CompilationInfo chain?
• (take the chain out of the CompilationMessage)
• LK: What if not every message has the same chain?
  • KN: Can make it nullable somehow
• How would this look? RequestCompilationInfo + GetCompilationInfo?
  • Compilation info already lives forever (with the shader module) but in the wire we would be holding a second copy alive (on the client side)
• (discussion)
• OK with removing extensibility from CompilationMessage
• (discussion of doing the previous proposal for AdapterInfo)
• KN: Not the worst if we say applications need to handle chain structs they don’t know about. (If we do that then CompilationMessage can be kept as extensible.)
• LK: Would need a way to request which AdapterInfo extensions you want anyway
  • Bitmask? Enum array? Dummy “prototype” struct chain?
  • AE: Prefer array
    • RM: agree, array of sTypes rather than creating new bitmasks
• — out of time —
• AE/KN offline: If we have a list of requests then actually applications will never get chains they don’t understand. This should work for both AdapterInfo and CompilationInfo.
  • The list could be STypes, or it could be a new enum specific to the request
• KN will write up a proposal on GitHub

[kainino0x]

Here it is:

// RequestAdapterInfo

typedef void (*WGPUAdapterRequestAdapterInfoCallback)(
    // This would be allowed to contain any of the allowedSTypes, but not others
    struct WGPUAdapterInfo const * adapterInfo,
    WGPU_NULLABLE void * userdata) WGPU_FUNCTION_ATTRIBUTE;

WGPU_EXPORT WGPUBool wgpuAdapterRequestAdapterInfo(WGPUAdapter adapter,
    // Alternatively instead of WGPUSType this could be
    // a new enum (allows finer granularity)
    // or a bitmask (may run out of bits).
    size_t allowedSTypeCount,
    WGPUSType const * allowedSTypes,
    WGPUAdapterRequestAdapterInfoCallback callback,
    WGPU_NULLABLE void * userdata) WGPU_FUNCTION_ATTRIBUTE;

// GetCompilationInfo

typedef void (*WGPUShaderModuleGetCompilationInfoCallback)(
    WGPUCompilationInfoRequestStatus status,
    struct WGPUCompilationInfo const * compilationInfo,
    WGPU_NULLABLE void * userdata) WGPU_FUNCTION_ATTRIBUTE;

WGPU_EXPORT void wgpuShaderModuleGetCompilationInfo(
    WGPUShaderModule shaderModule,
    // In this case, the list would contain the STypes allowed for both
    // WGPUCompilationInfo AND WGPUCompilationMessage.
    // (This is unambiguous because their valid STypes should be disjoint.)
    size_t allowedSTypeCount,
    WGPUSType const * allowedSTypes,
    WGPUShaderModuleGetCompilationInfoCallback callback,
    WGPU_NULLABLE void * userdata) WGPU_FUNCTION_ATTRIBUTE;

(These are the only two existing entrypoints that would need this.)

[kainino0x]

I guess another open question is whether we need extensibility in any of the others:

• errors (DeviceLost, UncapturedError, PopErrorScope)
  • note in WGPUFuture #199 (comment) PopErrorScope is gaining a status and won't be the same callback type as UncapturedError anymore
• MapAsync
• object creation (Create*PipelineAsync, RequestDevice, RequestAdapter)

They are probably all safe to leave as is.

[Kangz]

I'm not sure that I understand why we make the application pass in the list of sTypes it knows how to handle. Aren't they going to iterate over the sType chain with a switch anyway? If we provided some simple "FindInChain" utilities, maybe it would be sufficient to assume there won't be breakage when new sTypes are returned by the implementation.

It's just that the approach with a list of sTypes seems to be adding another mechanism on top of the extension mechanism, when the latter on its own could be sufficient.

[kainino0x]

I'm not sure that I understand why we make the application pass in the list of sTypes it knows how to handle. Aren't they going to iterate over the sType chain with a switch anyway? If we provided some simple "FindInChain" utilities, maybe it would be sufficient to assume there won't be breakage when new sTypes are returned by the implementation.

It's just that the approach with a list of sTypes seems to be adding another mechanism on top of the extension mechanism, when the latter on its own could be sufficient.

Fair point. I hadn't thought about the fact application code would be doing the iteration thing. If that's the case, then there's not much value in avoiding returning STypes they didn't ask for.

On the other hand, I think we COULD guarantee that the chain you get will be exactly the STypes you allowed in exactly the same order, then you could write code that knows exactly how far to go in the chain like (WGPUAdapterInfoVendorDeviceID*) info->nextInChain or (WGPUAdapterInfoVendorDeviceID*) info->nextInChain->next. Seems like overkill though.

So probably let's go back to just making the application deal with whatever chain we give them. (We also don't have to be mega concerned with compatibility either, because it'll only break if you update your dawn/wgpu-native/emscripten dependency. I've worked in the Web spec for too long.)

Providing FindInChain utilities would be nice, though not blocking. #278

[kainino0x]

Mar 7 meeting:

• Proposal to just do the original thing, pass the object in and make the application deal with unknown stuff in the chain
  • (recap of github comment)
• Sounds good
• CF: Could pass a sType=0xDEADBEEF in the chain in debug builds to make sure applications deal with it.
• CF: Also convenient for implementations because the chain setup can be static - just allocate it on the stack.

[kainino0x]

Which means no change is required.

[kainino0x]

"needs docs" to tell people how to iterate the chain.
(And maybe do the 0xDEADBEEF thing.)