Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
This was a great example. I started this change as just removing a `console.log` that seemed like it shouldn't be there but then I noticed the code didn't actually handle the case when 'timestamp-query' doesn't exist and ended up making a bunch of changes. I didn't mean to step on anyone's toes. Here's the change though. 1. Remove `console.log` from timestamp query sample This was just filling all of memory with strings. Probably not a big deal since no one is likely to run the sample for hours but still.... Maybe add a `<pre>` spot that holds the last N queries if you want to see a history? 2. no need for `hasOngoingTimestampReadback` `GPUBuffer` already has `mapState` which tells you this state so no need to track it separately. 3. There's no need for timestampCount It's already on `GPUQuerySet.count` 4. code didn't check if timestamps are supported before adding `timestamp` section to the render pass so the sample got a validation error if timestamp-query was not avaialble. Note: you can test this with the webgpu-dev-extension. Type 'timestamp-query' into the "Block Feature" field and reload the page. 5. Refactor not to use `readAsync` This seems like a problematic API. You call `readAsync` and if the map is pending then your callback is ignored. Ideally, if a callback was the right API design, then every callback you pass should get called and if it's pending then it would need to add your callback to a list of callbacks to be called when it's ready. Further, the timestamps passed back are only valid during the callback. If you tried to keep them they'd magically disappear as soon as your callback exited since they're directly the mapped buffer which will be unmapped as soon as you exit. Further, I'd argue for this sample at least, all you want is the last timing. So, changed the API to just give you the last times. No callbacks. 6. It's not clear what TimestampQueryManager was really managing. It wasn't adding the timestamps for you. You had to add them yourself and pull out the timestamp query object from internal fields inside TimestampQueryManager. Further, because you were adding them yourself you'd have to check yourself, if queries were supported then add them. Then, beacuse you were adding them yourself it was up to you to choose start and ends indices. It was also passing out BigInt but BitInt is hard to use correctly. I'd argue you don't need it. IIUC, Number.MAX_SAFE_INTEGER is 104 days of nanoseconds so it's fine to convert and give the user something easy to work with. So, I changed it to take a pairId and then use that to set both start and end. That way it can auto-subtract the pairs and give you Number. The user doesn't have to deal with BigInt. Other: I considered even getting rid of pairId and have `addTimestampWrite` just auto increment the indices to use. It would then reset to 0 when you call `update`. With that, it could easily add up the times and give you the total time across passes. That way you wouldn't have to manage the pairIds yourself. On the otherhand, if you wanted individual pass timings you need to know which pass got which pairId. It could pass the pairId back from `addTimestamp` but I was less sure about that change. Anyway, I hope this is considered an improvement.
- Loading branch information