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

nvapi-d3d: Just error out of GetCurrentSLIState #279

Merged
merged 1 commit into from
Feb 22, 2025
Merged

Conversation

K0bin
Copy link
Contributor

@K0bin K0bin commented Feb 21, 2025

Fallout New Vegas seems to think it's running on an SLI system unless we just error out here.

I also tried setting maxNumAFRGroups and numAFRGroups both to 0 but that didn't fix it either.
Additionally I tried just setting numAFRGroups to 0 and keeping maxNumAFRGroups at 1 but that didn't work either.

The Nvidia sample suggests what we have right now is a valid implementation:
bSLIEnabled = (gMaxNumAFRGroups > 1); but it appears that the check in Fallout isn't particularly robust.

So I suggest we just return an error when the function gets called. Alternatively we could return null pointers for the SLI functions in QueryInterface. I don't have a test setup for Nvapi on Windows right now, so I can't check what Nvidia does on Windows these days without a lot of effort.

Fixes: doitsujin/dxvk#4696
ValveSoftware/Proton#356 (comment)

@adamdmoss
Copy link

This feels wrong, at least for the purposes of fixing one game... NV docs suggest that an error from the nvapi call should even be interpreted by a game as 'prompt the player to upgrade their driver' which is counterproductive.

https://docs.nvidia.com/gameworks/content/technologies/desktop/sli_using_nvapi.htm

The fix would be to figure out why this game is happy with the result from real nvapi and unhappy with the result from dxvk-nvapi. dxvk-nvapi's implementation looks sensible at first glance but I wouldn't be surprised if a game expects numAFRGroups to be 0 instead of 1.

@K0bin
Copy link
Contributor Author

K0bin commented Feb 21, 2025

but I wouldn't be surprised if a game expects numAFRGroups to be 0 instead of 1.

I tested that. It did not fix it. I guess I need to see what Windows returns.

@K0bin
Copy link
Contributor Author

K0bin commented Feb 21, 2025

Okay I did thorough testing on Windows.

The docs don't list NVAPI_NO_ACTIVE_SLI_TOPOLOGY as a possible return value but that's exactly what I got.
The struct fields do not get initialized except numVRSLIGpus of version 2 of the struct. That one gets set to 0, the others don't get changed. The other errors (incompatible struct, invalid argument) do take precedence, so that part stays as it was.

NvAPI_D3D_ImplicitSLIControl does not return NVAPI_NO_ACTIVE_SLI_TOPOLOGY, regardless of the value passed for implicitSLIControl.

@K0bin
Copy link
Contributor Author

K0bin commented Feb 21, 2025

error: XDG_RUNTIME_DIR is invalid or not set in the environment.

The failed CI test isn't caused by the PR.

@Saancreed
Copy link
Collaborator

It is, you just looked at the wrong message:

D3D methods succeed
  GetCurrentSLIState succeeds
  GetCurrentSLIState with unknown struct version returns incompatible-struct-
  version
-------------------------------------------------------------------------------
../../../tests/nvapi_d3d.cpp:52
...............................................................................

../../../tests/nvapi_d3d.cpp:55: FAILED:
  REQUIRE( NvAPI_D3D_GetCurrentSLIState(&unknown, &state) == NVAPI_INCOMPATIBLE_STRUCT_VERSION )
with expansion:
  -113 == -9

===============================================================================
test cases:    20 |    [19](https://github.com/jp7677/dxvk-nvapi/actions/runs/13467194283/job/37635278839?pr=279#step:8:20) passed | 1 failed
assertions: 27591 | 27590 passed | 1 failed

@K0bin
Copy link
Contributor Author

K0bin commented Feb 22, 2025

Should be fixed now.

@adamdmoss
Copy link

Nice (and surprising) discoveries.

Copy link
Owner

@jp7677 jp7677 left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the research and fix, very cool!

Minor nit, otherwise LGTM! If you are at it, you could tweak the commit message slightly since it feels a bit more graceful now than just "error out". But also ok if you want to keep it like it is.

This is what NvAPI returns on Windows.

Fallout New Vegas seems to think it's running on an SLI
system if it get's SUCCESS as the returned status
from GetCurrentSLIState regardless of the contents
of the state struct.
@jp7677 jp7677 merged commit f898b3a into jp7677:master Feb 22, 2025
4 checks passed
@K0bin K0bin deleted the no-sli branch February 22, 2025 12:54
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.

Fallout: New Vegas HDR effect is too aggressive on NVidia
4 participants