-
-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug: Line does not produce correct path string with .d() #130
Comments
Svgpathtools is uses implied move values. Svgelements does not. The value for the line there isn't in error since it's fundamentally a path fragment. Paths start with Move. If they don't then they don't have a real meaning. >>> c = (Move(complex(0,9)) + Line(end=Point(5,4)))
>>> c.validate_connections()
>>> Path(Move(end=Point(0,9)), Line(start=Point(0,9), end=Point(5,4)))
>>> c
Path(Move(end=Point(0,9)), Line(start=Point(0,9), end=Point(5,4))) Would also work since you'd stick those fragments together and fix any connection problems they have. Though for a lot of fragments you can simply stick them together with Since paths are required to begin with moves, path fragments are a little bit strange. But, it's actually a design choice to not claim there's a Move() in the path when there is not a move in the path.
|
You do sort of need a start position to do things but you strictly speaking don't have the start position. But, you can still do more things with that segment than pretend its a full and correct path
In svgpathtools: Path("M0,0ZM1,1Z") isn't a coherent path while it is certainly permitted by the SVG spec (1.1, 2.0). Svgpathtools has no Move() segment or Close() segment so complex paths with closed and unclosed portions basically have no meaning. It's intended to be much more specific and accurate. So yeah, it doesn't have a Move there because it doesn't have a move there. You can use the |
OK I understad. Sorry for posting all these issues, it seems like I'm trying to get something done with as little knowledge about svg as I can :P Can you clarify how I could convert from svgelements.Path to svgpathtools.Path? |
It's absolutely fine. Getting things done without reading the spec multiple times (I have) should be considered a virtue. svgpathtools.Path(svgelements_path.d()) seems like it would work. rather effectively. While there are slight differences between them svgpathtools parses most valid svg paths correctly. And if you wanted to convert between them using the path_d info is going to be the easiest method. If you somehow have some compound paths in svgelements you would want to do: |
@Esser50K you may actually just be correct here and given #137 this is under review for a breaking change to change the path into inventing l = Line(start=complex(9,0), end=(5,4))
l.d() -> "L 5,4" Would still work in that manner, but: l = Path(Line(start=complex(9,0), end=(5,4)))
l.d() -> "M9,0L 5,4" Would happen if we put that line into a Path(). The single Line pathsegment would still give it's solo value, but as a Path would actually make the |
I am unsure here. It isn't helped by confusion between I think that the issue here is that PathSegments should NOT have start points. They only have start points as a programming "convenience" (though personally I am not sure it is really a convenience given the amount of code needed to confirm that the start point of one PathSegment is the same as the end point of the previous one). |
Consider the following piece of (valid) Python code:
From a python perspective, I have quite reasonably used the same
|
Hm. While that's certainly true, it's not exactly a fixable bug, rather it's a side effect of object manipulation and that sort of thing will always happen in most any program when you do things like that. |
Only fixable by taking a copy of objects before adding them to the path. And not sure whether that is worthwhile. I was only using this to illustrate that SVG PathSegments should not really have a Start point - but should take their start point every time from the previous segment. That does make coding more difficult, and it wouldn't be backwards compatible. |
There are clearly some instances of this stuff existing in the wild where the path goes from endpoint of previous link in the chain to the current one and falls into a proper path. The bigger issue is that the individual elements are meaningless without being in a path. In reality this decision was already made circa I'd maybe consider it as part of But it's far too late and too breaky to ever consider those changes here. |
This wouldn't fix the start and end point thing though. You could insert the same segment in twice and it would still have that side effect thing if you did an affine transform. It's not as much a bug as a consequence of reused memory in a language. |
I disagree. It is NOT reused memory - it is a deliberate consequence of objects. It is not reuse of memory - it is a link to the exact same object. And the fix is to copy the object when you add it to a path so that the two paths point to different objects rather than the same object.
Too late and breaky to remove the use of So it is a bug, though as I said before unless someone complains about it in the wild probably not one worth fixing.
So we probably ought to fix it. |
Making pointless copies of things aren't needed. If they wanted to avoid that they should have added in a fresh copy or something. It's certainly a do not fix. This code is intended for coders not general end users. If you do weird things you'll get weird results. There would be noteworthy side effects changing that since the particular line segments would no longer be the same line segment. If any change is made it should be to the readme to say don't reuse the same pathsegment in a path more than once, and certainly not preventing that from happening. |
I did say it was probably not worth fixing until you pointed out that these were problems cropping up in the wild. Copying is unlikely to lead to more memory usage as in most cases the original object will have zero references after a copy is added to the path and be garbage collected. But yes - one consequence would be |
|
This is a judgement call and I can see doing other things. It would mean pathsegments would have little functionality outside of paths, which might be preferable, but you couldn't do things like know the length of a line, since the length of the line is not just its But, the OP may be correct if we take a less fundamental shift that is more exclusive to |
Also, we can probably get around the whole
If we do this we can maintain backwards compatibility in use cases that do not currently break anyway. We could also provide an alternative Not sure whether this is worth the effort though. |
Original point was settled. |
The
Line
produces the wrong path string.The svgpathtools lib would produce ->
M 9,0 L 5,4
I think that is probably correct since you need the start position to do anything
The text was updated successfully, but these errors were encountered: