Put an address beside its own QR code, on one screen - #21
Merged
Conversation
A Bitcoin address is an unbroken run of base58 or bech32: no word shapes to count by, and characters chosen to look unlike each other rather than to group. It is exactly the shape a reader transcribing it loses their place in, and losing a place in an address is not a typo the reader finds out about cheaply. Drawn in groups of four now, with a gap at each boundary and the ink alternating between the body's white and the theme's orange - the same orange the back arrow and the selection bar already use, so this introduces no new colour. Two signals for one boundary on purpose: a reader who cannot tell the two inks apart still has the gap, and the grouping does not rest on colour alone. The address is never touched. Grouping happens at draw time, so what is compared, hashed and put in a QR is the same unbroken string it always was, and nothing downstream has to know to strip separators back out. Lines cut on a group boundary rather than at whatever pixel runs out, which is what keeps the alternation readable across a break - a line ending mid-group would put two same-coloured groups against each other with only the gap between them. The alternation is indexed from the start of the whole value, not from each line, for the same reason. The self-test walks four real addresses the way page_text_impl does and requires the pieces to rebuild the value character for character, plus the cut to land on a boundary. That is the invariant that matters: a group dropped or reordered at a line break would hand the reader an address that is not theirs, with nothing on screen to say so. Verified it fails when the boundary rounding is removed rather than passing vacuously. The line geometry is measured too - a grouped line is centred with its gaps counted in, so it can reach an edge a plain centred line would not. Only the address page groups. The xpub, the descriptor and the transcript are read rather than transcribed by hand, and the word list already has its own shape.
Every row of the address list is an address that does not fit, cut at whatever pixel ran out. A row that stops looks exactly like a row that ended, so the reader has nothing telling them the value continues - and on this list every value does. Rows that do not fit now end in an ellipsis. It is paid for out of the same width rather than drawn past it: the fit is recomputed against what is left after reserving room for the dots, so a truncated row is never wider than one that fits. This is in the list renderer rather than at the address list, so any row that overflows says so. In practice that is only this list - labels_fit_a_row already holds every menu label to fitting - which is why the change is narrow in effect while being general in the code. The self-test checks both halves, because either alone passes while the other is broken: that the dots stay inside the row's own column rather than running under the scrollbar, and that they are drawn at all. The second compares the rendered row against its own fit_row prefix - what a silent cut would have produced - and requires them to differ. Verified it fails when the ellipsis is removed.
A screen that hands the reader on to another one was drawing a tick, which is the glyph for agreeing to something. An address before its QR code asks nothing: the tick was answering a question the screen had not put. The confirm's look is a style now rather than a bool. Every one of the eighteen call sites set confirm_as_tick = true, so the flag distinguished nothing that existed - and a second bool for "and it points forward" would have made two flags encode three states, with one combination meaning nothing. SEEDTOOL_CONFIRM_BAR is the zero value, so a struct that names no style keeps what it had. The arrow itself is the back arrow walked from the other end: the triangle gained TRIANGLE_RIGHT, and DOWN and RIGHT are now the same mirroring of UP and LEFT. Reject on the left of the title bar and go on from the right, plainly one control pointing two ways rather than two glyphs to learn. Which screens get which: paged text always hands off - an address, an xpub or a descriptor to its QR, a transcript to the next stage - so it takes the arrow. A numbered word list is agreed to rather than passed through, and keeps the tick. Verified with the host self-test, the 52 Python tests and the self-test under ASan and UBSan, and by rendering the address screen off the framebuffer in both styles.
A QR screen was the one place in the firmware whose exit was drawn nowhere at all. The code sat against the left edge with the title in the margin beside it, which left no room on the left for an arrow, so the reader had to already know the chord left the screen. The code is right-aligned now and everything that is not the code - the title, and the arrow - shares the margin. The geometry computes the code's left edge rather than taking it from a constant, so all three QR screens move together: an address, the Compact SeedQR's region map, and a zoomed region. The arrow is drawn filled rather than carrying the chrome's three states. It is not a cursor position here: every QR screen leaves on the chord from wherever the reader is, so the arrow is a label for what that chord already does, not somewhere to move to. Verified with the host self-test, the 52 Python tests and the self-test under ASan and UBSan, and by rendering an address QR off the framebuffer before and after.
draw_grouped centred on the display and fit_grouped measured against it, both with the body face baked in. Neither assumption holds for a caller that has a margin rather than a screen - the address drawn beside its QR code in the commit above this one. Both keep their behaviour: the paged view passes the full width and the same face, and its own checks - which walk real addresses through fit_grouped and rebuild them character for character - still pass unchanged. Verified with the host self-test and the 52 Python tests.
Opening an address showed its text, and the code was a step further in. Scanning is what a reader almost always came for, so it is the screen they land on now - with the derivation path and the address itself in the margin the right-aligned code leaves, in the same groups of four the paged view uses. Two facts about one address were on two screens; they are on one. The grid is fixed at two groups to a line rather than as many as the margin holds, so the shape is the same every time and only the last line varies. Each column is as wide as the widest group in the value: the face is proportional, so groups of equal length are not of equal width, and centring each line would start every one at its own x. A last line that does not fill its columns is centred on the block instead of left in the first one. Not every address fits. A taproot address is 62 characters against the 48 two columns hold there, so seedtool_render_qr_address measures before it draws anything and refuses what will not fit - those fall back to the code alone and then the paged text, which has the width for them. An address cut off at the margin's end looks exactly like one that ended there, which is the whole reason this returns a bool rather than drawing as far as it reaches. That refusal is checked rather than argued: the new self-test alters an address's last group and requires the margin to change with it, so a tail that was never drawn fails, and it requires the taproot address to be refused outright. Both halves were confirmed against a deliberately broken build - removing the height guard fails it - because the existing grouped-paging check walks only the paged path at the body face and says nothing about groups drawn by any other route. page_grouped loses its only caller and goes; the paged grouped view it named is reached through page_text_impl now, for the values that need it. Verified with the host self-test, the 52 Python tests, the QR smoke test and the self-test under ASan and UBSan. Every address shape the firmware derives was rendered off the framebuffer and read.
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
2 participants
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.
Stacked on #20 — the two bottom commits (
Draw an address in fours, alternating ink,Say when a row was cut, instead of just stopping) are @sandman21vs's and will dropout of this diff once #20 merges and this branch is rebased onto
main. Review onlythe four commits above them; merge #20 first.
What changes
Opening an address used to cost two screens: a QR, then a forward step to the text.
Now one screen carries the QR on the right, the derivation path, and the address
itself in two columns of four characters, so nothing has to be remembered between
screens while checking it against a wallet.
in a checkbox, which reads as "I confirm" on screens that only advance. It is an
arrow now; the tick stays where assent is actually recorded.
right-aligned and carry a back arrow, freeing the left column for text.
@sandman21vs's grouping helper: it takes an explicit column and width instead of
assuming the full screen. No behaviour change — verified to build and self-test
on its own commit.
measures before it draws and returns false when the text will not fit, falling
back to the previous two-screen path — a taproot address is the case that does
not fit, at 62 characters against the 48 that two columns of four hold in this
margin. The first version of this screen truncated taproot to 48 characters and
looked complete; the guard exists because of that.
The address list keeps its plain
...elision.Verified by
./build-host/origo-simulator --self-testpython3 -m unittest discover -s tests— 52 passed-Wall -Wextra -Werrorgit rebase --exec)qr_address_draws_every_group— mutates the value's last group andrequires the drawn pixels to change, and requires a taproot address to be refused
rather than clipped
on a TTGO T-Display panel yet.