Skip to content

Minutes 2019 07 01

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

GPU Web 2019-07-01

Chair: Dean!

Scribe: Ken

Location: Google Meet

TL;DR

  • Three nominations for editor: Kai, Dzmitry and Justin. Proposals are still open for the remainder of the day. Corentin and Dean will discuss offline.
  • Went through all open pull requests. A handful were accepted.
  • Introduction to Myles’s binding API proposal #351 (to be continued next week)

Tentative agenda

  • PR Burndown
  • Myles’ API proposal
  • Agenda for next meeting

Attendance

  • Apple
    • Dean Jackson
    • Justin Fan
    • Myles C. Maxfield
  • Google
    • Austin Eng
    • Idan Raiter
    • Kai Ninomiya
    • Ken Russell
    • Shrek Shao
  • Intel
    • Yunchao He
  • Microsoft
    • Chas Boyd
    • Rafael Cintron
    • Damyan Pepper
  • Mozilla
    • Jeff Gilbert
  • Mehmet Oguz Derin
  • Timo de Kort

Admin

  • MM: My proposal came in late. I’m ok if you want to delay discussion.
  • KN: ok with discussing the register mapping proposal, but think there might be a rabbit-hole
  • JG: could time-box it, but could be useful to have some prelim discussion happen first on the issue and come back next week.
  • RC: I’m fine with that and/or proposing as long as we don’t make any decisions on it this meeting
  • DJ: had an AI to update the WG charter. Will be done soon after meeting.
  • DJ: have call for editors out. Have 3 victims. Dzmitry, Kai, Justin. Those three will be the three we have unless something happens in the next week.
    • KR: is it too late to make any adjustments?
    • MM: procedurally, it’s up to the chairs to determine the process.
    • DJ: Corentin and I will talk and finalize things afterward. Today could be taken as the deadline.

PR Burndown

  • Multiple Queues - https://github.com/gpuweb/gpuweb/pull/95
  • JG: I haven’t made progress on this.
  • MM: What’s the status?
  • JG: Just waiting on me.
  • DJ: Justin’s addition of reference demo #209 - https://github.com/gpuweb/gpuweb/pull/209
  • MM: licensing? Util file taken from another project
  • DJ: this is Brandon’s glMatrix. MIT or BSD licensed? Should be OK.
  • MM: this is the first sample. Creates samples dir.
  • JG: i.e. do we want this in the main repo?
  • MM: right. webgpu.io page has list of demos; none of these are in WebGPU’s space. Should we just have links to personal Github repos or post our own?
  • JG: think it would be useful to host some of our own to show best practices.
  • MM: one requirement: if there’s a sample to be hosted in this Github page, it should work on all implementations.
  • DJ: is what you’re getting at that there’s no way to detect how to do that yet with the shading language?
  • MM: either have to be separate but equal, or do some dynamic feature detection.
  • KR: or we determine the shading language in the short term and have one set of samples.
  • KN: I don’t think it’s necessary to resolve that before publishing some samples.
  • JG: think that could be good balance. Good to have some example code to show you what basic thing would look like.
  • JF: hard to do things without a shader. I have updated versions of a lot of the small demos with WHLSL shaders. That can wait.
  • JG: just because we haven’t agreed upon the two main paths for shaders, doesn’t mean we can’t use shaders. Can just point out this is a decision point for the CG.
  • KN: I don’t see a pressing need to integrate this into the repo as opposed to personal pages.
  • DJ: suggest creating subdirectories under “samples” so it doesn’t become a dumping ground.
  • KN: possible for you to publish Justin’s demo outside gpuweb group, or has to go into gpuweb group?
  • DJ: we could publish it anywhere.
  • KN: would be really good to publish and link them in that case. Nice to have parity on implementation samples page.
  • DJ: ok. So for actual request, just nits at the moment?
  • JG: seems that way. Main objection is that it creates the “samples” dir and maybe we’re not ready for that yet. My weak vote is to close this unmerged for the time being.
  • DJ: and put it in private repo and link to it.
  • JG: yes. Even maybe “gpuweb/experiments”.
  • MM: if we’re going to make new repo in the gpuweb org I don’t see much difference compared to putting it in our repo.
  • DJ: any strong thoughts?
  • RC: in a few months, when we’re super-rigorous, when we update the spec we should update the samples too.
  • KN: was going to do the same thing and suggest that we should put it in a separate repo because we don’t have that mapping yet.
  • JG: I’d argue the opposite because if they’re in the same repo they’re implicitly in sync.
  • KN: if this sample is in line with the spec as is I’d be fine with it. But we still haven’t converged with the spec in Chrome, not sure about Safari.
  • JF: no, not yet, have to sync over some changes.
  • KN: without reference impls it’s hard to tell whether this is in-line with spec.
  • JF: we’re missing features but think the demo is in line with the spec. Hard to tell.
  • MM: that’s why we made the PR; try to make us converge around an interpretation of the spec.
  • DJ: I think there’s value in putting a sample into the repo right now.
  • KN: if it turns out there are fixes needed to the demo, we can just make pull requests.
  • JF: in that case I’d like to push an update to this branch first before merging.
  • DJ: ok, we’ll wait for your update and review.
  • MM: do we have to have another discussion?
  • DJ: I don’t think so. At nits now. Review process: make sure it’s correct, then land.
  • JF: what about glMatrix?
  • JG: should be able to just include a <script> tag, linking Cloudflare CDN sources for popular libraries. Then we don’t have to host it.
  • KR: preference to including it in a third_party directory.
  • JG: preference to have the non-minified version.
  • AE: if we’re adding this sample we should add an analogous spir-v one?
  • KN: I think we should do a pull request to do feature detection. Would be nice if the API calls were the same.
  • JF: good idea.
  • KN: can’t do feature detection right now but can do user agent detection.
  • JG: can’t you just submit it and get an error?
  • JF: we don’t have errors right now.
  • JG: one takes ArrayBufferView, one takes String, but JS might convert ArrayBufferView.
  • MM: you can pushErrorScope, submit, popErrorScope.
  • JG: we don’t implement that yet. :)
  • JF: intent of demo was to match the spec but maybe not any impl yet. Could upload a version of this that doesn’t work in either one based on feature detection in the API.
  • JG: I’d prefer that rather than multiple implementations.
  • KN: I think it’ll be fairly easy to get it to work.
  • JG: pretty compelling to devs to try to get samples working.
  • DJ: OK. Justin will update PR to be compliant to spec right now. May not work in any existing impl. We can send PR if it’s wrong, but otherwise it’s encouragement to get it working.
  • JG: sounds good.
  • KN: sure.
  • Attachment Descriptor defaults - https://github.com/gpuweb/gpuweb/pull/268
    • DJ: this one’s approved.
    • KN: approved by you Dean, but not sure it’s been looked at by anybody else.
    • DJ: definitely conflicting now anyway.
    • MM: I should update this and we can discuss it next week.
  • LoadOp and Clear - https://github.com/gpuweb/gpuweb/pull/283
    • KN: don’t think it’s bit-rotted but it does block the previous one.
    • JG: I forgot about this one.
    • DJ: there’s also a reference to #284.
  • #284 Consider associating the texture with its clear color value
    • DJ: DM not here, let’s skip for this week.
  • Bits/Bitmasks - https://github.com/gpuweb/gpuweb/pull/292
    • JG: Assocated with #318. Think I have an AI to fix that one.
    • KN: two competing proposals. One not fully up to date.
    • JG: had to change “Flag” to “Flags” to match other APIs.
    • DJ: and that’s #318. #292 was original proposal, decided to go in completely other direction which is #318. Will #318 cover #292?
    • JG: yes. Once 318 is merged, we close 292.
    • KN: not actually possible to change these to plural due to conflicting definitions.
    • JG: you’d just change it to FLAG_field. “Associated with this other enum / prefix”.
    • JG: I’d prefer to close #292 after we merge #318.
  • GPURenderBundle - https://github.com/gpuweb/gpuweb/pull/301
    • JG: sounds like larger issue. Might want to talk about it as a meeting agenda item.
    • KN: also with the two people who were talking about this the most here.
    • DJ: let’s put this on the agenda for next week.
    • MM: this was mostly discussed on a call where I was on a plane. Are we going with recording in software, or hardware-backed?
    • KN: not yet resolved. That’s why we need agenda item.
    • MM: one interesting comment: at W3C web games summit I got some questions about bundle support, and executing command lists multiple times.
    • JG: possible to write that down?
    • MM: yes.
    • KN: any idea who asked about it?
    • MM: someone on the Edge team.
    • KN: David Catuhe?
    • RC: no, a peer of mine. Lead for rendering team. Babylon.js team wants this; it’s needed for their Mac samples to get better perf than WebGL.
    • MM: I’ll write a comment and we can discuss next week.
  • GPUDevice split + mixins - https://github.com/gpuweb/gpuweb/pull/316
    • KN: have people had a chance to look at it?
    • JG: not recently. Probably fine.
    • KN: it’s weird.
    • JG: could we not invert this somehow?
    • JG: ergonomically, could GPUDevice have everything on it, rather than GpuDeviceBase, and actual Device which has event targets on it.
    • MM: ???
    • KN: the way error handling works, things not captured by ErrorScope have to bubble out. Thought it would be annoying if that happened separately on every single worker. Instead, it all happens on a primary device object. All uncaptured error events bubble up. All come to the single thread. Can save it out for bug reporting. You shouldn’t have any use for this on the worker thread; just for bug reporting. This is the way the error handling doc is written. Could set it up so that there’s one event handler per worker. That’s also weird for separate reasons. In the section that’s diffed, this is discussed. ErrorScopeState is per-Device per-ExecutionContext, rather than per-GpuDevice object. Errors don’t always happen when you’re using the Device directly. If you had Buffer and tried to map it and it failed, have to report that. Also can’t have multiple error handlers on the same execution context. Need to know which it will go to. Need thing like: transfer device to worker -> copy of same device again, with all the same attachments, etc. Maybe possible, not sure. Entire thing is wacky.
    • MM: surely this is not the first time on the web platform we’ve had this problem?
    • KN: EventTargets typically aren’t cloneable. Possible to even do that?
    • MM: think this goes into how we’re going to deal with threading. Which objects transferable, etc. Would like to see that written down. Knowing that would be a requirement to move forward with this PR. Could have a discussion about that.
    • JG: should we add that to next week’s agenda?
    • MM: not unless we can write down somehting for discussion.
    • KN: I can do that.
    • MM: not sure that somebody writing down how they think should work is the best way forward. Maybe should investigate multiple options.
    • KN: I don’t think there are multiple ways this can work except maybe at the device level. Cloneable / not, transferable / not. On a per-device basis that’s pretty much the same for everything except the Device. It gets complicated there.
    • MM: another option: Metal’s ParallelRenderCommandEncoder. And if parallelism happens from command buffers, will they all have to be allocated on one thread and transferred?
    • KN: CommandBuffers are a good point and another toplc.
    • KN: there’s allocation parallelism and initialization parallelism. Would have to make a couple of objects Transferable.
    • MM: when I’m reviewing this, would be complicated to know how these 3 things interact. I should do this investigation because I’m the one concerned about this.
    • KN: how about I make a PR that says GPU buffers / textures should be Transferable. Can assume that’s the case if you want for the part that you’re working on.
    • MM: ok. So you make a PR and I’ll make one on top of it?
    • KN: yes. It’ll basically be 2 comments.
  • Texture dimensions - https://github.com/gpuweb/gpuweb/pull/339
    • MM: discussed last week. Nobody said was bad idea?
    • DJ: lots of comments from Kai and Dzmitry. DM’s updated it.
    • KN: think it’s ready to merge.
    • DJ: squashed and merged. (By Jeff.)
  • Use COPY instead of TRANSFER - https://github.com/gpuweb/gpuweb/pull/342
    • DJ: came in about a week ago from Francois.
    • MM: looks like a rename.
    • JG: pretty straightforward. Since you can only use this with COPY commands, maybe we should call it COPY instead of TRANSFER. Clear, etc. aren’t Copy commands but maybe apply to Transfer objects. I think Copy is slightly better than Transfer.
    • RC: I’m fine with it as well.
    • DJ: OK. Jeff, go for it.
  • GPUBufferBinding size optional - https://github.com/gpuweb/gpuweb/pull/346
    • DJ: another from Francois in the last week.
    • DJ: also approved. Any disagreements?
    • JG: I think it’s a little weird but whatever.
    • MM: in the one we just approved, instead of writing comment in the IDL, can we write some spec text?
    • JG: the editors would do this.
    • KN: doesn’t make sense to split the IDL block everywhere.
    • DJ: existing specs usually chunk entry points together and then have a list below.
    • KN: would need to chunk more finely grained then.
    • MM: can we agree it’s better to have prose rather than comments?
    • KN: seems to me the same thing and we can move it out later.
    • JG: MM just wants us to do it now.
    • KN: think it requires a larger refactoring.
    • MM: think you should do that refactoring.
  • Do not require rasterizationState - https://github.com/gpuweb/gpuweb/pull/347
    • DJ: another one from Francois.
    • DJ: since we have approval on this let’s accept it. And move forward with spec editing.
    • JG: I would prefer there be prose also.

