PhoenicisOrg / scripts

Phoenicis scripts
GNU Lesser General Public License v3.0
64 stars 49 forks source link

Update DXVK for nvidia GPU #982

Open Kreyren opened 5 years ago

Kreyren commented 5 years ago

You might need

Run your game with VK_ICD_FILENAMES=/usr/share/vulkan/icd.d/radeon_icd.x86_64.json for AMD 64 bit or with VK_ICD_FILENAMES=/usr/share/vulkan/icd.d/nvidia_icd.json for nvidia.

Works now -> DXVK verb update is required

Originally posted by @Kreyren in https://github.com/PhoenicisOrg/scripts/pull/965#issuecomment-499448221

VK_ICD_FILENAMES variable has to be set depending on end-user system for games to work.

ImperatorS79 commented 5 years ago

We need a way to guess the main GPU on a system and a way to update the _environment variable of the QuickScript from the DXVK verb.

Kreyren commented 5 years ago

@ImperatorS79 Why guess? we can grab the vendor from linux environment

EDIT: Worst case scenario we force user-input

EDIT2: probably if we are unable to deduce the vendor -> force user-input

ImperatorS79 commented 5 years ago

@plata How could I send the QuickScriptto the dxvk verb so that I could update the _environmentvar ?

ImperatorS79 commented 5 years ago

OK I have something that should work, we just have to change the prototype of preInstalland postInstallfunction:

this._preInstall(this, wine, setupWizard);
Kreyren commented 5 years ago

OK I have something that should work, we just have to change the prototype of preInstalland postInstallfunction:

this._preInstall(this, wine, setupWizard);

Will you MR it?

ImperatorS79 commented 5 years ago

It needs #956 first (which needs some changes in the GUI)

plata commented 5 years ago

I think we must query the user because there might be a multi GPU setup (will the system provide the correct GPU?).

ImperatorS79 commented 5 years ago

@plata That's what I am doing locally, the fix is ready, but I want your opinion about the preInstall change ^^.

plata commented 5 years ago

Hard to tell without actually seeing it.

ImperatorS79 commented 5 years ago

Just changing the prototype to this._preInstall(this, wine, setupWizard); instead of this._preInstall(wine, setupWizard); in any file using it so that I can access script variables.

plata commented 5 years ago

I got that part. I'm interested in the part where you use the script variable.

ImperatorS79 commented 5 years ago

I just get the script._environment variable (in my new shortcut pr), to append to it the VK_ICD_FILENAMES variable:

Wine.prototype.DXVK = function (quickscript, dxvkVersion) {
    ...
    //Update environment to ensure the right GPU is loaded
    var gpuType = this.wizard().menu("Please select the type of your main GPU", ["AMD", "Intel", "Nvidia"]);
    var envJSON = {};
    if (typeof quickscript._environment !== 'undefined'){
        envJSON = JSON.parse(quickscript._environment);
    }
    if (gpuType.text == "Nvidia") {
        envJSON["VK_ICD_FILENAMES"] = "/usr/share/vulkan/icd.d/nvidia_icd.json";
    }
    else if (gpuType.text == "AMD") {
        if (this.architecture() == "amd64"){
            envJSON["VK_ICD_FILENAMES"] = "/usr/share/vulkan/icd.d/radeon_icd.x86_64.json";
        }
        else{
            envJSON["VK_ICD_FILENAMES"] = "/usr/share/vulkan/icd.d/radeon_icd.x86.json"; //To check
        }
    }
    quickscript._environment = JSON.stringify(envJSON);
    ...
plata commented 5 years ago

Won't this cause issues if you do not use a QuickScript?

ImperatorS79 commented 5 years ago

All scripts inherit from QuickScript I think.

plata commented 5 years ago

No. It's convenient but not required.

ImperatorS79 commented 5 years ago

Which one does not inherit ? I just have to add the same logic for them.

Is there any reason why all scripts do not inherit from the same base ?

plata commented 5 years ago

For example: https://github.com/PhoenicisOrg/scripts/blob/master/Applications/Internet/Internet%20Explorer%207.0/Online/script.js. We want to have the possibility to write completely free scripts as well.

ImperatorS79 commented 5 years ago

Okay so I have to totally rethink how this could work :thinking:.

plata commented 5 years ago

I fear so, yes. I also have no great idea right now.

plata commented 5 years ago

Except: Can you not simply return the environment from the function?

ImperatorS79 commented 5 years ago

Yes, I could the json string or object! But I still need the modification of .preInstall ^^.

plata commented 5 years ago

Why? Once you are in the preInstall, you should be able to access a variable in the script. A bit like here: https://github.com/PhoenicisOrg/scripts/blob/master/Applications/Games/Rocksmith%202014/Steam/script.js

ImperatorS79 commented 5 years ago

In my implementation, the environment variables are stored in a variable of QuickScript(and set with .environment() function of QuickScript). Since DXVK is executed in the preInstall, how could I modify that "private" variable without this ?

plata commented 5 years ago

Can't you just call .environment() after .preInstall()?

ImperatorS79 commented 5 years ago

But how do I recover the variables outputted by DXVK inside the .preInstall ?

plata commented 5 years ago

Untested but can't you:

include("engines.wine.quick_script.steam_script");

var env = "";

var installerImplementation = {
    run: function () {
        new SteamScript()
            .preInstall(function (wine, wizard) {
                env =wine.DXVK();
            })
            .environment(env)
            .go();
    }
};
ImperatorS79 commented 5 years ago

That seems really weird but worth trying.

EDIT: this will not work since .preInstall only store the function which is executed in .go()

plata commented 4 years ago

What if environment() could also receive a function, like:

.environment(getDxvkEnvironment)

?

madoar commented 4 years ago

That sounds like a beautiful solution!