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

fix: Read environment variable configs in hello_triangle example #6698

Merged
merged 3 commits into from
Dec 13, 2024

Conversation

fryeb
Copy link
Contributor

@fryeb fryeb commented Dec 11, 2024

Connections
The hello_triangle doesn't support switching backends, leading to confusing errors like this one: #4247

Description
This adds support for the WGPU_* environment variables to the hello_triangle example, so that backend-specific issues can more quickly be narrowed down and fixes.

Testing
I've tested this manually, not sure if there is a better procedure for automatically testing these things

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
    • --target wasm32-unknown-emscripten
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@fryeb fryeb requested a review from a team as a code owner December 11, 2024 01:41
Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

LGTM, pending accidental .lock changes

(Reminder: please re-request a review from me once the changes are addressed to make sure I see it!)

Cargo.lock Outdated Show resolved Hide resolved
@cwfitzgerald cwfitzgerald self-assigned this Dec 11, 2024
@fryeb fryeb force-pushed the fix/configure-hello-triangle branch from 69b4842 to d71abfa Compare December 12, 2024 22:04
@fryeb fryeb requested a review from cwfitzgerald December 12, 2024 22:16
@fryeb
Copy link
Contributor Author

fryeb commented Dec 12, 2024

I've removed the accidentally-queued lock file stuff. Might be worth adding another utils method for getting the whole instance descriptor, but we'll see. Not worth blocking this quick fix imho

@fryeb
Copy link
Contributor Author

fryeb commented Dec 12, 2024

I had a spare couple of minutes so I factored the whole thing into the utilities package, hopefully that makes the examples a little more succinct.

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Now that's chefs kiss

@cwfitzgerald cwfitzgerald enabled auto-merge (squash) December 12, 2024 23:47
auto-merge was automatically disabled December 13, 2024 00:36

Head branch was pushed to by a user without write access

@fryeb fryeb requested a review from cwfitzgerald December 13, 2024 00:37
@fryeb
Copy link
Contributor Author

fryeb commented Dec 13, 2024

Apologies for re-requesting again. Just had to fix the clippy error

@cwfitzgerald cwfitzgerald merged commit 3918a09 into gfx-rs:trunk Dec 13, 2024
27 checks passed
@cwfitzgerald
Copy link
Member

No worries!

@fryeb fryeb deleted the fix/configure-hello-triangle branch December 13, 2024 02:42
kentslaney pushed a commit to kentslaney/wgpu that referenced this pull request Dec 16, 2024
…-rs#6698)

* fix: Read environment variable configs in hello_triangle example

* refactor: Add util method for creating entire instance descriptor from env

* fix: Fix clippy error/warning about "unneeded `return` statement"
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