Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Timestamp queries in render/compute pass descriptor #296

Merged
merged 7 commits into from
Sep 30, 2023

Conversation

eliemichel
Copy link
Contributor

@eliemichel eliemichel commented Sep 18, 2023

This is an attempt at adding support for the timestampWrites field of render and compute pass descriptors.

Notable remarks:

  • The API for timestamp writes in wgpu-core does not match the standard webgpu.h. In particular, it does not support different query sets in the same descriptor, nor multiple queries for the same timestamp location (although I admit there must be only very limited use case for these).
  • Resolving queries that have not been written leads to crash with a lost device with no error message at all (already the case prior to this PR).
  • The writing/resolving multiple times the same query suspicioulsy returns the same value all the time in my basic experiment (doc) (running on DX12 backend).
  • I do not check that the query set has the correct timestamp type, because none of the other functions manipulating query sets does so, hence I assume wgpu-core performs the check.
  • Not sure how idiomatic/minimal my code is, I am open to suggestion and improvment of my rust style ;)

(Bonus: This PR adds support for the new gles3_minor_version instance option.)

@eliemichel
Copy link
Contributor Author

eliemichel commented Sep 30, 2023

Updated to match the new API for timestamp writes in webgpu.h (see this change).

Not sure why CI fail on MacOS

@eliemichel eliemichel changed the title [WIP] Timestamp queries in render/compute pass descriptor Timestamp queries in render/compute pass descriptor Sep 30, 2023
@rajveermalviya
Copy link
Collaborator

Not sure why CI fail on MacOS

seems like cargo having some problems with wgpu-hal dep, I tested with this rev locally & I can reproduce it.
Changing it to current trunk of wgpu works fine, so let's just change it to that. (This is fine because, this is going to be released in 0.18, when wgpu does 0.18)

@rajveermalviya
Copy link
Collaborator

Need a rebase or merge trunk

@eliemichel
Copy link
Contributor Author

Done ;)

Copy link
Collaborator

@rajveermalviya rajveermalviya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks good overall!

Just a nit, but pushing through

src/lib.rs Outdated Show resolved Hide resolved
@rajveermalviya rajveermalviya merged commit e90c214 into gfx-rs:trunk Sep 30, 2023
15 checks passed
@eliemichel eliemichel deleted the eliemichel/timestamp-queries branch October 2, 2023 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants