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

misc: Enable DXGI present mode on NVIDIA when available. #6645

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

MutantAura
Copy link
Collaborator

@MutantAura MutantAura commented Apr 10, 2024

In driver 526.47 Nvidia added an option to control panel which allowed OpenGL/Vulkan applications to use a layered DXGI swapchain for some nice benefits.

  1. Improved compatibility with VRR technoligies such as GSync and FreeSync.
  2. Reduced display latency when in fullscreen.
    image

While users could already manually enable this themselves, the advantages here are massive and shouldn't be hidden away like this.
Breath of the wild atop the Plateau tower is used for the below examples.

Native present

Present latency (Vsync ON): 12.7ms
image

Present latency (Vsync OFF): 15.0ms
image

VRR possible: No
image

Layered DXGI

Present latency (VSync ON): 2.58ms
image

Present latency (VSync OFF): 5.44ms
image

VRR possible: Yes
image

I've taken the liberty of blanket enabling here but I'd also like feedback on if this should be an option.
The control panel tooltip states there could be some overhead when using this setting. While I saw no framerate or 1% low changes in my limited testing, lower end system tests could be good here.

Additionally adjusted the previous NVAPI helper code to be a little more option agnostic.

@github-actions github-actions bot added the gui Related to Ryujinx.Ui label Apr 10, 2024
@ryujinx-mako ryujinx-mako bot requested review from AcK77, emmauss, TSRBerry and a team April 10, 2024 21:51
Copy link
Member

@AcK77 AcK77 left a comment

Choose a reason for hiding this comment

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

Maybe we should do something for Nvapi/NvAPI differencies here and there. That could be better for consistencies.

}
catch
{
// NVAPI is not available, or couldn't change the application profile.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should log that using the logger? Idk if it's useful or not... (Same for L30)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've changed up this class a fair bit and added logging. Should hopefully be a bit cleaner.

src/Ryujinx.Common/GraphicsDriver/NVDriverHelper.cs Outdated Show resolved Hide resolved
src/Ryujinx.Common/GraphicsDriver/NVDriverHelper.cs Outdated Show resolved Hide resolved
@ryujinx-mako ryujinx-mako bot requested a review from a team April 10, 2024 22:13
@dxgldotorg

This comment was marked as resolved.

@MutantAura

This comment was marked as resolved.

@dxgldotorg

This comment was marked as resolved.

@MutantAura

This comment was marked as resolved.

@dxgldotorg

This comment was marked as resolved.

NVThreadedOptimization.SetThreadedOptimization(enabled);
if (id == NvapiSettingId.OglThreadControlId)
{
Environment.SetEnvironmentVariable("mesa_glthread", enabled.ToString().ToLower());
Copy link
Member

Choose a reason for hiding this comment

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

These settings are not for nvidia, yet the function has been renamed with "nv" in the name. I'm not sure if these are actually doing anything right now, due to how env vars can be cached. (maybe if the change happens before the driver is loaded...?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I suppose this is still somewhat tied to how this whole chain used to be setup.
Threading is currently toggled from:

DriverUtilities.ToggleOGLThreading(ConfigurationState.Instance.Graphics.BackendThreading == BackendThreading.Off);
which sets these environment variables and then goes into the NVAPI section.

In order to generalize how NVCP settings are set, I instead chose to pass the setting ID directly and this is the singular case (hence the if) that we also set environment variables alongside. Perhaps the environment variables should be set in a different method now this one isn't just to toggle threading?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, but the name for that method would probably be ToggleOGLThreading, which more indicates that the nv setting change should be called from this method rather than replacing it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right I see what you mean. Would this work better

public static class DriverUtilities
    {
        public static void ToggleOglThreading(bool enabled)
        {
            Environment.SetEnvironmentVariable("mesa_glthread", enabled.ToString().ToLower());
            Environment.SetEnvironmentVariable("__GL_THREADED_OPTIMIZATIONS", enabled ? "1" : "0");

            ToggleNvDriverSetting(NvapiSettingId.OglThreadControlId, enabled);
        }
        
        public static void ToggleNvDriverSetting(NvapiSettingId id, bool enabled)
        {
            try
            {
                NVDriverHelper.SetControlOption(id, enabled);
            }
            catch
            {
                Logger.Warning?.Print(LogClass.Application, "Failed to set NVIDIA driver settings. NVAPI may be unavailable.");
            }
        }
    }

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think that would cover this case well. I'm not sure we should be printing a warning here, since it will always fail on AMD.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alright I've swapped this up. Moved to a less aggressive info log because I really don't like an empty catch here.
I see little harm with this being info'd when we have stuff like Hypervisor and Colour space passthrough which do the same on unsupported systems.

Quick change if you insist though.

@ryujinx-mako ryujinx-mako bot requested a review from a team April 25, 2024 17:01
Copy link
Member

@riperiperi riperiperi left a comment

Choose a reason for hiding this comment

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

Enabling this setting makes ryujinx run with visible tearing on my system. Vsync should be left up to the application based on my global settings, but tearing is still present. My GPU requests Mailbox present mode, not immediate, so it should have vsync.

VK_PRESENT_MODE_MAILBOX_KHR specifies that the presentation engine waits for the next vertical blanking period to update the current image. Tearing cannot be observed.

image

My setup has two displays, display 1 at 144hz, display 2 at 60hz. I'm testing on display 2, though the tearing is also slightly visible on display 1. Due to the Ryujinx vsync timer, the tearing appears at nearly the same location in every frame, as it has pretty accurate timing for queuing presents at 60hz. At 144 it appears at a different location each time.

Forcing vsync to on in the driver sidesteps the issue, but means that disabling ryujinx vsync can't go fully uncapped, so it's not a great idea to do that.

I can't say this is a good default behaviour, needing to force vsync in the driver to match the target hardware will just leave a bad impression.

Not sure where to proceed from here. First step would probably be verifying that screen tearing also appears for other people when the driver vsync is set to auto, and they're using a 60hz monitor. Second would be testing if swapchain mode FifoKhr exhibits these issues, and how that might impact your present timing numbers. I'd imagine allowing tearing could result in a significant "boost" to those numbers, so you may already be experiencing it.

}
catch
{
// NVAPI is not available, or couldn't change the application profile.
Logger.Info?.Print(LogClass.Application, "NVAPI was unreachable, no changes were made.");
Copy link
Member

Choose a reason for hiding this comment

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

It'll still log this on every boot on non-nvidia PCs, since it doesn't guard against calling this based on vendor. Since it also tries to set the present mode, it'll even log it twice. I do think it should just not log anything at all.

@MutantAura
Copy link
Collaborator Author

Correct on all counts, disabling VRR and setting my display to 60Hz does show visible tearing with Mailbox but not with FifoKhr... It does complicate any path forward here because Fifo massively increases latency as you also expected. Up to highs of 50ms.
Can also reproduce in Cemu (Mailbox also used if available), so I can only assume it's something breaking in the shift to the DX presentation engine.

May be dead in the water as I really dont think there is much point duplicating this as an option from NVCP, especially with the caveats that you basically need VRR to make it worthwhile. If nothing else I'd like to keep the NVAPI adjustments as it makes any future interactions with it a little easier.

@MutantAura MutantAura marked this pull request as draft April 28, 2024 18:09
@dxgldotorg
Copy link

Correct on all counts, disabling VRR and setting my display to 60Hz does show visible tearing with Mailbox but not with FifoKhr... It does complicate any path forward here because Fifo massively increases latency as you also expected. Up to highs of 50ms. Can also reproduce in Cemu (Mailbox also used if available), so I can only assume it's something breaking in the shift to the DX presentation engine.

May be dead in the water as I really dont think there is much point duplicating this as an option from NVCP, especially with the caveats that you basically need VRR to make it worthwhile. If nothing else I'd like to keep the NVAPI adjustments as it makes any future interactions with it a little easier.

Fruit emulator did the same thing too. It must be a driver issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gui Related to Ryujinx.Ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants