Skip to content

Make auto row height calculations pixel perfect #67

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nchapman merged 2 commits into from
Dec 10, 2025
Merged

Make auto row height calculations pixel perfect #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4b7f3ac
Make auto row height calculations pixel perfect.
nchapman Dec 10, 2025
681aa16
Clean up remaining pixel issues.
nchapman Dec 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions tests/unit/all/common/test_ui_layout.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ void UI_initLayout(int screen_width, int screen_height, float diagonal_inches) {
ui.button_size = (best_pill * 2) / 3;
ui.button_margin = (best_pill - ui.button_size) / 2;
ui.button_padding = (best_pill * 2) / 5;
ui.text_baseline = (best_pill * 2) / 10;
}

void setUp(void) {
Expand All @@ -115,7 +114,6 @@ void setUp(void) {
ui.button_size = 20;
ui.button_margin = 5;
ui.button_padding = 12;
ui.text_baseline = 6;
}

void tearDown(void) {
Expand Down Expand Up @@ -271,15 +269,6 @@ void test_button_padding_proportional(void) {
TEST_ASSERT_EQUAL_INT(expected_padding, ui.button_padding);
}

void test_text_baseline_proportional(void) {
UI_initLayout(640, 480, 2.8f);

// text_baseline = (pill_height * 2) / 10, per UI_initLayout implementation
// For 30dp pill: (30 * 2) / 10 = 6
int expected_baseline = (ui.pill_height * 2) / 10;
TEST_ASSERT_EQUAL_INT(expected_baseline, ui.text_baseline);
}

///////////////////////////////
// Padding Tests
///////////////////////////////
Expand Down Expand Up @@ -368,7 +357,6 @@ int main(void) {
RUN_TEST(test_button_size_proportional);
RUN_TEST(test_button_margin_centers_button);
RUN_TEST(test_button_padding_proportional);
RUN_TEST(test_text_baseline_proportional);

// Padding
RUN_TEST(test_padding_consistent);
Expand Down
260 changes: 162 additions & 98 deletions workspace/all/common/api.c
View file
Open in desktop

Large diffs are not rendered by default.

73 changes: 56 additions & 17 deletions workspace/all/common/api.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,33 +142,72 @@ static inline void GFX_centerGlyph(int container_w, int container_h, TTF_Font* f
*
* These values are computed by UI_initLayout() based on screen dimensions
* to optimally fill the display without per-platform manual configuration.
*
* IMPORTANT: This struct contains BOTH Display Point (DP) values and pixel values.
* Understanding when to use each is critical:
*
* DP VALUES (int foo):
* - Used for PROPORTIONAL calculations (e.g., button_size = pill_height * 2/3)
* - Each DP() conversion involves rounding, so repeated conversions accumulate error
* - Example: ui.pill_height, ui.edge_padding, ui.option_size
*
* PIXEL VALUES (int foo_px):
* - Used for EXACT LAYOUT POSITIONING (e.g., y = edge_padding_px + row * pill_height_px)
* - These are pre-calculated ONCE to avoid rounding accumulation
* - Prevents overlap bugs caused by repeated DP() conversions
* - Example: ui.pill_height_px, ui.edge_padding_px, ui.option_size_px
*
* USAGE PATTERN:
* // WRONG - repeated DP() calls accumulate rounding error:
* for (int row = 0; row < 6; row++) {
* int y = DP(ui.edge_padding) + DP(row * ui.pill_height); // BAD!
* }
*
* // CORRECT - use pre-calculated pixel values for layout:
* for (int row = 0; row < 6; row++) {
* int y = ui.edge_padding_px + (row * ui.pill_height_px); // GOOD!
* }
*
* // CORRECT - use DP values for proportional calculations:
* int icon_size = (ui.pill_height * 2) / 3; // GOOD! (then convert once: DP(icon_size))
*/
typedef struct UI_Layout {
int screen_width; // Screen width in dp
int screen_height; // Screen height in dp
// Screen dimensions
int screen_width; // Screen width in dp (for proportional layout)
int screen_height; // Screen height in dp (for proportional layout)
int screen_width_px; // Screen width in pixels (cached for convenience)
int screen_height_px; // Screen height in pixels (cached for convenience)
int pill_height; // Height of menu pills in dp (28-32 typical)
int row_count; // Number of visible menu rows (6-8)

// Main menu pills (the large selectable rows)
int pill_height; // Pill height in dp (for proportional calculations like icon sizing)
int pill_height_px; // Pill height in EXACT pixels (for row positioning - avoids DP rounding drift)
int row_count; // Number of visible content rows (not including footer)

// Spacing and padding
int padding; // Internal spacing between UI elements in dp
int edge_padding; // Distance from screen edges in dp (reduced on devices with bezels)
int text_baseline; // Vertical offset for text centering in pill (DEPRECATED: use text_offset_px)
int edge_padding; // Distance from screen edges in dp (reduced on bezel devices)
int edge_padding_px; // Distance from screen edges in EXACT pixels (for positioning - avoids DP rounding drift)

// Button elements (action hints, icons)
int button_size; // Size of button icons in dp
int button_margin; // Margin around buttons in dp
int option_size; // Height of submenu option rows in dp
int option_baseline; // Vertical offset for label text (font.medium) in option rows (DEPRECATED)
int option_value_baseline; // Vertical offset for value text (font.small) in option rows (DEPRECATED)

// Pixel-based text centering offsets (computed from font metrics after font load)
// These are the Y offsets in pixels to center text in the respective row types
int text_offset_px; // Y offset in pixels to center font.large in pill_height
int option_offset_px; // Y offset in pixels to center font.medium in option_size
int option_value_offset_px; // Y offset in pixels to center font.small in option_size
int button_text_offset_px; // Y offset in pixels to center font.small in button_size (hints)
int button_label_offset_px; // Y offset in pixels to center font.tiny in button_size (MENU, POWER)
int button_padding; // Padding inside buttons in dp

// Submenu option rows (smaller than main pills, used in settings menus)
int option_size; // Option row height in dp (for proportional calculations)
int option_size_px; // Option row height in EXACT pixels (for positioning - avoids DP rounding drift)

// Settings indicators (brightness, volume sliders)
int settings_size; // Size of setting indicators in dp
int settings_width; // Width of setting indicators in dp

// Pixel-perfect text centering offsets (computed from font metrics after font load)
// These are Y offsets in pixels to vertically center text within their respective containers
int text_offset_px; // Y offset to center font.large in pill_height_px
int option_offset_px; // Y offset to center font.medium in option_size_px
int option_value_offset_px; // Y offset to center font.small in option_size_px (right-aligned values)
int button_text_offset_px; // Y offset to center font.small in button_size (action hints)
int button_label_offset_px; // Y offset to center font.tiny in button_size (MENU, POWER labels)
} UI_Layout;

extern UI_Layout ui;
Expand Down
51 changes: 27 additions & 24 deletions workspace/all/minarch/minarch.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4439,7 +4439,7 @@ int Menu_options(MenuList* list) {
}

int ox = DP_CENTER_PX(ui.screen_width, mw);
int oy = DP(ui.edge_padding + ui.pill_height);
int oy = ui.edge_padding_px + ui.pill_height_px;
int selected_row = nav.selected - nav.start;
for (int i = nav.start, j = 0; i < nav.end; i++, j++) {
MenuItem* loop_item = &items[i];
Expand All @@ -4454,24 +4454,25 @@ int Menu_options(MenuList* list) {

GFX_blitPill(
ASSET_OPTION_WHITE, screen,
&(SDL_Rect){ox, oy + DP(j * ui.option_size), w, DP(ui.option_size)});
&(SDL_Rect){ox, oy + (j * ui.option_size_px), w, ui.option_size_px});
text_color = COLOR_BLACK;

if (loop_item->desc)
desc = loop_item->desc;
}
text = TTF_RenderUTF8_Blended(font.medium, loop_item->name, text_color);
SDL_BlitSurface(text, NULL, screen,
&(SDL_Rect){ox + DP(OPTION_PADDING),
oy + DP(j * ui.option_size) + ui.option_offset_px});
SDL_BlitSurface(
text, NULL, screen,
&(SDL_Rect){ox + DP(OPTION_PADDING),
oy + (j * ui.option_size_px) + ui.option_offset_px});
SDL_FreeSurface(text);
}
} else if (type == MENU_FIXED) {
// NOTE: no need to calculate max width
int mw = DP(ui.screen_width - ui.edge_padding * 2);
int ox, oy;
ox = oy = DP(ui.edge_padding);
oy += DP(ui.pill_height);
ox = oy = ui.edge_padding_px;
oy += ui.pill_height_px;

int selected_row = nav.selected - nav.start;
for (int i = nav.start, j = 0; i < nav.end; i++, j++) {
Expand All @@ -4482,7 +4483,7 @@ int Menu_options(MenuList* list) {
// gray pill
GFX_blitPill(
ASSET_OPTION, screen,
&(SDL_Rect){ox, oy + DP(j * ui.option_size), mw, DP(ui.option_size)});
&(SDL_Rect){ox, oy + (j * ui.option_size_px), mw, ui.option_size_px});
}

// Calculate optimal widths using proportional truncation
Expand All @@ -4504,15 +4505,15 @@ int Menu_options(MenuList* list) {
SDL_BlitSurface(
text, NULL, screen,
&(SDL_Rect){ox + mw - text->w - DP(OPTION_PADDING),
oy + DP(j * ui.option_size) + ui.option_value_offset_px});
oy + (j * ui.option_size_px) + ui.option_value_offset_px});
SDL_FreeSurface(text);
}

if (j == selected_row) {
// white pill
GFX_blitPill(ASSET_OPTION_WHITE, screen,
&(SDL_Rect){ox, oy + DP(j * ui.option_size), label_w,
DP(ui.option_size)});
&(SDL_Rect){ox, oy + (j * ui.option_size_px), label_w,
ui.option_size_px});
text_color = COLOR_BLACK;

if (loop_item->desc)
Expand All @@ -4523,9 +4524,10 @@ int Menu_options(MenuList* list) {
GFX_truncateText(font.medium, loop_item->name, label_truncated, label_text_w,
0);
text = TTF_RenderUTF8_Blended(font.medium, label_truncated, text_color);
SDL_BlitSurface(text, NULL, screen,
&(SDL_Rect){ox + DP(OPTION_PADDING),
oy + DP(j * ui.option_size) + ui.option_offset_px});
SDL_BlitSurface(
text, NULL, screen,
&(SDL_Rect){ox + DP(OPTION_PADDING),
oy + (j * ui.option_size_px) + ui.option_offset_px});
SDL_FreeSurface(text);
}
} else if (type == MENU_VAR || type == MENU_INPUT) {
Expand Down Expand Up @@ -4562,7 +4564,7 @@ int Menu_options(MenuList* list) {
}

int ox = DP_CENTER_PX(ui.screen_width, mw);
int oy = DP(ui.edge_padding + ui.pill_height);
int oy = ui.edge_padding_px + ui.pill_height_px;
int selected_row = nav.selected - nav.start;
for (int i = nav.start, j = 0; i < nav.end; i++, j++) {
MenuItem* loop_item = &items[i];
Expand All @@ -4582,12 +4584,12 @@ int Menu_options(MenuList* list) {
// gray pill
GFX_blitPill(
ASSET_OPTION, screen,
&(SDL_Rect){ox, oy + DP(j * ui.option_size), mw, DP(ui.option_size)});
&(SDL_Rect){ox, oy + (j * ui.option_size_px), mw, ui.option_size_px});

// white pill
GFX_blitPill(ASSET_OPTION_WHITE, screen,
&(SDL_Rect){ox, oy + DP(j * ui.option_size), label_w,
DP(ui.option_size)});
&(SDL_Rect){ox, oy + (j * ui.option_size_px), label_w,
ui.option_size_px});
text_color = COLOR_BLACK;

if (loop_item->desc)
Expand All @@ -4598,9 +4600,10 @@ int Menu_options(MenuList* list) {
GFX_truncateText(font.medium, loop_item->name, label_truncated, label_text_w,
0);
text = TTF_RenderUTF8_Blended(font.medium, label_truncated, text_color);
SDL_BlitSurface(text, NULL, screen,
&(SDL_Rect){ox + DP(OPTION_PADDING),
oy + DP(j * ui.option_size) + ui.option_offset_px});
SDL_BlitSurface(
text, NULL, screen,
&(SDL_Rect){ox + DP(OPTION_PADDING),
oy + (j * ui.option_size_px) + ui.option_offset_px});
SDL_FreeSurface(text);

if (nav.await_input && j == selected_row) {
Expand All @@ -4614,7 +4617,7 @@ int Menu_options(MenuList* list) {
SDL_BlitSurface(
text, NULL, screen,
&(SDL_Rect){ox + mw - text->w - DP(OPTION_PADDING),
oy + DP(j * ui.option_size) + ui.option_value_offset_px});
oy + (j * ui.option_size_px) + ui.option_value_offset_px});
SDL_FreeSurface(text);
}
}
Expand All @@ -4625,8 +4628,8 @@ int Menu_options(MenuList* list) {
#define SCROLL_HEIGHT 4
#define SCROLL_MARGIN 4 // Tight spacing anchored to option list
int ox = (DP(ui.screen_width) - DP(SCROLL_WIDTH)) / 2;
int options_top = DP(ui.edge_padding + ui.pill_height);
int options_bottom = options_top + DP(max_visible_options * ui.option_size);
int options_top = ui.edge_padding_px + ui.pill_height_px;
int options_bottom = options_top + (max_visible_options * ui.option_size_px);

if (nav.start > 0)
GFX_blitAsset(ASSET_SCROLL_UP, NULL, screen,
Expand Down
47 changes: 24 additions & 23 deletions workspace/all/minarch/minarch_menu.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -621,13 +621,13 @@ static void Menu_loop_ctx(MinArchContext* ctx) {

SDL_Surface* text;
text = TTF_RenderUTF8_Blended(font.large, display_name, COLOR_WHITE);
GFX_blitPill(ASSET_BLACK_PILL, *scr,
&(SDL_Rect){DP(ui.edge_padding), DP(ui.edge_padding), max_width,
DP(ui.pill_height)});
GFX_blitPill(
ASSET_BLACK_PILL, *scr,
&(SDL_Rect){ui.edge_padding_px, ui.edge_padding_px, max_width, ui.pill_height_px});
SDL_BlitSurface(text, &(SDL_Rect){0, 0, max_width - DP(ui.button_padding * 2), text->h},
*scr,
&(SDL_Rect){DP(ui.edge_padding + ui.button_padding),
DP(ui.edge_padding) + ui.text_offset_px});
&(SDL_Rect){ui.edge_padding_px + DP(ui.button_padding),
ui.edge_padding_px + ui.text_offset_px});
SDL_FreeSurface(text);

if (show_setting && !cb->get_hdmi())
Expand All @@ -638,12 +638,13 @@ static void Menu_loop_ctx(MinArchContext* ctx) {
0);
GFX_blitButtonGroup((char*[]){"B", "BACK", "A", "OKAY", NULL}, 1, *scr, 1);

// Vertically center menu items
int header_offset = ui.edge_padding + ui.pill_height;
int footer_offset = ui.screen_height - ui.edge_padding - ui.pill_height;
int content_area_height = footer_offset - header_offset;
int menu_height_dp = MENU_ITEM_COUNT * ui.pill_height;
oy = header_offset + (content_area_height - menu_height_dp) / 2 - ui.padding;
// Vertically center menu items (calculate in pixel space to avoid rounding accumulation)
int header_offset_px = ui.edge_padding_px + ui.pill_height_px;
int footer_offset_px = ui.screen_height_px - ui.edge_padding_px - ui.pill_height_px;
int content_area_height_px = footer_offset_px - header_offset_px;
int menu_height_px = MENU_ITEM_COUNT * ui.pill_height_px;
int oy_px =
header_offset_px + (content_area_height_px - menu_height_px) / 2 - DP(ui.padding);

for (int i = 0; i < MENU_ITEM_COUNT; i++) {
char* item = m->items[i];
Expand All @@ -652,40 +653,40 @@ static void Menu_loop_ctx(MinArchContext* ctx) {
if (i == selected) {
if (m->total_discs > 1 && i == ITEM_CONT) {
GFX_blitPill(ASSET_DARK_GRAY_PILL, *scr,
&(SDL_Rect){DP(ui.edge_padding), DP(oy + ui.padding),
&(SDL_Rect){ui.edge_padding_px, oy_px + DP(ui.padding),
DP(ui.screen_width - ui.edge_padding * 2),
DP(ui.pill_height)});
ui.pill_height_px});
text = TTF_RenderUTF8_Blended(font.large, disc_name, COLOR_WHITE);
SDL_BlitSurface(
text, NULL, *scr,
&(SDL_Rect){DP(ui.screen_width - ui.edge_padding - ui.button_padding) -
text->w,
DP(oy + ui.padding) + ui.text_offset_px});
oy_px + DP(ui.padding) + ui.text_offset_px});
SDL_FreeSurface(text);
}

TTF_SizeUTF8(font.large, item, &ow, NULL);
ow += DP(ui.button_padding * 2);

GFX_blitPill(ASSET_WHITE_PILL, *scr,
&(SDL_Rect){DP(ui.edge_padding),
DP(oy + ui.padding + (i * ui.pill_height)), ow,
DP(ui.pill_height)});
&(SDL_Rect){ui.edge_padding_px,
oy_px + DP(ui.padding) + (i * ui.pill_height_px), ow,
ui.pill_height_px});
text_color = COLOR_BLACK;
} else {
text = TTF_RenderUTF8_Blended(font.large, item, COLOR_BLACK);
SDL_BlitSurface(text, NULL, *scr,
&(SDL_Rect){DP(2 + ui.edge_padding + ui.button_padding),
DP(1 + ui.padding + oy + i * ui.pill_height) +
ui.text_offset_px});
oy_px + DP(1 + ui.padding) +
(i * ui.pill_height_px) + ui.text_offset_px});
SDL_FreeSurface(text);
}

text = TTF_RenderUTF8_Blended(font.large, item, text_color);
SDL_BlitSurface(
text, NULL, *scr,
&(SDL_Rect){DP(ui.edge_padding + ui.button_padding),
DP(oy + ui.padding + i * ui.pill_height) + ui.text_offset_px});
SDL_BlitSurface(text, NULL, *scr,
&(SDL_Rect){ui.edge_padding_px + DP(ui.button_padding),
oy_px + DP(ui.padding) + (i * ui.pill_height_px) +
ui.text_offset_px});
SDL_FreeSurface(text);
}

Expand Down
Loading