Skip to content

backend/wayland/vulkan: Fix use of Vulkan (correctly this time) #292

Merged
mmstick merged 2 commits intomasterfrom
vulkan
Mar 13, 2026
Merged

backend/wayland/vulkan: Fix use of Vulkan (correctly this time) #292
mmstick merged 2 commits intomasterfrom
vulkan

Conversation

@ids1024
Copy link
Member

@ids1024 ids1024 commented Mar 12, 2026

It seems #280 was just wrong. I didn't test it correctly, and it actually prevented crashes simply because the Vulkan instance failed to create.

This failed because it's a device extension, not an instance extension (and we already check for the device extension).

The real issue seems to be simply the fact this was using the Instance after dropping the Entry. Which is easy enough to address.

  • I have disclosed use of any AI generated code in my commit messages.
    • If you are using an LLM, and do not fully understand the changes it is making to the code base, do not create a PR.
    • In our experience, AI generated code often results in overly complex code that lacks enough context for a proper fix or feature inclusion. This results in considerably longer code reviews. Due to this, AI authored or partially authored PRs may be closed without comment.
  • I understand these changes in full and will be able to respond to review comments.
  • My change is accurately described in the commit message.
  • My contribution is tested and working as described.
  • I have read the Developer Certificate of Origin and certify my contribution under its conditions.

ids1024 added 2 commits March 12, 2026 14:33
It seems #280 was
just wrong. I didn't test it correctly, and it actually prevented
crashes simply because the Vulkan instance failed to create.

This failed because it's a device extension, not an instance extension
(and we already check for the device extension).

The *real* issue seems to be simply the fact this was using the
`Instance` after dropping the `Entry`. Which is easy enough to address.
@ids1024
Copy link
Member Author

ids1024 commented Mar 12, 2026

The calloop change is probably good, but it's hard to test when there are other issues that are inconsistent and could look similar to and issue with that change. And it's not important. So I'll split that off into a draft: #293 (maybe combined with other fixes later).

@ids1024 ids1024 changed the title Fix Vulkan code (correctly this time) and use calloop executor backend/wayland/vulkan: Fix use of Vulkan (correctly this time) Mar 12, 2026
@ids1024
Copy link
Member Author

ids1024 commented Mar 13, 2026

This should be tested on a Meteor Lake system to make sure the workaround this is for is actually working to prevent graphical corruption (though RUST_LOG=info should also show if it failed to initialize Vulkan). And it may as well be tested together with pop-os/cosmic-comp#2185 to make sure Meteor Lake SHM capture is still working properly with both changes.

@ids1024 ids1024 marked this pull request as ready for review March 13, 2026 00:09
@ids1024 ids1024 requested review from a team March 13, 2026 00:10
Copy link
Member

@leviport leviport left a comment

Choose a reason for hiding this comment

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

Seems good on MTL darp10, as well as oryp13

@mmstick mmstick merged commit 45e01aa into master Mar 13, 2026
9 checks passed
@mmstick mmstick deleted the vulkan branch March 13, 2026 18:44
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.

3 participants