PotatoOfDoom / CyberFSR2

FidelityFx Super Resolution 2.0 for Cyberpunk
MIT License
620 stars 67 forks source link

Add support for an external configuration file #11

Closed mnxn closed 2 years ago

mnxn commented 2 years ago

This PR adds support for an optional external configuration file by reading settings from a nvngx.ini file.

Settings have been added to added to control the init parameters, sharpening, and view FOV/planes. When a setting is set to auto, missing from the nvngx.ini file, or the entire nvngx.ini file is absent, the mod will use settings from DLSS for the init parameters/sharpening and defaults for the view FOV/planes.

The default vertical FOV is 60, which is a horizontal FOV of about 91. I've noticed some image quality deterioration when the FOV is both too low and too high, so this should be a good middle ground. Users should still be encouraged to set the FOV to match what they're using in-game. If they only know the horizontal FOV, they can use a convertor to get the vertical FOV.

I've left the Cyberpunk2077.exe hook which will still be used for Cyberpunk unless the values are explicitly set in the config file. The output GetModuleHandleW is also checked so the mod will work for other games without crashing.

If this PR is merged, Cyberpunk 2077 players will be able to continue using the mod without the configuration file. My Dying Light 2 fork will be obsolete since the configuration file can be used to set SharpnessRange to negative and FOV to the player's FOV. I haven't used the mod with any other games, but users can now test it and modify the settings to attempt to fix graphical issues.

Notes

PotatoOfDoom commented 2 years ago

