Rename dpi_scale to display_scale - #581
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 284a2673c9
ℹ️ 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".
| * @related cf_app_set_size cf_app_get_position cf_app_set_position cf_app_get_width cf_app_get_height cf_app_get_display_scale cf_app_display_scale_was_changed | ||
| */ | ||
| CF_API float CF_CALL cf_app_get_dpi_scale(void); | ||
| CF_API float CF_CALL cf_app_get_display_scale(void); |
There was a problem hiding this comment.
Preserve the old DPI scale API while renaming
For any downstream C/C++ project that still calls cf_app_get_dpi_scale, this replaces the public declaration/export with cf_app_get_display_scale instead of keeping the old symbol as the implementation and adding the new name as an inline forwarder, so updating CF will produce compile or link failures; the paired cf_app_dpi_scale_was_changed rename needs the same compatibility treatment.
AGENTS.md reference: AGENTS.md:L122-L126
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Renames the app’s “dpi scale” terminology to “display scale” to better reflect that the value comes from SDL_GetWindowDisplayScale (OS points-to-pixels conversion for the window’s current display), and removes an unused previous-value field.
Changes:
- Rename internal
CF_Appfields/flags fromdpi_scale*todisplay_scale*and drop the unuseddpi_scale_prev. - Rename public API from
cf_app_get_dpi_scale/cf_app_dpi_scale_was_changedtocf_app_get_display_scale/cf_app_display_scale_was_changed(plus C++ inline wrappers). - Update the HiDPI topic doc to use the new terminology/link.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/internal/cute_app_internal.h | Renames stored scale state and removes unused *_prev field. |
| src/cute_input.cpp | Updates per-frame reset and SDL event handling to set display_scale/flag. |
| src/cute_app.cpp | Initializes display_scale and renames exported getter/changed-flag APIs. |
| include/cute_app.h | Renames public declarations and updates Doxygen references/wrappers. |
| docs/topics/hidpi.md | Updates HiDPI terminology and doc link to cf_app_get_display_scale. |
Suppressed comments (1)
include/cute_app.h:417
- The
@relatedlist for cf_app_display_scale_was_changed includes the function itself, which is redundant. Prefer listing only the corresponding getter.
* @related cf_app_get_display_scale cf_app_display_scale_was_changed
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| * @remarks On some devices (e.g. Apple Retina or iOS) pixels are clustered in 4x4 packs and abstracted as a single pixel | ||
| * called a "point". The intent is for applications to work in points, and scale their UI elements by a factor of 2x | ||
| * to aid in readability. These devices have very small pixels. Most of the time you should ignore dpi and let the OS | ||
| * handle this. CF enables DPI settings by default, but, you can see if this function returns 2.0f to let you know if | ||
| * pixels are clustered for you under the hood. |
| * handle this. CF enables DPI settings by default, but, you can see if this function returns 2.0f to let you know if | ||
| * pixels are clustered for you under the hood. | ||
| * @related cf_app_set_size cf_app_get_position cf_app_set_position cf_app_get_width cf_app_get_height cf_app_get_dpi_scale cf_app_dpi_scale_was_changed | ||
| * @related cf_app_set_size cf_app_get_position cf_app_set_position cf_app_get_width cf_app_get_height cf_app_get_display_scale cf_app_display_scale_was_changed |
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.
284a267 to
6cb8ddb
Compare
|
@RandyGaul Is there anything holding this PR? |
|
@pusewicz nope |
cf_app_get_dpi_scale/cf_app_dpi_scale_was_changedread like the OS's vague "suggested UI content scale" concept. The value is actuallySDL_GetWindowDisplayScale-- the OS's points-to-pixels conversion for the window's display -- so this renames it to say that.Breaking change: 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 #579 split -- pure rename, zero behavior change.