VSync and Max Framerate options implementation.#1124
VSync and Max Framerate options implementation.#1124HARMSTONE wants to merge 6 commits intosmartcmd:mainfrom
Conversation
codeHusky
left a comment
There was a problem hiding this comment.
There are a lot of unrelated changes to the topic of this PR. This is not mergable as-is.
Minecraft.Client/GameRenderer.cpp
Outdated
| renderLevel(a, lastNsTime + 1000000000 / maxFps); | ||
| } | ||
|
|
||
| renderLevel(a, 0); |
There was a problem hiding this comment.
How is the frame cap functional now? It looks like it's completely cut out of the code here
There was a problem hiding this comment.
I moved frame limiting to Windows64_Minecraft.cpp (line 1963-1980).
Minecraft.Client/GameRenderer.cpp
Outdated
| if (!player) | ||
| return false; | ||
| int active = 0; | ||
| int indexMap[NUM_LIGHT_TEXTURES] = {-1, -1, -1, -1}; |
There was a problem hiding this comment.
This is regarding all of the changes to this function -- why are you changing this? This seems completely irrelevant to your PR
There was a problem hiding this comment.
A lot of those changes were because i used GameRenderer and Windows64_Minecraft before the Modernize project codebase commit, but it should be fixed now.
Minecraft.Client/GameRenderer.cpp
Outdated
| for (int j = 0; j < XUSER_MAX_COUNT && j < NUM_LIGHT_TEXTURES; ++j) | ||
| m_cachedGammaPerPlayer[j] = gamma; | ||
| { | ||
| std::shared_ptr<MultiplayerLocalPlayer> player = Minecraft::GetInstance()->localplayers[j]; |
There was a problem hiding this comment.
Why is this being changed
| void UIScene_SettingsGraphicsMenu::updateComponents() | ||
| { | ||
| const bool bNotInGame=(Minecraft::GetInstance()->level==nullptr); | ||
| bool bNotInGame = (Minecraft::GetInstance()->level == NULL); |
There was a problem hiding this comment.
Why are you reverting to different types of null here?
|
|
||
| doHorizontalResizeCheck(); | ||
|
|
||
| bool bInGame = (Minecraft::GetInstance()->level != NULL); |
| int ClampFov(int value) | ||
| const int fpsCaps[] = {30, 60, 120, 0}; | ||
|
|
||
| int clampFov(int value) |
There was a problem hiding this comment.
Why is this suddenly lowerCamelCase?
|
|
||
| [[maybe_unused]] | ||
| int FovToSliderValue(float fov) | ||
| int fovToSliderValue(float fov) |
There was a problem hiding this comment.
Same here, why is this also lowerCamelCase now?
There was a problem hiding this comment.
it's pretty obvious it's ai generated. lol.
|
This implementation is too sloppy and there's numerous debug prints that are just using standard prints instead. We won't be merging this one. |
Description
Adds Max Framerate (FPS Cap) and VSync options to the graphics settings menu. Also personally encountered an issue where running in fullscreen with VSync forced on caused FPS to drop from ~1000 to 20-30, making the game unplayable, which motivated adding the VSync toggle.
(Also this is a reupload of the original fork because my account got suspended for unknown reason.)
Changes
Previous Behavior
There wasn't a Max Framerate or VSync option.
Root Cause
The graphics menu was lacking performance options.
Also I personally have an issue where running in fullscreen with VSync on caused FPS to drop from ~1000 to 20-30, making the game unplayable, which motivated adding the VSync toggle.
New Behavior
Now there are Max Framerate and VSync options in the graphics menu. VSync is a checkbox and Max Framerate is a slider (options are 30, 60, 120, Unlimited).
Showcase.mp4
Fix Implementation
Windows64_Minecraft.cpp now tracks frame time with QueryPerformanceCounter. It sleeps until the next frame is due, based on the selected FPS cap. The sync interval given to g_pSwapChain->Present is now controlled by the VSync setting.
AI Use Disclosure
Used for debugging.
Related Issues