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

Update wgpu dependency to 0.19.0. #567

Merged
merged 1 commit into from
Feb 4, 2024
Merged

Conversation

kpreid
Copy link
Contributor

@kpreid kpreid commented Jan 17, 2024

Checklist

  • CI Checked:
    • cargo fmt has been ran
    • cargo clippy reports no issues
    • cargo test succeeds
    • cargo rend3-doc has no warnings
    • cargo deny check issues have been fixed or added to deny.toml
  • Manually Checked:
    • relevant examples/test cases run
    • changes added to changelog
      • Add credit to yourself for each change: Added new functionality @githubname.

Description

Updates wgpu to the new release 0.19.0.

  • Updates wgpu-profiler to match.
  • rend3_gltf::util::format_components() is now implemented by calling wgpu::TextureFormat::components(), which seems to be the same function.

@kpreid
Copy link
Contributor Author

kpreid commented Jan 18, 2024

Incorporated updating wgpu-profiler.

@kpreid
Copy link
Contributor Author

kpreid commented Jan 20, 2024

It occurs to me that there's a subtlety here — with wgpu 0.19 no longer having the webgl feature imply disabling WebGPU, this means that as long as rend3 depends on wgpu with default features, it'll be impossible to compile rend3 applications for web without using web_sys_unstable_apis. So, I think this change will also have to disable default features on the wgpu dependency, or it'll put users in a bad situation. (Unfortunately, this means that users will have to think about wgpu backend feature selection explicitly…)

@kpreid
Copy link
Contributor Author

kpreid commented Jan 24, 2024

This should be ready to merge, in principle. Tests don't pass locally, though:

---- animation::test stdout ----
thread 'animation::test' panicked at /Users/kpreid/.cargo/registry/src/index.crates.io-6f17d22bba15001f/env_logger-0.10.2/src/logger.rs:479:14:
Builder::init should not be called after logger initialized: SetLoggerError(())
stack backtrace:
...
   4: env_logger::logger::Builder::init
             at /Users/kpreid/.cargo/registry/src/index.crates.io-6f17d22bba15001f/env_logger-0.10.2/src/logger.rs:478:9
   5: rend3_framework::App::register_logger
             at /Users/kpreid/Projects/rust/contributing/rend3/rend3-framework/src/lib.rs:74:9
   6: rend3_examples_package::tests::test_app::{{closure}}
             at ./src/tests.rs:17:5

I'm not sure what's up with that, but it seems to be a preexisting issue.

@kpreid kpreid marked this pull request as ready for review January 24, 2024 23:09
@cwfitzgerald
Copy link
Member

I'm not sure what's up with that, but it seems to be a preexisting issue.

You need cargo nextest - I should implement the same "scream at you" error as wgpu has.

@kpreid
Copy link
Contributor Author

kpreid commented Jan 25, 2024

You need cargo nextest

Then the testing steps in build.bash (and the PR template) are stale and should be updated or deleted, because they contain cargo test.

* Updates `wgpu-profiler` to match.
* Updates `naga` too to satisfy cargo-deny.
* `rend3_gltf::util::format_components()` is now implemented by calling
  `TextureFormat::components()`, which seems to be the same function.
@kpreid
Copy link
Contributor Author

kpreid commented Jan 25, 2024

Updated naga too to satisfy cargo-deny.

@kpreid
Copy link
Contributor Author

kpreid commented Feb 4, 2024

@cwfitzgerald The mac CI failure has no details — failure to start perhaps?

In any case, what does this PR need to move forward?

@cwfitzgerald
Copy link
Member

Yeah I pulled my local runner for security concerns - let's merge this and I'll fix it in a follow up.

@cwfitzgerald cwfitzgerald merged commit 28d9601 into BVE-Reborn:trunk Feb 4, 2024
5 of 6 checks passed
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