Skip to content

GPU Web 2020 04 06

Jump to bottom
François Daoust edited this page Dec 1, 2020 · 1 revision

Chair: Corentin

Scribe: Ken / Austin

Location: Google Meet

Tentative agenda

  • PR Burndown
    • Add callback entrypoints for per-frame Promise entrypoints. #626
    • Add pipeline shader module error enumeration. #646
    • Add validation rules for copyBufferToTexture #648
    • Add minimumBufferSize to BGL entry #678
  • Data upload #605, #649, #650** **and #680
  • Query API #614 and #619
  • Agenda for next meeting

Attendance

  • Apple
    • Dean Jackson
    • Justin Fan
    • Myles C. Maxfield
  • Google
    • Austin Eng
    • Brandon Jones
    • Corentin Wallez
    • Kai Ninomiya
    • Ken Russell
    • Shrek Shao
    • Ryan Harrisson
  • Microsoft
    • Damyan Pepper
    • Rafael Cintron
  • Mozilla
    • Dzmitry Malyshau
    • Jeff Gilbert
  • Matijs Toonen
  • Mehmet Oguz Derin
  • Timo de Kort

Administrative items

  • CW: Dean merged a change to the WG charter that addresses several comments. Charter has changed in a lot of minor ways. Would be good if people could run it past legal folks.
  • CW: Don't expect anything to be bad or changed significantly.
  • JG: do we plan on another round of revisions? Because if so it makes sense to defer legal re-validation.
  • DJ: On Apple's side only changes made were on request of legal.

PR Burndown

  • Add callback entrypoints for per-frame Promise entrypoints. #626
    • JG: consensus was needed to be part of a holistic approach to eliminating garbage from an example application. Useful to catalog how you might address this Promise-related garbage, but to want to take proposal more seriously, need to also address command buffer & encoder garbage per-frame. Until we address encoder garbage, useful to think about, but not useful to adopt. Holding pattern for that.
    • JG: I can take the AI about a no-garbage plan / investigation.
    • CW: Okay, so put it on hold for now.
    • MM: Is a new holistic approach going to share any lines of code with this patch?
    • JG: Yes.
    • CW: This one handles all the Promises, but the other part is the synchronous objects created. Seems pretty orthogonal.
    • JG: commandEncoder.reset() or similar.
  • Add pipeline shader module error enumeration. #646
    • CW: there was a question about to discuss this in this call or the WGSL call.
    • JG: It overlaps somewhat, and it depends on your philosophy about what the WGSL calls are about. This one is going to be an API entrypoint on the API side, so we should at least touch on it here. It matters on the whole API. The idea is to introduce some amount of shader module reflection, or details about why something failed. ex.) Could bubble up into ShaderToy and highlight why something failed.
    • MM: what's the relationship between this way of getting error messages and push/pop error scope?
    • JG: In OpenGL, the concept of shader compilation errors is separate from API usage errors. This is where the idea came from. It didn’t seem like it fit well with the push/pop error scope -- it would have to introduce a bunch of extra details. A Description message would be insufficient unless we specified the grammar of the error message. Would be better to have specific compilation messages that have attributes for line, etc.
    • CW: one way to get API error messages, and if you get an API error, it's pretty clear where it's coming from. For shading language, makes sense to have a very good error handling story / developer experience, as it's a major part of the WebGPU standard.
    • JG: in production we expect people to compile shaders / use pipelines. If they check those are complete, that should be done not serially with compiling. e.g. not "compile / check if error / compile / check if error". Should be possible with the push/pop error scope because that’s async, but feels less natural
    • MM: does that argue against push/pop?
    • JG: internal to Gecko's WebGL impl, we use "error stack" concept.
    • KN: in regard to error scopes for pipeline creation: have been thinking about taking that out of error scopes. Haven't thought about it too deeply. Pipeline creation is not exactly a GPU operation. Doesn't make that much sense to tie it to the pipeline.
    • CW: Vulkan RT added a concept of deferred operations. A thing the driver has to do but could take a while. Pipeline creation and building acceleration structure for RT are examples that do that.
    • RC: I see CompilationMessages is on GPUPipelineBase. Does that mean you need to get back the created pipeline to see them?
    • JG: you do always get back a created one. Might have to make a dummy one. I did intend to change this based on offline feedback. Thought it was better to put the compilation errors on the ShaderModule. Think there's stuff we care about in pipelines for linking, so analogous to WebGL's compile / link statuses. Expect PR that has entry points on both Pipeline and ShaderModule.
    • CW: Especially because in WebGL, there’s a single entrypoint, but WebGPU could have multiple which is selected at pipeline creation.
    • RC: if I get a pipeline back, and it has errors, it's an error to use the pipeline to do things with the API?
    • CW: Yes, it is an error object.
    • RC: do Pipelines return a Promise?
    • KN: not currently.
    • RC: possible alternative, Pipeline could be a Promise
    • MM: That actually makes a lot of sense to me.
    • KN: I don’t think it makes sense for us to force all pipeline creation to be async. Sometimes you want to make one and use it immediately without caring it may stall. We do want to have an async path though.
    • CW: maybe. Maybe you don't actually care. Maybe if you use the synchronous path it means you intend to use the pipeline this frame and don't have time to wait for the error messages. For the async case you could wait for the Promise to reject with the error messages.
    • KN: I guess the other question is: What’s the point of returning error messages in a structured way? Why shouldn’t they just go out to the error scope or uncaptured error callback?
    • CW: Good because tools like ShaderToy would surface exact errors on the line. WebGPU won’t have a compiler monoculture, so it’s harder to parse errors.
    • KN: We could still have structured error messages for shaders though.
    • MM: Seems worse than either entirely structure or entirely freeform.
    • KN: structured is fine with me.
    • CW: ok, structured error messages good. Do we want it on ShaderModule, Pipelines or both?
    • JG: I'll put it on both and if we don't want them then we can take them off.
    • KN: Little iffy on both for the same reason that OpenGL doesn’t define exactly.. don’t want to be too assumptive of the implementation which could have parsing differently. May happen on linking or not.
    • MM: there could be two places but no rule about which error comes from which place.
    • JG: in WebGL, best practice is to compile, but not check any errors until linking, because some impls can defer all compiles until linking. But: we have to parse comments at shader source input time. I'm generally in favor of deferring things until one true place when they're definitely wrong.
    • KN: It does seem unnatural, but if you think about the use case, I don’t know if there’s value in knowing if it’s on the ShaderModule or the Pipeline.
    • DM: you know it much earlier
    • KN: but it shouldn't happen in production.
    • DM: we're taking about the ShaderToy case, it's not "production".
    • KN: ShaderToy will have to create pipeline anyway.
    • JG: When it’s attached to the shader module, you know it’s coming from there exactly. We would have to add more structure so to specify which stage it failed to link for.
    • RC: I presume we'll only have things on the ShaderModule that are errors when they compile by themselves? Not things related to shader matching errors?
    • KN: correct.
    • JG: Think there’s a pseudo proposal to not have compile errors, and only link errors. At pipeline compilation, you would know your ex.) vertex stage failed to compile because xyz.
    • KN: ...
    • JG: I'm sympathetic to that. PR today already reflects that.
    • CW: What happens if I have a shader with a syntax error, but I also have a depthStencil format for colorAttachment? Which takes precedence? Do I get to see the compile errors even if the other descriptor members are bad?
    • JG: good question, I'd generally defer to whatever error you find first, in an undefined order.
    • KN: should probably be like every other error. We do return them through 2 separate channels, so a little weird. We give you two strings, undefined which you get. Now we have structured error & string. Do we say that you only get one of them?
    • MM: Would be nice if there were only one channel/
    • DM: It’s tempting to say pipeline linking errors are structural instead of just returning a string. We can say all are interface errors and we can define what they are. Would be better than an arbitrary string.
    • JG: worried those aren't actionable. Generally API errors are not considered actionable by the app. App should generally only generate out-of-memory or context lost errors. Diff here: we want to surface these to DevTools, at least, or other tools between user app and the API.
    • CW: I’d go further and say we’d want to support some shader authoring tool in the browser. Engines like to change materials on the fly. The ability to compile and receive a result is important for this use case. ShaderToy is a simple use case.
    • RC: looking at popErrorScope, we already have GPU memory & validation errors. Should we make this a new kind you get from popErrorScope? Then we can go through the same pushing/popping currently.
    • KN: It’s a possibility. It would mean there’s only one channel. A string or a structured error.
    • JG: it's like "1.5 channels" - one entry point, but different outputs you have to deal with in different ways.
    • MM: The semantic difference is that with the current proposal, these errors don’t have to be well-nested with pushing/popping. There is some semantic difference, not sure what it is. But there is value in having one channel.
    • JG: as soon as you start returning different types to the same entry point, it's not really the same entry point in my opinion. Seems cleanest to do a Promise full of Errors because this will always be a cold code path.
    • MM: not sure a JS dev will agree that because things are different types they’re different entry points.
    • KN: the reason I like it coming through one channel isn't because of handling it separately, but because you then can't get both errors at the same time.
    • JG: In this proposal, you would get an error that says pipeline creation failed, and in response to that or without checking, you can get a Promise asking for the errors.
    • KN: right. Makes sense to me. I'd like to pick some behavior for what happens when there are both pipeline creation errors from descriptor, etc. Can say you only see one of these.
    • MM: there is another difference: after you’ve popped an error scope, those errors are gone and can no longer get them. If this is a Promise on the pipeline, you can ask for the error message later. That may be valuable.
    • CW: yes, okay. Seems lot of discussions / thought that needs to happen offline. How about we do that and move on to next PR.
  • Add validation rules for copyBufferToTexture #648
    • CW: Many iterations on this. Jiawei has identified a number of issues that need to be resolved before validation can be completed. Editors, would you like to land as is and open issues to track each? Or resolve before landing?
    • KN: Depends on how quickly we can resolve. If we can just agree then doing that first would be great. I don’t remember the exact list of things.
    • KN: some concerns: bytesPerRow==0, rowsPerImage==0. Others?
    • CW: copy is empty, 1d and 3d, depth/stencil.
    • KN: 1D and 3D are mostly handled now. There's an issue for that anyway. PR already has issue for 1D/3D, and depth. Maybe we can agree on rowsPerImage / bytesPerRow==0 behavior now.
    • CW: OK. rowsPerImage==0: would be texture height?
    • KN: at least the texture height?
    • CW: yes. Or the copy size (?) height.
    • KN: does rowsPerImage==0 mean, rowsPerImage is the copy height, or it means it's 0 and only works if number of images / depth is 1?
    • CW: ==0 is a special case, it's in the spec already.
    • KN: that's what I proposed. The question is whether we keep it. ==0 would not be special any more - stride would be 0 and it would be illegal if it overlaps a second image.
    • JG: But it’s if and only if. If you have a depth of 1, then the imagePitch has to be zero
    • KN: that's another question.
    • JG: is it? You might have N slices where N==1, and ... (something) might be invalid operation.
    • KN: I wrote that and agree that it's wrong and we should lift that. We shouldn't have that rule for either bytesPerRow or rowsPerImage. Second question: if it is 0, does depth have to be 1?
    • JG: that's more reasonable - only alternative would be that it has to obey all of our other restrictions on pitch. Zero would have to be implicit rule that it's this other valid thing.
    • KN: exactly. I propose we make 0 not-special. Allow 0 values if depth==1. Should bytesPerRow follow the same rule?
    • JG: I wonder if you can spec this the other way. Subsequent rows & images can't overlap. Then 0 immediately becomes valid if you don't have more than one slice / row. That's what we're trying to spec. If we said that, then if you pass that restriction, you also have to follow alignment concerns.
    • CW: ok, bytesPerRow==0 only valid if copyHeight==1.
    • KN: yes.
    • JG: that behavior is good. Would be best to define non-overlapping behavior and have this be fallout.
    • CW: second point then is an empty copy size allowed? I think yes, because validating that copy size is empty is the same amount of work as making it a no-op.
    • JG: I have concerns about spec'ing no-ops. Nicer to spec it as a no-op. In WebGL's draw calls, we've made draws with 0 primitives no-ops, and it was painful because we had to enumerate all the other errors that we weren't going to fire, like doing a draw incorrectly with transform feedback enabled. It was more painful to make it a no-op rather than making it a zero-length draw call.
    • CW: right. No special handling.
    • JG: e.g. if the buffer is in the mapped state and copy 0 bytes out of it, that's a validation error.
    • CW: do we allow copying multiple layers ... ? Metal doesn't' support this.
    • KN: I'm confused. Metal docs say this should work.
    • JG: can we get clarity from Metal team?
    • KN: I don't think it matters whether it works or not. Can split them.
    • JG: ok, that sounds good.
    • CW: given we're merging the array layer count with the depth of the texture, it makes sense.
    • KN: our copy rules are less restrictive than D3D12's. We can already split one copy into multiple copies.
    • JG: should you be able to copy 200 layers in one command? You'd expect we'd not do 200 copies. Do we care about this?
    • CW: no. Even in Vk you can copy many sub-resources and the driver will iterate through them and do its thing.
    • JG: concerns from Apple?
    • JF: I'd be OK emulating it.
    • KN: I don't think emulating will actually be necessary.
    • CW: points 4 and 5 need more investigation. Can have them as issues in the spec until we have investigations.
    • JG: has anyone seen 1D textures used?
    • DP: they get used to do function lookups.
    • MM: our ML team uses them. Loading from texture cache hierarchy is better for their access patterns than from a buffer.
    • JG: Are there any people using 1D textures today who would not be fine using 2D textures that are actually 1D?
    • CW: I think the size limit is larger for 1D textures.
    • DP: They can possibly provide more optimal sampling, but I’m just guessing at that.
    • DM: could have a different layout for placement of texels. I use it for palettes in a project.
    • RC: have seen gradient folks use them too.
    • CW: so we need 1D textures.
    • JG: I don't think that's the takeaway. People using them because they happen to be around isn't interesting. You can do this with 2D textures.
    • MM: one diff is that you can wrap the memory around in buffers with 1D textures, and you can't do that with 2D textures.
    • DM: we discussed that in the past and you guys decided we didn't need that.
  • Add minimumBufferSize to BGL entry #678
    • DM: People are expected to use a lot of uniform buffers. Most content is in fields. We were thinking about ways to avoid doing bounds check on every field access. We can get it simply by the user providing a minimum size that has to cover the uniform buffer layout. This is an attempt to ask users for more information for us to do less runtime checking in the shaders.
    • CW: another alternative: do run-time checks on draw calls. Check size that the pipeline needs vs. size bound in buffers in bind groups. Also run-time work on the CPU rather than GPU.
    • DM: Both approaches would discard work trying to access those fields. Right now we have the model where you can access anything but it might be clamped or zeroed. Whatever way we pick to disallow these cases could potentially give faster shader code.
    • CW: would be great if we can get away with minimum buffer size in BGL. Worried how practical it is to ask that from applications. Not something they've ever done before. May put constraints on design of asset pipelines.
    • MM: is the idea you check the min buffer size in BGL at the time the pipeline is created? So checking isn't done at draw call time?
    • DM: correct. Two checks. One at BG creation, one at Pipeline creation. No draw call checks.
    • CW: at PL creation we check that it needs less than the minBufferSize and at BG creation time we check ti provides at least the minBufferSize. So that at draw time we know the buffers always have enough buffer space for what it might access.
    • MM: Okay, I understand.
    • CW: these two constraints together mean...
    • DM: OK, I understand.
    • JG: would we change how we compile shaders based on that?
    • DM: we don't recompile shaders.
    • JG: if you know it's always in bounds you can remove access checks.
    • DM: when shader's compiled it assumes you can always access all the fields.
    • CW: the way this PR is phrased, the min buffer size is a hard constraint. Can guarantee that all the buffer accesses except for some very specific ones are in bounds. No two different ways to compile pipelines.
    • JG: I think I need to think more about this. Would change how you'd compile the ShaderModule, and what checks you can elide.
    • CW: for each ShaderModule you'd know which checks are needed and not.
    • DM: If it's not an array field you know that no checks are needed. Fixed size array - add check if index is dynamic. If unsized array at end of uniform struct, then dynamic checks.
    • CW: I think that's only at end of shader storage, but same thing.

