Repository navigation
SVG: even-odd fills, gradients and inheritance in groups (from #46) with follow-up fixes - #49
Merged
Merged
Conversation
emitPath() knew 'fill', 'stroke' and 'fillstroke' only, so a caller had no way to ask for the even-odd rule. It takes 'fill-evenodd' (f*) and 'fillstroke-evenodd' (B*) now. An unknown mode still emits f. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SvgRenderer never read fill-rule, so every fill used the nonzero rule. A hole drawn the same way round as its outline — a ring made of two circles in one path, a single-path letter O, nested squares — filled solid, and a five-point star polygon with fill-rule="evenodd" kept its centre filled. fill-rule is resolved like the other inherited properties: the element, its inline style, a style-sheet rule, then the nearest group. Even-odd paths fill with f* (B* with a stroke); even-odd polygons go through emitPath with 'fill-evenodd'. Nonzero output is unchanged byte for byte. Rect, circle and ellipse are single simple outlines, where the two rules agree. clip-rule has nothing to apply to yet: the renderer has no clipPath. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
walkElement() reset the gradient table to its default (empty) argument on every <g>, so fill="url(#…)" on a rect inside any group printed black. The table is now set once per document and kept for the whole walk. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every shape read fill, stroke, stroke-width, fill-opacity and stroke-opacity from its own attributes only. A group's fill="#f00" was ignored and its shapes printed black; a group's stroke was dropped. These properties now resolve like the text properties already do: the element's inline style, its attribute, then the nearest ancestor. opacity stays per element, as SVG defines it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Under a transformed group, children were drawn with the group's mapping only; walkElementXY() never read their own transform attribute. A <g transform="scale(2)"><g transform="translate(2,2)">, as BaconQrCode and other generators write it, lost its translate, and a shape's own transform inside a transformed group was dropped. Each child's transform is now composed with its group's, and stroke widths follow the combined scale. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fill="url(#…)" worked on a plain <rect> only; paths, polygons, circles, ellipses and transformed rects printed black. They are now filled with a shading pattern through Page::fillPathWithPattern(). The pattern's matrix maps gradient space onto the page: the shape's user space (with its group transforms), its bounding box for objectBoundingBox units, then gradientTransform. gradientUnits="userSpaceOnUse" is honoured, so BaconQrCode's gradient QR codes come out as they do in a browser, and they scan. Even-odd paths fill with f*. The plain-rect gradient code and its output are unchanged; the stop handling moves to a shared shadingFunction(). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
resolveUseReferences() copied every reference in full, and a copy can hold <use> elements that were already expanded, so nested references multiply. A small SVG could expand into millions of shapes and keep a renderer busy for minutes. Copies are now counted before they are made, and a <use> that would take a document past SvgRenderer::MAX_USE_COPIES (50,000 elements) is left out; smaller ones after it are still drawn. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
<use> references resolve in one pass, so a cycle cannot loop, but each resolved <use> copies its group along with the copies made so far: n references from a group to itself draw 2^n copies. The cap already bounds that; this test keeps it bounded. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
We hit this drawing complex data visualisations: dense line charts and maps whose paths carry hundreds of thousands of points. parsePathD() split the whole d attribute with one preg_match_all (an array per command), built a second array per command, and only then wrote the path, so a 5 MB path with a million points peaked at about 511 MB. Path commands are now read in place with strcspn() and yielded one at a time, and emitPath() / fillPathWithPattern() take any iterable, writing a generator as it is read. The same path peaks at about 11 MB, mostly the content stream itself; time is about the same. A gradient fill, which reads the path twice, still builds the list. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each circle and ellipse was a 36-sided polygon, filled and then stroked as two paths. Large ones showed their facets, and scatter plots paid for 36 points a dot: about 640 bytes of content each. They are now four cubic Béziers (control distance 4/3 (sqrt 2 - 1), within 0.03% of the true outline), filled and stroked as one path through emitPath(), with gradients and fill-rule as for any path. A 25,000-dot scatter plot takes 1.2 s instead of 2.5 s, peaks at 19 MB instead of 46 MB, and its PDF is 1.8 MB instead of 5.2 MB. A zero or negative radius now draws nothing, as SVG defines. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Inherited fill and stroke made every shape look up about six properties, and each lookup ran its own ancestor-or-self XPath query: 30,000 rects went from 0.086 s to 0.157 s. The ancestor chain is now kept for the element being drawn (keyed by the object every lookup receives) and dropped when the render ends; the same rects take 0.114 s. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
resolveUseReferences() counted the referenced element's descendants for every <use>, even when the copy was then left out, so thousands of <use> elements on a group above MAX_USE_COPIES took seconds. A target that does not fit is remembered and skipped at once: it only grows and the room left only shrinks. The loop stops once the cap is reached. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
renderRect() matched fill="url(#id)" with its own pattern, which took
neither quotes nor spaces, so fill="url('#g')" filled a path but printed
a plain <rect> black. It now asks gradientFor(), as the other shapes do.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…points An objectBoundingBox gradient on a path was stretched over a box that took in every control point, wider than the curve whenever a control point sits off it, so the colours landed elsewhere than in a browser. The box now takes the end points and, for each cubic segment, the points where x or y turns (the roots of the derivative). Quadratics and arcs are cubics by then, so they bound tightly too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tors fillGradient() registered the shading pattern before it knew whether the path had anything to fill, so an empty path with a userSpaceOnUse gradient left an unused /Pattern in the page's resources. emitPath() and fillPathWithPattern() now say that a generator is consumed once. Also: the ellipse docblock no longer speaks of a 36-sided polygon, and a stray blank line is gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Only plain rects, circles, ellipses and polygons applied opacity, fill-opacity and stroke-opacity: a shape under any transform, and every <path> and <line>, printed opaque, inherited fill-opacity included. Those renderers now draw inside the shape's opacity; an opaque shape writes no graphics state, as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nd-gradients # Conflicts: # CHANGELOG.md
…nd-gradients # Conflicts: # CHANGELOG.md
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.
This carries the eleven commits of #46 by @serpentblade unchanged, authorship kept, and adds six follow-up commits from the review. Thank you for this work — real-world SVGs (styled QR codes, dense charts) were exactly where the renderer fell short, and every fix came with a test and a clear explanation.
The follow-ups:
<use>cap without the quadratic cost. The cap stopped the explosion, but every<use>still counted its target's subtree, even when the copy was then refused: 20,000<use>of a 60,000-element group took ~3 s, and a few-megabyte file still minutes. Refused ids are now remembered and skipped at once (an element only grows and the remaining room only shrinks), and the loop stops when the cap is reached. The test file runs in 0.09 s.<rect>reads the gradient throughgradientFor(), sofill="url('#g')"with quotes or spaces works on a rect as on every other shape.objectBoundingBoxgradients were sized by the control points of curves, so they stretched wider than in a browser. Bounds now come from segment ends and the points where a cubic turns in x or y (quadratics and arcs are cubics by then).opacity,fill-opacityandstroke-opacitywere applied only to untransformed rects, circles and polygons; transformed shapes and every<path>and<line>printed opaque. An opaque shape still writes no transparency.emitPath()/fillPathWithPattern()document that a generator is consumed once; a stale "36 segments" comment.[Unreleased]covers all of it.Note for later: PDF/A-1 forbids transparency, and the library does not yet stop an SVG's opacity from reaching a PDF/A-1 document (this predates both PRs; the conformance set has no SVG). That deserves its own change.
Shading objects are still created per shape; reusing them across shapes with one gradient is left for a separate PR.
tests/Svg 146, ContentStreamTest 10, HtmlInlineSvgTest 9 pass; PHPStan clean on the touched files; veraPDF 1.30.0 passes all nine PDF/A documents of the conformance set.
Closes #46.
🤖 Generated with Claude Code