Skip to content

Conversation

suleyth
Copy link
Contributor

@suleyth suleyth commented Aug 16, 2025

Fixes (in the Vulkan backend) the issue in the later comments of #12682.

Description

I turned deviceRank into an Uint16 to avoid overflow issues if the VRAM is too large. It may still have weird issues if someone has more than 65 gigabytes of VRAM, but I doubt that's going to happen any time soon.

Testing on the GPU examples shows this change correctly picks my more powerful GPU. This is important because, since SDL will not expose having the user choose the GPU directly, players having issues with games forcibly using their less powerful GPU and without any possible fix would be terrible.

To make sure this truly works though I'd advice finding another weirdo with multiple dedicated GPUs in their computer to test. :)

Existing Issue(s)

I sadly don't have access to a Windows or Mac PC, so I wouldn't be able to test this change in the other backends. However it should be reasonably simple to implement.

@slime73
Copy link
Contributor

slime73 commented Aug 16, 2025

I turned deviceRank into an Uint16 to avoid overflow issues if the VRAM is too large. It may still have weird issues if someone has more than 65 gigabytes of VRAM, but I doubt that's going to happen any time soon.

The nvidia 5090 has 32 GB of VRAM already, there's nothing to suggest consumer GPUs won't have > 64 GB of VRAM during SDL3's lifetime.

Why not make the device rank 64 bits instead of 16 bits?

@suleyth
Copy link
Contributor Author

suleyth commented Aug 16, 2025

I turned deviceRank into an Uint16 to avoid overflow issues if the VRAM is too large. It may still have weird issues if someone has more than 65 gigabytes of VRAM, but I doubt that's going to happen any time soon.

The nvidia 5090 has 32 GB of VRAM already, there's nothing to suggest consumer GPUs won't have > 64 GB of VRAM during SDL3's lifetime.

Why not make the device rank 64 bits instead of 16 bits?

Yep! You are right! I'll get on it tomorrow.

@suleyth
Copy link
Contributor Author

suleyth commented Aug 17, 2025

deviceRank is now an Uint64 to future-proof the code in case a GPU with more than 64 GB of VRAM comes out during SDL3's lifetime. Aditionally, now we only add the VRAM to deviceRank if we don't prefer a low power device and we do it after checking the device supports all the needed properties.

This should make it work just like I said: if there's multiple dedicated GPUs on the computer, pick the one that meets all requirements. If all of them meet the requirements, pick the one with the highest VRAM (which theoretically should be the most powerful one)

@thatcosmonaut
Copy link
Collaborator

Looks right to me, just want to hear from @flibitijibibo and then it can go in.

Copy link
Collaborator

@flibitijibibo flibitijibibo left a comment

Choose a reason for hiding this comment

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

With the low-power check this makes sense, can be merged when CI comes out green (started it just now!)

@flibitijibibo
Copy link
Collaborator

The tests actually passed, macOS just stalled on the last part...

@flibitijibibo flibitijibibo merged commit bad7075 into libsdl-org:main Aug 18, 2025
40 of 41 checks passed
@suleyth suleyth deleted the rank-vulkan-physical-devices branch August 18, 2025 07:47
@suleyth
Copy link
Contributor Author

suleyth commented Aug 18, 2025

By the way this is actually my first merged PR ever! 🥳

@sechshelme
Copy link

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.

5 participants