Digit height becomes content-width-aware: fit-to-width clamp (issue #121) #123

Merged
robert merged 1 commit from area/digit-height-content-aware into main 2026-09-06 12:56:16 +02:00
Owner

Closes #121.

The problem, checked against the shipped constants

page_render_digit_height(template, slot) returned a fixed ceiling with no idea how wide the string
it was about to draw actually was, or how wide the cell it was drawing into actually was. Combined
with page_render_glyph_width()'s fixed digit=0.6x/dot=0.2x/gap=0.1x ratio, a value's run width scales
linearly with digit height regardless of the cell's real width.

PR #120 flagged two confirmed clips rather than patch them: HERO1 at 140px clips FIELD_SPEED's
"28.4" on both emery (needs 322px against 200px content) and basalt (144px content); QUAD's
existing 56/76px (D29, sized against emery's 100x104 cell) overflows basalt's real 72x74 cell the
same way.

Checking those two surfaced two more that D55 (readiness review) had already predicted but PR #87's
implementation never re-verified once it picked its own glyph-width ratio (0.6) "by eye" rather than
D55's own recommended <=0.50: on emery itself, HERO2's 96px hero needs 217px of run for its own
default field's real value ("28.4") against a 200px hero, and QUAD's 56px ordinary ceiling needs
109-149px for a realistic 3-digit whole number (HR/POWER_3S/AVG_POWER's "148"/"213"/"198")
against a 100px cell — both on the two pages (Ride, Effort) already shipping today. These never read
as obviously broken in a screenshot the way HERO1's/basalt QUAD's several-times-over overflow did; this
PR found them by running the exact numbers, the same way PR #120 found its own two.

The mechanism

page_render_fit_digit_height(text, max_width, requested_height) (page_render_geometry.c/.h) —
approach (a) from the issue. Binary search over the existing page_render_text_width() rather than a
closed-form estimate, so it can never disagree with what that function itself says fits (text_width is
non-decreasing in height, so the search is valid); ~8 comparisons worst case, not the ~140-iteration
linear scan a naive countdown would need for HERO1.

  • Returns requested_height unchanged when it already fits. Never raises, and never shrinks a value
    that didn't need it
    — a two-digit cadence value stays at its slot's full ceiling even if some other
    value on some other slot would have overflowed.
  • Only clamps downward, per value, when the real run would overflow.
  • page_render_digit_height() itself is untouched — this is a second function alongside it, exactly as
    the issue's own option (a) describes, not a signature change. Every existing caller (including every
    host test asserting its per-template numbers) keeps working unchanged.
  • Called from page_render.c's prv_draw_cell() — the one place that has both the live formatted
    text (field_format() was just called) and the cell's real width.

Why a per-value clamp instead of lowering the glyph-width ratio globally

D55 already computed that a global ratio of <=0.50 (instead of the shipped 0.6) would make everything
except HERO1 fit by construction. That was rejected here because it would narrow every digit,
including values that already fit today (QUAD's two-digit CADENCE, HERO2's own lower cells) — the
ceiling a template requests is still the right size for a value short enough to need it, and a global
ratio change can't tell a two-digit value from a four-character one. The per-value clamp keeps every
already-fitting value at its existing size and only narrows the specific values that would overflow.

HERO1's own 140px figure

Left exactly as-is, on purpose: it's a ceiling, not an always-drawn size. A genuinely short value (a
one- or two-digit field) still draws at the full 140px; "28.4"-shaped values clamp down to ~89px on
emery, ~64px on basalt. This does not get its own new D-numbered decision — D55 already
predicted this exact split ("HERO1: two glyphs, ever" at 140-170px) as part of its own Decided
analysis; I appended a dated correction to D55 instead (append-only, D48), the same pattern D24's own
DPI correction used, with the full arithmetic for all four findings above. docs/DESIGN.md section 3
gets a one-line footnote pointing at it rather than a rewritten table — the "~140px"/"56px" etc.
figures now mean "ceiling", consistent with how the DPI number was corrected without rewriting the
table cell itself.

Confirmed: HERO2/QUAD's emery sizing is not silently frozen at old (wrong) numbers

I want to be upfront about this rather than let the PR title imply "unchanged": HERO2's hero and QUAD's
ordinary ceiling do not stay bit-for-bit identical on emery for their own real default values,
because — checked above — those values were never actually fitting in the first place. What's
preserved is the property the issue asked for: nothing that already fit shrinks (QUAD's CADENCE
"87" stays at 56px; HERO2's lower-cell "24.1"/"42.7" stay at 44px — both asserted in the new tests),
and nothing that didn't fit before still doesn't after. See test_fit_leaves_already_fitting_emery_ values_unchanged vs. test_quad_fit_corrects_the_pre_existing_emery_three_digit_overflow /
test_hero2_hero_fit_corrects_the_pre_existing_emery_overflow in the diff for the exact split.

Flagged, not fixed here: gabbro's HERO1 still overshoots the bezel

Measured directly (not just eyeballed): fitting "28.4" to gabbro's reported 260px content width and
centring it lands the outermost glyphs' corners ~146px from screen centre, against a 130px radius —
about 16px of real overshoot, confirmed in the gabbro emulator screenshot. Root cause:
page_render_content_rect() reports the full rectangular bounding box on round, not the ~184x184 a
round screen actually shows (D24). Every template has this same gap on round, not only HERO1 — the
round-aware content rect that fixes it for all of them at once is #62's job; a HERO1-only patch here
would fix one template's number while leaving every other template's wrong the same way. Documented in
a code comment on page_render_hero1_hero_rect() and in the D55 correction.

Verification

  • Host tests: 9 new assertions in test_page_render_geometry.c (47 total, up from 38) — hand-
    compiled with gcc (no cmake in this sandbox): gcc -std=c11 -Wall -Wextra -Werror -I src/c -I tests tests/test_page_render_geometry.c src/c/page_render_geometry.c -o /tmp/test_binary && /tmp/test_binary
    — 47/47 pass. Also re-ran test_page/test_state/test_backlight/test_fields unchanged, all
    green (test_page's own DESIGN.md-matching assertion is what caught my temporary HERO1 page-swap
    scaffolding not yet being reverted, exactly as it should).
  • Real pebble build: clean on emery, basalt, gabbro (pebble build, all three targets in
    targetPlatforms).
  • Real emulator screenshots, via the same temporary field-seeding + page-template-swap technique
    PR #87/#120 used (marked _TEMP in main.c/page.c, fully reverted before this diff — git diff main...area/digit-height-content-aware --stat only touches page_render.c,
    page_render_geometry.c/.h, the test file, and the two docs):
    • HERO1 + FIELD_SPEED "28.4" on emery: fits cleanly, no clipping (full-width digits, comfortable
      margin).
    • HERO1 + "28.4" on basalt: fits cleanly.
    • HERO1 + "28.4" on gabbro: dramatically better than the ~62px-of-overflow before this PR, but see
      the flagged bezel-overshoot finding above — not fully clip-free on round yet.
    • QUAD (Effort: HR/POWER_3S/CADENCE/AVG_POWER) on basalt: every value contained in its own cell,
      no overlap into the neighbour — confirmed both visually and by computing every glyph run's x-range
      against its cell's x-range in a standalone probe (all four OK-WITHIN-CELL).
    • QUAD (Progress: REMAINING/DISTANCE/ELAPSED/ETA, including a worst-case 7-character "2:34:17"
      duration) on basalt: same, all four cells OK-WITHIN-CELL.
    • QUAD (Effort) on emery: the newly-discovered HR/POWER emphasised-slot overflow (149px against
      100px at the un-clamped 76px height) now renders contained, confirming the fix rather than just
      the two issue-cited cases.

Files touched: watchapp/src/c/page_render_geometry.c, watchapp/src/c/page_render_geometry.h,
watchapp/src/c/page_render.c, watchapp/tests/test_page_render_geometry.c, docs/DESIGN.md,
docs/DECISIONS.md.

https://claude.ai/code/session_01DAoXbRmJUf2uxNYBfdAXPt

Closes #121. ## The problem, checked against the shipped constants `page_render_digit_height(template, slot)` returned a fixed ceiling with no idea how wide the string it was about to draw actually was, or how wide the cell it was drawing into actually was. Combined with `page_render_glyph_width()`'s fixed digit=0.6x/dot=0.2x/gap=0.1x ratio, a value's run width scales linearly with digit height regardless of the cell's real width. PR #120 flagged two confirmed clips rather than patch them: HERO1 at 140px clips `FIELD_SPEED`'s "28.4" on both `emery` (needs 322px against 200px content) and `basalt` (144px content); QUAD's existing 56/76px (D29, sized against `emery`'s 100x104 cell) overflows `basalt`'s real 72x74 cell the same way. Checking those two surfaced two more that D55 (readiness review) had already predicted but PR #87's implementation never re-verified once it picked its own glyph-width ratio (0.6) "by eye" rather than D55's own recommended <=0.50: **on `emery` itself**, HERO2's 96px hero needs 217px of run for its own default field's real value ("28.4") against a 200px hero, and QUAD's 56px ordinary ceiling needs 109-149px for a realistic 3-digit whole number (`HR`/`POWER_3S`/`AVG_POWER`'s "148"/"213"/"198") against a 100px cell — both on the two pages (Ride, Effort) already shipping today. These never read as obviously broken in a screenshot the way HERO1's/basalt QUAD's several-times-over overflow did; this PR found them by running the exact numbers, the same way PR #120 found its own two. ## The mechanism `page_render_fit_digit_height(text, max_width, requested_height)` (`page_render_geometry.c`/`.h`) — approach (a) from the issue. Binary search over the existing `page_render_text_width()` rather than a closed-form estimate, so it can never disagree with what that function itself says fits (text_width is non-decreasing in height, so the search is valid); ~8 comparisons worst case, not the ~140-iteration linear scan a naive countdown would need for HERO1. - Returns `requested_height` unchanged when it already fits. **Never raises, and never shrinks a value that didn't need it** — a two-digit cadence value stays at its slot's full ceiling even if some other value on some other slot would have overflowed. - Only clamps downward, per *value*, when the real run would overflow. - `page_render_digit_height()` itself is untouched — this is a second function alongside it, exactly as the issue's own option (a) describes, not a signature change. Every existing caller (including every host test asserting its per-template numbers) keeps working unchanged. - Called from `page_render.c`'s `prv_draw_cell()` — the one place that has both the live formatted text (`field_format()` was just called) and the cell's real width. ## Why a per-value clamp instead of lowering the glyph-width ratio globally D55 already computed that a global ratio of <=0.50 (instead of the shipped 0.6) would make everything except HERO1 fit by construction. That was rejected here because it would narrow *every* digit, including values that already fit today (QUAD's two-digit CADENCE, HERO2's own lower cells) — the ceiling a template requests is still the right size for a value short enough to need it, and a global ratio change can't tell a two-digit value from a four-character one. The per-value clamp keeps every already-fitting value at its existing size and only narrows the specific values that would overflow. ## HERO1's own 140px figure Left exactly as-is, on purpose: it's a ceiling, not an always-drawn size. A genuinely short value (a one- or two-digit field) still draws at the full 140px; "28.4"-shaped values clamp down to ~89px on `emery`, ~64px on `basalt`. This does **not** get its own new D-numbered decision — D55 already predicted this exact split ("HERO1: two glyphs, ever" at 140-170px) as part of its own Decided analysis; I appended a dated correction to D55 instead (append-only, D48), the same pattern D24's own DPI correction used, with the full arithmetic for all four findings above. `docs/DESIGN.md` section 3 gets a one-line footnote pointing at it rather than a rewritten table — the "~140px"/"56px" etc. figures now mean "ceiling", consistent with how the DPI number was corrected without rewriting the table cell itself. ## Confirmed: HERO2/QUAD's emery sizing is *not* silently frozen at old (wrong) numbers I want to be upfront about this rather than let the PR title imply "unchanged": HERO2's hero and QUAD's ordinary ceiling do **not** stay bit-for-bit identical on `emery` for their own real default values, because — checked above — those values were never actually fitting in the first place. What's preserved is the *property* the issue asked for: nothing that already fit shrinks (QUAD's CADENCE "87" stays at 56px; HERO2's lower-cell "24.1"/"42.7" stay at 44px — both asserted in the new tests), and nothing that didn't fit before still doesn't after. See `test_fit_leaves_already_fitting_emery_ values_unchanged` vs. `test_quad_fit_corrects_the_pre_existing_emery_three_digit_overflow` / `test_hero2_hero_fit_corrects_the_pre_existing_emery_overflow` in the diff for the exact split. ## Flagged, not fixed here: gabbro's HERO1 still overshoots the bezel Measured directly (not just eyeballed): fitting "28.4" to gabbro's reported 260px content width and centring it lands the outermost glyphs' corners ~146px from screen centre, against a 130px radius — about 16px of real overshoot, confirmed in the gabbro emulator screenshot. Root cause: `page_render_content_rect()` reports the full rectangular bounding box on round, not the ~184x184 a round screen actually shows (D24). Every template has this same gap on round, not only HERO1 — the round-aware content rect that fixes it for all of them at once is #62's job; a HERO1-only patch here would fix one template's number while leaving every other template's wrong the same way. Documented in a code comment on `page_render_hero1_hero_rect()` and in the D55 correction. ## Verification - **Host tests**: 9 new assertions in `test_page_render_geometry.c` (47 total, up from 38) — hand- compiled with gcc (no cmake in this sandbox): `gcc -std=c11 -Wall -Wextra -Werror -I src/c -I tests tests/test_page_render_geometry.c src/c/page_render_geometry.c -o /tmp/test_binary && /tmp/test_binary` — 47/47 pass. Also re-ran `test_page`/`test_state`/`test_backlight`/`test_fields` unchanged, all green (`test_page`'s own DESIGN.md-matching assertion is what caught my temporary HERO1 page-swap scaffolding not yet being reverted, exactly as it should). - **Real `pebble build`**: clean on `emery`, `basalt`, `gabbro` (`pebble build`, all three targets in `targetPlatforms`). - **Real emulator screenshots**, via the same temporary field-seeding + page-template-swap technique PR #87/#120 used (marked `_TEMP` in `main.c`/`page.c`, fully reverted before this diff — `git diff main...area/digit-height-content-aware --stat` only touches `page_render.c`, `page_render_geometry.c`/`.h`, the test file, and the two docs): - HERO1 + `FIELD_SPEED` "28.4" on `emery`: fits cleanly, no clipping (full-width digits, comfortable margin). - HERO1 + "28.4" on `basalt`: fits cleanly. - HERO1 + "28.4" on `gabbro`: dramatically better than the ~62px-of-overflow before this PR, but see the flagged bezel-overshoot finding above — not fully clip-free on round yet. - QUAD (Effort: HR/POWER_3S/CADENCE/AVG_POWER) on `basalt`: every value contained in its own cell, no overlap into the neighbour — confirmed both visually and by computing every glyph run's x-range against its cell's x-range in a standalone probe (all four `OK-WITHIN-CELL`). - QUAD (Progress: REMAINING/DISTANCE/ELAPSED/ETA, including a worst-case 7-character "2:34:17" duration) on `basalt`: same, all four cells `OK-WITHIN-CELL`. - QUAD (Effort) on `emery`: the newly-discovered HR/POWER emphasised-slot overflow (149px against 100px at the un-clamped 76px height) now renders contained, confirming the fix rather than just the two issue-cited cases. Files touched: `watchapp/src/c/page_render_geometry.c`, `watchapp/src/c/page_render_geometry.h`, `watchapp/src/c/page_render.c`, `watchapp/tests/test_page_render_geometry.c`, `docs/DESIGN.md`, `docs/DECISIONS.md`. https://claude.ai/code/session_01DAoXbRmJUf2uxNYBfdAXPt
Digit height becomes content-width-aware: fit-to-width clamp (issue #121)
Some checks failed
dev-artifact / build-pbw (push) Failing after 0s
dev-artifact / build-apk (push) Failing after 0s
dev-artifact / publish (push) Has been skipped
fast-lane / host-c-tests (push) Failing after 0s
fast-lane / jvm-tests (push) Failing after 0s
fast-lane / pebble-build (push) Failing after 0s
fast-lane / lint-and-secrets (push) Failing after 0s
fast-lane / meta-declares-required-jobs (push) Failing after 0s
fast-lane / host-c-tests (pull_request) Failing after 0s
fast-lane / jvm-tests (pull_request) Failing after 0s
fast-lane / pebble-build (pull_request) Failing after 0s
fast-lane / lint-and-secrets (pull_request) Failing after 0s
fast-lane / meta-declares-required-jobs (pull_request) Failing after 0s
fec3d75c58
Fixes the mechanism gap PR #120 flagged rather than patched: page_render_
digit_height(template, slot) returned a fixed ceiling with no idea how wide
the string it was about to draw actually was, or how wide the cell actually
was. Two confirmed clips resulted (HERO1 at 140px on emery/basalt, QUAD's
56/76px on basalt's 72x74 cell) - and checking those two surfaced two more,
on emery itself, that PR #87 never re-verified once it picked its own 0.6
glyph-width ratio "by eye" rather than D55's own recommended <=0.50: HERO2's
96px hero needs 217px for its own default SPEED value ("28.4") against a
200px hero, and QUAD's 56px needs 109-149px for a realistic 3-digit value
(HR/POWER_3S/AVG_POWER's "148"/"213"/"198") against a 100px cell.

- page_render_fit_digit_height(text, max_width, requested_height)
  (page_render_geometry.c/.h): binary search over the existing
  page_render_text_width(), so it can never disagree with what that
  function itself says fits. Returns requested_height unchanged when it
  already fits (never raises, never shrinks what didn't need it); only
  clamps downward, per value, when it would overflow. page_render_digit_
  height() itself is untouched - this is a second function alongside it,
  not a signature change, so every existing caller (including every host
  test asserting its per-template numbers) keeps working unchanged.
- page_render.c's prv_draw_cell(): the one call site with both the live
  formatted text and the cell width in hand, right after field_format().
- 9 new host tests (test_page_render_geometry.c, 47 total up from 38): a
  real "does this text fit this cell at this height" assertion for both
  flagged findings, the two newly-discovered emery overflows, the
  already-fits-unchanged property, and the function's own edge cases.
  Hand-compiled with gcc (no cmake in this environment), all passing.
- docs/DECISIONS.md: appended a 2026-09-06 correction to D55 (append-only,
  D48) with the full arithmetic and the reasoning for a per-value clamp
  over a smaller global ratio. docs/DESIGN.md section 3 gets a footnote to
  it, same treatment D24's DPI correction got - the "~140px"/"56px" etc.
  figures are ceilings now, not always-drawn sizes.
- Flagged, not fixed here: gabbro's HERO1 still overshoots the physical
  bezel by ~16px of radius, because page_render_content_rect() reports the
  full 260px rectangular bounding box rather than the ~184x184 a round
  screen actually shows (D24) - every template has this same gap on round,
  not just HERO1, and the round-aware content rect that fixes it for all
  of them at once is #62's job.

Verified with a real `pebble build` on emery/basalt/gabbro and real
emulator screenshots (HERO1 on all three; QUAD Effort/Progress on basalt;
QUAD Effort on emery as a regression check) via the same temporary
field-seeding/page-swap technique PR #87/#120 used, marked _TEMP and fully
reverted before this diff - `git diff --stat` against main touches only
page_render.c/page_render_geometry.c/.h, the test file, and the two docs.

Claude-Session: https://claude.ai/code/session_01DAoXbRmJUf2uxNYBfdAXPt
robert merged commit c9ded78609 into main 2026-09-06 12:56:16 +02:00
Sign in to join this conversation.
No description provided.