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

Unify FPS and V-Sync controls and behavior between renderer #5074

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

Spodi
Copy link
Contributor

@Spodi Spodi commented Feb 16, 2025

  • DirectX's "Match Refresh Rate" is now a checkbox like in OpenGL and Metal.
  • Clamp max FPS to refresh rate when V-Sync is enabled or can't be disabled, regardles of the FPS sliders setting, to avoid slowdown in Metal / OpenGL and dropping frames in DirectX as much as possible.
  • FPS slider allows up to 360 FPS in every renderer. This needs V-Sync toggle in SDL (OpenGL + Metal) Kenix3/libultraship#822, or it doesn't make much sense (currently included in this PR to allow testing).

Build Artifacts

@larsy1995
Copy link
Contributor

Did some quick testing on macOS and it seems to work just as intended on both OpenGL and Metal.

@aMannus
Copy link
Contributor

aMannus commented Feb 20, 2025

FYI I want to get this in but we're in the middle of moving over to the modern menu, so this may have to be parked until that's done.

Copy link
Contributor

@briaguya-ai briaguya-ai left a comment

Choose a reason for hiding this comment

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

this is still showing LUS changes. as of #5092 everything this needs from the LUS side should be in, so there should be no changes to the LUS submodule

@Spodi Spodi requested a review from briaguya-ai February 21, 2025 14:14
@Spodi
Copy link
Contributor Author

Spodi commented Feb 21, 2025

Oops, I just clicked something on my phone. There is nothing to review yet xD

@Spodi
Copy link
Contributor Author

Spodi commented Feb 21, 2025

Included LUS changes should be removed now

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.

4 participants