fix(treemap): fail closed for CJK label widths (#121)
* Fix CJK treemap label width verification * Strengthen CJK treemap width regressions
This commit is contained in:
Cathryn Lavery
@@ -1,7 +1,7 @@
|
||||
{
|
||||
"name": "diagram-design",
|
||||
"description": "Create branded architecture, IT current-state, flowchart, sequence, state machine, ER/data model, timeline, swimlane, quadrant, radar/spider, loop/flywheel, nested, tree, org chart, layer stack, Venn, pyramid/funnel, treemap, bar, line, Gantt and scatter charts, high-level, process, medallion, data flow, DP integration, DP security matrix, sankey, fishbone, Wardley map, kanban, user journey, deployment, dependency graph, UML class, story map, or database schema diagrams as standalone HTML/SVG/PNG. Redraw .drawio/.drawio.png/.drawio.svg or Mermaid .mmd sources at a chosen size/detail; onboard brand tokens from a website; add semantic patterns, callouts, accessible motion, or sketchy/hand-drawn styling.",
|
||||
"version": "2.5.11",
|
||||
"version": "2.5.13",
|
||||
"author": {
|
||||
"name": "Cathryn Lavery",
|
||||
"url": "https://github.com/cathrynlavery"
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
{
|
||||
"name": "diagram-design",
|
||||
"description": "Create branded architecture, IT current-state, flowchart, sequence, state machine, ER/data model, timeline, swimlane, quadrant, radar/spider, loop/flywheel, nested, tree, org chart, layer stack, Venn, pyramid/funnel, treemap, bar, line, Gantt and scatter charts, high-level, process, medallion, data flow, DP integration, DP security matrix, sankey, fishbone, Wardley map, kanban, user journey, deployment, dependency graph, UML class, story map, or database schema diagrams as standalone HTML/SVG/PNG. Redraw .drawio/.drawio.png/.drawio.svg or Mermaid .mmd sources at a chosen size/detail; onboard brand tokens from a website; add semantic patterns, callouts, accessible motion, or sketchy/hand-drawn styling.",
|
||||
"version": "2.5.11",
|
||||
"version": "2.5.13",
|
||||
"author": {
|
||||
"name": "Cathryn Lavery",
|
||||
"url": "https://github.com/cathrynlavery"
|
||||
|
||||
@@ -12,6 +12,7 @@ Exit: 0 all pass, 1 a case failed.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import runpy
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
@@ -20,6 +21,12 @@ from pathlib import Path
|
||||
ROOT = Path(__file__).resolve().parent.parent
|
||||
CHECKER = ROOT / "scripts/verify-treemap.py"
|
||||
GOOD = ROOT / "skills/diagram-design/assets/example-treemap.html"
|
||||
SHIPPED = sorted(GOOD.parent.glob("example-treemap*.html"))
|
||||
ESTIMATED_ADVANCE = runpy.run_path(str(CHECKER), run_name="verify_treemap_test")[
|
||||
"estimated_advance"
|
||||
]
|
||||
KOREAN_LABEL = "남아메리카 인구 구성 비율 요약"
|
||||
JAPANESE_LABEL = "南アメリカ人口構成比率の概要"
|
||||
|
||||
|
||||
def run(path: Path) -> tuple[int, str]:
|
||||
@@ -43,15 +50,35 @@ def main() -> int:
|
||||
source = GOOD.read_text(encoding="utf-8")
|
||||
failures: list[str] = []
|
||||
|
||||
# Pin the estimator independently from CLI diagnostics. The Korean fixture
|
||||
# has 13 wide glyphs and four narrow spaces; the Japanese fixture has 14
|
||||
# wide glyphs. This catches a width-contract regression without coupling
|
||||
# the overflow fixture to its human-readable pixel message.
|
||||
for label, expected, language in (
|
||||
(KOREAN_LABEL, 15.4, "Korean"),
|
||||
(JAPANESE_LABEL, 14.0, "Japanese"),
|
||||
):
|
||||
actual = ESTIMATED_ADVANCE(label, False)
|
||||
if abs(actual - expected) > 1e-9:
|
||||
failures.append(
|
||||
f"{language} estimator contract moved: expected {expected:g}em, got {actual:g}em"
|
||||
)
|
||||
else:
|
||||
print(f"OK: {language} wide-glyph estimator contract is {expected:g}em")
|
||||
|
||||
with tempfile.TemporaryDirectory() as raw:
|
||||
directory = Path(raw)
|
||||
|
||||
# 1. The shipped example must pass untouched.
|
||||
code, output = run(write(directory, "clean.html", source))
|
||||
if code != 0:
|
||||
failures.append(f"clean example was rejected: {output.strip()}")
|
||||
else:
|
||||
print("OK: the shipped treemap passes")
|
||||
# 1. Every shipped Latin example must pass untouched, proving the
|
||||
# Unicode estimator did not move the calibrated Latin baseline.
|
||||
shipped_clean = True
|
||||
for path in SHIPPED:
|
||||
code, output = run(path)
|
||||
if code != 0:
|
||||
shipped_clean = False
|
||||
failures.append(f"clean example {path.name} was rejected: {output.strip()}")
|
||||
if shipped_clean:
|
||||
print(f"OK: all {len(SHIPPED)} shipped Latin treemaps pass")
|
||||
|
||||
# 2. Shrink a labelled cell below the share it prints. This is the
|
||||
# defect that shipped in review — a cell drawn smaller than it claims
|
||||
@@ -104,7 +131,51 @@ def main() -> int:
|
||||
else:
|
||||
print("OK: a label wider than its cell is rejected")
|
||||
|
||||
# 4b. Overflow off the LEFT edge, via an end-anchored label. Checking
|
||||
# 4a. Reproduce #115: most glyphs in this Korean translation are
|
||||
# full-width. The old 0.60em character-count estimate called it
|
||||
# 122.4px wide and accepted it in 132px of available space, while
|
||||
# Chromium measured 146.2px. A conservative 1em wide-glyph advance
|
||||
# must fail closed without changing the Latin estimate.
|
||||
korean = source.replace(
|
||||
'<text x="804" y="324" fill="#2d3142" font-size="12" font-weight="600" '
|
||||
'font-family="\'Geist\', sans-serif">South America</text>',
|
||||
'<text x="804" y="324" fill="#2d3142" font-size="12" font-weight="600" '
|
||||
'font-family="\'Geist\', \'Apple SD Gothic Neo\', \'Noto Sans KR\', '
|
||||
f'\'Malgun Gothic\', sans-serif">{KOREAN_LABEL}</text>',
|
||||
)
|
||||
if korean == source:
|
||||
failures.append("could not build the Korean-overflow fixture (anchor moved)")
|
||||
else:
|
||||
code, output = run(write(directory, "korean-overflow.html", korean))
|
||||
if code == 0:
|
||||
failures.append("full-width Korean label overflowing its cell was accepted")
|
||||
elif "label" not in output or "overflows" not in output:
|
||||
failures.append(f"Korean fixture lacked a label-overflow finding: {output.strip()}")
|
||||
else:
|
||||
print("OK: a Korean label in the old-estimator boundary gap is rejected")
|
||||
|
||||
# 4b. Japanese Han and kana are also Unicode-wide. Cover the documented
|
||||
# CJK contract beyond Hangul with another label that fits under the
|
||||
# old 0.60em estimate but overflows under the conservative contract.
|
||||
japanese = source.replace(
|
||||
'<text x="804" y="324" fill="#2d3142" font-size="12" font-weight="600" '
|
||||
'font-family="\'Geist\', sans-serif">South America</text>',
|
||||
'<text x="804" y="324" fill="#2d3142" font-size="12" font-weight="600" '
|
||||
'font-family="\'Geist\', \'Hiragino Sans\', \'Noto Sans JP\', '
|
||||
f'\'Yu Gothic\', sans-serif">{JAPANESE_LABEL}</text>',
|
||||
)
|
||||
if japanese == source:
|
||||
failures.append("could not build the Japanese-overflow fixture (anchor moved)")
|
||||
else:
|
||||
code, output = run(write(directory, "japanese-overflow.html", japanese))
|
||||
if code == 0:
|
||||
failures.append("full-width Japanese label overflowing its cell was accepted")
|
||||
elif "label" not in output or "overflows" not in output:
|
||||
failures.append(f"Japanese fixture lacked a label-overflow finding: {output.strip()}")
|
||||
else:
|
||||
print("OK: a Japanese full-width label is rejected when it overflows")
|
||||
|
||||
# 4c. Overflow off the LEFT edge, via an end-anchored label. Checking
|
||||
# only the right and bottom edges would pass this silently, and the
|
||||
# text would read as belonging to the cell next door.
|
||||
left_over = source.replace(
|
||||
@@ -122,7 +193,7 @@ def main() -> int:
|
||||
else:
|
||||
print("OK: a label overflowing the left edge is rejected")
|
||||
|
||||
# 4c. Overflow off the TOP edge — the label's ascent clears the cell.
|
||||
# 4d. Overflow off the TOP edge — the label's ascent clears the cell.
|
||||
top_over = source.replace(
|
||||
'<text x="592" y="68" fill="#2d3142" font-size="13"',
|
||||
'<text x="592" y="44" fill="#2d3142" font-size="13"',
|
||||
@@ -138,7 +209,7 @@ def main() -> int:
|
||||
else:
|
||||
print("OK: a label overflowing the top edge is rejected")
|
||||
|
||||
# 4d. A sliver can be narrower than the fixed 10px information-mark
|
||||
# 4e. A sliver can be narrower than the fixed 10px information-mark
|
||||
# disc. The marker must never cross into the neighbouring cell.
|
||||
marker_over = source.replace(
|
||||
'<rect x="940" y="296" width="16" height="124"',
|
||||
@@ -157,6 +228,21 @@ def main() -> int:
|
||||
else:
|
||||
print("OK: an information mark overflowing its cell is rejected")
|
||||
|
||||
# 4f. Combining marks do not advance independently. Eighteen decomposed
|
||||
# accented glyphs fit by the unchanged Latin baseline (129.6px in
|
||||
# 132px), but counting every acute accent as a character would
|
||||
# incorrectly double the estimate and reject the label.
|
||||
decomposed = ("e\N{COMBINING ACUTE ACCENT}" * 18)
|
||||
combining = source.replace(">South America</text>", f">{decomposed}</text>")
|
||||
if combining == source:
|
||||
failures.append("could not build the combining-mark fixture (anchor moved)")
|
||||
else:
|
||||
code, output = run(write(directory, "combining-marks.html", combining))
|
||||
if code != 0:
|
||||
failures.append(f"non-advancing combining marks were overcounted: {output.strip()}")
|
||||
else:
|
||||
print("OK: combining marks do not create phantom label overflow")
|
||||
|
||||
# 5. The shipped example leaves its smallest cell deliberately
|
||||
# unlabelled. That must not switch the area check off for the cells
|
||||
# that ARE labelled — the failure mode where a checker reports clean
|
||||
|
||||
@@ -22,9 +22,10 @@ later-painted nodes) reads a cell against the number printed inside it.
|
||||
|
||||
Text extent is estimated from font metrics rather than measured in a browser,
|
||||
deliberately: every other gate here is pure Python and runs in CI with no
|
||||
browser. The advances below are calibrated against Chromium renderings of the
|
||||
shipped Geist / Geist Mono faces and rounded UP, so the estimate reports
|
||||
overflow slightly before real overflow, never after.
|
||||
browser. The Latin advances below are calibrated against Chromium renderings of
|
||||
the shipped Geist / Geist Mono faces and rounded UP. Unicode wide/full-width
|
||||
characters use a conservative 1em advance, so both estimates report overflow
|
||||
slightly before real overflow, never after.
|
||||
|
||||
Usage:
|
||||
python3 scripts/verify-treemap.py --all
|
||||
@@ -39,6 +40,7 @@ import argparse
|
||||
import html
|
||||
import re
|
||||
import sys
|
||||
import unicodedata
|
||||
from pathlib import Path
|
||||
|
||||
ROOT = Path(__file__).resolve().parent.parent
|
||||
@@ -66,6 +68,7 @@ VALUE_RE = re.compile(r"(?P<num>\d[\d,]*(?:\.\d+)?)\s*(?P<unit>[BMK])\b")
|
||||
|
||||
MONO_ADVANCE = 0.62
|
||||
SANS_ADVANCE = 0.60
|
||||
WIDE_ADVANCE = 1.00
|
||||
ASCENT = 0.74
|
||||
UNITS = {None: 1.0, "K": 1e3, "M": 1e6, "B": 1e9}
|
||||
|
||||
@@ -116,6 +119,23 @@ def plain(body: str) -> str:
|
||||
return html.unescape(TAG_RE.sub("", body)).strip()
|
||||
|
||||
|
||||
def estimated_advance(text: str, mono: bool) -> float:
|
||||
"""Conservative text advance in em, preserving the calibrated Latin baseline."""
|
||||
narrow_advance = MONO_ADVANCE if mono else SANS_ADVANCE
|
||||
advance = 0.0
|
||||
for char in text:
|
||||
# Nonspacing and enclosing marks modify the preceding glyph; counting
|
||||
# them as another glyph rejects labels that do fit. Spacing combining
|
||||
# marks (Mc) retain the narrow estimate because they can advance.
|
||||
if unicodedata.category(char) in {"Mn", "Me"}:
|
||||
continue
|
||||
if unicodedata.east_asian_width(char) in {"W", "F"}:
|
||||
advance += WIDE_ADVANCE
|
||||
else:
|
||||
advance += narrow_advance
|
||||
return advance
|
||||
|
||||
|
||||
def parse_cells(source: str) -> list[Box]:
|
||||
"""Deduped cell rects. Each cell is painted twice: paper mask, then body."""
|
||||
seen: list[Box] = []
|
||||
@@ -149,11 +169,11 @@ def label_box(attrs: dict[str, str], body: str) -> Box | None:
|
||||
size = float(attrs.get("font-size", "12"))
|
||||
except (KeyError, ValueError):
|
||||
return None
|
||||
chars = len(plain(body))
|
||||
if not chars:
|
||||
text = plain(body)
|
||||
if not text:
|
||||
return None
|
||||
mono = "mono" in attrs.get("font-family", "").lower()
|
||||
width = chars * size * (MONO_ADVANCE if mono else SANS_ADVANCE)
|
||||
width = size * estimated_advance(text, mono)
|
||||
anchor = attrs.get("text-anchor", "start")
|
||||
|
||||
transform = attrs.get("transform", "")
|
||||
|
||||
@@ -145,7 +145,7 @@ Geist has no CJK coverage. When labels contain Japanese, Chinese, or Korean text
|
||||
<text font-family="'Geist', 'Apple SD Gothic Neo', 'Noto Sans KR', 'Malgun Gothic', sans-serif">인증 서비스</text>
|
||||
```
|
||||
|
||||
The Hiragino/Yu Gothic stack carries no Hangul glyphs, so Korean labels need the Korean stack — don't reuse the Japanese one. For mono sublabels use `'Geist Mono', 'Noto Sans Mono CJK JP', monospace` (Japanese) or `'Geist Mono', 'Noto Sans Mono CJK KR', monospace` (Korean). CJK glyphs render ~10% wider than Latin at the same size — budget box width accordingly, and prefer 12px names over 8px sublabels for CJK, which goes muddy below 10px.
|
||||
The Hiragino/Yu Gothic stack carries no Hangul glyphs, so Korean labels need the Korean stack — don't reuse the Japanese one. For mono sublabels use `'Geist Mono', 'Noto Sans Mono CJK JP', monospace` (Japanese) or `'Geist Mono', 'Noto Sans Mono CJK KR', monospace` (Korean). Budget **1em per full-width CJK glyph**, not a small percentage over the average Latin glyph; `verify-treemap.py` uses that conservative contract for Unicode wide/full-width characters and treats combining marks as non-advancing. Prefer 12px names over 8px sublabels for CJK, which goes muddy below 10px. Actual width still varies by fallback font, so run the relevant geometry verifier after translating labels.
|
||||
|
||||
---
|
||||
|
||||
|
||||
Reference in New Issue
Block a user