Modify path effects open closed paths

In addition simplepath.py (incorrectly) assumes that 'Z' followed by a coordinate pair should be read as 'Z L'. This could be a feature, but if it is, it is inconsistent with what Inkscape does (Inkscape correctly comes to the conclusion that the path data has a syntax error and discards the rest).

I'm getting the same bug too, paths become open after effect added. Using Ubuntu Hardy

This bug has bitten me - if you Add Nodes on a path, then run Visualize->Number Nodes, there is a division by zero error. The attached file is a circular path after adding nodes - running the Number Nodes extension causes a crash. (Version 0.47 stable)

I don't have a current bzr checkout, so this patch is against 0.47 stable (I'm betting that nothing's changed in cubicsuperpath.py since then). I've tested[0] on quite a few shapes and get the results I expect - though I must confess that I'm not 100% sure it's valid SVG. It could use some testing.

[0] only with the 'Add Nodes Extension' so far

patch tested with Inkscape 0.47 and 0.47+devel r9442 on OS X 10.5.8 with Python 2.5.1 and 2.6.2

1) 'Add Nodes…' now fails for closed paths (rectangle&star converted to path, closed path drawn with pen tool):

Traceback (most recent call last):
  File "addnodes.py", line 113, in <module>
    e.affect()
  File "/Volumes/blue/src/Inkscape/src/inkscape-repo/mp-x11/Build/share/inkscape/extensions/inkex.py", line 215, in affect
    self.effect()
  File "addnodes.py", line 109, in effect
    node.set('d',cubicsuperpath.formatPath(new))
  File "/Users/suv/.config/inkscape.extensions/LeWitt/cubicsuperpath.py", line 171, in formatPath
    return simplepath.formatPath(unCubicSuperPath(p))
  File "/Users/suv/.config/inkscape.extensions/LeWitt/cubicsuperpath.py", line 162, in unCubicSuperPath
    if (closed):
UnboundLocalError: local variable 'closed' referenced before assignment

2) 'Number Nodes…' still fails for paths created by converting an ellipse and adding nodes (e.g. with Number of segments: 2):

Traceback (most recent call last):
  File "dots.py", line 108, in <module>
    e.affect()
  File "/Volumes/blue/src/Inkscape/src/inkscape-repo/mp-x11/Build/share/inkscape/extensions/inkex.py", line 215, in affect
    self.effect()
  File "dots.py", line 73, in effect
    self.separateLastAndFirst(p)
  File "dots.py", line 48, in separateLastAndFirst
    x = dx/dist
ZeroDivisionError: float division

Ellipses converted to path appear to be a special case (i.e. differing from other shapes converted to path): they have both coinciding start and end nodes and a 'z' segment. 'Add Nodes…' inserts nodes to the 'z' segment too, in this special case producing 3 coinciding nodes (start, second to last, last node). Adding a 'Z' in 'cubicsuperpath.py' doesn't prevent 'Number Nodes…' to fail, because apparently it doesn't handle the doubled end nodes - with or without 'z'.

OK - new bzr patch attached. It's still not 100%, but should be OK for open paths now (fixes issue #1). I still need to dig into issue #2 a bit to see if I can reliably determine if there are extra nodes to be removed. The method I'm using now seems a bit hacky.

At any rate, this patch should cause paths to keep the 'closed' command (Z), whether they were made from shape or drawing tools.

OK - this seems to handle every shape now, and removes redundant nodes.

Removes "redundant" nodes? Could you elaborate on that?

related (exposed in part due to changes in r9076 and 9079?):
bug #542260 “Motion extension ignores 'z' segments”
bug #586597 “Fractalize extension removes segment”

In response to #8:

Draw an ellipse, convert to path (or draw a closed curve)
Extensions->Modify->add Nodes.

In r9448 the path is now open, and there are two extra (redundant) nodes added at the end of the path. The patch removes those and closes the path. Linear shapes don't get any extra nodes, but are also left open, the patch closes them (removing the last node, which is now redundant and closing with Z).

That sounds like a slightly dangerous fix (we used something similar inside the Inkscape core as well and it led to some subtle bugs). Still, thanks for having a go at fixing this. Is someone reviewing/committing this patch already?

~suv is helping (thanks!), but unfortunately my computer has died, so it will be some days before I can return to this.

And yes - it's pretty hairy. But there's no reason to break a closed path just because it was converted to cubicsuper and back to simple. I may need to try a different approach though (keep the extra nodes from being created in the first place).

