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

[Merged by Bors] - Update wgpu to 0.12 and naga to 0.8 #3375

Closed
wants to merge 19 commits into from

Conversation

vabka
Copy link
Contributor

@vabka vabka commented Dec 18, 2021

Objective

Fixes #3352
Fixes #3208

Solution

  • Update wgpu to 0.12
  • Update naga to 0.8
  • Resolve compilation errors
  • Remove [[block]] from WGSL shaders (because it is depracated and now wgpu cant parse it)
  • Replace elseif with else if in pbr.wgsl

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Dec 18, 2021
crates/bevy_gltf/Cargo.toml Outdated Show resolved Hide resolved
comparison: true,
filtering: true,
},
ty: BindingType::Sampler(SamplerBindingType::Comparison),
Copy link
Contributor

Choose a reason for hiding this comment

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

Can someone verify if this isn't a behavorial change.

Copy link
Contributor Author

@vabka vabka Dec 18, 2021

Choose a reason for hiding this comment

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

+1
Also not sure

Copy link
Contributor Author

@vabka vabka Dec 18, 2021

Choose a reason for hiding this comment

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

I dont know what these options should affect. But with SamplingBindingType::Filtering in panics on start.
Result looks pretty same.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think comparison is fine for now. We might be able to rein this in, but this seems like the right choice for the current impl.

crates/bevy_gltf/Cargo.toml Outdated Show resolved Hide resolved
crates/bevy_ui/Cargo.toml Outdated Show resolved Hide resolved
@bjorn3
Copy link
Contributor

bjorn3 commented Dec 18, 2021

Thanks for the PR! I have a couple of nits and one thing someone else needs to check, but apart from that I don't see any issues.

@cart cart added this to the Bevy 0.6 milestone Dec 18, 2021
Copy link
Contributor

@bjorn3 bjorn3 left a comment

Choose a reason for hiding this comment

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

Looks like two files still have extra newlines. The rest has been fixed.

crates/bevy_pbr/src/render/mesh_view_bind_group.wgsl Outdated Show resolved Hide resolved
crates/bevy_render/src/render_resource/shader.rs Outdated Show resolved Hide resolved
Co-authored-by: bjorn3 <bjorn3@users.noreply.github.com>
@cart cart changed the title Fix crash on amd GPUs Update wgpu to 0.12 and naga to 0.8 Dec 18, 2021
@vabka
Copy link
Contributor Author

vabka commented Dec 18, 2021

finally😅. Removed all extra blank lines

@mockersf
Copy link
Member

Could you also remove this line?

bevy/deny.toml

Line 61 in 8c25091

{ name = "raw-window-handle", version = "0.3" }, # from wgpu v0.11.1

It's a duplicate dependency that's no longer duplicated with the wgpu update 👍

@vabka
Copy link
Contributor Author

vabka commented Dec 18, 2021

@mockersf Why it was added? What 'raw-window-handle' does?
(Sorry if it is a stupid question. I am a newbie in rust)

@mockersf
Copy link
Member

Not a stupid question, no worries!

Bevy is using cargo-deny to help control the dependency list. With it, we can make CI fail for things like unmaintained dependencies, or duplicated dependencies.

In this case, raw-window-handle is a dependency added by both wgpu and winit, and it was updated in Bevy with the last winit update, while still depending on the previous version through wgpu. Now that wgpu is also updated, the last version of raw-window-handle is used everywhere so we don't need to accept it as a duplicated dependency.

raw-window-handle is... I don't know exactly, but it seems it's used to handle windows on all OS

Copy link
Member

@mockersf mockersf left a comment

Choose a reason for hiding this comment

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

works on macOS

@vabka
Copy link
Contributor Author

vabka commented Dec 18, 2021

Thank you for such explanation :)

@mockersf mockersf added A-Rendering Drawing game state to the screen C-Dependencies A change to the crates that Bevy depends on and removed S-Needs-Triage This issue needs to be labelled labels Dec 18, 2021
@Josh015
Copy link
Contributor

Josh015 commented Dec 18, 2021

@vabka @mockersf raw-window-handle just lets a user pass a window handle to a graphics API so it knows where to render. Specifically, it hides the platform-specific details of this operation and moves that responsibility out of the user's code and into the crates they depend upon.

@vabka
Copy link
Contributor Author

vabka commented Dec 19, 2021

@vabka @mockersf raw-window-handle just lets a user pass a window handle to a graphics API so it knows where to render. Specifically, it hides the platform-specific details of this operation and moves that responsibility out of the user's code and into the crates they depend upon.

I mean "what bad thing it does so it was banned". I know what is window handle.
But thanks :)

@cart
Copy link
Member

cart commented Dec 19, 2021

It wasn't banned, the "failure on duplicate dependency" was just suppressed.

@cart
Copy link
Member

cart commented Dec 19, 2021

bors r+

bors bot pushed a commit that referenced this pull request Dec 19, 2021
# Objective

Fixes #3352
Fixes #3208

## Solution

- Update wgpu to 0.12
- Update naga to 0.8
- Resolve compilation errors
- Remove [[block]] from WGSL shaders (because it is depracated and now wgpu cant parse it)
- Replace `elseif` with `else if` in pbr.wgsl
@bors bors bot changed the title Update wgpu to 0.12 and naga to 0.8 [Merged by Bors] - Update wgpu to 0.12 and naga to 0.8 Dec 19, 2021
@bors bors bot closed this Dec 19, 2021
@vabka vabka deleted the fix-amd-gpu branch December 19, 2021 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Dependencies A change to the crates that Bevy depends on
Projects
None yet
6 participants