Fix generated HWP table geometry serialization#923
Closed
ubermensch1218 wants to merge 4 commits into
Closed
Conversation
The writer path already has a native export entry point, but it lacked a small public-facing contract that proves a blank editable HWP can serialize, reload, and preserve mixed text after insertion. This adds that seam at the WASM/native wrapper level without expanding writer implementation scope. Constraint: Follow-on writer work should target .hwp output first and keep Swift/iOS work on top of a stable Rust writer contract. Rejected: Use HWPX generation as the first contract | the current milestone is HWP writer introduction, not HWPX export. Confidence: high Scope-risk: narrow Directive: Do not weaken the FileHeader, DocInfo, and BodyText/Section0 assertions without replacing them with an equivalent HWP validity check. Tested: cargo test --lib hwp_writer_contract -- --nocapture Tested: cargo test --lib test_export_hwp_empty -- --nocapture Tested: git diff --check (cherry picked from commit 807758c)
The CSAT HWP generator needs passage markers that are real table controls with square wrapping. setTableProperties previously updated attr bits and raw bytes inconsistently, so reopened files lost relation, offset, margin, and anchor state. Constraint: HWP table controls serialize object placement through CommonObjAttr. Rejected: Patch generated files after export | leaves rhwp unable to round-trip table placement. Confidence: high Scope-risk: narrow Directive: Keep table.common, table.attr, and raw_ctrl_data synchronized when adding table placement fields. Tested: cargo test test_set_table_properties_syncs_square_wrapping Not-tested: Full rhwp test suite (cherry picked from commit dfa2d4a95e693aaf4b26a8ea9790cbba3c035124)
Generated CSAT-style HWP output was relying on nested table geometry, but the binary table metadata could drift from the visible cell settings. This keeps table dimensions and row metadata aligned with the HWP CommonObjAttr layout before ggugeo consumes the WASM build. Constraint: Hancom reads serialized CommonObjAttr and table row-size records, not just high-level cell JSON Rejected: Keep patching only ggugeo marker constants | the width symptoms came from rhwp geometry serialization as well Confidence: medium Scope-risk: moderate Directive: Do not change CommonObjAttr offsets without checking parser/control/shape.rs layout Tested: cargo test test_update_ctrl_dimensions_writes_common_obj_width_offsets Tested: cargo test test_set_cell_properties_keeps_width_with_border_widths Tested: cargo test test_set_table_properties_syncs_square_wrapping (cherry picked from commit 25d0d86c6e9d509872f55667a6f98d78b7ff1efa)
Hancom ignores the intended narrow floating-table width when the serialized CommonObjAttr keeps the HWPTAG_TABLE record attr. The writer now forces CommonObjAttr packing from the common object fields, refreshes cell paragraph geometry after dimension changes, and gives square-wrapped generated tables matching line segment offsets. Constraint: Current fix was validated against an editable HWP CSAT marker case where Hancom only honored width after CommonObjAttr serialization stopped preserving the table record attr. Rejected: Caller-side width tuning only | Hancom continued to open the marker table too wide while the raw object attr stayed polluted. Confidence: medium Scope-risk: moderate Directive: Do not copy HWPTAG_TABLE attr into CommonObjAttr before serialization; those bitfields are distinct. Tested: cargo test test_set_table_properties_syncs_square_wrapping Tested: cargo test test_update_ctrl_dimensions_writes_common_obj_width_offsets Tested: cargo test test_set_cell_properties_keeps_width_with_border_widths Tested: git diff --check -- src/document_core/commands/table_ops.rs src/document_core/commands/text_editing.rs src/renderer/composer/line_breaking.rs src/wasm_api/tests.rs Not-tested: full cargo test suite (cherry picked from commit 2c8ae0d00adf108c9e25fca7a4ae9b8962fc227f)
Owner
|
Thanks for the follow-up PR, @ubermensch1218. However, this PR targets Additionally, the test checklist shows To proceed
We'll review the new PR once it's ready. The table geometry fix looks valuable — just needs the correct base branch. |
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
2 participants
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.
변경 요약
Generated narrow floating tables could open too wide in Hancom because
set_table_properties_nativecopied the HWPTAG_TABLE record attr intoCommonObjAttrbefore serialization. Since the serializer preserves non-zero raw attrs, the generated HWP kept a polluted object attr instead of packing width/height/wrap fields from the common object fields.This PR keeps generated table geometry stable by:
table.attr관련 이슈
Closes #921
테스트
cargo test test_set_table_properties_syncs_square_wrappingcargo test test_update_ctrl_dimensions_writes_common_obj_width_offsetscargo test test_set_cell_properties_keeps_width_with_border_widthsgit diff --check origin/main...HEADcargo testcargo clippy -- -D warnings스크린샷
Manual downstream Hancom check: an editable CSAT-style
[A]floating table marker only started honoring the narrow generated width after the CommonObjAttr serialization fix. No screenshot is attached because the regression is covered here with writer roundtrip assertions rather than renderer output.