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

Support variable refresh rate display #8022

Closed
dmex opened this issue Oct 23, 2020 · 7 comments
Closed

Support variable refresh rate display #8022

dmex opened this issue Oct 23, 2020 · 7 comments
Labels
Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting

Comments

@dmex
Copy link

dmex commented Oct 23, 2020

The Windows Terminal is missing a flag required for variable refresh rate display support which ends up directly causing two additional bugs in the dx renderer.

hr = _dxgiSwapChain->Present1(1, 0, &_presentParams);

and here:

hr = _dxgiSwapChain->Present(1, 0);

Must to be changed from 1 to 0.

Those two changes are needed for fixing both #649 and #7147 and mentioned in the nvidia documentation:

image

@dmex dmex added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Oct 23, 2020
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Oct 23, 2020
@DHowett
Copy link
Member

DHowett commented Oct 23, 2020

I believe @lhecker tried both of these things and documented them in #649. Do you recall, Leonard?

This thread may not need to be separate from 649; comments about the behavior and why this may not be in WT’s purview start here: #649 (comment)

@DHowett
Copy link
Member

DHowett commented Oct 23, 2020

If you have tested and confirmed that this works, we’d love to evaluate it! 😄

@lhecker
Copy link
Member

lhecker commented Oct 23, 2020

There's a slight chance that I did something wrong, when I tried to implement Variable Refresh Rate (VRR) support, but I honestly doubt that, since I had full Nvidia G-Sync support locally.

But what I said in #649 (comment) basically explains why VRR is the entirely wrong approach for us, a non-game application which composites with the rest of the desktop.
A VRR application controls the frame rate of the monitor. If the VRR application draws at 100 FPS, the monitor will run at 100 Hz. If the application draws at 20 FPS, well the monitor will run at 20 Hz, etc.
A monitor running at 20 Hz, will make your cursor look stuttery, the browser you have open side-by-side and so on. To prevent these unsightly side effects we'll have to draw at full speed all the time. That would fix #649, but drain laptop batteries extremely fast just by the application being open.
VRR was designed to prevent screen tearing when applications dip below the maximum refresh rate of a monitor, without having the (about) 1-frame overhead of V-Sync. This comes at the sacrifice that you cannot do on-demand rendering anymore and that applications cannot properly composite with the rest of the desktop.
Considering that most people will not run Windows Terminal (WT) in an exclusive fullscreen mode, VRR support will not be beneficial for this project. The only way it could, is if we add a VRR setting, detect VRR capable hardware and only enable VRR when WT is in an exclusive fullscreen mode (I don't know how to do the second step).

If you could tell me how to make Nvidia's drivers stop recognizing this application as one that wants G-Sync I'd gladly implement that on the other hand, because we could then finally close #649. 😄

@zadjii-msft
Copy link
Member

Thanks for the additional details @lhecker. From the sounds of this, since there's nothing else actionable here, we should probably close this thread as a dupe of #649, yea?

@dmex
Copy link
Author

dmex commented Oct 24, 2020

VRR support will not be beneficial for this project. The only way it could, is if we add a VRR setting

The issue affects other applications when you're using multiple monitors with Discord, Skype, Slack, Twitch, Visual Studio, Youtube etc... you can't use those screens and applications when the Terminal window has focus.

if we add a VRR setting, detect VRR capable hardware

The best solution was using "UseDx": false but there should be a setting for disabling vsync similar to games.

only enable VRR when WT is in an exclusive fullscreen mode (I don't know how to do the second step).

If it's the same with IDesktopWindowXamlSourceNative as the d3d12 samples IslandWindow::OnSize would need GetFullscreenState: https://github.com/microsoft/DirectX-Graphics-Samples/blob/master/Samples/Desktop/D3D12Fullscreen/src/D3D12Fullscreen.cpp#L636-L646

The sample passes the flag to Present() without vsync here:
https://github.com/microsoft/DirectX-Graphics-Samples/blob/master/Samples/Desktop/D3D12Fullscreen/src/D3D12Fullscreen.cpp#L568-L575

close this thread as a dupe of

Yep, we can close it 👍

@dmex dmex closed this as completed Oct 24, 2020
@Luckz
Copy link

Luckz commented Oct 28, 2020

Correct me if I'm wrong, please: wouldn't the correct fix be G-Sync being disabled for all affected MS Store apps (without affecting games)? Either via the app communicating to the driver that it is not a game and will not benefit from variable refresh rate, or the driver detecting through profiles or algorithms that this is the case.

The behaviour in that case with a 59/60 fps game (internal limiter) and a 144 Hz Windows Terminal (Nvidia profile disabling G-Sync for Terminal):
if the game is in focus, and Terminal is out of sight, the screen will be at 59/60 Hz.
With Terminal in focus, the screen will be at 144 Hz.
With both overlapping in a specific way, and the game on top, the OSD-displayed refresh rate varies wildly and seems to be around 100 Hz. A hybrid of both if you so want.

@lhecker
Copy link
Member

lhecker commented Oct 28, 2020

Correct me if I'm wrong, please: wouldn't the correct fix be G-Sync being disabled for all affected MS Store apps (without affecting games)?

Yep that's exactly what I tried to convey @Luckz.
While VRR is really nice, it doesn't make a lot of sense the way it works for (non-fullscreen) windowed applications.
Nvidia synchronizes the monitor refresh rate with the frame rate of the topmost/focused application.
While that works well enough for fullscreen applications it breaks down for windowed ones. For instance if you have a Youtube video running side-by-side with Windows Terminal the video will now get displayed at 2 FPS.
What VRR drivers like Nvidia's should do instead is refresh the monitor at the highest frame rate of all visible applications.
Alternatively they should stop assuming all UWP applications want to use VRR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting
Projects
None yet
Development

No branches or pull requests

5 participants