Digit height becomes content-width-aware: fit-to-width clamp (issue #121) #123
No reviewers
Labels
No labels
area:companion
area:docs
area:shared
area:tooling
area:watchapp
blocker
kind:chore
kind:feature
kind:spike
kind:test
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
robert/PedalPebble!123
Loading…
Reference in a new issue
No description provided.
Delete branch "area/digit-height-content-aware"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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 stringit 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 scaleslinearly 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) andbasalt(144px content); QUAD'sexisting 56/76px (D29, sized against
emery's 100x104 cell) overflowsbasalt's real 72x74 cell thesame 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
emeryitself, HERO2's 96px hero needs 217px of run for its owndefault 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 aclosed-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.
requested_heightunchanged when it already fits. Never raises, and never shrinks a valuethat 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.
page_render_digit_height()itself is untouched — this is a second function alongside it, exactly asthe 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.
page_render.c'sprv_draw_cell()— the one place that has both the live formattedtext (
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 onbasalt. This does not get its own new D-numbered decision — D55 alreadypredicted 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.mdsection 3gets 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
emeryfor 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_unchangedvs.test_quad_fit_corrects_the_pre_existing_emery_three_digit_overflow/test_hero2_hero_fit_corrects_the_pre_existing_emery_overflowin 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 around 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
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_fieldsunchanged, allgreen (
test_page's own DESIGN.md-matching assertion is what caught my temporary HERO1 page-swapscaffolding not yet being reverted, exactly as it should).
pebble build: clean onemery,basalt,gabbro(pebble build, all three targets intargetPlatforms).PR #87/#120 used (marked
_TEMPinmain.c/page.c, fully reverted before this diff —git diff main...area/digit-height-content-aware --statonly touchespage_render.c,page_render_geometry.c/.h, the test file, and the two docs):FIELD_SPEED"28.4" onemery: fits cleanly, no clipping (full-width digits, comfortablemargin).
basalt: fits cleanly.gabbro: dramatically better than the ~62px-of-overflow before this PR, but seethe flagged bezel-overshoot finding above — not fully clip-free on round yet.
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).duration) on
basalt: same, all four cellsOK-WITHIN-CELL.emery: the newly-discovered HR/POWER emphasised-slot overflow (149px against100px 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