Skip to content

Change a window’s VSync without disturbing another window - #186

Open
relh wants to merge 5 commits into
treeform:masterfrom
relh:codex/portable-runtime-vsync
Open

Change a window’s VSync without disturbing another window#186
relh wants to merge 5 commits into
treeform:masterfrom
relh:codex/portable-runtime-vsync

Conversation

@relh

@relh relh commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

A game may need to change OpenGL VSync without recreating its window. The previous PR could switch the current rendering context, report a change on a backend it did not control, or call a GLX extension the server did not support.

The setter now preserves the caller’s context. macOS and Windows read the driver’s interval; Linux checks advertised extensions and verifies EXT/MESA readback. An initially disabled Linux window works without swap control. Unsupported enable requests raise an error; SGI cannot turn an already enabled interval off. CPU, Metal, DirectX, and Vulkan setters reject changes because the application owns presentation there. Browser VSync remains controlled by the browser.

Validation:

  • On a native Mac, the old setter changed the peer window’s current context. The fixed test preserves it and reads intervals 0, 1, 0. On the final integrated tree, 30 rendered swaps took 33 ms, 311 ms, and 6 ms respectively.
  • With Mesa/Xvfb, the old setter accepted an enable request despite the server advertising no swap-control extension. The fixed test creates a disabled window, rejects unsupported enable, and keeps the peer context. This server has no supported swap-control extension, so it does not validate EXT/MESA/SGI success or physical frame pacing.
  • The native Mac CPU backend rejects changes without changing its reported option.
  • A focused runtime test replaces the old compile-only API check. It is wired into existing Mac/Linux CI, Windows CPU CI, and a Windows OpenGL run using Mesa. The final hosted Windows OpenGL/Mesa test, Windows CPU test, Mac OpenGL/CPU tests, and Linux unsupported-extension test have passed; the complete final build matrix has also passed.

Physical Windows/Linux pacing, SGI/MESA success paths, and native non-CPU alternative backends still need runtime evidence. These limits are not covered by the Mac result.

Current CI run passed on Linux, macOS, and Windows. The runtime limits above still apply.

@treeform

treeform commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Thanks, the direction is right: swap interval is a per-context runtime knob on all three platforms, so a live window.vsync = setter is the correct shape. A few things need fixing before this can merge, and it needs real per-platform runtime tests rather than nim check.

Issues

1. Linux regression: newWindow(vsync = false) can now raise.
newWindow now always goes through applyVsync(vsync). On a driver with no swap-control extension that raises "VSync control is not supported", and on SGI-only drivers it raises "Disabling VSync is not supported". On master vsync = false simply skipped the call and worked everywhere. Turning vsync off when nothing can be set should be a silent no-op (the driver default is already "off" for GLX). Only an explicit enable that cannot be honoured should error.

2. Non-GL builds record a change that did not happen.
On Windows with useDirectX/useVulkan/useCpu and on macOS with useMetal4/useCpu, the setter flips vsyncEnabled without touching anything. That is the same "state change that did not happen" problem the description calls out for GLX. Either raise WindyError on those builds (windy does not own presentation there, the app does), or leave the flag untouched and document that the property is OpenGL-only.

3. makeContextCurrent as a side effect.
Both the setter and applyVsync switch the current context. In a multi-window app the caller's current context is silently changed. Either document it, or save/restore the previous context around the swap-interval call.

4. Getter reports the cached bool, not the driver.
vsync returns vsyncEnabled. Where the platform can read the interval back, the getter (or at least the tests) should use it so a failed or ignored call cannot leave the cached value lying.

Tests needed per platform

The CI matrix only compile-checks. This change is behavioural, so each platform needs a runtime test that creates a window, toggles vsync false → true → false, and verifies two things: the driver-side interval round-trips, and frame pacing actually changes.

Linux / X11

  • Run under Xvfb with Mesa llvmpipe (LIBGL_ALWAYS_SOFTWARE=1) so it works on the GitHub runner.
  • Read back with glXQueryDrawable(display, window.handle, GLX_SWAP_INTERVAL_EXT, addr value) after each set and assert it matches.
  • Cover the fallback branches: force glXSwapIntervalEXT = nil to hit the MESA path, and nil both to hit SGI/none, then assert newWindow(vsync = false) still succeeds and vsync = false does not raise.
  • Note that Xvfb has no real vblank, so the "enable/disable/re-enable proof" there only shows the call is accepted. A frame-time check (N swaps with vsync on should take ≥ N/refresh seconds, off should be much faster) needs a real X server and belongs in a manually-run example, not CI.

Windows

  • GitHub Windows runners only expose OpenGL 1.1 via GDI. Drop Mesa's opengl32.dll (mesa-dist-win / llvmpipe) next to the test binary so wglSwapIntervalEXT exists.
  • Read back with wglGetSwapIntervalEXT() after each set and assert.
  • Test the useDirectX/useVulkan/useCpu builds: the setter must either raise or leave vsync unchanged (see issue 2), not silently report the new value.
  • Frame-time check on real hardware, manual.

macOS

  • GitHub macOS runners have a WindowServer session, so a real NSOpenGLView window can be created in CI.
  • Read back with openGLContext.getValues(addr value, NSOpenGLContextParameterSwapInterval) and assert.
  • Test the useMetal4 and useCpu builds the same way as the Windows non-GL builds.
  • Frame-time check on real hardware (note flushBuffer with interval 1 blocks on vblank, so this one is easy to measure), manual.

Emscripten

  • Already a no-op setter with getter fixed at true. Add the same test file and assert the getter does not change, so the four backends share one test.

A single tests/test_vsync.nim that does the toggle loop and calls a small per-platform readBackSwapInterval() helper would cover all of this and can run on the existing three-OS matrix.

@relh relh left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[Codex, acting on behalf of relh]

Reviewed 604d266. The issues in André's existing review still apply at this head: Linux now attempts VSync setup even for vsync = false and can reject otherwise usable contexts without a supported extension; non-OpenGL backends can report a changed cached state without changing presentation; and setters can change the current window context. I found no additional distinct issue to add inline. The three-OS compile checks do not exercise those runtime/backend paths; this pass was source review only.

@relh relh changed the title Add portable runtime VSync control Change a window’s VSync without disturbing another window Sep 9, 2026
@relh

relh commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

The review was right: the earlier compile-only checks did not establish runtime behavior. I fixed the context side effects, rejection of changes on application-owned presentation backends, and driver readback on Mac/Windows and GLX EXT/MESA. Linux now checks advertised extensions; the old branch accepted enable on my Mesa/Xvfb setup even though no swap-control extension was advertised. My first probe then crashed while attempting unsupported driver readback; that probe crash is not evidence that the setter itself crashed.

The final integrated Mac test preserves a peer context, reads driver intervals 0 → 1 → 0, and renders 30 swaps in 33 ms → 311 ms → 6 ms. The native CPU build rejects the setter without changing its option. On Mesa/Xvfb, creating and retaining a disabled window works and unsupported enable is rejected. That server has no swap-control extension, so it cannot prove the successful EXT/MESA/SGI paths.

The single focused runtime test is in the existing CI matrix, including Windows CPU and an OpenGL run with Mesa. The first Windows attempt exposed a test setup error: nim r put its executable in the cache, away from the Mesa DLLs. The updated command places it beside the DLLs. I am checking the final run rather than treating compile success as validation.

Physical Windows/Linux pacing, successful SGI/MESA operation, and non-CPU alternative presentation backends still need runtime evidence. I have kept those limits in the PR description instead of marking the whole review satisfied.

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.

2 participants