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

winevulkan: Add FSR for fshack. #116

Closed

Conversation

DadSchoorse
Copy link
Contributor

@DadSchoorse DadSchoorse commented Jul 18, 2021

This is an implementation of AMD's FSR https://gpuopen.com/fidelityfx-superresolution/ in fshack, which allows users to chose it instead of linear/nearest filtering for upscaling. It can be enabled using WINE_FULLSCREEN_FSR=1 and a lower in game resolution than the display's native one. WINE_FULLSCREEN_FSR_STRENGTH controls the amount of sharpening (not to be confused with amd's quality presets, those are about the input resolution), 0 is the maximum sharpness, higher values mean less sharpening. The default is 5. The default is now 2 because that's AMD's rough recommendation.

Caveats:

  • Only works in vulkan games (dxvk and vkd3d-proton included).
  • Some games upscale on their own, without using fshack, when you select a lower resolution in game. FSR won't work in those.
  • Obviously applying FSR to the final image is not ideal, it should be applied before the UI is drawn and before some post processing effects like film grain.
  • AMD also recommends a negative LOD bias, which is not possible in a generic way without problems.
  • This is only the slow fp32 version, if accepted I might follow up with the fp16 version.
  • The patchset is a bit of a mess. If the huge commit is a problem I can try to split it up a bit.
  • An environment variable to add custom resolution modes would be nice to more directly control the quality. e.g. 2048x1152

@DadSchoorse DadSchoorse changed the title Fsr clean winevulkan: Add FSR for fshack. Jul 18, 2021
@DadSchoorse DadSchoorse force-pushed the fsr-clean branch 2 times, most recently from 5dabee1 to 9e1debe Compare July 21, 2021 20:58
@DadSchoorse
Copy link
Contributor Author

DadSchoorse commented Jul 21, 2021

I added another commit for a negative mip bias env var, you can use WINE_VULKAN_NEGATIVE_MIP_BIAS=N for a bias of N/100. For 1080p -> 1440p upscaling AMD recommends a negative bias between 0.38 and 0.58, so something like WINE_VULKAN_NEGATIVE_MIP_BIAS=45. By default, this is only applied to samplers with anisotropic filtering enabled, you can use WINE_VULKAN_BIAS_ALL_SAMPLERS=1 to force it for all samplers.

Do note that forcing a mip bias can break game graphics!

@gabriele2000
Copy link

gabriele2000 commented Jul 21, 2021

Replying to #116 (comment)

Will it be possible to choose the various presets through and env var?
I'm currently playing the witcher 3 from 720p to 1080p and the result is very interesting, still I sense that we could do a lot more

@DadSchoorse
Copy link
Contributor Author

@gabriele2000 I'm not sure what you mean with presets, you control the quality by selecting the in game resolution and you adjust sharpness/mip bias a bit with the environment variables if you want.

@gabriele2000
Copy link

gabriele2000 commented Jul 22, 2021 via email

@DadSchoorse
Copy link
Contributor Author

DadSchoorse commented Jul 22, 2021

Like I said, the quality presets of AMD just mean different game resolutions that get upscaled to your display's native resolution.

@gabriele2000
Copy link

gabriele2000 commented Jul 22, 2021 via email

@palaashatri
Copy link

might sound like a noob question, but how will we be able to choose the ingame render resolution that will be upscaled to our native monitor resolution by fsr? for example, some old games like hitman blood money for example, which do not have an ingame resolution slider, running through dxvk?

@Kron4ek
Copy link

Kron4ek commented Jul 22, 2021

From 720p to 1080p what is the preset?

Quality, according to the info from this page.

@have-a-boy
Copy link

might sound like a noob question, but how will we be able to choose the ingame render resolution that will be upscaled to our native monitor resolution by fsr? for example, some old games like hitman blood money for example, which do not have an ingame resolution slider, running through dxvk?

Afaik, that should just be the output resolution you set in the games' display options. With the caveat that some games implement their own upscaling from whatever resolution you set, to the target resolution of your window/screen, which prevents upscaling after that point.

@aeikum
Copy link
Collaborator

aeikum commented Jul 22, 2021

Looks cool. I added some nit-picky comments on the review. Some other thoughts:

-Since it touches so much fshack code, which is fragile, this'll need a full QA pass on our end. Just FYI, it may take a little while to get merged. Excited to see those stupid DOOM comments gone, though.

-I'm not a big fan of options. Are all of those environment variables actually useful? There isn't just a good set of hard-coded values we can use?

-If we really need the options, then at the very least these need to be documented. The documentation should include valid values (and accompanying validity checks in code), and what the settings do and why a user would set them. I'm not sure where that documentation would go, maybe a new file in proton.git/docs/? But again, I'd prefer if the options just didn't exist.

-How does this interact with the Integer scaling mode, e.g. if both are set? A good solution here may be to provide a new Proton env var which correctly selects between them. E.g. see this (completely untested) diff for a rough concept:

diff --git a/README.md b/README.md
index 92b008c..32c76da 100644
--- a/README.md
+++ b/README.md
@@ -313,7 +313,7 @@ the Wine prefix. Removing the option will revert to the previous behavior.
 | <tt>vkd3dfl12</tt>    |                                | Force the Direct3D 12 feature level to 12, regardless of driver support. |
 | <tt>vkd3dbindlesstb</tt>|                              | Put `force_bindless_texel_buffer` into `VKD3D_CONFIG`. |
 | <tt>hidenvgpu</tt>    | <tt>PROTON_HIDE_NVIDIA_GPU</tt>| Force Nvidia GPUs to always be reported as AMD GPUs. Some games require this if they depend on Windows-only Nvidia driver functionality. See also DXVK's nvapiHack config, which only affects reporting from Direct3D. |
-|                       | <tt>WINE_FULLSCREEN_INTEGER_SCALING</tt> | Enable integer scaling mode, to give sharp pixels when upscaling. |
+|                       | <tt>PROTON_FS_SCALING_MODE</tt>| Use one of the following scaling modes: `integer` scaling to give sharp pixels; AMD's `[fsr](docs/FSR.md)` scaling. |
 | <tt>cmdlineappend:</tt>|                               | Append the string after the colon as an argument to the game command. May be specified more than once. Escape commas and backslashes with a backslash. |
 | <tt>nowritewatch</tt> | <tt>PROTON_NO_WRITE_WATCH</tt> | Disable support for memory write watches in ntdll. This is a very dangerous hack and should only be applied if you have verified that the game can operate without write watches. This improves performance for some very specific games (e.g. CoreRT-based games). |
 | <tt>seccomp</tt>      | <tt>PROTON_USE_SECCOMP</tt>    | **Note: Obsoleted in Proton 5.13.** In older versions, enable seccomp-bpf filter to emulate native syscalls, required for some DRM protections to work. |
diff --git a/proton b/proton
index 82a32e9..f113d7f 100755
--- a/proton
+++ b/proton
@@ -909,6 +909,14 @@ class Session:
         #disable XIM support until libx11 >= 1.7 is widespread
         self.env.setdefault("WINE_ALLOW_XIM", "0")
 
+        if "PROTON_FS_SCALING_MODE" in self.env:
+            if self.env["PROTON_FS_SCALING_MODE"] == "integer":
+                self.env["WINE_FULLSCREEN_INTEGER_SCALING"] = "1"
+                self.env["WINE_FULLSCREEN_FSR"] = "0"
+            elif self.env["PROTON_FS_SCALING_MODE"] == "fsr":
+                self.env["WINE_FULLSCREEN_INTEGER_SCALING"] = "0"
+                self.env["WINE_FULLSCREEN_FSR"] = "1"
+
         if "wined3d11" in self.compat_config:
             self.compat_config.add("wined3d")

dlls/winevulkan/vulkan.c Show resolved Hide resolved
dlls/winevulkan/vulkan.c Show resolved Hide resolved
dlls/winevulkan/vulkan.c Outdated Show resolved Hide resolved
@@ -826,7 +826,7 @@ static VkSurfaceKHR X11DRV_wine_get_native_surface(VkSurfaceKHR surface)
}

