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

Change default wgpu backend from all to primary on windows #7733

Conversation

LiamGallagher737
Copy link
Member

Objective

Temporary fix for #7620

Solution

Revert the default wgpu backends back to PRIMARY from all().
Reverting #7481

I personally believe this is a temporary fix for 0.10, if gfx-rs/wgpu#2540 is fixed before 0.10 I think if should be fine to leave it on all() but that issue hasn't had much activity.

I think we should also keep in mind wither more people are running into the issue of Unable to find a GPU! than there are people suffering from #7620 to try and please the most users.

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen labels Feb 17, 2023
@alice-i-cecile alice-i-cecile added this to the 0.10 milestone Feb 17, 2023
@alice-i-cecile alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label Feb 17, 2023
@mockersf
Copy link
Member

Could we use PRIMARY on windows, and all() on other platforms?

From my understanding of #7620 (comment) it's more likely to happen on windows, while the Unable to find a GPU! error was happening more on linux.

crates/bevy_render/src/settings.rs Outdated Show resolved Hide resolved
Co-authored-by: Robert Swain <robert.swain@gmail.com>
@LiamGallagher737 LiamGallagher737 changed the title Change default wgpu backend from all to primary Change default wgpu backend from all to primary on windows Feb 20, 2023
Co-authored-by: James Liu <contact@jamessliu.com>
@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Feb 20, 2023
Comment on lines 48 to 55
let default_backends = if cfg!(feature = "webgl") {
Backends::GL
// TODO: When https://github.com/gfx-rs/wgpu/issues/2540 is fixed, Windows can also use all()
} else if cfg!(target_os = "windows") {
Backends::PRIMARY
} else {
Backends::all()
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps this logic should be moved to a Default impl on Backends?

@LiamGallagher737
Copy link
Member Author

LiamGallagher737 commented Feb 21, 2023

A new glow patch is out which should fix the issue so hold off on merging this.

@alice-i-cecile
Copy link
Member

This is now released: closing this PR.

@LiamGallagher737 LiamGallagher737 deleted the default-to-primary-wgpu-backend branch March 8, 2023 23:57
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-Bug An unexpected or incorrect behavior S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants