Resolution independent UI #38
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Dynamic row fitting: Calculate rows based on available screen space instead of fixed 6-8 range - +1 tier asset scaling: Load one tier higher than needed for better antialiasing quality - Use RGBA8888 for temporary surfaces during asset scaling Note: Transparency issue on rg35xxplus (RGB565) still being debugged. See docs/transparency-debugging.md for investigation history.
Keep asset surfaces in RGBA8888 format throughout the scaling pipeline instead of converting to loaded_assets->format. Disable SDL_SRCALPHA before intermediate blits to copy RGBA data directly rather than alpha-blending (which causes transparent pixels to blend to black when the destination is uninitialized). SDL handles RGBA→RGB565 conversion with proper alpha blending at final screen blit time.
The previous 160 PPI baseline (Android MDPI) was designed for smartphones held at arm's length. Retro handhelds are held closer (12-18") and prioritize readability for game lists over information density. The 144 PPI baseline (12²) provides: - ~11% larger UI elements across all devices - Better readability for gaming-focused interfaces - Typographically favorable number with excellent divisibility - Single constant works perfectly across 47+ device configurations Also adjusted desktop platform's virtual screen diagonal from 4.0" to 2.78" to maintain dp_scale ≈ 2.0 for consistent development environment.
The previous algorithm had several issues: 1. Used FOOTER_ROWS constant that double-counted padding 2. Had arbitrary 8-row maximum limit 3. Required even pixels as hard constraint, causing failures New algorithm: - Treats footer as another row with same height (simpler model) - Calculates max possible rows from MIN_PILL (no arbitrary limit) - Prefers even pixels but accepts odd as fallback - Early exit optimization when pill > MAX_PILL Results across 33 devices: - 98.5% average fill (was ~88% with gaps) - 5-13 rows depending on screen size - Only 7.7px average waste Also fixed desktop development environment: - Changed PLATFORM from "macos" to "desktop" to match .pak directory - Added keyboard controls help to dev-run output - Fixed FAKESD_PATH to use pwd -P for symlink resolution Added scripts/analyze-row-fitting.py for testing algorithm against all supported handheld devices before implementation.
…cro to all platforms.
There was a problem hiding this comment.
Pull request overview
This PR implements a resolution-independent UI system using Display Points (DP), replacing fixed scaling factors with dynamic calculations based on screen PPI. The system automatically calculates optimal UI element sizes for each device, eliminating per-platform manual tuning.
Key changes:
- Introduces DP scaling system (similar to Android's dp) with 144 PPI baseline
- Removes hardcoded
FIXED_SCALE,MAIN_ROW_COUNT,PADDINGconstants - Adds
SCREEN_DIAGONALto all platform headers for PPI calculation - Implements runtime UI layout calculation in
UI_initLayout() - Converts all UI code to use DP macros (
DP(),DP2(), etc.)
Reviewed changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| workspace/*/platform/platform.h | Added PLATFORM define and SCREEN_DIAGONAL; removed FIXED_SCALE and UI layout constants |
| workspace/*/platform/platform.c | Removed platform-specific overlay implementations; cleaned up commented debug code |
| workspace/all/common/api.h | Added DP system (macros, UI_Layout struct, wrapper functions) |
| workspace/all/common/api.c | Implemented UI_initLayout with PPI calculation and asset scaling; added fallback overlay |
| workspace/all/minui/minui.c | Converted UI layout from SCALE macros to DP with ui.* values |
| workspace/all/minarch/minarch.c | Converted menu rendering to use DP-based coordinates |
| workspace/all/minput/minput.c | Converted button layout to use DP system |
| workspace/all/say/say.c | Updated message display to use DP wrapper |
| workspace/all/clock/clock.c | Converted clock rendering to DP coordinates |
| workspace/all/common/defines.h | Removed SCALE macros; moved display constants; added FIXED_BPP/PITCH calculations |
| workspace/all/common/scaler.c | Changed include from platform.h to defines.h |
| tests/unit/all/common/test_ui_layout.c | Added comprehensive unit tests for DP system |
| scripts/.sh, scripts/.py | Added development tools for deployment and analysis |
| makefile.qa, makefile.dev, makefile | Updated build system for new tests and dev workflows |
| docs/*.md | Added extensive documentation for DP system and debugging |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1086
to
+1089
| if (!logged) { | ||
| LOG_info("Battery bolt: offset=(%d,%d) from battery corner\n", fill_x_offset, | ||
| fill_y_offset); | ||
| } |
There was a problem hiding this comment.
The logged variable needs to be set to 1 before the check on line 1086 to ensure all log messages print on the first call. Currently, logged is set on line 1073, but the condition on line 1086 checks if (!logged) again, which will always be false after the first battery render. This means the charging bolt offset log on lines 1087-1089 will never print.
Move the logged = 1; assignment to after all logging calls (after line 1103) to ensure all debug messages print on the first invocation.
| # Perfect: in range AND even | ||
| best_pill = pill | ||
| best_rows = content_rows | ||
| best_is_even = True |
There was a problem hiding this comment.
Variable best_is_even is not used.
Comment on lines
+10
to
+11
| import re | ||
|
|
There was a problem hiding this comment.
Import of 're' is not used.
Suggested change
| import re |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.