SVG Groups are ignored. Especially transformations acting on the paths inside the group.

[alphanoob1337]

Example in an SVG created in Inkscape:

<g
id="g3450-3"
transform="matrix(-1e-4,-1,1,-1e-4,670.77002,132.92999)">
<path
inkscape:connector-curvature="0"
d="m 28.35,0 c 0,15.65621 -12.69107,28.3485 -28.34728,28.35"
style="fill:none;stroke:#000000;stroke-width:0.72000003;stroke-linecap:round;stroke-linejoin:round;stroke-miterlimit:10;stroke-opacity:1;stroke-dasharray:none"
id="path3452-8" />

The group transformation is ignored when calling svgpathtools.svg2paths().

[mathandy]

I'd like to add this functionality. I probably won't get to this in the near future though.

Here's how to do this (in case some helpful awesome person wants to make a pull-request):
You can quite easily take an affine transformation of lines, quadratics, and cubics by simply transforming the coefficients. E.g.,

p = parse_path("m 28.35,0 c 0,15.65621 -12.69107,28.3485 -28.34728,28.35")
A = np.array([[-1e-4, -1], [1, -1e-4]])
b = np.array([670.77002, 132.92999])

# these convert between real 2-vectors and complex numbers
xy = lambda z: np.array([z.real, z.imag])
z = lambda xy: xy[0] + 1j*xy[1]


transformed_path = Path(*[bpoints2bezier([z(np.dot(xy(pt),A) + b)
							for pt in seg.bpoints()]) 
								for seg in p])

