Skip to content

Minutes 2019 04 01

Jump to bottom
Corentin Wallez edited this page Jan 4, 2022 · 1 revision

GPU Web 2019-04-01

Chair: Corentin

Scribe: Ken

Location: Google Meet

TL;DR

  • Next F2F will be hosted by Google contrary to what was announced before
  • Bind group inheritance #171
    • Consensus to not have Vulkan-style bindgroup invalidation rules but have persistent bindgroups and validate layouts on draw/dispatch
    • Mostly agreement to use by-value semantics for bind group layout compatibility
  • Chromium has a prototype integration of WebGPU that can do things
    • Discussed feedback from trying the API in JS: need a way to upload ImageBitmaps, will discuss it in two weeks.
  • PR Burndown
    • #227 - use inheritance for debug labels and add them to descriptors
      • Approved with nits
    • #241 - Default values for most dictionary fields
      • Approved with some more contentious defaults removed
    • #249 - Vertex inputs: push-model with sparse arrays
      • Approved with a naming change
    • #256 - Replaced “sample-texture” with “ro-texture”
      • Consensus to not do rw/ro-texture. DM will open a PR to discuss changing sampled-texture to just texture.
    • #262 - Move SwapChain methods from GPUDevice to GPUCanvasContext
      • Approved with naming change.

Tentative agenda

  • Bind group inheritance #171
  • <your topic here>
  • PR burndown
  • Agenda for next meeting

Attendance

  • Apple
    • Dean Jackson
    • Justin Fan
    • Myles C. Maxfield
  • Google
    • Austin Eng
    • Corentin Wallez
    • Dan Sinclair
    • Kai Ninomiya
    • Ken Russell
    • Shrek Shao
  • Microsoft
    • Chas Boyd
    • Rafael Cintron
  • Mozilla
    • Dzmitry Malyshau
    • Jeff Gilbert
  • Joshua Groves

Administrative items

  • CW: Mozilla can’t host the F2F so Google will.
  • DJ: Same for WebGL?
  • KR: Yes, should have both WebGL/WebGPU question is in which building it is.
  • KN: Might be in Sunnyvale in the worst case. (dino - wow, burn on Sunnyvale)
  • CW: can we land Dean’s Bikeshed spec so we can rebase on top of it?
    • KR: we should land it.
  • DJ: I’ll check to make sure nothing came in since my latest push, then land it. Sketch IDL will be replaced with a one-line redirect. In-progress pull requests might need to be rebased.
    • CW: sounds good.
  • CW: would everyone be comfortable taking a snapshot during the next F2F?
    • JG: let’s talk about it at F2F.
    • KN: we need a lot of PRs reviewed and landed before then.