static VkBool32 X11DRV_query_fs_hack(VkSurfaceKHR surface, VkExtent2D *real_sz, VkExtent2D *user_sz,
VkRect2D *dst_blit, VkFilter *filter)
VkRect2D *dst_blit, VkFilter *filter, BOOL *fsr, float *sharpness)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't a blocker for your series, but I wonder if we can make this API suck less (e.g. use a struct instead of a bunch of arguments).

dlls/winex11.drv/fs.c Outdated Show resolved Hide resolved
dlls/winevulkan/vulkan.c Outdated Show resolved Hide resolved
dlls/winevulkan/vulkan.c Show resolved Hide resolved
@DadSchoorse DadSchoorse force-pushed the fsr-clean branch 2 times, most recently from c0905e2 to 07ca57a Compare July 23, 2021 10:52
@DadSchoorse
Copy link
Contributor Author

-I'm not a big fan of options. Are all of those environment variables actually useful? There isn't just a good set of hard-coded values we can use?

-If we really need the options, then at the very least these need to be documented. The documentation should include valid values (and accompanying validity checks in code), and what the settings do and why a user would set them. I'm not sure where that documentation would go, maybe a new file in proton.git/docs/? But again, I'd prefer if the options just didn't exist.

For sharpness we could leave it at the default, but I would like to have an option since how much sharpening looks good can depend on the blurriness of the input image.

The mip bias stuff has to be entirely optional, it effects everything, even if fsr is not enabled.

-How does this interact with the Integer scaling mode, e.g. if both are set? A good solution here may be to provide a new Proton env var which correctly selects between them. E.g. see this (completely untested) diff for a rough concept: [...]

Combining integer scaling with fsr would break the effect, yes. I think your idea sounds good.

@DieFeM
Copy link

DieFeM commented Jul 23, 2021

I've installed Fedora 34 and luckily I've been able to successfully compile and install proton with your changes (never compiled it before), tested it with Skyrim SE, scaling from 720 to 1080 and it makes it looks like native 1080 (very small differences, almost unnoticeable). This is great, thank you!

PS: I don't understand why this is not yet included as a global filter in the video settings, even if it is not the best implementation, it is way better than most texture filtering options.

@SkewedZeppelin
Copy link

Anyone who wants an easy way to try this and is already using Steam through Flatpak can:

flatpak install --user https://dl.flathub.org/build-repo/52165/com.valvesoftware.Steam.CompatibilityTool.Proton-Exp.flatpakref

@tuxtergames
Copy link

Sorry put a question here, but my monitor is FHD resolution 1920x1080, to work fsr according amd site I need to left resolution 1477 x 831 to work, is that right?
cause a lot of games I have here don't accept this input resolution, tks all.....

@DadSchoorse
Copy link
Contributor Author

@tuxtergames You would need that in-game resolution to match what AMD calls "Ultra Quality". 1280x720 input would equivalent to "Quality". And yes, you won't be able to select 1477x831 (at least at the moment) because fshack doesn't expose them as valid modes, see https://github.com/ValveSoftware/wine/blob/experimental_6.3/dlls/winex11.drv/fs.c#L63 for all currently reported resolutions.

@tuxtergames
Copy link

@tuxtergames You would need that in-game resolution to match what AMD calls "Ultra Quality". 1280x720 input would equivalent to "Quality". And yes, you won't be able to select 1477x831 (at least at the moment) because fshack doesn't expose them as valid modes, see https://github.com/ValveSoftware/wine/blob/experimental_6.3/dlls/winex11.drv/fs.c#L63 for all currently reported resolutions.

@DadSchoorse tks for reply xD

@ren2r
Copy link

ren2r commented Jul 28, 2021

The steam overlay stops working when the WINE_FULLSCREEN_FSR is enabled.
When pressing shift-tab on a game running with a 900p resolution on a 1080p screen, the overlay keeps blinking and doesn't shows up.
Removing the FSR flag or reverting the game to the native resolution makes the overlay work again.
[Edit]
Also, Alt-tab seems to revert to the default scaling method even with the flag enabled.

@mvrednov
Copy link

mvrednov commented Jul 28, 2021

Since there is a list of games for which the FSR developers have made support. Will WINE_FULLSCREEN_FSR = 1 and WINE_FULLSCREEN_FSR_STRENGTH keep FSR running for games that have not been supported by the FSR developers ?!

@Atemu
Copy link

Atemu commented Jul 28, 2021

@ren2r I believe I saw something about the ordering of overlays somewhere where the Steam overlay would work if it's at the end or something. Might be worth a Google.

@mvrednov the game would FSR up to its render res (i.e. 720p) and fshack would then FSR that to your native res.

@rcelyte
Copy link

rcelyte commented Jul 29, 2021

Does/will this support SteamVR games (i.e. Beat Saber)?

@Izaic
Copy link

Izaic commented Aug 4, 2021

@tuxtergames You would need that in-game resolution to match what AMD calls "Ultra Quality". 1280x720 input would equivalent to "Quality". And yes, you won't be able to select 1477x831 (at least at the moment) because fshack doesn't expose them as valid modes, see https://github.com/ValveSoftware/wine/blob/experimental_6.3/dlls/winex11.drv/fs.c#L63 for all currently reported resolutions.

Is there something blocking it from going with 1477x831 for Ultra Quality 1080P output FSR?

Signed-off-by: Georg Lehmann <dadschoorse@gmail.com>
Signed-off-by: Georg Lehmann <dadschoorse@gmail.com>
Signed-off-by: Georg Lehmann <dadschoorse@gmail.com>
@Zeioth
Copy link

Zeioth commented Sep 25, 2021

May this work in borderless mode in the future? Or is Fullscreen a limitation of FSR?

@kode54
Copy link

kode54 commented Sep 25, 2021

May this work in borderless mode in the future? Or is Fullscreen a limitation of FSR?

It needs some way for the game resolution to be different from the display resolution, so it only works with the full screen hack, which replaces actual mode switching with upscaling to the desktop resolution.

It stands to reason it could also be used for upscaling with Gamescope, if implemented into that compositor. Doing it for arbitrary windows would require hooking the exact display window and reporting the client visible area as a fraction of the actual size.

@pchome
Copy link

pchome commented Nov 14, 2021

Well, I patched FS-hack code to add "cinematic" ultrawide (1080p) resolutions, but I can see how quickly fs_monitor_sizes[] can go brr. So ...

  • An environment variable to add custom resolution modes would be nice to more directly control the quality. e.g. 2048x1152

maybe WINE_FULLSCREEN_FSR environment variable should accept scale factor? According to FSR info page it should work with scale factors from 1 to 4 (presets are 1.3x/1.5x/1.7x/2x from Ultra down to Performance).

In addition to fixed scaling, FSR may be used in “arbitrary scaling” mode, whereby any area scale factor between 1x and 4x is supported. This mode is typically used for Dynamic Resolution Scaling, whereby source resolution is determined by a fixed performance budget to achieve a minimum frame rate.

POC patch, but w/ additional env var:

