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

sysinfo: Replace deprecated Vulkan macros #173

Merged
merged 2 commits into from
May 17, 2024
Merged

sysinfo: Replace deprecated Vulkan macros #173

merged 2 commits into from
May 17, 2024

Conversation

jp7677
Copy link
Owner

@jp7677 jp7677 commented May 15, 2024

Unfortunately we cannot just use the newer Vulkan macros since the major version for the NVIDIA driver needs 10 bits whereas the newer Vulkan macro for the major version truncates to 7 bits.

From vulkan_core.h:

// DEPRECATED: This define is deprecated. VK_API_VERSION_MAJOR should be used instead.
#define VK_VERSION_MAJOR(version) ((uint32_t)(version) >> 22U)

#define VK_API_VERSION_MAJOR(version) (((uint32_t)(version) >> 22U) & 0x7FU)

The newer VK_API_VERSION_MAJOR cuts after 7 bits due to introduction of the variant number, this conflicts with the 10 bits needed for the NVIDIA major version.

Inspired by doitsujin/dxvk#4001

@jp7677 jp7677 requested a review from Saancreed May 15, 2024 19:32
Copy link
Collaborator

@Saancreed Saancreed left a comment

Choose a reason for hiding this comment

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

Sure. I have one suggestion but otherwise LGTM.

Is this something worth writing tests for?

src/util/util_version.h Outdated Show resolved Hide resolved
@jp7677
Copy link
Owner Author

jp7677 commented May 15, 2024

Is this something worth writing tests for?

We already somewhat cover it indirectly https://github.com/jp7677/dxvk-nvapi/blob/master/tests/nvapi_sysinfo.cpp#L202 but yeah good idea actually

@jp7677 jp7677 force-pushed the vulkan-macros branch 2 times, most recently from 4e317f0 to 67331ec Compare May 15, 2024 21:18
Unfortunately we cannot just use the newer Vulkan
macros since the major version for the NVIDIA driver
needs 10 bits whereas the newer Vulkan macro for the
major version returns only 7 bits.
@jp7677 jp7677 requested a review from Saancreed May 17, 2024 14:20
@jp7677 jp7677 merged commit aae4902 into master May 17, 2024
2 checks passed
@jp7677 jp7677 deleted the vulkan-macros branch May 17, 2024 14:48
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.

2 participants