I'll put several possible refactorings here, for the loading code path; they have all seemed attractive at some point
Summary of changes, feel free to modify stuff here:
- handle in set_atts
- Defs is messy - replace with SvgResources
- Remove interior mutability in Handle
- Make Fragment store an AllowedUrl, not a string (#622)
- Cleanup filter/image.rs to not store a reference to LoadOptions
- Resolve references / external resources in an extra tree traversal
- Once resolve_resources is in an extra trasversal, use for NodeUse and maybe others
handle in set_atts
The only places where the
handle attribute of
NodeTrait::set_atts is used are
filters/image.rs. This is so that they can call handle::load_image_to_surface(), and that function uses the
LoadOptions and the
base_uri which are stored in the handle.
At some point I wanted to move the base uri into
LoadOptions, and pass that struct around instead of the full handle.
(For the very first pass... can
set_atts take a
&Handle instead of a raw pointer? Will this force us to not store back-references to the handle in random places?)
Defs is messy
Defs stores two completely different things: the id -> Node mapping for a single handle ("one SVG file"), and a cache of URI -> handle for all the SVGs that were loaded from the original one.
I think we should separate these responsibilities. The id->Node mapping would go naturally in
I kind of want to leave
Svg as it is, with the
CssStyles, representing a single SVG file.
However, for the case when an SVG file causes other ones to be loaded, maybe there should be
SvgResources thing that basically maps URIs to either
Svg structs or to raster image data? Then, the
RsvgHandle in the public API would just hold an
SvgResources internally as the One True Storage of Everything?
(Maybe this can let us fix #313 once and for all?)
Add a step to resolve references
What if the initial loading stage is "just parse the XML". An
SvgResources gets built up. When something requires a new URI to be loaded, it gets parsed (in parallel?) and added to the
And at the end, there is a tree traversal, analogous to the current
.cascade(), but this one is called
.resolve_references(). This turns
Fragments into actual
Rc<Node> references. This is also where the code would detect reference cycles, probably. The point is that during the
resolve_references stage, there is no I/O going on, since everything has already been loaded into the
SvgResources by the loading stage.