--- a/dlls/winex11.drv/fs.c
+++ b/dlls/winex11.drv/fs.c
@@ -177,6 +177,8 @@ static void add_fs_mode(struct fs_monito
 static BOOL fs_monitor_add_modes(struct fs_monitor *fs_monitor)
 {
     DEVMODEW *real_modes, *real_mode, current_mode;
+    const char *fsr_scale_env;
+    double fsr_scale_factor;
     UINT real_mode_count;
     DWORD width, height;
     ULONG_PTR real_id;
@@ -203,6 +205,22 @@ static BOOL fs_monitor_add_modes(struct
         return FALSE;
     }
 
+    /* For FSR add one mode in range 1x up to 4x */
+    if ((fsr_scale_env = getenv("WINE_FULLSCREEN_FSR_SCALE")))
+    {
+        fsr_scale_factor = atof(fsr_scale_env);
+        if (fsr_scale_factor > 1 && fsr_scale_factor <= 4)
+        {
+            add_fs_mode(fs_monitor, current_mode.dmBitsPerPel,
+                (UINT)ceil(current_mode.dmPelsWidth / fsr_scale_factor),
+                (UINT)ceil(current_mode.dmPelsHeight / fsr_scale_factor),
+                current_mode.dmDisplayFrequency,
+                current_mode.u1.s2.dmDisplayOrientation);
+
+            real_settings_handler.free_modes(real_modes);
+            return TRUE;
+        }
+    }
     /* Add the current mode early, in case we have to limit */
     add_fs_mode(fs_monitor, current_mode.dmBitsPerPel, current_mode.dmPelsWidth,
                 current_mode.dmPelsHeight, current_mode.dmDisplayFrequency,

NOTE: for users who will try this patch, this will add only one supported resolution and will likely fail for games which expect some standard modes to be present.

EDIT: My original goal was to limit available modes when FSR enabled to only "presets" and native/current resolution. But I tested a few random games w/ this patch and didn't even touched in-game configuration. They started in desired mode and all I had to do is start new game. I sometimes see some Wine errors in terminal output, but that was expected.

EDIT2: I removed the screenshot, appeared not only shadows were broken but also FSR. Anyway it was for demonstration that WINE_FULLSCREEN_FSR_SCALE=4 work, image resolution was 2560x1080 while 640x270 was used by the benchmark.

@DadSchoorse
Copy link
Contributor Author

Closing this since I don't think it will get merged given the limitations. If you want FSR I think ValveSoftware/gamescope#360 is the better option since it will work with every game.

@gabriele2000
Copy link

Closing this since I don't think it will get merged given the limitations. If you want FSR I think Plagman/gamescope#360 is the better option since it will work with every game.

What about RSR? Same tech, driver-integrated, perhaps it could be integrated in mesa, to make it global even for native linux games?

@DadSchoorse
Copy link
Contributor Author

DadSchoorse commented Jan 13, 2022

Well afaiu RSR is exactly that, FSR without per game integration. No need to put it in the driver, the correct place to do upscaling is the compositor. And you should be able to use gamescope's FSR with all linux games, even native.

@Atemu
Copy link

Atemu commented Jan 14, 2022

I agree that the compositor is the best place for this but having it in fshack is still useful to those who cannot use gamescope for one reason or another.

@DadSchoorse
Copy link
Contributor Author

@Atemu feel free to keep using this code then, I think TKG/GE still maintain a rebased version of it.

@Atemu
Copy link

Atemu commented Jan 26, 2022

That's pretty much the status quo anyways. The point of all of this is upstream Proton inclusion of the feature though which can only happen here.

@DadSchoorse
Copy link
Contributor Author

At this point it's pretty clear that it won't be accepted into upstream Proton.

@mercuriete
Copy link

I think a problem here (please correct me if I am wrong) is that gamescope is Wayland only.
There are some users that just because they wanted to use X11 or any non-justifiable reason they don't want to migrate to Wayland.
Given said that... if gamescope becomes popular I will move to Wayland in the long run.

@DadSchoorse
Copy link
Contributor Author

DadSchoorse commented Jan 26, 2022

Gamescope works just fine in nested mode on X11. The real (hopefully temporary) problem is that it does not work on nvidia because they don't support some required system integration extensions.

@aeikum
Copy link
Collaborator

aeikum commented Jan 26, 2022

At this point it's pretty clear that it won't be accepted into upstream Proton.

FWIW the only outstanding issues from my review above were how best to handle and document the environment variable options. But, it sounds like it's more appropriate if it lives at a lower level than Proton itself anyhow.

@pchome
Copy link

pchome commented Jan 26, 2022

I wonder if its possible to move Vulkan part (so all tuning) to vulkan layer. Then Wine can do it's part if such layer was loaded. All documentation will be on layer's side. Maybe even implementation (FSR, NVIDIA's thing, whatever else will be invented) could be easily switched by loading different vulkan layer.

For example if an implicit layer launching by BLAHBLAH_FSR=1 environment variable then it can be added to Steam command options like BLAHBLAH_FSR=1 %command% (iirc). Wine will check that env var or loaded layer name or else detection mechanics, maybe will read some additionally required data from environment or from layer (iirc layer can provide such data).
Also such layers could be used by other applications or compositors, they will need to do their part with window (if I understand this correctly).

@kakra

This comment has been minimized.

@DadSchoorse

This comment has been minimized.

@arrowgent
Copy link

for anyone that continues to monitor the progress of wine/proton AMD FSR

please see community patches here:

https://github.com/Frogging-Family/community-patches/blob/master/wine-tkg-git/amd_fsr_fshack.mypatch

if you know of other community maintained patches, post em up!

thanks a ton for dadschoorse for his contribution

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.

None yet