Please note that I can't _review_ the changes - my skills are limited to testing new patches, reporting back any unexpected results (and occasionally I dig for links to other reports or commits).

OK - so I have my hands on some new hardware and a spare moment to try and address this. Here's my understanding of the issue:

Extensions send SVG data to cubicsuperpath, which converts the SVG into a simple list of points. Which means the 'Z' attribute is lost when csp returns the list to simplepath for formatting as SVG again.

I've attached a proposed fix. It makes CubicSuperPath a class (instead of a function) with two attributes: csp (the exact same list a returned by the original function) and closed (boolean). There is also new code that removes the one or two extra nodes at the end of the path depending on if it was closed with a line or a curve then replaces the 'Z'. These are only removed if there was a 'Z' command to begin with, so it will not delete points on a non-closed path that just happens to have start and end points at the exact same spot.

The good: we can reliably know whether a path is closed or not.
The bad: I'd have to tweak every extension that makes a call to CubicSuperPath(). This patch contains the changes needed to make the Add Nodes extension work properly with the changes. It's very likely to cause other extensions to fail. However, the changes to the other extensions should be fairly trivial.

There's also a small tweak to simplepath.py to correct the command spacing in the SVG output (looking at the output was driving me insane).

So I'd like to get a little feedback before going ahead and adjusting every extension using cubicsuperpath.

Very cool! I'll review the patch today if at all possible. This would be extremely cool to have in the next version of Inkscape.

Paths do remain closed, but: there are some issues with subdividing the closing segment (when subdividing by number it seems to generate one segment less on the closing segment), and it likely doesn't work with multiple subpaths.

My recommendation would be to flag each line segment as closing or non-closing. There is even a chance that this wouldn't actually break any extensions (at least not more than they already are).

(wipes egg off face) I did not test with enough parameter variations of Add Nodes. This new patch seems to handle every shape correctly (curves, straight, combo, compound and closed/open versions of all the above) with a range of parameter settings.

One thing - if you have a path with two points directly on top of each other, Add Nodes will happily add nodes 'between' them, resulting in a stack of nodes :) This seems to me to be the correct behavior, though I'm not sure it's particularly useful.

Sorry for taking a while to review your latest patch (btw, did not mean to throw any eggs, you're doing great work!). Your last patch is definitely better, but can you track closedness per subpath? As it is now, if you have two subpaths, one closed and one open they will both become closed (not a major problem, but it would be a shame to introduce a new, albeit less problematic, bug).

I cannot reproduce with 0.48.3.1 or trunk r11080.

And ~suv gave me steps I could reproduce...

Bug reproduced on Trisquel 7.0 LTS with 0.91 trunk Inkscape and Jitter Nodes. I have assigned it to myself in order to get into the Inkscape developers group.

I found the problem. The format of the cubic-super-path before I came was totally losing/inadequate (i.e. it had no capability of indicating the closed-ness of paths). I invented a new format where None signifies an open end; the extensions relying on cubicsuperpath.py will need to be changed in light of this. The new C-S-P Python file is attached.

And here is an example of an extension in Modify Path (Jitter nodes) adapted to the new format. I put "SVG units" instead of "px" due to the ongoing dispute over how Inkscape handles units…

Attaching diffs against trunk r13750 of cubicsuperpath.py (comment #22)
and radiusrand.{inx,py} (comment #23) for reviewing.

At last, the whole things! The new CSP format uses None to denote that the endpoints are open.

We apparently forgot to test it for 0.92.
Could someone take a look for 0.93 (some strings changes, so it can't be targeted to 0.92.1)?

I would like to open a bezier shape path I created using "Create Shapes From Vector Layer."

This was originally a closed path in the Illustrator file but I would like to open it in AE.

Is this possible? The "Closed" path indicator is checked and greyed out; I cannot seem to modify it either through the Layer menu or by right clicking on the shape path.

When I delete points from the path, the path always remains closed regardless of whether I directly select the points and delete, or use the pen-minus tool.

Hi - thanks for reporting this bug, I've manually migrated it to Inkscape's new
bug tracker on GitLab, and closed it here.

Please feel free to file new bugs about the issues you're seeing at
http://inkscape.org/report.

Moved to: https://gitlab.com/inkscape/inbox/-/issues/3093
Closed by: https://gitlab.com/mray