Error management in unconfigured Surface methods

[eliemichel]

A Surface object is created from an Instance, so it has no associated Device by default, until its Configure method gets called.

However, the typical management of errors is done through the Device's UncapturedErrorCallback.

So how should a Surface report an error when a method like Present is called before the surface was configured? This concern holds for all methods that were inherited from the former SwapChain object.

[kainino0x]

Following the pattern of #225 and #115 (comment) I would propose we make wgpuSurfacePresent return a WGPUStatus (or whatever we call this Success/Failure enum)
and following various issues, a textual message would also come in through "implementation-defined logging" (ErrorCallback in Dawn), which we intend to standardize at some point in the future.

Another option would be to have present do nothing in this case as that is what would happen in a browser, when it "Present"s implicitly at the end of a frame.

[kainino0x]

Jan 4 meeting:

• RM: Other methods that can fail?
  • CreateSurface returns null and logged error. Probably fine
  • Adapter methods? These are probably OK
  • Present without GetCurrentTexture?
• (...)
• Return the Status (Success/Failure) enum (Handling invalid/not-enabled chained structs on extensible Out-structs #115 (comment)), with implementation defined logging, for any error on Present (even if there’s a device, don’t send the error to the error scope). If similar future things come up, do something similar probably

We don't have WGPUStatus (or whatever it's going to be called) yet but it can be added anytime.

[Kangz]

Given that we have a WGPUStatus now, should WGPUSurfaceGetCurrentTextureStatus be merged into it?

[kainino0x]

WGPUSurfaceGetCurrentTextureStatus has more specific statuses, so if we did that we'd have to have another way to report those.

• Success -> Success, Suboptimal (Consider renaming SwapChainSurface.suboptimal to optimal. #257)
• Error -> Outdated, Lost, OutOfMemory, DeviceLost

[Kangz]

It's just a bit weird that there are multiple status enums when there is also a generic one that's just a two-value enum. But folding everything into a single enum also means that each entrypoint will have to say which subset of it is valid, and application will have to know that wgpu::Suboptimal is still a success. So staying as is seems fine.

[kainino0x]

May 23 meeting (sorry for delay):

• KN/AE: Considering a case where you return a status and write out to a state.
• KN: Don’t like the possibility - if we return two status enums - of returning a success status when there’s really an error.
  • Also don't like reusing WGPUStatus_Error to indicate when there was an error other than calling the function incorrectly (with a bad chain).
• KN: Suggest keeping as is - we just added an Error status (WGPUSurfaceGetCurrentTextureStatus needs a value for "surface not configured" #291 (comment)), which we can reuse for when there’s a chaining error.
• No objections (IIRC)

[kainino0x]

• KN: Suggest keeping as is - we just added an Error status (WGPUSurfaceGetCurrentTextureStatus needs a value for "surface not configured" #291 (comment)), which we can reuse for when there’s a chaining error.

To clarify: I'm fine with this because WGPUSurfaceGetCurrentTextureStatus_Error doesn't strongly indicate one particular type of error. WGPUStatus_Error is strongly associated with chaining errors (or at least "you called the function wrong"), because we use it that way everywhere else.

[kainino0x]

Actually I'm rereading the other notes on this issue and realizing that my position there doesn't make sense. We did say we would use WGPUStatus_Error for state errors in wgpuSurfacePresent():

• Return the Status (Success/Failure) enum (Handling invalid/not-enabled chained structs on extensible Out-structs #115 (comment)), with implementation defined logging, for any error on Present (even if there’s a device, don’t send the error to the error scope). If similar future things come up, do something similar probably

That's fine though. Main point was that we didn't want to return two possibly-conflicting status enums, and that constraint is best satisfied by using WGPUSurfaceGetCurrentTextureStatus to indicate both types of error.