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

Use VK_EXT_robustness2 only when not using an outdated intel iGPU driver #4602

Merged
merged 11 commits into from
Dec 7, 2023

Conversation

TheoDulka
Copy link
Contributor

Checklist

  • Run cargo clippy.
  • Run cargo clippy --target wasm32-unknown-unknown if applicable.
  • Add change to CHANGELOG.md. See simple instructions inside file.

Connections
Fixes #4599, fixes bevyengine/bevy#8037 and fixes bevyengine/bevy#8395

Description
Intel iGPUs with outdated drivers can break rendering if VK_EXT_robustness2 is pushed.
The solution is to not push if its driver version is before 31.0.101.2115, because this one works. There's probably an earlier functional version as it was just the earliest version I could find and try (due to Intel's End Of Life on 6th gen proc).

Testing
Not really something testable beyond getting an Intel iGPU laptop and seeing if it is unable to render. I used the shadow example for my testing.

@TheoDulka TheoDulka requested a review from a team as a code owner October 28, 2023 05:43
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.

Unfortunately I don't think this will work directly. You can't just remove the extension from the list.

We should instead just set the feature flag to false - in adapter.rs:236, we can just flip it false on this condition.

wgpu-hal/src/vulkan/adapter.rs Outdated Show resolved Hide resolved
wgpu-hal/src/vulkan/adapter.rs Outdated Show resolved Hide resolved
@TheoDulka
Copy link
Contributor Author

Unfortunately I don't think this will work directly. You can't just remove the extension from the list.

Yeah I realize that now, but I don't understand how I would do this:

We should instead just set the feature flag to false - in adapter.rs:236, we can just flip it false on this condition.

… than removing the extension. Also added another check for intel driver and not mesa
@TheoDulka TheoDulka changed the title Push VK_EXT_robustness2 to extensions only when not using an outdated intel iGPU driver Use VK_EXT_robustness2 only when not using an outdated intel iGPU driver Oct 29, 2023
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.

Test code looks good, just some changes on the enabling code.

Please re-request a review from me once the changes are addressed to make sure I see it!

wgpu-hal/src/vulkan/adapter.rs Outdated Show resolved Hide resolved
Removing the ExtRobustness2 from the supported_extensions
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.

Nothing wrong with the code, just want teo to have a final look before we merge

wgpu-hal/src/vulkan/adapter.rs Outdated Show resolved Hide resolved
wgpu-hal/src/vulkan/adapter.rs Outdated Show resolved Hide resolved
wgpu-hal/src/vulkan/adapter.rs Outdated Show resolved Hide resolved
Copy link
Member

@teoxoy teoxoy left a comment

Choose a reason for hiding this comment

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

Good stuff, thanks!

@teoxoy teoxoy enabled auto-merge (squash) December 7, 2023 18:09
@teoxoy teoxoy merged commit cf8e11e into gfx-rs:trunk Dec 7, 2023
27 checks passed
@TheoDulka TheoDulka deleted the intel-igpu-VK_EXT_robustness2 branch December 7, 2023 19:10
bradwerth pushed a commit to bradwerth/wgpu that referenced this pull request Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants