feat(document)!: collapse the drawing elements into frame - #821
Merged
Conversation
andiwand
force-pushed
the
deprecate-drawing-elements
branch
from
September 5, 2026 20:26
af7c644 to
1f06b83
Compare
andiwand
force-pushed
the
collapse-drawing-elements
branch
from
September 5, 2026 20:55
07c1351 to
060502b
Compare
andiwand
force-pushed
the
collapse-drawing-elements
branch
2 times, most recently
from
September 6, 2026 11:43
d3ffac5 to
d73cb46
Compare
BREAKING CHANGE: `Rect`, `Line`, `Circle` and `CustomShape` are gone, with their `ElementType` values and `Element::as_rect`/`as_line`/`as_circle`/ `as_custom_shape`. A shape is a `Frame` naming itself through the new `ShapeType Frame::shape_type()`, and carrying `Frame::path()` and the new `Frame::line()`. Closes #773. The four types were an odf-parser artifact — the other five engines already funnel every shape into `frame` — and cost a public class, an abstract adapter, a dispatcher, a style writer and three binding mirrors each. Shapes now carry `anchor_type()` and `z_index()`, which only frames had; the renderer still places them absolutely, so no emitted html moves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pd6AtUjaEzVT4FXMQdXLjT
andiwand
force-pushed
the
collapse-drawing-elements
branch
from
September 6, 2026 12:36
d73cb46 to
69f033e
Compare
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.
🤖 Generated with Claude Code
Closes #773. Rebased onto
mainafter the deprecation PR (#818) was dropped:this is now one commit that goes straight to the breaking change, and the
ODR_DEPRECATED/ODR_INTERNAL_BUILDmechanism #818 introduced is gone withit.
Breaking, and the API-shape prerequisite for #771: it turns "add a shape kind"
from a vertical slice through four binding layers into one parser-table line.
The model
Gone:
Rect,Line,Circle,CustomShape, their fourElementTypevalues,Element::as_rect/as_line/as_circle/as_custom_shape, fourabstract::*Adapterinterfaces with their dispatchers, eighteen near-duplicateodf readers, three near-identical style writers, and the Java/ObjC/Python
mirrors of all of it.
In their place,
Frame:ShapeTypecarries the five kinds the parser can actually tell apart today,not the nine the issue sketched —
draw:polygon,draw:pathand friendsalready reach the renderer as a
customwith a realDrawingPath, so aseparate enumerator for each would carry no information. New kinds are one
enumerator plus one table line when a renderer branch wants them.
A line is the one shape ODF states as two points rather than a box, so it keeps
them, in a
DrawingLinealongside the existingDrawingPath/DrawingTransform.FrameAdapterdefaults the three new readers (none,{},{}): only odfever produces a shape, and the other five engines would otherwise carry fifteen
stubs saying so. That default needs the complete types, so
internal/abstract/document.hppnow includes<odr/document_element.hpp>instead of forward-declaring ten of its types.
odf
create_shape_elementrecords the kind in aSideTable<ShapeType>beside theelement — no extra bytes in
ElementRegistry::Element, which is size-critical— and the parser table maps
draw:rect/draw:caption→rect,draw:line/draw:measure→line,draw:circle/draw:ellipse→ellipse(or
customwhendraw:kindcuts it), and everything drawn rather than named→
custom.draw:frameanddraw:gstay plain frames.The merged
frame_x/y/width/heightare the oldcustom_shape_*: readthe
svg:attribute, else fall back to the connector's own path box.Rendering is untouched
translate_frameswitches onshape_type()into the five bodies that weretranslate_frame/_rect/_line/_circle/_custom_shape, and the threeidentical shape-box writers became one
translate_shape_properties. Shapeskeep their absolute placement rather than picking up
translate_frame_properties,so the reference output is byte-identical — verified below — and no pin
moves.
That does leave the anchoring/z-index bug §3 of #773 describes open: a
draw:custom-shapewithtext:anchor-type="paragraph"still lands relative tothe page. The fix is now one line (send shapes through
translate_frame_propertiestoo), but it moves pixels on ~300 reference files,so it belongs with #771 and its regen rather than here.
Rebased onto #823 and #829
The three red CI legs were the branch's base, not this change: #819 gave
PageLayoutadirectionthatiwork_document.cppdid not name (gcc-14 andclang-18
-Wmissing-field-initializers), and an odf test's raw string satinside a macro argument msvc's traditional preprocessor will not read.
mainfixed both in #825. A fourth leg,
test, was failing onmaintoo — thereference-output pins lagged #824 — and #829 has since advanced them.
The rebase lands on #823, which moved the per-type
*_adapter(id)dispatchersinto
internal::ElementAdapterand the element store intointernal::ElementRegistry. Resolved to keep both: odf'sAdapterBasedropsthe four shape adapters from its pack,
internal::ElementAdapterloses thefour dispatchers it generated for them, and the odf registry's
m_shape_typesbecomes a
SortedSideTable<ShapeType, StoredId>beside the other four — it iswritten when the element is created, so the binary search fits.
Verified on this rebase, at
4a80837f-WerrorRelWithDebInfo build of the library, tests and CLI: clean.diff -rqof a fresh run against both reference-output trees checked out atthe revisions
test/data.cmakenow pins —29fc194andd7aa0dc, refactor(html): share one style writer between the sheet and pdf views #829's:0 differences, both trees. Nothing this PR touches moves a byte.
scripts/format: no changes.bugprone-derived-method-shadowing-base-methodhits on untouched headers.Carried over from before the rebase, not re-run here: the JNI junit and
pyodrpytest suites,
javap -sagainst the jar, and the separate-Werrorsyntaxcheck of
ODRDocumentElement.mm. CI covers all four.test/src/internal/odf/odf_frame_test.cpp: everydraw:*shape is aframe naming its kind, a rect keeps the anchor and z-index a frame has, a
line states its two ends, a custom shape carries its outline.