Status enums still have too many values

[kainino0x]

Following on #396 we should check if there are any other extraneous status values. Checked box = definitely good, or we decided what to do about it.

These look good:

• WGPUPopErrorScopeStatus

  • WGPUPopErrorScopeStatus_Success
  • WGPUPopErrorScopeStatus_InstanceDropped

• WGPUErrorType (not really a status but we'll include it)

  • WGPUErrorType_NoError
  • WGPUErrorType_Validation
  • WGPUErrorType_OutOfMemory
  • WGPUErrorType_Internal
  • WGPUErrorType_Unknown - this is a bit niche but it's used if the browser returns some new status code and Wasm bindings don't understand it and need to translate it into a WGPUErrorType. It could be renamed/renumbered to Undefined=0 but I don't think there's any real reason to do that.

• WGPUAdapterType (this really isn't a status but it also has an "unknown")

  • WGPUAdapterType_Unknown - this could also be renamed/renumbered to Undefined=0

• WGPUDeviceLostReason (also not technically a status) (see also Review entire header for design consistency #299 (comment))

  • WGPUDeviceLostReason_Unknown - this is from the JS spec
  • WGPUDeviceLostReason_Destroyed
  • WGPUDeviceLostReason_InstanceDropped
  • WGPUDeviceLostReason_FailedCreation

• WGPUWaitStatus

  • WGPUWaitStatus_Success
  • WGPUWaitStatus_TimedOut
  • WGPUWaitStatus_UnsupportedTimeout
  • WGPUWaitStatus_UnsupportedCount
  • WGPUWaitStatus_UnsupportedMixedSources

These I'm not sure about:

• WGPURequestAdapterStatus

  • WGPURequestAdapterStatus_Success
  • WGPURequestAdapterStatus_InstanceDropped
  • WGPURequestAdapterStatus_Unavailable - presumably this is used when no adapter is available, but JS treats this as "success" and returns a null adapter. Should we really have a separate status for this? (Probably yes.)
    Keep
  • WGPURequestAdapterStatus_Error (validation error)
  • WGPURequestAdapterStatus_Unknown - Dawn uses this when there is a wire error. Should it return InstanceDropped, or pretend an adapter is unavailable instead? (or crash)
    Remove

• WGPURequestDeviceStatus

  • WGPURequestDeviceStatus_Success
  • WGPURequestDeviceStatus_InstanceDropped
  • WGPURequestDeviceStatus_Error (validation error)
  • WGPURequestDeviceStatus_Unknown - Dawn uses this when there is a wire error. Should it return InstanceDropped, or return a lost device instead? (or crash)
    Remove

• WGPUQueueWorkDoneStatus

  • WGPUQueueWorkDoneStatus_Success
  • WGPUQueueWorkDoneStatus_InstanceDropped
  • WGPUQueueWorkDoneStatus_Error (validation error) - Happens when the queue is invalid. I don't think this is possible now; it will be, but should we keep this status in the meantime?
    Remove
  • WGPUQueueWorkDoneStatus_Unknown - When would we use this instead of faking success? I don't think Dawn uses this. (Not sure what it does on wire errors.)
    Remove

• WGPUMapAsyncStatus

  • WGPUMapAsyncStatus_Success
  • WGPUMapAsyncStatus_InstanceDropped
  • WGPUMapAsyncStatus_Error (validation error)
  • WGPUMapAsyncStatus_Aborted
  • WGPUMapAsyncStatus_Unknown - Dawn uses this when there is a wire error. Should it return InstanceDropped, or lose the device and return Aborted instead?
    Remove

• WGPUCompilationInfoRequestStatus

  • WGPUCompilationInfoRequestStatus_Success
  • WGPUCompilationInfoRequestStatus_InstanceDropped
  • WGPUCompilationInfoRequestStatus_Error - there are no possible validation errors here unless we're validating the CallbackMode (which I think we're supposed to crash on when invalid). So what does this mean? I don't think Dawn uses this.
    Remove
  • WGPUCompilationInfoRequestStatus_Unknown - when would we use this instead of Success or crashing? I don't think Dawn uses this. (Not sure what it does on wire errors.)
    Remove

• WGPUCreatePipelineAsyncStatus

  • WGPUCreatePipelineAsyncStatus_Success
  • WGPUCreatePipelineAsyncStatus_InstanceDropped
  • WGPUCreatePipelineAsyncStatus_ValidationError
  • WGPUCreatePipelineAsyncStatus_InternalError
  • WGPUCreatePipelineAsyncStatus_Unknown - Dawn uses this when there is a wire error. Should it return InstanceDropped, or lose the device and return Success instead?
    Remove

• WGPUSurfaceGetCurrentTextureStatus

  • WGPUSurfaceGetCurrentTextureStatus_SuccessOptimal
  • WGPUSurfaceGetCurrentTextureStatus_SuccessSuboptimal
  • WGPUSurfaceGetCurrentTextureStatus_Timeout
  • WGPUSurfaceGetCurrentTextureStatus_Outdated
  • WGPUSurfaceGetCurrentTextureStatus_Lost
  • WGPUSurfaceGetCurrentTextureStatus_OutOfMemory - TBH not entirely certain how this would be used. I don't think Dawn uses this.
    Remove probably
  • WGPUSurfaceGetCurrentTextureStatus_DeviceLost - already slated for removal in Remove DeviceLost status from WGPUSurfaceGetCurrentTextureStatus #397
  • WGPUSurfaceGetCurrentTextureStatus_Error (validation error)

[kainino0x]

@lokokung you would know better what the wire is doing in the futures code, could you review the notes above?
@Kangz PTAL as well

[kainino0x]

Forgot the synchronous statuses WGPUWaitStatus and WGPUSurfaceGetCurrentTextureStatus. Added above.

[Kangz]

All _Unknown are used by Dawn wire when the connection drops, but we should instead pretend that all devices are lost similar to JS.

So my suggestion would be to keep just WGPURequestAdapterStatus_Unavailable for RequestAdapter failing to get an adapter, like when the instance is lost.

[lokokung]

IIRC, when the wire connection drops, the current implementation uses InstanceDropped for all the callbacks, including RequestAdapter. For wire errors, maybe we should generalize something like Unrecoverable and use that for InstanceDropped and other weird errors? Not sure..., but right now InstanceDropped is probably too specific of a name given that it's used in multiple scenarios, i.e. wire drop.

[kainino0x]

Dropping the wire is kind of like dropping the instance, in that you no longer really hold a reference to the (server-side) instance, except that it can happen outside of our control (e.g. server process crashes).

Losing the wire connection doesn't totally kill futures though, we still track them client side and can still pretend they succeed or whatever. So I think it makes more sense to do that rather than return InstanceDropped on disconnect?

[kainino0x]

Just a note, WGPUErrorType also has Unknown which could maybe be called Undefined=0 but I don't think there's any reason to do that. Same could be done for WGPUAdapterType.

[lokokung]

While we can still track the futures on the client side, they will never be completed anymore, which is why I used InstanceDropped in the first place. They are only set to be ready when the server responds after all.

[kainino0x]

Couldn't we just as well pretend they succeed just like we do on device loss, and call the callback with Success? (except for MapAsync which gets Aborted)

[lokokung]

Hmm, we could? But it'll be a silent failure since I think we said we wanted the StringView msg to be empty if we resolve with Success in most callbacks.

[kainino0x]

I think a silent failure makes sense. We do that on device loss, we just have another channel for reporting the device loss happened. Probably we would report device loss if the wire disconnects, too.

[kainino0x]

Dec 9 meeting:

• KN: "Unknown" statuses were used in Dawn for wire disconnection, those should presumably be either Success or InstanceDropped - which one?
  • CF: Probably fine, but @rajveermalviya would know if wgpu-native needs these
  • But Dawn will probably try to remove, and that should prove it's logically possible
• KN: RequestAdapter distinguishes "Success" from "Unavailable" whereas JS's Promise<GPUAdapter> just returns null if unavailable. Should we keep the enum distinction? Probably, why not.
  • Keep the distinction, otherwise unclear (moreso than in JS). Also better in C++ because don't have to figure out how to check nullness.
• KN: WGPUQueueWorkDoneStatus_Error is not possible because queues can't be invalid. Keep anyway for when we have multiqueue, or remove until then?
  • Remove
• KN: WGPUCompilationInfoRequestStatus_Error is not possible. Remove?
  • Remove
  • (if CallbackMode is invalid, that returns a null future, not an error status)
• KN: WGPUSurfaceGetCurrentTextureStatus_OutOfMemory - when would this actually be used?
  • CF: Swapchain ran out of memory for some reason. Whole world explodes? It does happen in wgpu.
  • KN: Also weird because not possible to report in async context (e.g. wire) because it's synchronous.
  • CF: Some places where we don't really know what happened and report out of memory. Surfaces very complicated though, especially Linux.
  • KN: Remove it from upstream header for now and wgpu can see if it can remove it, or just keep it as an extension?
  • CF: OK.
  • LK: Could maybe return a message string? (Though requires a FreeMembers, or a static string.)
  • (... we didn't really discuss this much but I think we thought it wasn't worth it)

[kainino0x]

Opened two PRs for the remaining things, I'll just close this and use those to track work.