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

WGPUSurfaceGetCurrentTextureStatus needs a value for "surface not configured" #291

Closed
kainino0x opened this issue May 17, 2024 · 3 comments · Fixed by #377
Closed

WGPUSurfaceGetCurrentTextureStatus needs a value for "surface not configured" #291

kainino0x opened this issue May 17, 2024 · 3 comments · Fixed by #377
Labels
has resolution Issue is resolved, just needs to be done presentation Presenting images to surfaces like windows and canvases

Comments

@kainino0x
Copy link
Collaborator

There's just one error case in getCurrentTexture() in the JS spec:

  1. If this.[[configuration]] is null:
    1. Throw an InvalidStateError and stop.

We don't have a WGPUSurfaceGetCurrentTextureStatus to represent this currently.

typedef enum WGPUSurfaceGetCurrentTextureStatus {
    WGPUSurfaceGetCurrentTextureStatus_Success = 0x00000000,
    WGPUSurfaceGetCurrentTextureStatus_Timeout = 0x00000001,
    WGPUSurfaceGetCurrentTextureStatus_Outdated = 0x00000002,
    WGPUSurfaceGetCurrentTextureStatus_Lost = 0x00000003,
    WGPUSurfaceGetCurrentTextureStatus_OutOfMemory = 0x00000004,
    WGPUSurfaceGetCurrentTextureStatus_DeviceLost = 0x00000005,
    WGPUSurfaceGetCurrentTextureStatus_Force32 = 0x7FFFFFFF
} WGPUSurfaceGetCurrentTextureStatus WGPU_ENUM_ATTRIBUTE;

Should we add a new one, or merge with one of those?

Followup issue to #203. cc @rajveermalviya @Kangz @eliemichel

@kainino0x kainino0x added the presentation Presenting images to surfaces like windows and canvases label May 17, 2024
@kainino0x
Copy link
Collaborator Author

FWIW, because there is only one error case in JS, there will be only two possible return values in Wasm (Success and surface not configured).

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

kainino0x commented Jun 6, 2024

May 23 meeting (sorry for delay):

  • KN: Surface lost is probably close enough? What is surface lost anyways?
  • AE: If it’s anything like device lost where something went pretty wrong, maybe we should use something different?
  • KN: Should we call it something like invalid state so that we could reuse it for something else if necessary? e.g. wasm will get an InvalidState error from the JS API and have to translate it into the enum.
  • AE: So maybe we use invalid state with or without implementation defined logging?
  • AE: What if we just called it error or something like that?
  • KN: Seems fine, but if we ever add any other errors then it might be a bit weird cause we could have different error types
  • KN: Just call it WGPUSurfaceGetCurrentTextureStatus_Error

Note in #255 (comment) we decided to also use the same status for errors in struct chaining (for which we would normally use WGPUStatus_Error), so it does end up being slightly overloaded. Both can be considered "programmer error" so it seems OK.

@kainino0x kainino0x added the has resolution Issue is resolved, just needs to be done label Jun 6, 2024
copybara-service bot pushed a commit to google/dawn that referenced this issue Jun 10, 2024
…ppens

See webgpu-native/webgpu-headers#291

Bug: 42241264
Change-Id: Ia8669fe9838c98f86942af9adaf835fddaf36fa6
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/192185
Commit-Queue: Corentin Wallez <[email protected]>
Reviewed-by: Kai Ninomiya <[email protected]>
Reviewed-by: Austin Eng <[email protected]>
@Kangz
Copy link
Collaborator

Kangz commented Aug 28, 2024

Done in #319.

@Kangz Kangz closed this as completed Aug 28, 2024
kainino0x added a commit to kainino0x/webgpu-headers that referenced this issue Oct 19, 2024
kainino0x added a commit to kainino0x/webgpu-headers that referenced this issue Oct 19, 2024
kainino0x added a commit to kainino0x/webgpu-headers that referenced this issue Oct 19, 2024
kainino0x added a commit to kainino0x/webgpu-headers that referenced this issue Oct 19, 2024
kainino0x added a commit to kainino0x/webgpu-headers that referenced this issue Oct 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has resolution Issue is resolved, just needs to be done presentation Presenting images to surfaces like windows and canvases
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants