Skip to content

Fix: TargetFrameRate setter used stale value and ignored vsync#88

Open
AshishT112203 wants to merge 1 commit into
TheFlyingFoool:masterfrom
AshishT112203:fix/fps-target-setter
Open

Fix: TargetFrameRate setter used stale value and ignored vsync#88
AshishT112203 wants to merge 1 commit into
TheFlyingFoool:masterfrom
AshishT112203:fix/fps-target-setter

Conversation

@AshishT112203

@AshishT112203 AshishT112203 commented May 26, 2026

Copy link
Copy Markdown
Contributor

Closes #59.

Two distinct bugs in the FPS settings path:

Bug A — TargetFrameRate setter used a stale backing-field value

The setter assigned Program.main.DrawRateLimiterTarget = S_TargetFrameRate before storing the new value into S_TargetFrameRate, so the limiter target was always one step behind the slider. Also unconditionally set UseDrawRateLimiter = true, ignoring vsync. Rewrote to store value first, use Math.Max(S_TargetFrameRate, 60) for parity with InitalizeFPSThings(), and respect UncappedFPS.

Bug B — Rate limiter disabled at startup when vsync is on

InitalizeFPSThings() set UseDrawRateLimiter = !UseVSync && UncappedFPS && TargetFrameRate >= 60 — explicitly disabling the limiter when vsync is on, trusting vsync alone to cap FPS. On the reporter's setup (and likely many others — adaptive sync / G-Sync / FreeSync / certain windowed configs), vsync silently fails to actually cap and the user sees 3000+ FPS at startup.

The reporter's existing workaround — "nudge the slider 240 → 180 → 240" — works precisely because the buggy TargetFrameRate setter force-enabled the rate limiter. We make the rate limiter a safety net on the load path too: enabled whenever UncappedFPS && TargetFrameRate >= 60, regardless of vsync. UseVSync setter updated to match so the three apply paths (UseVSync setter, TargetFrameRate setter, InitalizeFPSThings) all use the same calculation.

Also updated the V-Sync menu description (which said it "overrides FPS Target") and fixed the "Verticaly" typo while there.

Note: with this PR, FPS Target and V-Sync coexist (lower cap wins) instead of vsync overriding the target. For users on working-vsync setups, this is a no-op unless they set FPS Target lower than their refresh rate. If you'd prefer to keep the existing semantics and address #59 a different way (e.g. log when ApplyChanges() fails to honor vsync), happy to rework.

Test plan

  • Builds clean (~200 warnings as expected, no new ones)
  • Set FPS Target = 240, restart with V-Sync ON → cap holds at 240 (was 3000+)
  • Set FPS Target = 240, restart with V-Sync OFF → cap holds at 240
  • Set FPS Target = 0, restart → uncapped (limiter disabled)
  • Set FPS Target = 240 → 180 → 240 live without restart → cap follows slider exactly each step

…yingFoool#59)

Two distinct bugs in the FPS settings path:

Bug A — TargetFrameRate setter read S_TargetFrameRate before storing
the new value, so the limiter target was always one step behind the
slider. Rewrote the setter to store value first, use Math.Max for
parity with InitalizeFPSThings, and respect UncappedFPS.

Bug B — InitalizeFPSThings explicitly disabled the rate limiter when
vsync was on, trusting vsync alone to cap FPS. On setups where vsync
silently fails to cap (G-Sync / FreeSync / certain windowed configs)
this leaves FPS uncapped at startup — closes TheFlyingFoool#59. Made the rate
limiter a safety net: enabled whenever UncappedFPS && TargetFrameRate
>= 60, regardless of vsync. UseVSync setter updated to match so the
two apply paths stay consistent.

Updated the V-Sync menu description to reflect the new semantics
(and fixed the "Verticaly" typo).
@AshishT112203 AshishT112203 force-pushed the fix/fps-target-setter branch from a0a35cb to 57fe511 Compare May 26, 2026 06:22
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.

FPS settings not sticking properly

1 participant