Data upload #605, #649, #650** **and #680

  • CW: has been some discussion, new PRs and investigations into what engines do. Let's keep discussing offline, hopefully reach agreement by next week.
  • CW: I looked at Godot open-source engine and Unreal Engine. They don't use subrange mapping, essentially. Godot has exactly the ring buffer of buffers. Not sure I can talk about Unreal, but they essentially map buffers as a whole.
  • DM: so you mean they don't use parts of the buffer on the GPU at the same time. Mapping the whole buffer is common for persistent mappings, but we don't have that. You mean they don’t simultaneously use the persistent mapped buffer for something else.
  • CW: Yes, that.

Query API #614 and #619

  • CW: no update since last week, so skip it.

Agenda for next meeting

  • CW: Data upload
  • CW: iterate on two PRs, compilation error and min buffer size
  • CW: if there's some update for query API can chat about that too.
  • DM: not sure what to iterate on the min buffer size, so please comment on it.
    • MM: I have a proposal on how to improve it, will comment.
  • From editors meeting:
    • #546 Zero-sized buffers
    • #686 Binding to zero-sized range of a buffer
    • #652 Copying of depth24plus formats
  • From KN after editors meeting:
    • #639 Disallow storage-buffers/textures in fragment shaders in core
Clone this wiki locally