unwanted circular loops in Spiro Spline LPE [Patch #2]

Bug #415079 reported by Alvin Penner
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Inkscape
Fix Released
Undecided
Unassigned

Bug Description

The spiro spline LPE occasionally becomes unstable, in which case it may show multiple, nearly perfectly circular, loops in one segment. These are sometimes characterized by a segment in which both ends of the segment are rotated nearly exactly 180 degrees from where they were expected to be. Sample attached.

Revision history for this message
Alvin Penner (apenner) wrote :
Revision history for this message
Alvin Penner (apenner) wrote :

Attached is a proposed patch which attempts to deal with this. It consists of a clamp on the angular error at each vertex, forcing it to be in the range (-pi, pi). This type of clamp is normally applied to all calculated values of all angles. However, since the angular error at a vertex is the sum of a number of different contributions, it appears to be sometimes necessary to re-apply the same clamp, just to be sure.

The patch has been tested offline in a stand-alone calculation, to confirm that it works; but it has not been tested inside Inkscape. Also, there is still some possible residual bad behaviour, but I believe that is a separate issue.

Revision history for this message
bbyak (buliabyak) wrote :

Thanks - I tested this on a large collection of valid spiros and got no differences in rendering, whereas your sample was improved by the patch (still not what I would call natural shape, but at least not as wild as before). So I committed it. In any case, such patches that can potentially change rendering of Spiros must all be committed before the release - after we release the first stable version with Spiros, we will have to maintain its rendering of Spiros exactly the same forever so as to not break people's files.

Changed in inkscape:
status: New → Fix Released
Revision history for this message
Alvin Penner (apenner) wrote :

    thanks, glad to hear that it behaved okay. I had only worked with two examples so far, one 'bad' and one 'good'.
    there is still a residual problem in my 'bad' case. It shows a discontinuity in the slope at a location that is definitely not a node. I'll investigate this, but it will not get fixed before the release of 0.47.

Revision history for this message
Pablo Trabajos (pajarico) wrote :

> after we release the first stable version with Spiros, we will have
> to maintain its rendering of Spiros exactly the same forever so
> as to not break people's files.
even for those cases that are "bad" math? I see that this problem affects copy & paste too. I created a spiro path (see attachment) which gives problems by just moving it or making a copy, changing the look drastically. The example attached by Alvin is not affected by moving but it is by copying.

Under such circumstances, spiro becomes terribly broken and makes me wonder if it is a good idea to include it with these problems (I haven't tried Alvin's patch). If I remember correctly, even Raph Levien was aware of this instability but I don't recall him giving any particular solution. Does anybody has contacted Levien?

Revision history for this message
bbyak (buliabyak) wrote :

> even for those cases that are "bad" math?

Yes - what is "bad math" for you may be used by someone for a creative effect, and we must never change rendering of old files. The only way is to use versioned spiro effects, so the old files default to the old version of spiro which remains exactly the same as in 0.47, but for new files we can introduce a new version of spiro with a changed math.

Revision history for this message
Pablo Trabajos (pajarico) wrote :

> Yes - what is "bad math" for you may be used by someone for a creative effect
Don't misunderstand me, bulya :) This is a valid point. Artist like to exploit tools in ways that the developers never intended or imagined. They also expect to have backwards compatibility.

Rather, my point is that this situation might be a collateral effect of what to my eyes looks like a somewhat broken code released into the public too early. What if Mr. Levien finally corrects his algorithm and this changes the rendering of many other examples? According to his thesis page he will be done by the end of this year.

You suggest the policy of versioning LPEs, is this already implemented? How does it work? What happens when you open an "old" LPE version on a newer release? Gets converted to a path? Translated into the new version transparently?

I'm not a programmer so I can't give a consistent opinion. I'm concerned in a broader sense about the implications that having different version might bring.

Regards.

Revision history for this message
bbyak (buliabyak) wrote :

> released into the public too early. What if Mr. Levien finally corrects his algorithm and this changes the rendering of many other examples? According to his thesis page he will be done by the end of this year.

Very good - if he does we will do a new LPE version with his changes then.

New version is simply a new LPE, still called "Spiro" in the UI but using a different id, for example "spiro2", in the SVG. The old spiro gets hidden so it's not in the list and cannot be added, but otherwise remains fully functional for those who have old files with these effects.

Revision history for this message
Alvin Penner (apenner) wrote :

    re-opening this report in order to propose a second clamp, this time on the variable ks[0]. In his thesis, Raph Levien has shown, in Equation 8.24, that ks[0] is the sum of two independent angles. Since these angles will be in the range (-pi, pi), therefore the sum must be in the range (-2*pi, 2*pi).
    the attached patch imposes this constraint. I have tested this patch 'offline' on the file bad5.svg and it appears to work well there. I also tested it on the file spiro.svg above. In this case the results were somewhat inconclusive. The calculation initially did not converge, and I had to increase the number of iterations to get it to converge. When it finally converged, it produced a reasonable result. However, I would not recommend increasing the CPU time just to solve this problem. A simpler solution would be to add an extra node, because the source of the problem is the collinearity of three of the nodes.
    I believe this is the end of the clamping proposals, because now both the independent and the dependent variables are clamped.

Changed in inkscape:
status: Fix Released → New
summary: - unwanted circular loops in Spiro Spline LPE [Patch]
+ unwanted circular loops in Spiro Spline LPE [Patch #2]
Revision history for this message
Pablo Trabajos (pajarico) wrote :

Byak: That answers my question, thanks. Although I wonder what would happen when the UI for a LPE changes drastically between versions (not the case for spiro). This would mean that it'll have to store two different UIs and show them according to LPE's version, right?
Penner: I understand that my example spiro.svg might be an unrealistic case. I'm not as concerned for the result as I'm for the fact that it is impossible to move or copy reliably. Is this related to this bug or should I file another?

Revision history for this message
Alvin Penner (apenner) wrote :

I suspect it is the same bug, won't know for sure until I've had a chance to experiment with it in Inkscape.

Revision history for this message
Alvin Penner (apenner) wrote :

closing this to give more visibility to Bug 417179.
sorry about the confusion, I just didn't want the second patch to get lost in the shuffle.

Changed in inkscape:
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.