-
Notifications
You must be signed in to change notification settings - Fork 459
Implement low latency provider #6666
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
base: master
Are you sure you want to change the base?
Conversation
Susko3
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
frameCount is accessed in a thread-unsafe manner, this could mean that the tests done in ppy/osu#35678 might be invalid.
The way low latency providers are initialised and used with all the try-catch-ignore-don't log seems convoluted and prone to hiding problems. It also doesn't consider a game providing multiple different low latency providers for different renderers. I would remove all public methods from GameHost and instead allow games to supply a list of ILowLatencyProviders via HostOptions.
osu.Framework/Platform/GameHost.cs
Outdated
|
|
||
| try | ||
| { | ||
| LatencyMark(LatencyMarker.RenderSubmitStart, frameCount); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The frameCount is set from the update thread and read here from the draw thread. This can cause a multitude of problems when running in multi-threaded:
- calls to
LatencyMark()in the same draw frame having different frame counts - frame count not being the one used when generating the
buffer(as the update thread runs twice as fast)
To make this correct, the frameCount should be stored in the buffer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 6163e21 (#6666) and 225a4e5 (#6666). Let me know if you had a different solution in mind.
I wonder if this might fix the frametime spikes I observed predominantly in the Reflex On and Boost modes.

(green is off, orange is on, blue is with boost)
| { | ||
| bool IsAvailable { get; } | ||
|
|
||
| void Initialize(IntPtr nativeDeviceHandle); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would move this to IDirect3D11LowLatencyProvider, to avoid the problem of passing the wrong device handle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 5c02d43 (#6666)
osu.Framework/Platform/GameHost.cs
Outdated
| } | ||
| } | ||
|
|
||
| public void SetLowLatencyMode(LatencyMode mode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't need to be public, FrameworkSetting.LatencyMode can and should be set through the FrameworkConfigManager.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in f250d43 (#6666)
|
Ahhh completely forgot about threading.. Pretty embarrassing, will go through and clean that up soon, along with the other review comments.
I was going to make a small write-up in the osu-sided PR talking about this as I felt it was also noteworthy to speak about. I agree that sending exceptions on the Say we get an exception in
It's probably more than enough to ensure every other piece is working, and to blindly trust that |
This PR implements
LowLatencyProvider, an interface that can be used by any latency reduction API to enable just-in-time rendering. Currently this is being implemented for use with NVIDIA Reflex, but it can easily be used with LatencyFlex as well.Opening as a draft as this PR is still incomplete, pending the addition of frame limiting logic. However the current code is more or less good for review.