Skip to content

[Feature] Download DXVK 1.10.3 if no Vulkan 1.3 support is detected #2717

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

Merged
merged 2 commits into from
Jun 30, 2023

Conversation

CommandMC
Copy link
Collaborator

@CommandMC CommandMC commented May 19, 2023

First and foremost: Yes, this idea is borrowed from Lutris' most recent release. I have however been meaning to do something like this for a long time, since I own several PCs affected by this as well

A lot of older GPUs don't support Vulkan 1.3, which DXVK versions > 1.X.X requires. For those users, the "Auto-Install DXVK" function was entirely useless, as the DXVK Heroic installs will never work on their system.
Now, Heroic will query the user's GPUs for their Vulkan support and install the older 1.10.3 version of DXVK if no Vulkan 1.3 support is detected.

Internally, this is done by:

  • accepting not just a string as a download URL for tools, but also a function returning a string
  • DXVK's url function querying Vulkan support and choosing either the latest release or 1.10.3 based on that
  • new utility functions being added to interface with Vulkan directly

Vulkan functions are called using a helper binary (https://github.com/imLinguin/vulkan-helper-rs)


Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@CommandMC CommandMC added the pr:ready-for-review Feature-complete, ready for the grind! :P label May 19, 2023
@CommandMC CommandMC requested a review from a team May 19, 2023 18:08
@CommandMC CommandMC self-assigned this May 19, 2023
@CommandMC CommandMC requested review from arielj, flavioislima, Nocccer and imLinguin and removed request for a team May 19, 2023 18:08
Copy link
Member

@imLinguin imLinguin left a comment

Choose a reason for hiding this comment

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

Wow that was fast. Looks good to me. We can merge after Lint and Tests pass.

@CommandMC
Copy link
Collaborator Author

The tests look strange to me, it seems like Jest doesn't reset the module's state between runs (even with resetModules enabled)

Copy link
Member

@Etaash-mathamsetty Etaash-mathamsetty left a comment

Choose a reason for hiding this comment

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

looks fine other than this
and also maybe add some stuff for downloading older vkd3d-proton

@CommandMC CommandMC force-pushed the feat/vulkan-detection branch 2 times, most recently from 64ae099 to 93dea84 Compare May 20, 2023 10:53
@flavioislima flavioislima added pr:wip WIP, don't merge. and removed pr:ready-for-review Feature-complete, ready for the grind! :P labels May 29, 2023
@CommandMC CommandMC force-pushed the feat/vulkan-detection branch from 93dea84 to 5facaf7 Compare June 24, 2023 13:16
@CommandMC CommandMC added pr:ready-for-review Feature-complete, ready for the grind! :P and removed pr:wip WIP, don't merge. labels Jun 24, 2023
A lot of older GPUs don't support Vulkan 1.3, which DXVK
versions > 1.X.X requires. For those users, the "Auto-Install
DXVK" function was entirely useless, as the DXVK Heroic installs
will never work on their system.
Now, Heroic will query the user's GPUs for their Vulkan support
and install the older 1.10.3 version of DXVK if no Vulkan 1.3 support
is detected.

Internally, this is done by:
- accepting not just a string as a download URL for tools, but also
  a function returning a string
- DXVK's url function querying Vulkan support and choosing
  either the latest release or 1.10.3 based on that
- new utility functions being added to interface with Vulkan directly

Vulkan functions are called using a helper binary (see
https://github.com/imLinguin/vulkan-helper-rs)

Co-authored-by: Paweł Lidwin <[email protected]>
@CommandMC CommandMC force-pushed the feat/vulkan-detection branch from 5facaf7 to b4f1e86 Compare June 24, 2023 13:26
@@ -43,7 +48,36 @@ export const DXVK = {
},
{
name: 'dxvk',
url: 'https://api.github.com/repos/doitsujin/dxvk/releases/latest',
url: () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe this function can be moved outside the const tools = [... so we would just set url as a string, and then we don't need the const download_url = typeof tool.url === 'string' ? tool.url : tool.url() line if it's always a string

Copy link
Collaborator

@arielj arielj Jun 24, 2023

Choose a reason for hiding this comment

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

if you do that you can even do something like if (download_url) { tools.push({name: 'dxvk', url: dxvkUrl, etc...}) } so you can handle the case when DXVK cannot be used (we have the FIXME saying we can't handle that case) by not adding it to the array of tools

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Simply not downloading DXVK isn't quite enough, the frontend/other parts of the backend would still try to install it (which would then fail, since the folder isn't there)

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh I see

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think my first comment still applies though, to not have to do the if typeof === 'string' conditional

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've moved it into its own function with aebb5fe now. This is definitely clearer, thanks for the suggestion

@flavioislima flavioislima merged commit a24adca into main Jun 30, 2023
@flavioislima flavioislima deleted the feat/vulkan-detection branch June 30, 2023 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:ready-for-review Feature-complete, ready for the grind! :P
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants