Repository navigation
fix(pages): make a line taller by the borders its characters' shapes draw above and below - #653
Conversation
airmang
left a comment
airmang
left a comment
There was a problem hiding this comment.
Reviewed the border-width lookup/cache, held-character metrics, ending-run height, and cache refusal against the new 26-page synthetic fixture. Exact-head CI passes 7,614 tests and static gates; 27 focused local regressions pass. An independent public-API reproduction found a supported reflow path that drops the new border metrics (details inline). This needs correction despite green CI. Integration with latest main 907c54b also conflicts in CHANGELOG.md, so merging is held.
| name = _local(child) | ||
| if name == "t": | ||
| for offset in range(len(_t_text(child))) if boxed else (): | ||
| held[place + offset + 1] = max(held.get(place + offset + 1, 0), box) |
There was a problem hiding this comment.
[P2] Preserve the new border metrics when paragraphs reflow
The added border height lives only in held, but _paragraph replaces its computed metrics through _Reflow.at for page.unequal, and _Reflow neither stores held nor forwards it to _line_metrics. The new height therefore disappears on this supported layout path. Reproduction using this PR's pages_character_border_line_height.hwpx: remove line caches and encode two identical 21260-unit columns either as sameSz="1", sameGap="0", colCount="2", or as sameSz="0", colCount="2" with two colSz width="16384" gap="0" children. Both resolve to exactly the same actual column widths, but estimate_pages reports 66 different paragraph positions: paragraph 13 is at 3928 with equal-column encoding and 3840 with explicit-column encoding. The 10pt/0.1mm bordered line falls from height/advance 1056/1688 to 1000/1600 during same-width reflow. Carry border/held metrics through _Reflow.at and its banded path (rebasing offsets), or fail closed for combinations that cannot preserve them, and add a public regression for equivalent column-width encodings.
6b56c0a to
b5f2e60
Compare
airmang
left a comment
airmang
left a comment
There was a problem hiding this comment.
Re-reviewed b5f2e60 after the main update. The border feature's source/test delta and 26-page fixture are identical to the previously reviewed head. All 13 exact-head checks pass (7,616 tests/static gates); 27 local focused tests pass. The existing unresolved P2 is still independently reproducible on this head: equivalent two-column encodings at 21260/21260 units yield 66 different paragraph positions because reflow loses the border metrics (paragraph 13: 3928 vs 3840). The original inline finding is retained without a duplicate. Hold for that fix plus the current-main a3269cb CHANGELOG.md conflict.
b5f2e60 to
25f5dd8
Compare
airmang
left a comment
airmang
left a comment
There was a problem hiding this comment.
Re-reviewed 25f5dd8 after the latest-main update. The border feature's source/test delta and 26-page fixture remain identical to the previous reviewed head, retaining #630/#649/#654. All 13 exact-head checks pass (7,624 tests and static gates); 27 focused local tests pass. The original unresolved P2 still reproduces on this new head via public estimate_pages: equivalent 21260/21260-unit column encodings produce 66 different paragraph positions (paragraph 13: 3928 vs 3840) because reflow drops border heights. The original inline issue is retained without a duplicate finding. Merge remains held pending preservation of border/held metrics through reflow or an explicit unsupported result, with a public regression.
…draw above and below A character shape with a border above or below (hh:borderFill of its charPr) makes the line its characters stand on that much taller on each side, its line spacing reckoned from that height, as Hancom lays it out; an empty run ending the paragraph counts as well. Left and right borders take no room. The line cache leaves such paragraphs to Hancom, which writes their heights and baselines otherwise. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…cument Looking up each character shape's border fill through the document's border_fills property parsed the whole header again on every call, making line-cache writing half as slow again. The widths above and below each border fill are now read once from the header element the measure holds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… released Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
25f5dd8 to
6a80127
Compare
airmang
left a comment
airmang
left a comment
There was a problem hiding this comment.
Re-reviewed 6a80127 after incorporating #655/#657 and resolving the current-main conflict. The border feature's source/test delta and 26-page fixture are identical to the prior review; the intervening stack-spacing and moved-table changes do not fix the reflow path. Exact-head CI passes all 13 checks, including 7,628 tests plus Ruff/mypy/Pyright. The original P2 still reproduces on this exact head: equivalent 21260/21260-unit column encodings yield 66 different paragraph positions (paragraph 13: 3928 vs 3840) because border metrics are dropped during reflow. The existing inline finding remains unresolved; no duplicate inline is added. Merge remains held until that path preserves the border/held metrics or returns unsupported, with a public regression.
요약
글자 모양에 위·아래 테두리(
charPr@borderFillIDRef의topBorder·bottomBorder)가 있으면, 한/글은 그 글자가 선 줄을 테두리 두께만큼 위아래로 키우고 줄 간격도 그 높이로 잽니다. 문단 끝 빈 run에 테두리가 있어도 마찬가지입니다. 왼·오른 테두리는 줄 높이에 영향이 없습니다.쪽 수 추정은 글자 테두리를 보지 않아서, 그런 줄과 그 뒤 문단을 테두리 두께만큼 위에 놓았습니다.
바뀐 점
src/hwpx/layout/pages.py_Measure.char_box를 더했습니다. 글자 높이에 위·아래 테두리 두께(mm를 HWPUNIT으로 바꾼 값)를 더한 크기이고, 글자 모양마다 한 번만 셉니다._held_sizes: 테두리 있는 글자마다 그 줄을char_box만큼 키웁니다. 이 경로는 줄 높이와 간격에만 들어가고, 글자 폭과 줄 나눔은 그대로입니다._end_size: 문단 끝 빈 run의 크기를char_box로 잽니다.src/hwpx/layout/line_cache.py: 테두리 있는 run이 있는 문단은 줄 캐시를 쓰지 않습니다. 한/글은 그 줄의 높이(vertsize)와 기준선(baseline)을 테두리 자리에 따라 다르게 적습니다.pages_character_border_line_height.hwpx(26쪽, 한/글 저장본)를 넣었습니다.CHANGELOG.md항목을 더했습니다.확인
🤖 Generated with Claude Code