Awesome, I didn't expect anyone to make something like that so fast. Your solution looks nice but is there a reason why you didn't just use something like SimpleIni? I think something like that would be much easier to use and would handle errors better. (I don't think your solution accounts for a missing ini file.)

Also I am not quite sure about the sharpness part. I haven't tried your dying light 2 mod but is there a problem with the sharpness? Everything in Dx12ParameterImpl should be properties set by the game itself and not by a config file. Did the developers set the wrong sharpness values? If you are modifying the sharpness I would suggest you only do it here (like you are already doing for the other sharpness values). This would also allow you to remove the singleton property of the config class and turn it into an instance of the feature context. Then players could just change config file values by simply disabling and reenabling dlss without rebooting the game.

I also think the best way regarding the ViewMatrixHook thing would be to use inheritance with a master ViewMatrixHook, a derived cyberpunk2077 ViewMatrixHook class and a derived config ViewMatrixHook class too. This would easily allow us to extend the system with more custom code (like game specific signature scanning) for other games. We could then use the config file to select specific inherited ViewMatrixHook classes or "profiles" for different games.

RealIndica commented 2 years ago

Awesome, I didn't expect anyone to make something like that so fast. Your solution looks nice but is there a reason why you didn't just use something like SimpleIni? I think something like that would be much easier to use and would handle errors better. (I don't think your solution accounts for a missing ini file.)

Also I am not quite sure about the sharpness part. I haven't tried your dying light 2 mod but is there a problem with the sharpness? Everything in Dx12ParameterImpl should be properties set by the game itself and not by a config file. Did the developers set the wrong sharpness values? If you are modifying the sharpness I would suggest you only do it here (like you are already doing for the other sharpness values). This would also allow you to remove the singleton property of the config class and turn it into an instance of the feature context. Then players could just change config file values by simply disabling and reenabling dlss without rebooting the game.

I also think the best way regarding the ViewMatrixHook thing would be to use inheritance with a master ViewMatrixHook, a derived cyberpunk2077 ViewMatrixHook class and a derived config ViewMatrixHook class too. This would easily allow us to extend the system with more custom code (like game specific signature scanning) for other games. We could then use the config file to select specific inherited ViewMatrixHook classes or "profiles" for different games.

100% agree. I was thinking of making this almost universal by implementing an application specific ini file which contains configurations for each game's ViewMatrix (such as base address/signature and offsets). However, with some games such as RDR2 it requires a bypass for nvngx to be loaded in the first place and its possible to load nvngx without editing the registry by changing 1 byte as proven in my fork. At the end of the day, simply dragging and dropping this mod into your desired game would be incredibly user-friendly and was a personal goal in my fork.

mnxn commented 2 years ago

Hi @PotatoOfDoom, thank you for the comments. I'll make the changes over the next few days.

Your solution looks nice but is there a reason why you didn't just use something like SimpleIni? I think something like that would be much easier to use and would handle errors better. (I don't think your solution accounts for a missing ini file.)

There was a typo, but GetPrivateProfileString returns the lpDefault parameter when the file is absent. simpleini looks like the better solution, so I'll rewrite this PR to use that instead. Should the mod report errors like invalid values to the user then? It could be done by combining all the config errors together and show one error message with a MessageBox.

Did the developers set the wrong sharpness values?

You're right that the sharpness changes shouldn't be in Dx12ParameterImpl.

I had to add the sharpness changes for Dying Light 2 since it was sending sharpness values of -0.99 when the in-game slider was at 0. FSR 2.0 only takes sharpness values between 0 and 1 so there had to be a conversion. I saw that the RDR2 fork had the same conversion and thought it would be worthwhile to add it as a configuration option. If that sharpness range only goes negative in Dying Light 2, it's probably only a bug with that game and I'll remove the configuration option. @RealIndica: can you confirm whether RDR2 has the same sharpness problem or if the conversion is just a remnant of the Dying Light 2 code?

This would also allow you to remove the singleton property of the config class and turn it into an instance of the feature context.

Do you mean that the feature context should have a field for a Config object or that the Config class should be removed and the feature context should have all of the configuration fields itself?

PotatoOfDoom commented 2 years ago

There was a typo, but GetPrivateProfileString returns the lpDefault parameter when the file is absent. simpleini looks like the better solution, so I'll rewrite this PR to use that instead. Should the mod report errors like invalid values to the user then? It could be done by combining all the config errors together and show one error message with a MessageBox.

I am not sure. I thought of something like an autodetection system for games where the dll detects the game and uses the proper configuration (without any interaction or config file). Not sure if this is feasible though. Also, this is obviously not possible if there is no known configuration so maybe we output a messagebox only if we don't know the game and can't find a config file or so. On the other hand, from my experience messageboxes and fullscreen games don't mix very well because they occasionally appear behind the game itself. Maybe someone in here has a better idea.

I had to add the sharpness changes for Dying Light 2 since it was sending sharpness values of -0.99 when the in-game slider was at 0.

Oh okay, my bad. That part is even defined in the dlss spec (here on page 18). Sharpness Values go from -1 to 1. So yeah, the conversion from the nvidia format to the amd 0 to 1 is a very good idea. Though this should mean that we don't really need the "inverse sharpness" right? It just seems to be a bug in our implementation.

Do you mean that the feature context should have a field for a Config object or that the Config class should be removed and the feature context should have all of the configuration fields itself?

I thought about a unique_ptr to the config class like the references to Fsr2Context or ViewMatrixHook.

JacekJagosz commented 2 years ago

I think the .ini has one major feature behind it, that you can add new game profiles or change the current ones without the need of compiling again. That makes it much simpler for contributors and for people who want to start. So it would be awesome if there was a way to change parameters without need of recompiling.

mnxn commented 2 years ago

Oh okay, my bad. That part is even defined in the dlss spec (here on page 18). Sharpness Values go from -1 to 1. So yeah, the conversion from the nvidia format to the amd 0 to 1 is a very good idea. Though this should mean that we don't really need the "inverse sharpness" right? It just seems to be a bug in our implementation.

Thank you for the documentation link. The reason I originally thought it was a bug was because Cyberpunk doesn't allow softening and only uses the 0 to 1 range. Therefore we can't always do the conversion because FSR would get a sharpening value of 0.5 when the Cyberpunk in-game slider is at 0. Another option would be to only pass sharpening values that are at least zero from DLSS to FSR, but then that would make the first half of the Dying Light 2 slider ineffective.

I would prefer to leave an option for whether to do a conversion or not, though it should probably have better value names than normal and negative.

NintendoManiac64 commented 2 years ago

On the subject of a more "universal DLSS to FSR2" translator, I can't help but get vibes similar to DXVK or the like which subsequently makes me wonder about how realistic at some point in the (near? far?) future something similar couldn't be hooked into WINE and/or Proton directly, particularly benefiting Steam Deck users (I would imagine that the manual DLL replacement method is a bit cumbersome to do on SteamOS).

JacekJagosz commented 2 years ago

@NintendoManiac64 I actually think this situation is more similar to SpecialK, which has a system-wide automatic .dll loader and special versions for some games. I also think this mod can't just work with any game. If all the features get implemented each game would still need a few options to be specified, like the sharpness range for example or addresses. And there is no way do do some of those automatically, someone will need to do that for each game. So in this way it won't be like DXVK, but I guess in the future there could be an automatic loader and updater made to simplify things for users. But that is all far future.

mnxn commented 2 years ago

I've made the changes, including the start of game-specific profiles based on executable name.

I tried to do error reporting with MessageBox but it was a bad solution. The created message box window appeared behind both the Cyberpunk 2077 and Dying Light 2 windows. Even worse, it is blocking so it would stop the game until the message box is closed (which is very hard to do when it's behind the game window). Another solution is to add logging, but these changes are already quite large in scope, so I think it should wait until another PR.

Let me know if there's something I missed or anything else that needs to be done.

Side note: this PR has a lot of commits now so I would recommend merging it with a squashed commit ("Squash and merge") once it's ready.

PotatoOfDoom commented 2 years ago

LGTM. Next step would be to find a more elegant way to handle the different game configs but this is good for now.