Minutes 2022 09 14

GPU Web 2022-09-14

Chair: Corentin

Scribe: Corentin/Kai

Location: Google Meet

Tentative agenda

• Administrivia
• CTS Update
• “Tacit Resolution” Queue
• Colorspace conversions for GPUExternalTexture are arbitrarily complex #3293
• createPipelineAsync() doesn't reject its promise, even if the created pipeline is invalid #3296
• Add multi draw indirect feature #2315
  • (From Editors’ meeting. Get this landed, or leave for later?)
• Change ArrayBuffer arguments to ArrayBuffer + srcOffset #244
  • (From Editors’ meeting. Keep as is, or remove overloads?)
• GPURenderBundleEncoder.setPipeline() not implementable on MTLIndirectCommandBuffer #3423
• Items from Editors’ Meeting that might be too substantial to discuss on short notice
  • Frame advancement of canvases that aren't visible #3295
  • mapAsync(WRITE) zero-fills the relevant region of the buffer #2926
• Agenda for next meeting

Attendance

• Apple
  • Myles C. Maxfield
• Google
  • Corentin Wallez
  • Kai Ninomiya
• Microsoft
  • Rafael Cintron
• Mozilla
  • Jim Blandy
  • Kelsey Gilbert

Administrivia

• CW: We have a charter, going to send it for review. Take a last look (at this stage) and I’ll send it over to tidoust at the end of the week.

CTS Update

• KN: Igalia looking at operation tests. Showed them how to handle one test. So they might start porting dawn_end2end_tests to CTS for what's missing coverage.
• KN: Progress on builtin precision stuff.

“Tacit Resolution” Queue

• Rename maxColorAttachmentBytesPerPixel to *PerSample #3434
  • KN: Debated about this limit name, since the count is per sample, then let's have it per sample in the name. Alternatively considered “PerFragment”.
  • Consensus: Cool! Land it.

Colorspace conversions for GPUExternalTexture are arbitrarily complex #3293

• MM: Think that we should just close this issue since everybody agrees.
• Everybody agrees: close.

createPipelineAsync() doesn't reject its promise, even if the created pipeline is invalid #3296

• KN: The problem with this API is that we are trying to mix the async of our API with the async of Javascript more tightly than other parts of the API. The two examples from Myles show that one alternative is much better: the one where we expose failure with a rejection is better for ergonomics. However, I would like to consider reshaping with "isReady" like we proposed a long time ago. That requires a bunch of implementation complexity to use the pipeline object if it isn't ready yet. But we can add validation on the content side that setPipeline must have a pipeline that's ready. So refactor at the surface level of the API. Resolve the weirdness where it isn't clear what happens in some corner cases.
• MM: Don't think that the semantic should change depending on whether the application looked at the promise or not. Should be opt-in.
• KN: Agreed, would be a creation parameter.
• MM: Maybe two different objects with the descriptors that inherit from each other and the objects inherit from each other.
• KN: Was thinking to put an already resolved promise there.
• MM: Promises are always resolved at microtask boundary so you can't use the pipeline in the same microtask.
• KN: Same as current API - intentional, changes only the surface API
• CW: Concerned about creating a special case of pipeline object [where it throws an exception on use]
• MM: https://github.com/gpuweb/gpuweb/issues/3296#issuecomment-1244818365
• …
• CW: What would happen if the pipeline is used when isReady is not resolved?
• KN: You'd get an exception so you need less implementation complexity in the GPU process.
• MM: Other Web APIs have something - not necessarily advocating - rather than saying it’s illegal to use an object until it’s ready, phrase it as “object cannot be used until the application has asked for a promise and it has been resolved”. Prevents the case where you just wait long enough without waiting for .ready and it ends up working.
• KN: Think we should do that.
• KG:
• CW: Can we reject with whatever we want? Reject with an error pipeline?
• KG: I suggested that last week. It’s possible.
• KG: All I would want as a user is the ability to create a pipeline, then wait for ready, then do things with it. If I mess that up and use it before its ready… I don’t have access to it. What if createPipelineAsync handed you a pipeline and it had an isDone on it.
• MM: In the 3 code snippets I posted a few days ago, in the last 2 code snippets, you can only use the pipeline is when it succeeds, and the only place you can use the pipeline is when it fails. Whereas in the 1st, you hold onto both objects at the same time. Would prefer if, when there’s an error, there’s no pipeline object you can see.
• KG: End up further diverging the way error handling works for pipelines.
• MM:
• KN: Question was whether to report to error scope or as rejection. With isReady you would still report to the error scope. So would need to hold onto that.
• MM: Would be consistent to say that functions that are async return errors via rejection.
• KG: Would be ok with having polymorphism / shared interface between pipelines and async pipelines.
• MM: Saying that the only way to get error from createPipelineAsync is through rejection.
• CW: Could we add getCompilationMessage to the rejection object?
• KN: Would be weird because the rejection is like a throw so need to inherit from an Error object. But we can hang whatever we want off of the Error object.
• MM: Benefit of having the error object and non-error object look the same is so the calling application doesn’t have to distinguish. If we’re going to use rejection, then the calling application will be explicitly handling it anyway, so better to have them be different types.
• CW: GPUPipepilineBase has no compilation message? Could make GPUPipelineCompilationError extends Error.
• KG: Should have GPUPipelineCompilationError and see later?
• KN: Still prefer isReady but interacts with error scopes. Also createPipelineAsync doesn't bubble to uncapturederror.
• CW: Let's move on and discuss on the issue.

