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

[d3d9] Refactor D3D9ShaderValidator and enable input count validation only for The Void #4538

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

WinterSnowfall
Copy link
Contributor

Fixes #4537. I had hoped the input count validation would work globally, but apparently it regresses that particular game.

There's no nicer way to handle this unfortunately, since we're dealing with entirely undocumented behavior, so I've hidden the offending validation behind a config option and enabled it only for The Void, the only known game that actually needs it.

Also part of this PR, I've refactored the code to sit in its own .cpp file (as I should have done from the start really).

@WinterSnowfall WinterSnowfall force-pushed the d3d9-shadervalrework branch 2 times, most recently from 478f504 to 3537475 Compare December 22, 2024 09:22
@WinterSnowfall WinterSnowfall marked this pull request as ready for review December 22, 2024 16:02
@doitsujin
Copy link
Owner

doitsujin commented Dec 30, 2024

What exactly does the validation in question do, what do the shaders in The Void look like that cause it to fail, and why does it break the other game?

@WinterSnowfall
Copy link
Contributor Author

WinterSnowfall commented Dec 30, 2024

What exactly does the validation in question do, what do the shaders in The Void look like that cause it to fail, and why does it break the other game?

The validation in question fails if a PS 3.0 uses more than 10 (higher than index 9) input registers. This is (apparently) what The Void relies on to work correctly.

As to why the game on #4537 takes issue with it, no idea, as it's no longer available for purchase, but since the validation is pretty specific, I'm sure it uses a higher than 9 index in a PS 3.0 for some reason. As of now it's the only other game (with the exception of The Void of course) that we know of to do that, but in its case it doesn't expect the validation to fail.

TLDR, this entire thing is a mess even by d3d9 standards, but since in practice only The Void needs it, I think the PR isn't too far of an ask. I lost count how many config options we have specifically for Sims 2, so we might as well have a very dubious one for The Void, at least until (if ever) we get an exact idea what the intended behavior is here, if it differs based on OS versions etc.

@doitsujin
Copy link
Owner

My concern here is that we're essentially making irreversible changes since it's going to be impossible to even try to understand what's going on. I'd prefer to ask the user to dump shaders (with a build that has the workaround), so we can at least try and see where we fail.

@WinterSnowfall
Copy link
Contributor Author

WinterSnowfall commented Jan 5, 2025

My concern here is that we're essentially making irreversible changes since it's going to be impossible to even try to understand what's going on.

Not sure how anything is irreversible at this point? The offending validation is already in effect on master, this PR only restricts it to The Void.

I'd prefer to ask the user to dump shaders (with a build that has the workaround), so we can at least try and see where we fail.

I can do that, but as I said the validation is very specific and only fails if an input register index above 9 is used with a PS 3.0. It won't fail on anything else.

I can try to write a test for it, but not sure what good that will do except maybe to confirm how native behaves. Based on the fact that The Void and the affected game in the issue expect different things, I doubt it's going to be consistent anyway.

@doitsujin
Copy link
Owner

I can do that, but as I said the validation is very specific and only fails if an input register index above 9 is used with a PS 3.0. It won't fail on anything else.

I would like to at least see what the offending shader is and make sure we're not missing something silly (built-in input or something like that?). If it happens to be a scenario that should actually fail but doesn't for reasons we don't know then we can still do an app profile, but I'd just like to at least investigate it first.

@WinterSnowfall
Copy link
Contributor Author

WinterSnowfall commented Jan 5, 2025

@Blisto91 has asked the reporter to dump the shaders, but the annoying thing is the offended game is a) obscure, b) of the MMO variety, c) no longer available for purchase.

The alternative is to simply not care about it, but I am concerned it just might affect other things as well long term. And I'd rather not do that just to fix The Void, which was really the intention here.

@WinterSnowfall
Copy link
Contributor Author

Dumped shaders are here: #4537 (comment)

There seems to be quite a lot of PS 3.0 in there, and some are using up to register index 14 as input, if I'm reading it correctly... which should really fail validation based on our assumptions, but for some reason that game expects them not to?

Since we've not seen/hit this in many other games we've tested that actually validate PS 3.0, I'm not sure what's going on to be honest. I've probably hit a "goes above my head" wall at this point, so would appreciate your input.

It might be the validation calls have a slient failure mode of sorts, where they return the right error strings but pass validation? Kind of hard to tell when there's no actual documentation on any of this, and I went with what misyltoad was able to divine as ground truth.

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.

DXVK 2.5.2 causes FPS drop due to shader issues
2 participants