This should work (I didn't test it, but just wanted to give the idea) for everything but paths containing Arc objects. Arc objects shouldn't be too hard to deal with either (see Arc.rotated() and Arc.translated()).
... it might be nice to just add a Path.transformed() method to deal with affine transformations in general.

[alphanoob1337]

I took a look at the code by now. Yes. The transformation should be fairly easy to implement and I have started working on it. On Bézier curves, can I also just do a matrix transformation of the start, end and the two control nodes?

The issue is more that currently there is only a list of d strings created and then it is parsed. This way all information about the XML tree is lost. Because groups can be nested, the file has to be recursively parsed and transformed. If I am happy with my solution to the problem and it doesn't affect performance too much, I can do a pull request myself, if you want to have a look at it. It is still work in progress, though.

[mathandy]

On Bézier curves, can I also just do a matrix transformation of the start, end and the two control nodes?

That's exactly what the above code is doing.

The issue is more that currently there is only a list of d strings created and then it is parsed. This way all information about the XML tree is lost.

Keeping track of the group structure could be accomplished to some extent by simply adding an attribute to Path to keep track of any relevant information. I'm not sure what the desired behavior would be though when a Path is split into two, copied, tested for equality, etc.
Why retain the group structure? If all the path objects are transformed, then is it necessary to retain the groupings?

[alphanoob1337]

OK. I included the code now with a few changes to keep it closer to this documentation https://developer.mozilla.org/de/docs/Web/SVG/Attribute/transform
Now I didn't want to retain the group structure. But to apply the transformations in the right order and to the right set of graphs you need to walk the tree instead of using minidom to get all the tags with a certain tag name.

My changes are ready for a pull request now. However I was not able to create a new branch. I get a Permission to mathandy/svgpathtools.git denied to alphanoob1337.

[mathandy]

@alphanoob1337 to make a pull request when you don't have commit permission you first need to create a fork (on your own repository), commit to that, then click the "create pull request" button on this (the svgpathtools) repository and you should see your own fork as a "compare" option.
changing-the-branch-range-and-destination-repository

[alphanoob1337]

So let's give this a fresh start. I think we both agree by now, that it is not that simple. If you, @mathandy want to implement it in another way, that is totally fine. I just think it doesn't make sense for me to do it, if you already have very specific ideas about how you want it to be done. It will be more effort for you to tell me how you would like it to be implemented.

Although I am by no means a professional (I just code out of necessity for hobby and professional projects), I do care about coding style. Not about PEP8, but about thinking before implementing. So please let me explain my thoughts about the solution to this issue:

I was assuming that SVGs are typically composed the following way:

1. There is usually another group and/or a path in a group. So empty groups are rare in general.
2. Usually there is more than one path object in a group.

The file I had, had 200 paths in 86 groups nested up to four levels deep.

So for the sake of this argument, let's assume it has 200 paths in 62 groups in three layers. Then, in the first layer it would be 50 groups with 4 paths in each them, the second layer would be 10 groups of 5 groups of the first layer and the third layer would be 2 groups of 5 groups of the second layer.

I see two main ways of implementing the group transformations. Let me call them 'top-down approach' and 'bottom-up approach'. The top-down approach is what I preferred and implemented. It is recursively walking through the XML tree of the SVG and parsing the paths and groups along the way. I call this top-down because one goes from the root to the branch. In a hierarchy this is called 'top-down'. The bottom-up approach would be to walk through the XML tree once and identify all the path objects along the way. Then one loops over the parent nodes for each path to parse and apply the group transformations. This is parsing the groups starting from the leaves to the root. Hence 'bottom-up'.

As always, both approaches have their benefits: The top-down works best if the structure is deeply nested and if there are multiple paths in one group. However, empty groups are bad for performance. The bottom-up approach doesn't parse empty groups. Also it works well if there is one group per path. Therefore this approach is the better choice for shallow structures. One can also implement hybrid approaches, where the groups are parsed top-down and the paths independently of that. Then the transformation matrices are applied afterwards.

Apart from the thoughts about the best way to do it, there are also practical considerations, that play a role during the choice. The ones I see as most prominent are the following: In the current implementation the structure of the SVG file does not play a role so far. So it was perfectly valid, to extract the paths and work on them after they have been extracted from the XML tree. Since there were library functions available for the task, which can be assumed to be somewhat optimized, I agree that this was the best choice at that time. So to implement the top-down approach, one has to change existing code unnecessarily and the order in which the objects are returned by the function changes. The bottom-up had the clear disadvantage for me, that it was not the best approach for my example described above: in total, 600 group transformations would have had to be parsed and applied. In the top-down it is only 62 groups have to be parsed. Of course this is highly non-linear with the complexity of the file. In addition it allowed for all the other objects that are parsed into a path object, to be dealt with in the same walk through the tree. In principle there is only a small performance penalty for a hybrid approach. But there is a huge practical drawback: I was not able to come up with a good way to identify the individual path objects during the walk through the tree: paths usually have ID attributes, but you can not rely on this. The order in which minidom returns the objects in getElementsByTagName is not documented and therefore also unreliable (in the current version the elements are returned in order of appearance in the file). Of course one could say that it is unlikely that the order in which getElementsByTagName returns the nodes changes and rely on the current implementation. But still it would make the code more complex to read if two different approaches are mixed together.

Since for the svgpathtools the order in which the paths and converted objects are returned was not documented either and also was subject to change with minidom anyway, I decided this is undesirable but not a huge issue. The change of existing code was also undesirable to me, because it requires additional work, but in my opinion it was worth the hassle because it is the more elegant choice and it avoids walking the XML tree unnecessarily often and allows to directly cross-parse other nodes into paths in the same walk, which reduces the number of walks even compared to the current implementation without any groups. Also other complex features, like parsing the other group attributes and overwriting parts of the path attributes where needed, can be implemented in the same XML tree walk, if that is desired in the future.

Of course this is a question of style and therefore it comes down to personal opinions and preferences. Since you are maintaining the project it should be your choice. However it was not really clear to me from your comments, which approach you prefer. Is it the bottom-up or the hybrid one? I went ahead with the top-down approach, because it appeared to be the most logical one to me and my assumptions about SVG files. Also it complied best to my personal preferences, as I value simplicity, scalability and performance over keeping well-tested code. But as I said: it is fully up to you to decide which changes are unnecessary and which are not. I am just explaining here why I decided to implement it the way I did in my fork.

[mathandy]

Thanks for the explanation of your thinking @alphanoob1337.
There are many improvements I'd like to make to the svg2paths function and I'm not opposed to the top-down approach you describe. That said any drastic changes we make to well-tested code should come with thorough unittests.
You could have implemented this in a way that would have very little impact on current functionality, but you didn't. So how do I know the code still works for all cases that it worked for before? Am I just supposed to trust you? I could certainly add thorough unittests myself, I'd have to add unittests for the old and the new functionality since you replaced the old code, but I could do that. It would take a lot of my time though.

Does that make sense? If so, what would you suggest my next step be?

P.S. The recursive structure in your code is aesthetically an excellent choice, but I'm worried about its performance and scalability compared to other options. What do you think?

[mathandy]

@alphanoob1337 would you mind me using some of the code from your suggested fix to add support for groups? I opened a branch for this issue. If you make a new pull-request to the issue16 branch, I'll merge it and use your code as a base for the new functionality.

[alphanoob1337]

Sorry for the slow response, but I am fairly busy at the moment. Yes, please use as much of my code, as you want to. I have a five hour train ride today, where I plan to clean up the PEP8 compatibility and create a pull request.

Yes. Python sometimes behaves funny in terms of performance. Appending the paths to a list in the loop as is my main concern at the moment since copying the list of paths around is very costly in terms of performance. However I was not really able to figure out a faster way to do this, but I am confident, that it still is fast enough for most every-day situations: I checked the code against a large SVG file from Wikimedia Commons: Geological map of East-Westfalia created by the author Hagar66 and it took my laptop 6.31 to 8.60 seconds to run the svg2paths command on it, although the file has 22,920 paths and also some groups with transformations in it.

According to my experience often it is more efficient, to parse a file of unknown size twice: once to count the number of objects in it and then allocate the memory needed. Then, in a second run, the data is actually parsed and written to the allocated memory. This works well i.e. with large numerical data files. In this case however, the space needed in memory to store a path object is not clear a priori. So the pre-parsing and serialization will probably create more overhead than it speeds up the code.

So I think there is different ways of continuing from here for the group implementation:

1. The quick solution would be to do a minimal implementation that only applies transformations to the d string. This should be well documented so that it will not cause confusion why other group features are not working.
2. The full package requires a lot of work and will open a Pandora's box. Implementing full attribute parsing and overwriting path attributes by group attributes where needed would be the desired behavior in the long run, since it allows the user of the library to further process attributes like line width or color. However this easily becomes a very complex task, since some attributes might be overwritten only in part. And there are complex group features like clipping of paths.

So those are my thoughts on this. What do you think about how to proceed?

[mathandy]

@alphanoob1337 as soon as I've fixed the bug I'm currently working on (hopefully today, but I don't know -- I've got a lot on my plate), I'm going to play around with an implementation of your (1) -- I'll try a bottom-up approach to make it as self-contained as possible.

As far as (2) goes... I'm not sure how inheritance works in SVGs.
In general it looks like child attributes override parent attributes, but then there are examples where this is not true, e.g.

• when a child attribute is set to 'inherit' (see relavant stackoverflow post)
• transforms (the transformations are composed)

If these are the only two exceptions to the children-override-parents rule, then implementing (2) doesn't sound so bad... though that's a pretty big if.

To anyone reading: Do you know of any documentation where inheritance in SVGs is well-explained?

@alphanoob1337 like you say, it seems like (2) is opening pandora's box a bit, but it might be worthwhile to add an experimental implementation that comes with a warning in the documentation. On a somewhat related note, if your original top-down approach offers speed benefits, it might be worthwhile to offer it also as another "experimental feature".

[mathandy]

Just another quick note:
Since we're talking about flattening attributes (so our xml tree becomes a list of paths), it will likely be necessary to check that duplicate attributes are handled properly.
E.g.
<circle cx="25" cy="25" r="15" transform="translate(0,-10)" transform="translate(0,10)"/>
In a quick test, Chrome/JSFiddle ignores the second transform (the same happens with other attributes I tried).

[alphanoob1337]

Hi @mathandy,

I was mostly concerned with the style attribute, because it requires full CSS parsing to resolve the inheritances. A group can have style properties, that the path overwrites but the others also apply to the path. Also there is style classes which can be defined for groups and path elements, that are also inherited according to rules. And last but not least, there is inline stylesheets in SVG that can address specific elements (groups or paths) by their respective ID and change their style.

For SVG 2 it is required for the user agent to parse the CSS. In the SVG 1.1 documentation I found the hint that the CSS cascade must be obeyed if CSS is parsed (this was optional for SVG 1.1). The inheritance rules for CSS are given in chapter 6 of the the CSS documentation. Those can get really complicated and this fact alone was reason for me not to deal with inheritance at all. But maybe there is some Python code for CSS parsing around, you can just import as a dependency...

And another thought: I would intuitively say that clip-paths are inherited similar to transformations so that all of them apply to the path in the respective order...

Cheers
alphanoob1337

[sbliven]

Support for transformations will be very useful. Inkscape uses transformation matrices on many objects by default, resulting in completely misplaced paths. I hope the code-style issues get resolved quickly.