Fix duplicate clip-path attribute and restore piped stdin input - #52
Open
turbinefish wants to merge 2 commits into
Open
Fix duplicate clip-path attribute and restore piped stdin input#52turbinefish wants to merge 2 commits into
turbinefish wants to merge 2 commits into
Conversation
added 2 commits
August 28, 2026 21:27
startPathDraw() already writes the clip-path attribute when it opens the
element. endFormDraw() wrote it a second time on the same element, so any
record that goes through both (the pie/chord/arc family) produced
<path clip-path="url(#clip-N)" ... clip-path="url(#clip-N)" />
which is not well-formed XML (duplicate attribute). A conforming SVG
renderer rejects the whole document, so a single such record makes the
entire figure unrenderable -- not merely mis-drawn.
Dropping the duplicated call in endFormDraw() is enough: the attribute is
still emitted once, by startPathDraw().
Verified on 12 real-world EMF files taken from Office documents: before
the change 11/12 rendered and 1/12 was rejected outright; after it 12/12
render, and the 11 previously working outputs are byte-for-byte identical,
so the change only affects the records that were producing invalid markup.
The CLI sized its input buffer with seekg(end) + tellg(). That only works on a seekable stream: when the input is a pipe -- which is what "-i /dev/stdin" is in the common `producer | emf2svg-conv -i /dev/stdin` shape -- tellg() returns -1, the value wraps around when stored in a size_t, and `new char[size]` throws std::bad_alloc, so the process aborts before reading a single byte. Reading through std::istreambuf_iterator (the variant that was already sitting commented out right below) needs no seeking and handles regular files and pipes alike. 1.1.0 accepted piped input; the seek-based sizing introduced in the 1.8 series regressed it. Ownership of `contents` is unchanged (still new[]/delete[]), so the rest of the function is untouched.
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.
Two independent fixes found while converting EMF figures extracted from real Office documents. Both are reproducible on current master and on the 1.8.1 tag.
1.
endFormDraw()emits a secondclip-pathattributestartPathDraw()already writesclip-pathwhen it opens the element.endFormDraw()wrote it a second time on the same element, so any record that goes through both (the pie/chord/arc family) produced:That is not well-formed XML (duplicate attribute), so a conforming SVG renderer rejects the whole document. One such record makes the entire figure unrenderable, not merely mis-drawn:
Dropping the duplicated call in
endFormDraw()is enough; the attribute is still emitted once, bystartPathDraw().2.
emf2svg-convcan no longer read a pipeThe converter seeks to the end of the input to size its buffer before reading.
That works for a regular file but not for a pipe, where the reported size is 0,so the tool aborts (
std::bad_alloc) on:Reading the stream sequentially restores the pre-1.8 behaviour. Ownership of the buffer is unchanged, so the rest of
main()is untouched.Verification
Tested on 12 EMF files extracted from real-world Office documents (a Word design
document and a legacy PowerPoint deck), all of them dual EMF/EMF+ files:
cat f.emf | emf2svg-conv -i /dev/stdin -o /dev/stdout | rsvg-convert -b white.Happy to split this into two pull requests if you prefer them separate.