Myles’ Bind Point Renaming API proposal 351

  • MM: we discussed binding models a while ago. WHLSL / HLSL have a binding model not compatible with WebGPU’s. Months ago we decided there should be an adapter in the JS API to bridge them. Pretty simple API addition. Allows author to provide map from “strings” (the semantic in HLSL / WHLSL) to the bind group. Map from parameters in shader to IDs in the WebGPU binding model. It’s optional. If there’s a semantic in the shader which doesn’t have an entry, it’s autogenerated, and there’s an algorithm. If you have source which adheres to WebGPU binding model you don’t have to do anything. If not then you can supply this mapping just for the parameters which need to be modified - or all of them.
  • JG: do we want to limit which ones you can rebind? E.g., can only rebind the ambiguous ones? Or let people rebinding things around?
  • MM: the old way means two things actually conflict.
  • RC: would it be possible to make it so we don’t have to parse these things? Make them easier to parse?
  • MM: did consider that. Not married to either way. Little explanation at bottom of “conversation”. Think this approach is better: 1) putting little snippets of HLSL as keys in this map lets us extend things better. 2) way you decide this map in the IDL is by record type, which are strings. If you want to use dict, have to make more complex API with more helper objects. 3) because there’s an exact copy between shader and this map, obvious for author which piece is mapped to which entry. This is what I think; if it’s important it not work this way I’m fine with changing it.
  • RC: didn’t think of all of those. Just opens up possibility for more interop issues. Have mini-language we have to parse.
  • MM: do you think a solution for that is change it from e.g. string to dictionary?
  • RC: would prefer it be more of a dictionary-like thing.
  • JG: reminds me of bindAttribLocation API. Would be nice if this said “this is an array of dicts” and the dictionary has binding, bind group, and name / index. Then don’t have to worry about it being a string. Seems weird to put that in keys.
  • MM: and rules about collisions.
  • JG: yes.
  • MM: OK. I’m OK with that.
  • JG: would prefer to not have to parse things in our impl. Would prefer it be an actual dictionary rather than a string. Would rather validate a dictionary than a string.
  • KR: Can we also consider putting this as a preamble on the shaders?
  • KN: HLSL already has a very strange syntax for this.
  • KN: I have comments about this too complex to discuss right now. Not about the shape of the thing but whether it’s necessary. Will discuss online.
  • JG: OK. This is on next week’s agenda.

Agenda for next meeting

  • #301 Add GPURenderBundle - needs Corentin and Dzmitry
  • Continue discussion on #351
  • DJ: is this also the due date for talking about shaders? Let’s leave this as is. Those two topics will be enough.
Clone this wiki locally