feat(capi): frontend-supplied text measurement (fixes #56) - #57
Merged
Conversation
The command-buffer screen's TextSize estimated label width as bytes x fontSize/2. Real fonts (e.g. Segoe UI wide glyphs) advance up to 2x that, and the error compounds down the label shunting chain in DasherViewSquare::DoDelayedText - users saw deep-zoom text degenerate into jumbled overlaps as sentences grew (Dasher-Windows #28; v5, with real platform font metrics, never overlapped). - dasher_set_text_size_callback: frontends measure text with the SAME font they draw opcode-5 commands with. Results cached per label and font size; steady-state frames do not re-measure. Registering before dasher_set_screen_size is fine (the callback is forwarded at screen creation). - dasher_text_metrics_changed: invalidates cached measurements after the canvas font changes (SP_DASHER_FONT / font picker). - Fallback estimate now counts UTF-8 code points, not bytes: accented text previously measured 2x its glyph count. A failing measurement falls back to the estimate without caching, so a later frame retries. - Wrapped labels (lock message) stay on the estimate: the callback contract has no wrap parameter. Tests: tests/test_text_metrics.cpp (callback consulted + per-pair cache, invalidation re-measures, pre-screen registration, failure fallback, null-callback path). Docs: dasher.h + C_API.md incl. Important Notes #7. Signed-off-by: will wade <willwade@gmail.com>
CI formats whole files with clang-format 18, which indents lambda-as-argument continuations differently from the local 21. Bind the shared callback to a named function and split the structured-binding loop so both versions agree. Signed-off-by: will wade <willwade@gmail.com>
Review finding on #57: CHECK(st.calls > 0) was vacuous after warm-up — calls were already positive, so a no-op invalidation would pass. Compare before/after counts for both total calls and per-pair maxima, then assert the fresh measurements are cached again. Signed-off-by: will wade <willwade@gmail.com>
Author
|
Valid — fixed in 5a2670f. Both invalidation assertions now compare before/after (total calls and per-pair maxima, so a no-op invalidation fails on either axis), followed by an exact-equality check that the fresh measurements are cached again. Tests: 3/3, 12 assertions. |
13 tasks
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.
Fixes #56.
What
Implements option 1 from the issue: frontends can supply real text metrics for the font they actually draw with, so the engine's label layout (the anti-overlap shunting in
DasherViewSquare::DoDelayedText) stops estimating.API
Design points:
Label× font size, invalidated by a generation counter (dasher_text_metrics_changedor re-registering the callback). Steady-state frames make zero callbacks — verified by test.dasher_set_screen_sizeforwards it to the screen.Consumer contract (per frontend)
Measure with the same font used for opcode-5 text commands (
SP_DASHER_FONT+ per-command size); pixels; call on thedasher_framethread (it's the UI thread everywhere today); calldasher_text_metrics_changedwhen the font picker changes the family.Tests (
tests/test_text_metrics.cpp, all via public C API)dasher_set_screen_sizestill takes effectFull suite: 39/39.
clang-formatclean vs main.Follow-ups (not in this PR)
Frontend wiring: Dasher-Windows #28 (the affected user), Dasher-GTK (Cairo/Pango), Dasher-Android (Paint.measureText via JNI), dasher-web. Each is small: one callback + invalidate call on font change.
DCO signed.
Greptile Summary
The PR adds frontend-provided text measurement to improve label layout while retaining a UTF-8-aware fallback estimate.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Sequence Diagram
sequenceDiagram participant Frontend participant CAPI as dasher_ctx / C API participant Screen as CommandScreen participant Cache as Label measurement cache Frontend->>CAPI: dasher_set_text_size_callback(callback, user_data) CAPI->>Screen: Forward callback when screen exists or is created Frontend->>CAPI: dasher_frame(...) CAPI->>Screen: TextSize(label, font_size) Screen->>Cache: Lookup current generation alt Cached measurement exists Cache-->>Screen: width, height else Measurement required Screen->>Frontend: callback(text, font_size, outputs, user_data) alt Callback succeeds Frontend-->>Screen: width, height Screen->>Cache: Store measurement for generation else Callback fails Frontend-->>Screen: non-zero or invalid dimensions Screen->>Screen: Use UTF-8 code-point estimate without caching end end Frontend->>CAPI: dasher_text_metrics_changed() CAPI->>Screen: Increment metrics generationReviews (2): Last reviewed commit: "test: make the invalidation assertions e..." | Re-trigger Greptile