Repository navigation
SVG: inherit the stops and attributes of the gradient an href references - #2169
Conversation
andersonhc
left a comment
andersonhc
left a comment
There was a problem hiding this comment.
I noticed two things worth addressing before merging:
- Long reference chains get expensive because each gradient rebuilds the chain using list searches. A roughly 82 KB SVG with 2,000 linked gradients took 42 seconds locally, compared with 0.038 seconds on the base. Could we cache the resolved templates and use a set for cycle detection?
- _gradient_stops() falls back to the template when local stops exist but none parse successfully. Could we check whether local elements exist before deciding to inherit? That would prevent invalid local colors from unexpectedly picking up the template’s colors.
The chain of a gradient was rebuilt for every gradient, with list searches, so 2,000 linked gradients took seconds; each gradient is now resolved once and cached, with a set for cycles. A gradient with <stop> elements no longer takes the stops of its template when none of its own can be read.
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
|
Thanks! Each gradient's href chain is now resolved once and cached ( |
# Conflicts: # CHANGELOG.md
The cycle-cache case still reproduces on the latest head: with a -> b -> a, where only a has stops, defining a first causes b to be skipped. Reversing the definitions makes both gradients resolve. Could we avoid caching that incomplete result and add a regression test covering both definition orders? |
With a -> b -> a where only a has stops, resolving a first cached b before a's stops were known, so b was skipped; defined the other way round both resolved. The members of a cycle now each follow the cycle from themselves: going round it backwards twice gives each one the nearest attributes and stops after it, still in linear time.
|
Thanks, you're right: resolving a first cached b before a's stops were known. The members of a cycle now each follow the cycle from themselves (going round it backwards twice, still linear: a 2,000-gradient cycle takes 0.08 s here), so a -> b -> a resolves both gradients in either order. test_gradient_href_cycle_does_not_depend_on_order covers both orders; the a-first one failed before. |
Inkscape writes every gradient as two elements: one with the stops, and one with the coordinates that references it with
xlink:href:fpdf2 ignores the reference, so it logs "Linear gradient '#linearGradient2' has no valid stops, skipping" and the shapes filled with it are not painted. matplotlib's toolbar icon
mpl-data/images/hand.svgis such a file: its 5 linear and 3 radial gradients are skipped and the cross in it comes out hollow instead of filled in teal.As the specification says for
hrefon gradients, a gradient now takes the attributes it does not set (x1...r,fx,fr,gradientUnits,spreadMethod,gradientTransform) and, if it has no stops, the stops from the gradient it references, following a chain of references, also to a gradient defined later, and stopping at a cycle.Added
test_gradient_inherits_stops_with_href(Inkscape's form) andtest_gradient_inherits_attributes_with_href(a chain of radial gradients, the last one defined after the first); both fail on master.test_gradient_href_cycle_without_stops_is_skippedchecks that a cycle ends. The SVG tests, black, pylint and mypy pass