Bind group inheritance #171

  • CW: If pipelines 0/1 match, then pipelines compatible for them, and only BindGroups 2,3 are invalidated.
  • DM: on pipeline change: the descriptor sets are invalidated when you bind descriptor set. If you bind 2, everything past 2 is invalidated.
  • CW: correct. I’m wrong. For simplicity, on draw operation, we check that layout of currently bound descriptor sets match currently bound pipeline. Not 100% optimal, but whatever.
  • DM: would help Metal in particular. If Metal uses arg buffers they don’t have the same invalidation constraints as Vulkan. We use it in gfx-rs and it works well so I think this new proposal would work well.
  • CW: I think it makes sense. Was originally concerned about validation cost.
  • DM: my proposal is we just compare the bind group IDs. No forced deduplication.
  • CW: by-value or by-pointer? Is everyone not doing the inheritance the Vulkan way?
  • KR: what’s the impact on application code?
  • CW: will make validation less stateful. Apps can just set bind group 3, or 0, and assume the others are still bound.
  • KR: Does it mean that if you have two BGL that are the same by value but not by pointer that validation fails?
  • CW: that’s the thing we might disagree about. I think it should be by value. If middleware implements its own bind groups, then you don’t need to deduplicate between different parts of app. Really, that caching is trivial to do.
  • DM: I don’t quite see that. Point of BindGroup is that diff layers can be responsible for diff things. BindGroup / BindGroupLayout can be used and you don’t have to worry. Think it just complicates the logic for no reason.
  • CW: if the BGL specifies a C structure, if you have 2 structure definitions then they’re compatible, right?
  • DM: you don’t bake this definition anywhere in C.
  • KR: What if one library knows a bind group layout but doesn’t expose it, how can an application specify that bind group layout?
  • DM: if the library’s written for apps that are supposed to provide data, and doesn’t provide the BGL, then it’s just a bad library. I don’t think trying to guess what the library binds is the right solution for user code.
  • KR: How about debugging and introspection libraries that want to tweak inputs to program but can’t because they can’t get the baked thing?
  • DM: if they intercept the calls they’ll know what the BGLs were when they were created.
  • CW: what’s your concern with having things by value? Is it that the spec has more text? Don’t want to do deduplication in your impl?
  • DM: I want us to have simpler internals. A unique ID lets you work more efficiently on the client side.
  • CW: on the client side they can be 2 different objects. Looking at your GPU command buf equivalent, app makes 2 BGLs, 2 different JS objects, and can use them interchangeably. On GPU service, both handles associated with same object.
  • DM: I see what you mean. Have we got any evidence that this will make anyone’s life better? Is it needed for anyone to do this?
  • CW: the Vulkan spec says it’s by value.
  • DM: descriptor set layout might not match.
  • CW: two DSLs that are created the same way must match. Only one way to get the layout, whatever it is. Seems like artificial limitation. No allocation with a layout (except std::vector for something). No GPU memory address associated with it. No drawback to deduplication.
  • MM: I wish I could tiebreak but have no opinion.
  • DM: you are probably right that Vk treats it the same. With context of where DS is, they’re the same only at the same index of the DS. Do you want it to be the same when they’re not the same indices?
  • CW: when you make a DS in Vk, you don’t specify the index you’re going to bind to.
  • DM: but that DS doesn’t make sense if anything earlier different is bound.
  • CW: the compat is always between the DS and its index. Pipeline layout / BindGroup layout are different at different indices. We don’t care about compat between #1 and #2 in the validation rules. Layout, if you specify it twice, is the same layout. There’s a set of all the layouts, and you get a handle to that unique argument.
  • DM: seems like a compelling argument to me. We can go with that.
  • CW: ok, so we settle for deduplication.
  • DM: agreed.
  • KR: I could have sworn we talked about this earlier and settled on the deduplication rule. So good.

Chromium’s F2F hackathon Results

  • CW: made a Chrome fork and hacked on it until we had something working on Mac. Working with TF.js and other teams to see what it looked like and what their feedback was. Got some good feedback. Will be spending more time to land it.
  • KN: showed couple of demos; ComputeBoids and a spinning cube.
  • CW: SwapChain gets copied into texture at the end. Based heavily on Justin’s RotatingCube example.
  • KR: Wanted to talk about this because we had a number of issues raised by the customers around data upload / download, also ImageBitmap.
  • KN: Agreed to start with only ImageBitmap.
  • KR: Best way forward probably because image decode is already async so most efficient both. Other question is whether we want to have the equivalent of TexSubImage2D.
  • CW: Will be good topic in 2 weeks. Uploading from browser images is a missing area.
  • MM: fine for now but eventually we’ll want more ways to upload image data.
  • KN: video, definitely. But I don’t think we want too many different ways because most of them can funnel through ImageBitmap.
  • MM: interesting concern, ImageBitmap usually exists on GPU already. Uploading ImageBitmap to GPU is usually not an upload. Pushing the expensive cost into the ImageBitmap which might not be portable performance, or what we want.
  • CW: trying to understand the concern. Create Texture from ImageBitmap, single layer, etc., but it’s in GPU memory already?
  • MM: think it’s fine. Just don’t want to say it’s an upload because devs may do the wrong thing.
  • KN: it might be an upload. Potential cost there.
  • JG: it’s also tricky if we want to support selecting which GPU you want to run on. Might be tricky. Might expect it to be uploaded to the GPU already.
  • CW: you can view ImageBitmap like an IOSurface. On all the GPUs and the CPU at the same time. Browser will put it in the right place.
  • JG: that is the dream. Not making promises about that. Might be a problem.
  • RC: I suppose we’ll spec ImageBitmap so its semantic is, it’s uploaded too?
  • CW: the semantic is as if the ImageBitmap is copied. It’s an impl detail if it is copied. Right now Imagebitmap is read-only usage. Copy-on-write in the browser.
  • RC: yes, that’s my point.
  • DM: have texture usage. If you create Texture from ImageBitmap I assume it wouldn’t be modified.
  • JG: for videos we’ll need a way to sample textures that you can’t modify. Feels like that’s what we’re running against in WebGL. Fastest way to upload data is to say, the browser already has the data in some form. One problem with WebGL entry points is you say, I want it in this format. Have to convert to that, maybe. Might be worth trying saying, we provide you with a resource you can sample from.
  • MM: so make it not a texture?
  • JG: a read-only texture?
  • CW: you could have ImageBitmap, give me WebGPU texture for it, gives you texture along with format and usage. My suggestion would be to create a SwapChain for it. If you don’t use what the browser tells you to, it would be slower.
  • MM: probably important that set of usages would be between different browsers. Want to avoid situation where there’s content that runs in just one browser.
  • KR: I’ll volunteer to put something together with help from Kai, Austin, Corentin or some subset thereof.

PRs

  • CW: let’s skip getSubData
  • #227 - use inheritance for debug labels and add them to descriptors
    • KN: surprised if we have any comments on this.
    • DM: does dictionary inheritance map to wasm?
    • CW: no. This is JS dictionaries. Go from WASM structure to dictionary in Emscripten. C structure in Dawn: add “label” to all structures internally since you can’t do any inheritance in C.
    • DM: why do we have DescriptorBase?
    • CW: that contains the label.
    • JG: so does GpuObjectBase.
    • RC: there’s one at the end.
    • DM: partial dictionary, wow.
    • CW: to note: this adds 2 descriptors for compute pass, can also have descriptors and also have names.
    • RC: where do you pass GpuObjectBase to?
    • CW: it has an optional label. DOMString is put in ObjectBase. Can modify it later.
    • MM: can we make this easier to read?
    • CW: OK. I will make it look nicer and we can discuss it more once it looks nicer.
    • MM: no change in functionality right?
    • CW: it adds labels to descriptors.
    • MM: sounds like a great idea.
    • KN: I think we should just merge it after cleanup.
    • CW: let’s have Dean do the sketch IDL and then merge it.
  • #241 - Default values for most dictionary fields
    • MM: did my homework - updated it. Think I did all the feedback.
    • DM: the texture format is still default. Wasn’t that supposed to be fixed?
    • MM: did I miss something?
    • KN: make everything required that we hadn’t agreed upon yet. That includes default format, and a few other things. 6 or so comments on the most recent version of the PR.
    • CW: sounds like only minor concerns on this one. Let’s remove the last defaults that people have discussions about, land most of them, and then discuss default by default. Seems like it’s almost there. 4-5 members still being discussed. Can you make a list of members?
    • MM: yes.
    • DM: any thoughts about clearDepth / clearStencil semantics? Suggested to make them optional.
    • KN: would like to see a PR for that after Myles’ lands. In favor of it, as mentioned on Github.
    • DM: will make one them.
  • #249 - Vertex inputs: push-model with sparse arrays
    • KN: any comments?
    • CW: basically what we discussed. Uses sparse arrays / sequences.
    • JG: why is one a sequence of nullables and the other sequence of requireds?
    • KN: attributes has shader location in descriptor.
    • DM: doesn’t matter - that’s why it doesn’t have to be optional.
    • RC: difference between attribute sequence and shader location?
    • KN: location in attribute sequence doesn’t matter.
    • JG: can we call it “attribute set”? Nice to have some indication. There were 2 of us confused by this being a sequence.
    • KN: good point.
    • RC: shader location is what then?
    • KN: it’s the “layout location=” in GLSL. Semantics in D3D. Shading language binding.
    • RC: offhand this seems fine. We’ll have to validate that all shader locations are accounted for, or you get e.g. zeros, or it’s invalid.
    • CW: pipeline validation rules will be that you have to have one attribute for every vertex input in the vertex shader.
    • KN: I think so too.
    • RC: offset offsets into the vertex buffer?
    • CW: vertex buffer stride.
    • KN: offset has to be less than the stride.
    • CW: sounds like no concern except naming of attribute -> attributeSet.
    • KN: OK. I’ll make that change and merge it.
  • Skipping #253. Documentation change. Do offline.
    • KN: need review from RC.
    • RC: I’ll review.
  • #256 - Replaced “sample-texture” with “ro-texture”
    • KN: suggested by Myles a while ago. Got lost. Closed old one, opened this one.
    • MM: Think I was just confused by the names.
    • KN: opened new PR to discuss the naming convention.
    • JG: seems strange to me that rw vs. ro is store vs. sample. Generally, RW surprising that it’s not just a RO texture + writes.
    • CB: they use a different swizzle pattern.
    • CW: also some metadata plane like “clear” or “textureCompression” that wouldn’t be present in the UAV case.
    • KN: I think I’m in favor of old name, sampleTexture and then storageTexture.
    • DM: I would rather not have that. Texture for that which 95% of people use. StorageTexture or WriteableTexture for the thing that others need.
    • CW: I buy that. It’s a bit weird that you spec something as a texture but not all textures can be used there. Ways to use the texture in non-texture ways.
    • KN: think that’s pretty minor.
    • RC: different kinds of textures? Put in name of what it’s for. “Texture” and then a whole bunch of other ones, which is the most / least powerful?
    • MM: GLSL has this with Barriers. “Barrier” + 17 modifications of it.
    • CW: sounds like you’ve had to deal with that before.
    • JG: prevailing opinion is to reject this PR?
    • CW: think people agree to do so. Last question is sampleTexture vs. roTexture. Think I agree with Rafael. Can discuss more.
    • DM: it’s not the textures you provide there, but the view. But don’t want to make it more complicated.
    • JG: sampled view of a texture sounds great to me.
    • DM: was referring to using same texture for 2 purposes. But they’re actually views.
    • CW: can make a PR to discuss more, but this one for sure we will close.
  • #260 - GPUUncapturedErrorEventInit is supposed to have a GPUError, not string
    • KN: just an error. GPUError is a unification of two interfaces designed to give you info about an error.
    • JG: let’s review this one offline.
    • CW: OK, but please do.
  • #262 - Move SwapChain methods from GPUDevice to GPUCanvasContext
    • CW: only reason for this: when making the samples, was not super clear to Francois that creating a new SwapChain on the canvas, that previous one was detached. By putting swap chain on context makes it clearer.
    • JG: shouldn’t it be replaceSwapChain?
    • CW: maybe. But replace when there’s no swap chain seems weird.
    • JG: addSwapChain? Would be nice for name to indicate replacement. “Set”. “Update”.
    • MM: “Configure”.
    • DM: fine with Configure. In Dawn do you already have Configure?
    • CW: they’re really specific now. No Dawn equivalents. Can discuss WGPU and Dawn convergence offline.
    • Silent yes agreeing to this.
  • CW: remaining: ArrayBuffer GC issue, and GPU event constructors, maybe another one.
  • CW: great work. Got backlog under control

Agenda for next meeting

  • CW: next week: programming model for WebGPU. Issues which David and Robin worked through together. More specifics?
  • DS: For shader stuff, David and Robin were working through lots of stuff related to memory models. Both need to be here.
  • CW / DJ: let’s discuss with Robin to set more precise agenda.
  • CW: for week after: there’s ImageBitmap thing we need to work through. getSubData / setSubData. We should revisit discardStoreOperation. Seems important and we don’t have it.
    • KN: should I request that we take the default out for StoreOp?
    • CW: yes. Imp’t for apps to know what they’re requesting.
Clone this wiki locally