Explicit user-controlled pixel scale and HiDPI fix - #579
Conversation
b90e4ae to
d62ec37
Compare
cf_app_get_dpi_scale / cf_app_dpi_scale_was_changed read like the OS's vague "suggested UI content scale" concept. The value is actually SDL_GetWindowDisplayScale -- the OS's points-to-pixels conversion for the window's display -- so name it that. Hard rename, no compat alias. Also drops CF_App::dpi_scale_prev, which was written once at init and never read. First slice of the RandyGaul#579 split.
bf973ba to
49a6b0c
Compare
…ion) Experiment, not a proposal: remove all event-driven HiDPI automagic in favor of user-controlled state, to see what the model looks like. - cf_app_set_pixel_scale: pixel scale is now a plain user value (AA + glyph density); the canvas is resized explicitly via set_canvas_size. - cf_app_pixel_scale_was_changed + cf_app_get_natural_pixel_scale; a density change only raises dpi_scale_was_changed, never applies. - Startup is the single automatic step: canvas at natural density, default 2d projection from the logical window size -- both set once. - cf_draw_projection is now truly sticky: window resizes, canvas recreation, and MSAA changes never touch the projection. - hidpi sample carries the copy-paste resize/density recipe and forced 1x/2x/4x switching; test_hidpi reworked for the new contracts. 350/350 tests pass on a 2x display (master baseline: 319/339 -- the 20 Retina failures were the half-size regression, which dies structurally here since nothing ever rebuilds the projection in pixel units). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LHq3Cx7xcjAQhji9m1ADJn
bullno1's follow-up suggestion on the PR RandyGaul#575 thread: package the recurring three-step reaction (set pixel scale, resize canvas to window * scale, rebuild the logical-points projection) into a single public helper. The window size is read internally rather than passed in -- it is always current by the time user code reacts to an event, and passing it would only invite stale values. The sample, the shared test-app sweep, and the hidpi tests all shrink to one call each, which was the tell that the helper deserved to be public API. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LHq3Cx7xcjAQhji9m1ADJn
Terminology follows the SDL functions underneath, per the PR thread: - cf_app_get_display_scale / cf_app_display_scale_was_changed (backed by SDL_GetWindowDisplayScale): what the OS wants, point-to-pixel, consistent across platforms. Replaces both the dpi_scale pair (term dropped) and cf_app_get_natural_pixel_scale (SDL_GetWindowPixelDensity is 1.0 on Windows/X11 where the scale lives in content-scale, so display scale is the right value to follow everywhere). - cf_app_update_display(scale): renamed from cf_app_apply_pixel_scale. - pixel_scale keeps its name: how fonts/shapes are scaled, arbitrary or following the reported display scale. - Initial pixel_scale now comes from display scale (identical on Mac, correct on Windows where density alone under-reports). Window size and mouse coordinates are now normalized to logical points at the SDL boundary via SDL_GetDisplayContentScale (window create, set_size, resize events, mouse events; touch already derives from app->w/h). A no-op on macOS where content scale is 1.0 -- the normalization needs verification on Windows and X11 at >100% scaling. 351/351 tests, docsparser clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LHq3Cx7xcjAQhji9m1ADJn
cf_destroy_draw freed s_draw but never nulled it (the make-error path did), so cf_app_set_pixel_scale on a NO_GFX app created after a gfx app's destruction passed the hook's s_draw guard with a dangling pointer -- an instant access violation on Windows, silently-readable freed memory on POSIX. The synthetic WINDOW_RESIZED event in test_hidpi pushed 500 raw, but SDL resize events carry raw window coordinates which CF now divides by the display content scale -- not 1.0 on the X11 CI runners. Push points * content_scale like a real event, and compare against the handler's exact round-trip. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LHq3Cx7xcjAQhji9m1ADJn
…ic event On X11 the ConfigureNotify confirming test_make_app's SDL_SetWindowSize can arrive late and land in the same pump as the synthetic resize event, stomping app->w after it. Drain it first -- the same settling dance the one-shot test used on the previous branch, lost in the rewrite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LHq3Cx7xcjAQhji9m1ADJn
Shows raw screen coords next to cf_screen_to_world's translation, so the HiDPI mouse mapping can be eyeballed against known shape positions at any forced pixel scale. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The pixel scale only ever changes when user code calls cf_app_set_pixel_scale, so this flag just echoed the caller's own action back one frame later -- no OS event ever set it. The real event remains cf_app_display_scale_was_changed, and anyone wanting change detection can compare cf_app_get_pixel_scale against a cached value. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S4anUDPNkAio27kQbWAqrn
49a6b0c to
c481e04
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core cross-platform window/input scaling semantics and HiDPI behavior in the app loop, so it needs human validation across multiple OS/display configurations despite the new tests.
Pull request overview
Reworks Cute Framework’s HiDPI model to remove event-driven/automatic canvas + projection updates and instead make pixel scale an explicitly user-controlled value, with a single helper (cf_app_update_display) for the common “resize/scale-change recipe”. This is a core behavioral shift intended to prevent projection/canvas mismatches (e.g., the #575 half-size regression) by structurally eliminating hidden refresh paths.
Changes:
- Introduces explicit pixel-scale APIs (
cf_app_set_pixel_scale,cf_app_update_display) and updates app/canvas/projection contracts + docs accordingly. - Normalizes SDL-reported window/mouse coordinates into CF logical points via display content scale at the SDL boundary (resize + mouse events, plus window sizing at creation /
cf_app_set_size). - Adds a new
test_hidpisuite and updates existing app/canvas persistence tests; refreshes thehidpisample to demonstrate the manual model and scale-following vs forced scaling.
File summaries
| File | Description |
|---|---|
| test/test_hidpi.cpp | Adds readback-based tests for the new HiDPI/pixel-scale contracts and resize/scale-change behavior. |
| test/test_app.cpp | Updates tests to assert canvas size persistence across resize and MSAA changes. |
| test/test_app_shared.cpp | Updates shared app reset logic to restore startup-equivalent display settings via cf_app_update_display. |
| test/main.cpp | Registers the new test_hidpi suite. |
| test/CMakeLists.txt | Adds test_hidpi.cpp to the test build. |
| src/internal/cute_draw_internal.h | Replaces the “app canvas resized” hook with a “pixel scale changed” hook for AA refresh only. |
| src/internal/cute_app_internal.h | Adds internal cf_app_get_content_scale() (SDL content scale) accessor and updates internal scale semantics. |
| src/cute_input.cpp | Removes automatic canvas/projection rebuilds on resize/scale events; converts SDL raw coordinates to logical points. |
| src/cute_draw.cpp | Removes automatic projection refresh on canvas resize; adds cf_draw_on_pixel_scale_changed() for AA recalculation. |
| src/cute_app.cpp | Implements explicit pixel-scale APIs, changes window sizing to logical points (via content scale), and makes canvas sizing persistent unless explicitly changed. |
| samples/hidpi.c | Updates sample to demonstrate the new manual recipe and adds interactive “follow vs forced” scale toggles + mouse coordinate readout. |
| include/cute_app.h | Documents the new public API and the revised HiDPI contract (display scale informational; pixel scale user-controlled). |
Review details
Suppressed comments (1)
src/cute_input.cpp:672
- Indentation/formatting in the SDL_EVENT_MOUSE_BUTTON_UP case has misaligned braces and space indentation; it should match the surrounding tab-indented style for consistency.
} else if (event.button.clicks == 2) {
app->mouse.click_type = CF_MOUSE_CLICK_DOUBLE;
}
} break;
- Files reviewed: 12/12 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } | ||
| break; | ||
| } break; |
| * @function cf_app_set_size | ||
| * @category app | ||
| * @brief Sets the size of the window in pixels. | ||
| * @param w The width of the window in pixels. | ||
| * @param h The height of the window in pixels. | ||
| * @related cf_app_get_size cf_app_get_position cf_app_set_position | ||
| * @brief Sets the size of the window in logical points. | ||
| * @param w The width of the window in logical points. | ||
| * @param h The height of the window in logical points. | ||
| * @remarks Only the window changes. The app canvas and the default 2d projection keep their current size -- | ||
| * update them alongside if desired, e.g. `cf_app_set_canvas_size` and `cf_draw_projection`; see the | ||
| * hidpi sample for the recipe. | ||
| * @related cf_app_get_size cf_app_get_position cf_app_set_position cf_app_set_canvas_size |
| Cute Framework | ||
| Copyright (C) 2026 Randy Gaul https://randygaul.github.io/ |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62fadad2fd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * The canvas keeps this size until the next `cf_app_set_canvas_size` call -- nothing resizes it behind | ||
| * your back. It is created once at startup at window size (in points) times the display's pixel density; | ||
| * after that, window resizes and display density changes only raise `cf_app_was_resized` / | ||
| * `cf_app_display_scale_was_changed`, and resizing the canvas in response is up to you (see the hidpi sample | ||
| * for the recipe). |
There was a problem hiding this comment.
Update stale HiDPI docs for the new resize contract
With this new persistent-canvas contract, the published docs still tell users the opposite: docs/topics/hidpi.md:24-26 says CF recreates the app canvas on resize/scale changes and cf_app_set_canvas_size is one-shot, docs/topics/application_window.md:100 says the same, and include/cute_draw.h:2214-2215 still says CF resets the projection on resize. Users following those pages won't call cf_app_update_display() after cf_app_was_resized(), so their resizable windows keep the old canvas/projection and render stretched or clipped.
Useful? React with 👍 / 👎.
| SDL_SetNumberProperty(props, SDL_PROP_WINDOW_CREATE_WIDTH_NUMBER, (int)CF_ROUNDF(w * creation_content_scale)); | ||
| SDL_SetNumberProperty(props, SDL_PROP_WINDOW_CREATE_HEIGHT_NUMBER, (int)CF_ROUNDF(h * creation_content_scale)); |
There was a problem hiding this comment.
Align cf_make_app docs with logical-point sizing
After this change, cf_make_app treats w/h as logical points and multiplies by the content scale before creating the SDL window. On Windows/X11 at 150% scaling, callers following include/cute_app.h:224-225 (pixels) who request 640x480 now get a 960x720 physical window; either the constructor docs/examples need to move to logical points along with cf_app_set_size, or this scaling should not happen here.
Useful? React with 👍 / 👎.
| HidpiGuard guard; | ||
|
|
||
| REQUIRE(cf_app_get_pixel_scale() == 1.0f); | ||
| REQUIRE(cf_app_get_display_scale() == 1.0f); |
There was a problem hiding this comment.
Stop forcing display scale to 1 for NO_HIGH_DPI
In the CF_APP_OPTIONS_NO_HIGH_DPI_BIT case, this only pins the applied pixel scale/backbuffer; SDL still reports the OS display/content scale on Windows and X11 (e.g. 1.5 at 150% desktop scaling), and cf_make_app stores that value in app->display_scale. This assertion will fail on those supported HiDPI desktops and would force cf_app_get_display_scale() to stop reporting the OS scale that the new API docs tell users to follow.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@RandyGaul @bullno1 I trimmed the PR. Please have a look and let me know what needs clarifying. |
Working demo of @bullno1's proposal from the #575 review thread (including the follow-up naming refinements): remove all event-driven HiDPI behavior and make pixel scale a plain user-controlled value. Draft on purpose — it exists to give that design discussion something concrete to poke at.
window_size * display_scale, default 2d projection from the logical window size, both set once. Resizes and scale changes only raise flags (cf_app_was_resized/cf_app_display_scale_was_changed).cf_app_get_display_scale(+_was_changed) isSDL_GetWindowDisplayScale— what the OS wants, consistent across platforms; thedpinames are gone.cf_app_set_pixel_scaleis how fonts/shapes scale — arbitrary, or following the reported display scale; it does not touch the canvas (cf_app_set_canvas_sizeis separate).cf_app_update_display(scale)is the all-in-one helper bringing canvas, projection, and pixel scale together, usable on both resize and scale change — the w/h params are omitted since the window size is always current by the time user code reacts.SDL_GetDisplayContentScale(window creation,cf_app_set_size, resize events, mouse events; touch derives from window size). This is a no-op on macOS (content scale 1.0) and needs verification on Windows/X11 at >100% scaling — I can only test on a Mac.cf_draw_projectionbecomes truly sticky andcf_app_set_canvas_sizepersistent. The half-size regression Fix HiDPI half-size regression: default 2d projection is logical points #575 fixes dies structurally — no code path can rebuild the projection in pixel units — and all of Fix HiDPI half-size regression: default 2d projection is logical points #575's deferral/guard machinery becomes unnecessary. 351/351 tests on a 2x display (master: 319/339, the 20 failures being that regression).Cost, stated plainly: breaking change. 44 of 76 samples use resizable windows and each needs the two-line recipe (deliberately not yet updated); canvas_modes.c needs a rewrite.
NO_HIGH_DPI_BITis kept: it is the only handle on the creation-time 1x swapchain (memory + final-blit bandwidth), whichcf_app_update_display(1)cannot replicate.Before
After
🤖 Generated with Claude Code
https://claude.ai/code/session_01LHq3Cx7xcjAQhji9m1ADJn