Add multi draw indirect feature #2315

• KN: What should we do with this PR? Leave it open, get it into the spec? Work mostly done so maybe can land but still needs editorial work.
• KG: Apologize but say post-V1.

Change ArrayBuffer arguments to ArrayBuffer + srcOffset #244

• KG: Should keep this because likely useful and if we're wrong then we have forever cruft.
• MM: Two arguments to discuss it: nobody shipped yet so it being in spec doesn't imply any inertia. Second is that this is pure performance and so needs to have justification to be in the spec.
• KG: Justification is that "experts in the field" (WebGL implementers, Jukka) think that's enough if experts are in agreement. Web platform experts historically have not understood Web graphics sub-millisecond requirements as well.
• KN: In WebGL this change made it so you could use WebGL entirely without garbage. We have hard performance data from WebGL that showed this was very important. WebGPU has forced garbage in other places. However we may be seeing more overhead than just garbage in array buffer views, maybe because they are more special objects than JS primitives? Could be something to fix in JS engines, but the cost of fixing JS engines is large compared to this trivial override. Likely important for us and it is trivial.
• KG: This is very simple and may help people. Maybe the best API doesn't have this but the cost to add this is tiny and prevents having downsides.
• MM: Ok let's agree to close this issue then.

GPURenderBundleEncoder.setPipeline() not implementable on MTLIndirectCommandBuffer #3423

• MM: WebGPU pipelines include depth-stencil state but can't set that state in MTLIndirectCommandBuffer
• CW: Three solutions:
  • 1. Make implementations split if needed.
  • 2. Make implementations validate that the depth-stencil state is all the same in render bundles.
  • 3. remove setPipeline from render bundle encoder.
• MM: Think we should add the validation for <didn't catch that>
• KN: A bit more complicated because executeBundles needs to set the depth stencil state before the first inlining of MTLIndirectCommandBuffer.
• MM: If you split you transform a constant sized object into a variable-size object though.
• KG: The philosophy about render bundles in WebGPU seemed to be less about implementability in backends and more on usefulness for devs. There's a bunch of things that D3D12 bundles can't do.
• JB: What wgpu-core is doing at the moment is treating the bundles as pre-recorded sets of instructions we’ve done some validation on, and replaying it. Not using the platform bundles at all, only get to save some validation.
• MM: Our design is to never/almost never record commands except directly into the backend.
• CW: Jim, is this the design wgpu has now or that it intends to keep in the future?
• JB: Haven’t made a decision about what we’ll do in the future. If we can get better performance using platform APIs then we’ll do that.
• KG: I think we weren’t as concerned about direct implementability at least in our implementation. Potentially risky that we end up having a narrow fast path of success. Like if you want to use a single Metal bundle then you’ll have to be very careful not to fall off that path.
• MM: In the worst case will turn N commands into 2N commands. But don’t know whether that’s bad or not.
• MM: I also don’t want to snatch defeat from the jaws of victory here. If the group wants to do splitting then that’s a positive outcome. Just worried about removing the ability to implement using Metal bundles.
• CW: Feels we need a performance-best-case “linter” for WebGPU.
• CW: Reason I feel implementation should do splitting is, say you’re bundling a bunch of objects, in the common case they’ll all end up using the same depth stencil state. But developers won’t worry about it, and then at some point it’ll come to bite them that they wanted to render one object that had a different depth stencil state.
• MM: The natural reaction to solving that problem would be to put additional state into the render bundle descriptor. Not sure exactly what to put there, can’t put a pipeline state, but would be natural.
• KG: Design-wise, curious if we could have the render bundle API better “show” you the fast path. You might naively think it’s free to change the depth-stencil state in a bundle when it’s not.
• RC: With regard to D3D render bundles, I’ve been told that they typically don’t give you the performance you expect. Sometimes get better perf from replaying commands in userspace.
• RC: Babylon.js team really really like WebGPU bundles. The times they’ve been able to organize a whole scene into one or a few bundles, since they’re very JS/CPU bound, having the browser cache that and replay that gives better performance than them replay (and revalidate) all of those commands. So, they would really just like all capabilities in bundles, not limited to what platform APIs can do.
• JB: Sounds like it’s mostly a JS win.
• (wide agreement)
• MM: I think you’ve changed my mind, we should allow them to put a whole scene into a render bundle.
• KG: The cost of JS bindings is very different from e.g. D3D, where the API overhead is small, and you’re expected to be doing things in a multithreaded way anyway.
• CW: The ANGLE team was interested in using secondary command buffers in Vulkan. But it turned out to be a performance loss in most cases.
• CW: They also use a soft command buffer internally, that they replay on submit. They find this faster, for reasons that aren’t clear. Possibly icache and dcache are better split because not churning as much between ANGLE code and Vulkan driver.

Items from Editors’ Meeting that might be too substantial to discuss on short notice

• Frame advancement of canvases that aren't visible #3295
• mapAsync(WRITE) zero-fills the relevant region of the buffer #2926

Agenda for next meeting