Fix the drawn icons and centre the digit field - #19
Merged
Conversation
The confirm tick derived both legs from one width: width/3 for the short one, the remainder for the long one. With NAV_ARROW_WIDTH=7 that integer division gives 2, so the short leg was two overlapping squares - barely wider than the low point itself - and the mark read as a diagonal bar. The legs are fixed lengths now, 3 and 6, roughly the 1:2 ratio a check needs to be recognisable, in a box of their own rather than the arrow's: the arrow is tall and narrow to read as a triangle, a check this small has to be wider than tall. The box is derived from the legs and stroke rather than declared beside them, so changing a leg cannot leave the tick silently off-centre. Verified with the host self-test and the 52 Python tests. Both states - idle white-on-black and selected black-on-orange - were rendered off the framebuffer and compared against the old shape; costs 0 bytes of flash. Simulator only, not yet flashed.
The back arrow and the digit field's scroll hints were two functions drawing the same shape on different axes - the back arrow stepping columns, the digit arrows stepping rows. Same arithmetic, transposed. They had drifted apart in the rounding, which is how this surfaced. The back arrow's half-width is (step + 1) * across / (2 * steps); the digit arrows' left out the + 1, so their first two rows both rounded to zero and the apex came out as a 1px-wide, 2px-tall spike rather than a point. One function now, taking a direction. The back arrow's own pixels are unchanged - the unified formula reproduces its 1, 3, 5, 7, 7, 9, 11 exactly, checked by rendering the header before and after and comparing the framebuffer byte for byte. The digit arrows also drop from seven rows to six. Half-widths step in whole pixels, so an 11-wide triangle has exactly six distinct rows; a seventh has no width of its own left and repeats one, which reads as a flat spot in the diagonal. Verified with the host self-test and the 52 Python tests.
The field sat 13px above the centre of the band between its title and its footer - measured, not eyeballed: the stack of arrows and boxes ran 26..77, centre 51, in a band of 18..110 whose centre is 64. DIGIT_BOX_Y and the two arrow rows were pinned literals, so nothing moved when the arrows changed height and nothing could notice they were off-centre to begin with. They are derived from the stack's own parts now, and from the band it sits in. That band is not the same on both screens that draw this stack, which is why one pinned y could never suit both: the digit field has only the footer under it, while the value box shares its screen with the entropy quality bar at DICE_BAR_Y. Each centres against the thing that actually bounds it. The value box moves down 2px as a result - it was not centred above the bar either, just less visibly. Two things this turned up: The assert meant to keep the down arrow off the footer compared against LIST_FOOTER_Y, which belongs to the list screens and sits 11px below the footer these actually draw. It would have permitted the overlap it was written to catch. Both placements are asserted now, each against its own bound. The footer's y was the literal 111 in three renderers; it is SCREEN_FOOTER_Y, which is what the centring has to measure against. Verified with the host self-test and the 52 Python tests, and by measuring the rendered framebuffer: the stack now centres at 63 against the band's 64, the 1px being the odd height's rounding.
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
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.
Three things wrong with shapes this firmware draws by hand, found by looking at them scaled up rather than on the device.
The tick was a diagonal bar
It derived both legs from one width:
width/3for the short one. WithNAV_ARROW_WIDTH=7that integer division gives 2, so the short leg was two overlapping squares — barely wider than the low point — and the mark read as a bar, not a check.Fixed lengths now (3 and 6, roughly the 1:2 a check needs), in a box of its own: the arrow is tall and narrow to read as a triangle, a check this small has to be wider than tall. The box is derived from the legs so it cannot drift from them.
The two arrows were two functions that had drifted
The back arrow and the digit field's scroll hints draw the same shape on different axes — one stepping columns, one stepping rows. Same arithmetic, transposed.
They had drifted in the rounding, which is how this surfaced: the back arrow's half-width is
(step + 1) * across / (2 * steps); the digit arrows' left out the+ 1, so their first two rows both rounded to zero and the apex came out as a 1px-wide, 2px-tall spike.One function now, taking a direction. The back arrow's pixels are unchanged — the unified formula reproduces its
1, 3, 5, 7, 7, 9, 11exactly, checked by rendering the header before and after and comparing the framebuffer byte for byte. The digit arrows drop to six rows, since an 11-wide triangle has exactly six distinct half-widths and a seventh row repeats one.The digit field sat 13px above centre
Measured, not eyeballed: the stack of arrows and boxes ran y 26..77, centre 51, in a band of 18..110 whose centre is 64.
DIGIT_BOX_Yand the arrow rows were pinned literals, so nothing moved when the arrows changed height and nothing could notice they were off-centre to begin with. They are derived now — from the stack's own parts, and from the band it sits in.That band differs between the two screens that draw this stack, which is why one pinned y could never suit both: the digit field has only the footer under it, the value box shares its screen with the entropy quality bar. Each centres against what actually bounds it.
The value box moves down 2px as a result. It was not centred above the bar either, just less visibly.
Two things this turned up
The assert meant to keep the down arrow off the footer compared against
LIST_FOOTER_Y— the list screens' footer, 11px below the one these actually draw. It would have permitted the overlap it was written to catch. Both placements are asserted now, each against its own bound.The footer's y was the literal
111in three renderers; it isSCREEN_FOOTER_Y.Size
111 insertions, 58 deletions — of which net executable code is +6 lines. The rest is comments, named constants and asserts. +32 bytes of flash (303952, from 303920).
Verification
Host self-test and the 52 Python tests, on every commit individually via
git rebase --exec, not just the tip. Each changed shape was rendered off the framebuffer and read at 20x; the digit field's centring was measured from the rendered pixels rather than judged by eye.Flashed to a TTGO T-Display and walked on the panel: the digit field, the tick in both states, the unchanged back arrow, the orange in each, and the value box's 2px shift all confirmed there.