Repository navigation
fix(render): unify the clock layout grid - #126
Merged
Merged
Conversation
tarakanof
commented
Sep 26, 2026
tarakanof
left a comment
tarakanof
left a comment
Owner
Author
There was a problem hiding this comment.
Review: #126 (closes #108)
Verdict: OK to merge after fixing 1. Item 2 is a design call for the owner. The headline bug is fixed on every payload path. There are no blockers.
Checks run:
- Merged locally onto the current
overhaul/ui-ng-2026-09(17335f8, which now includes #118 and #122). The merge is clean. go vet ./...andgo test ./... -racepass.gofmt -lflags onlycmd/ember/device_display_test.go, which already fails on the base branch.- Frames were rendered with my own
-overlayharness on the base and on the merged tree. - Device access was read-only GETs:
/api/v1/versionreturns1.1.2, plus/api/v1/apps. Nothing was written to the device.
Verified
-
Every row-7 bar starts at col 8. I checked each path:
ComposeFrame(session, rate and usage bars). The source-carddrawOpsAroundpath puts col 8 in the left op.detailPayload(tool card and locked attention card).RenderIdleUsagePayload.- Weather tile: drawn, native-icon (op from col 8) and moon variants.
- Air tile.
- Pomodoro: the device's native progress at x=8, and the preview.
/v1/preview,/v1/weather/previewand/v1/pomodoro/preview, which render the same frames.- The tool card has no grid in
/v1/preview: macOS draws it as a text line, so there is no second layout to drift. - The only other row-7 painter is the forecast tile. It is full-panel with no icon, which is intended.
-
9-wide
iconOpmasking col 8. NG documents the render order (text first, thendraw,textInFront:false). It does not say in so many words that bitmap zeros are opaque. The live shots support it:- s03, s14, s25 and s26 are tool cards caught at different scroll positions, and cols 0-7 are byte-identical in all four.
- s26 has text reaching col 8, so text was passing under the icon. Yet no glyph pixel shows through the icon's holes (row 1 cols 0/7, row 5 cols 2-5).
- So zeros paint black, and the zero col 8 will mask the gap. After the change, my scroll sweep shows text at x=3/5/6 cut at col 9. Before, a glyph lit col 8.
-
textFadeMs.- It is in the NG payload key table (
reference/payload.md, Text section, "Sinusoidal fade period"). migrating-from-awtrix3.mdlists it as the rename offadeText.- The cached docs top out at v1.1.2, which is the version on the device.
- Its sibling
textBlinkMsalready ships indetailPayload. - The 422 risk is low. It is still not verified on the device, because I sent no writes.
- It is in the NG payload key table (
-
Payload size (
json.Marshal, base → PR):Payload Base PR Change tool card, session bar 473 B 572 B +99 tool card, rate bar 473 B 703 B +230 attention card 484 B 583 B +99 weather tile 1198 B 1082 B -116 weather native 930 B 814 B -116 air tile 1151 B 1068 B -83 usage-reset face 1152 B 1032 B -120 usage 5h, rate mode 1283 B 1163 B -120 Pomodoro, paused 189 B 207 B +18 Everything still fits in one TCP segment with headers, so the lossy link sends the same number of packets. No concern.
Findings
- should-fix:
ngGlyphWgets some widths wrong, one of them in the unsafe direction.- NG's docs say ASCII in the
smallfont is drawn with the AWTRIX panel font. AWTRIX3'ssrc/AwtrixFont.hgives these ink widths (xAdvance − 1): M 5, W 5, N 4, Q 4, I 1, space 1,:/./'1. - Live s02 matches this: the M in
M4has the sameX...X/XX.XXshape at cols 9-13. - Q is underestimated (3 instead of 4).
QQQQpasses as 15 px but is really 19 px and runs under the glass op at col 25. The stated invariant ("never runs under the glass") does not hold. - N is overestimated, so
NWN(15 px, fits) is cut toNW. I is overestimated too. - The comment says the function "errs wide for runes it does not know", but the default is 3, the common width. That is not erring wide, and non-ASCII comes from Matrix-Fonts with unknown widths.
- Fix: a small table (M/W 5, N/Q 4, I and narrow punctuation 1, space 1, other ASCII 3) and a wide default (5) for non-ASCII. Pin
QQQQandNWNin the test.
- NG's docs say ASCII in the
- Judgement call I disagree with: the HH:MM reset face without a unit. In session-bar mode the
usage-resetcard is now a bare17:30beside the robot. NG's built-in Time app, which is in the same rotation, also shows a bare HH:MM, so the reset time reads as the current time. The old gray5hwas the only cue. Tightening can't free columns, but the slot has room for a marker: the clock ends at col 23, so the 3-px hourglass⧗at cols 27-29 leaves 3 blank columns and won't read as17:305h. At minimum, get an explicit OK from the owner. The other three calls (1-px strip, unchanged alignment, 1-px forecast bars) I agree with. - nit: a stale code comment.
render.go:409-413(drawOpsArounddoc) still says a full-panel op "suppresses the firmware's text layer completely". This is the wrong mechanism the PR fixes in ARCHITECTURE.md (#108 item 11). Reword it to match the z-order explanation. - nit: the hourly strip stretch is uneven when 24 isn't a multiple of n. With the 22 values seen live,
x*n/barWdoubles two hours (cols 8-9 and 20-21) while the other hours get one column each. The same PR fixes this unevenness on the forecast tile. It is hard to see at 1 px, so this is optional (for example, pad instead of stretch when n > 12). - nit / suggestion: forecast tile columns. At 24h the bars sit at cols 4-27. Cols 8-31 would put each hour directly under its column in the weather strip the rotation just showed, and they match
barX0/barWexactly. Optional.
Frames (my harness; native text approximated in 3×5)
Agent tool card, 2 sessions (waiting + running). Row 7 was empty before; it now carries the session bar:
before 7 ................................
after 7 ........CB......................
Agent tool card, text scrolled to x=6. Before, a glyph lit col 8; after, col 8 is masked:
before 2 .ABAABA.B.B.B.B...B.B.B.......B.
after 2 .ABAABA...B.B.B...B.B.B.......B.
Agent source card, session bar:
before 7 ...........DC...................
after 7 ........DC......................
Weather, 22h (live count). Digits move from rows 0-4 to 1-5, and the strip moves from 9-30 (2 rows) to 8-31 (1 row):
before 0 ...............AAA..A..AA....... after 0 ................................
before 6 .........CDEFGHJKLMNPCDEFGHJKLM. after 6 ................................
before 7 .........CDEFGHJKLMNPCDEFGHJKLM. after 7 ........CCDEFGHJKLMNPPCDEFGHJKLM
Pomodoro preview (focus 17:30):
before 1 ..AAA.....B..BBB...BBB.BBB...... after 1 ..AAA........B..BBB...BBB.BBB...
before 7 BBBBBBBBBBBBBBBBBBBBBBCCCCCCCCCC after 7 ........BBBBBBBBBBBBBBBBBCCCCCCC
The preview time (col 12) now matches NG's centring of the 17-px native MM:SS in cols 9-31. The long break shows the mug, as the device does.
The Agents app's session bar started at col 11, a leftover from the pre-#57 layout when the robot was wider. The rate bar, the usage bar and NG's native progress all start at col 8, so the bar jumped three columns between cards of the same app and cols 8-10 stayed dark. Put the panel grid in one const block (layout.go) and draw every agent bar from barX0 = 8 over barW = 24 columns. A new test pins that every row-7 bar starts at barX0. Refs #108
The tool/activity card and the locked attention card are cards of the same ember app as the source and usage cards, but detailPayload sent only the 8x8 icon op. Every 6 s rotation turned row 7 off and on again. detailPayload now appends a 24x1 row-7 op with the same session or rate bar ComposeFrame paints (extracted as drawBottomBar). NG draws text in rows 1-5, so the op never covers the label. The context glass stays off on these cards: it would mask scrolling text at cols 25-31. Refs #108
A payload with a drawn icon and no native icon lets NG scroll its text across all 32 columns, then paints the draw ops on top. The 8x8 icon op hid cols 0-7 but left the gap column 8 exposed, so glyphs ran into the icon mid-scroll (seen live on the tool card). Every text payload with a drawn icon (agent tool/attention cards, weather/air/sun/limit-reset/reminder popups, meeting tile and popup) now sends a 9x8 op, iconOp, whose col 8 is zeros. Bitmap zeros are opaque, so the text now disappears at col 9, which matches NG's own reserved 9 px icon column. Refs #108
Both tiles drew their digits on rows 0-4 while every other app uses rows 1-5, so the numbers jumped a row as the rotation moved from the agents to weather. Their 2-row hourly strips started at col 9, one right of every other bottom bar, and painted one pixel per hour into 23 columns: a 24 h window silently lost its last hour and a short window (forecast_hours=6) ended in a 6 px stub with a dark tail. Digits now sit on the text row, and the strips are a 1 px bottom bar on row 7, cols 8-31, with the window stretched evenly over all 24 columns (drawHourlyStrip). Row 6 becomes the same blank spacer the agent app has. I picked the 1 px strip over keeping two rows because two rows would touch the digits with no spacer, and a 1 px bar matches every other app's bottom bar. Refs #108
The menu's Pomodoro preview (RenderPomodoro) drew a different layout from the device's native payload: progress across cols 0-31 under the icon, digits left-aligned at col 9, and a moon for the long break. The device shows the coffee icon for both breaks, the MM:SS centred in cols 9-31 and NG's native progress from col 8. The preview now uses the shared grid: time centred (col 12 for a 17 px MM:SS), progress on the bottom bar from barX0, and the mug for both breaks. The device payload also adds textFadeMs while paused, because the animated native icon stays at full brightness and dimming the text alone was a weak cue. Refs #108
Three problems on the usage faces: - The HH:MM reset clock ends at col 23 and the gray "5h" unit started at col 25. One blank column is the same as the gap between glyphs, so it read as "17:305h". Clock faces now drop the unit. The 5h percentage is on the previous face or on the bar below, and the hourglass fallback, which is short, keeps its "5h". - Percent digits used rateColor, which reuses the agent state colours, while the bar under them used usageThreshold. One percentage came out in two hues, and an amber 87 % looked like a waiting agent. Digits, bars and reset urgency now share usageThreshold, and rateColor is gone. - The hourglass glyph was the same bitmap as 'I'. It now has a waist. Refs #108
sourceCardText kept 4 runes, which is 3x5-font logic. NG draws the name in its own font, where M/N/W are 5 px wide, so "MWMW" runs to about 23 px, past col 24, and the right-hand glass op cut the last letter mid-glyph. Truncate by estimated NG width (5 px for M/N/W, 3 px for other runes, 1 px spacers) to 15 px, which keeps col 24 blank before the glass. Names of 3 px letters still show four glyphs. Refs #108
Three small glyph and colour defects found in the #108 audit: - The forecast tile spread N hours over 32 columns, so 24 h came out as a jagged mix of 1 px and 2 px bars. Every bar is now 32/N columns wide and the remainder becomes equal margins. - The degree sign was a solid 2x2 block that read as a blob, and its 2 px of ink put centred temperatures 1 px left of centre. It is now a 3x3 ring. - The EEA print colours for VERY POOR (#960032) and EXTREME (#7D2181) were very dim on the LED, which is when the reading matters most. They keep their hue at full value (#FF1060, #C040FF). Refs #108
The display-layout section never gave the session-bar columns, and the gotcha said a full-panel bitmap "suppresses" text. NG documents textInFront (default false): text is painted first and draw ops on top, so the bitmap's opaque zeros overwrite the text. Add the shared column map (layout.go), explain the 9-wide icon op and the bottom bar on every agent card, and update the weather/air/forecast, usage-unit and Pomodoro descriptions to match #108. Refs #108
The width table guessed: Q counted 3 px, so "QQQQ" (19 px) passed as 15 and ran under the glass op. N and I were overestimated, so "NWN" was cut although it fits. Non-ASCII runes defaulted to the common 3 px, which does not err wide. Take the ink widths of all printable ASCII from AWTRIX3's AwtrixFont.h, the panel font NG uses for ASCII, and count every other rune as 5 px, because NG draws those from Matrix-Fonts, whose widths we don't have. Refs #108
Review of #126: with the "5h" unit dropped, the reset card was a bare 17:30 beside the robot. NG's built-in Time app shows a bare HH:MM in the same rotation, so it read as the current time. Draw the gray hourglass glyph at cols 27-29. That leaves three blank columns after the clock, which ends at col 23, so it can't run into the digits the way "5h" at col 25 did. Refs #108
Review of #126: - The strip stretch (x*n/24) gave two hours a double column and the rest a single one when the window doesn't divide 24. That is the 22 values seen live. - The forecast tile centred its 24 bars in cols 4-27, so an hour did not sit under its own column in the weather strip shown just before. Both now use hourSlot: hour i of an N-hour window owns 24/N whole columns, laid out left to right from col 8. Windows that divide 24 fill the bar exactly. Other windows leave a dark tail at the right (22 h: cols 30-31) instead of an uneven stretch. That is the least surprising option: hour 0 is always at col 8, and every hour is the same width on every tile. Refs #108
tarakanof
force-pushed
the
fix/108-clock-layout-grid
branch
from
September 26, 2026 07:13
3315b83 to
c489071
Compare
Owner
Author
|
Review addressed after rebasing onto overhaul/ui-ng-2026-09:
vet and test -race pass. |
This was referenced Sep 26, 2026
Merged
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Closes #108
What changed and why
The headline bug: the Agents app's session bar started at col 11, a value left over from before the #57 redesign. The rate bar, the usage bar and NG's native progress all start at col 8, so the bar jumped three columns between cards and cols 8-10 stayed dark. The fix is a shared grid in one place, so these values cannot drift apart again.
internal/render/layout.go: one const block for the column grid, copied from NG's own layout for an app with an 8px icon: icon 0-7, gap 8, content from 9, right slot 25-31, bottom bar row 7 cols 8-31 (iconW,iconOpW,contentX,textRow,rightSlotX,barX0,barW,barRow). It replacesnumStart,unitStart,barStartand the hard-coded 8/9/11s.TestEveryBottomBarStartsAtBarX0asserts that 18 variants start their row-7 bar atbarX0: agent session, rate, usage, idle, tool and attention bars; weather drawn, 6h, native-icon and moon tiles; air; Pomodoro device and preview.drawBottomBar), so row 7 no longer blinks as the cards rotate. The glass stays off on those cards because it would mask scrolling text.iconOp) whose col 8 is zeros, so scrolling text disappears at col 9 instead of touching the icon. This covers the agent cards, the weather/air/sun/reminder/limit-reset popups and the meeting tile and popup.hourSlot): each hour gets24/Nwhole columns. A 24h window is one column per hour, so the last hour is no longer dropped.textFadeMs: 2000, because the animated icon stays at full brightness.5hunit with a gray hourglass at cols 27-29, leaving 3 blank columns after the clock (see below). Digits, bars and reset urgency share one threshold palette,usageThreshold, kept apart from the agent-state colours, andrateColoris deleted. The hourglass glyph was the same bitmap asIand now has a waist.AwtrixFont.h(the panel font NG uses for ASCII): M/W 5, N/Q 4, I and most punctuation 1, other letters 3. Non-ASCII counts as 5 px, erring wide because NG draws it from Matrix-Fonts. Tests coverQQQQ→QQQandNWN→NWN.textInFrontz-order (text first, draw ops over it, opaque zeros), and the weather/air/forecast, usage-unit and Pomodoro descriptions are updated.cmd/ember/weather.go: one comment updated (the air strip is 24 columns, not 23).Design choices worth a look
24/Ncolumns from col 8 on the weather strip, the air strip and the forecast tile. Windows that divide 24 (6, 8, 12, 24 h) fill the bar. Any other window leaves an even dark tail on the right rather than doubling some hours: 22h darkens cols 30-31, and 16h/20h use cols 8-23 and 8-27. I picked this because hour 0 is always at col 8 and no hour is wider than another. The trade-off is the tail on the non-divisor windows.17:30read as the time of day next to NG's Time app, and5hone column after the clock read as17:305h. The clock faces now show a gray hourglass at cols 27-29, with 3 blank columns after the clock.5hstays on the pct face and the hourglass fallback.Out of scope / not done
textInFront, NG text draw ops andbarChartare left to feat(render): adopt NG 1.1.x native features on the clock #109.textCase) were not in this issue's fix list.coordinator.goanddevice_settings.goare untouched.cmd/ember/device_display_test.goalready failsgofmt -lon the base branch. I left it alone because it is outside this scope.Needs on-device check
textFadeMson the paused Pomodoro. It is documented in the NG payload reference, but Ember hasn't sent it before, and an unknown key would 422 the whole push.Tests
gofmt -l internalis clean.go vet ./...andgo test ./... -racepass. New tests are inlayout_test.go: bar start column, icon-gap mask, strips filling the bar, digits on the text row. There are also new tests for even forecast widths, the usage palette, the degree ring, AQI LED brightness, source-name width truncation and the Pomodoro device layout.Review follow-ups
After rebasing onto
overhaul/ui-ng-2026-09(#118, #121, #122), these review items are addressed:drawOpsAroundis fixed.Before / after frames
Simulated with the audit's frame-dump harness (
go test -overlay). Native text is approximated in the 3×5 font.AGENT source card, session bar (2 sessions), glass 47%
Before:
After:
AGENT tool/activity card (detailPayload)
Before:
After:
AGENT usage card usage-reset (session-bar mode)
Before:
After:
AGENT usage 5h face (rate-bar mode: clock in slot)
Before:
After:
AGENT source card long name (truncated to 4)
Before:
After:
WEATHER tile drawn 21°, 24h
Before:
After:
WEATHER tile drawn -12°, 6h
Before:
After:
AIR tile 105 (3 digits)
Before:
After:
WEATHER tile drawn 21°, 22h
(no before frame: new harness case)
After:
FORECAST tile 24h
Before:
After:
FORECAST tile 16h
Before:
After:
POMODORO drawn RenderPomodoro (preview only)
Before:
After: