Skip to content

fix(text): honor element color/anchor on parse_colors path and drop O(n²) fitting#28

Merged
g4bri3lDev merged 1 commit into
OpenDisplay:mainfrom
balloob:fix/text-rendering
Jul 6, 2026
Merged

fix(text): honor element color/anchor on parse_colors path and drop O(n²) fitting#28
g4bri3lDev merged 1 commit into
OpenDisplay:mainfrom
balloob:fix/text-rendering

Conversation

@balloob

@balloob balloob commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Correctness and performance fixes for the text and multiline elements, focused on the parse_colors path and on text fitting.

Findings addressed

  • A6 — parse_colors ignored element color. parse_colored_text hardcoded black for text outside any [color] markup, so {value: "plain text", color: red, parse_colors: true} rendered black. The helper now takes a default_color argument and draw_text/draw_multiline pass the element color through.
  • A7 — parse_colors dropped the vertical anchor for single-line text. The single-line branch drew at the raw y with anchor="lt", so anchor: mm/mb were silently ignored and the text shifted down by half its height the moment parse_colors was enabled. The vertical anchor offset is now applied, matching the plain path.
  • B2 — O(n²) truncation/wrapping. truncate: true shrank the string one character at a time, re-measuring the whole string each step (measured at ~3s for a 1600-word string — that blocks HA's event loop). It now binary-searches the cut point (O(n log n)). Word-wrap accumulates precomputed word widths instead of re-measuring the whole current line per word.
  • B7 — redundant textbbox after draw.text. draw_multiline called textbbox twice and threw the result away (pure waste); those are removed. The per-segment textbbox calls on the parse_colors paths of draw_text are replaced with cheap font-metric / precomputed block-height math for the pos_y cursor.

Note / deferred

Finding A7 also mentions a related inconsistency: on the parse_colors path, calculate_segment_positions applies both align and the horizontal anchor component, so anchor: mm + align: center double-offsets horizontally. Fixing that cleanly depends on the intended semantics of align vs anchor on this path (Pillow treats align as intra-block justification only), so I've left it out of this PR to avoid guessing at behavior — happy to follow up if you confirm the desired semantics.

Tests

Adds regression tests for A6 (element color applied), A7 (vertical anchor respected), and unit tests for the B2 truncate/wrap helpers (including a check that binary-search truncation matches the old linear result). No new failures in the suite (pre-existing tests/visual/* byte-snapshot failures under Pillow 12 are unrelated).

🤖 Generated with Claude Code

…(n^2) fitting

- A6: unmarked text on the parse_colors path now uses the element-level color
  instead of a hardcoded black. parse_colored_text() gains a default_color arg.
- A7: the vertical anchor component (e.g. 'mm', 'mb') is now applied to
  single-line parse_colors text, matching the plain path — previously it drew at
  the raw y with a 'lt' anchor and shifted down by half the text height.
- B2: replace the character-at-a-time truncate (O(n^2), seconds on the event
  loop for long strings) with a binary-search cut (O(n log n)), and wrap by
  accumulating precomputed word widths instead of re-measuring the whole line
  per word.
- B7: remove the two textbbox() calls in draw_multiline whose results were
  discarded, and the per-segment textbbox() calls on the parse_colors paths of
  draw_text (pos_y now derives from font metrics / precomputed block height).

Adds regression tests for A6, A7, and the B2 helpers.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UVpLN4Rzdv7y3ud7QzutVJ
@balloob balloob requested a review from g4bri3lDev as a code owner July 4, 2026 07:19
@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@g4bri3lDev g4bri3lDev merged commit e4d6b05 into OpenDisplay:main Jul 6, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants