I would like to draw your attention on this 2geom commit: http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/10385 Should this be pushed to 2geom? I'm not too happy with the solution I chose for fixing part c) of this bug: https://bugs.launchpad.net/inkscape/+bug/640985
-Julien
On 30-06-11 02:51, Gellule Xg wrote:
I would like to draw your attention on this 2geom commit: http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/10385 Should this be pushed to 2geom? I'm not too happy with the solution I chose for fixing part c) of this bug: https://bugs.launchpad.net/inkscape/+bug/640985
How does a little instability cause a crash? Weird results, okay, but a crash? As far as I'm concerned your change is a bit dubious, but pragmatic and commonplace, but it should not solve a crash. So can you explain what causes the crash?
On 6/29/11 8:58 PM, Jasper van de Gronde wrote:
On 30-06-11 02:51, Gellule Xg wrote:
I would like to draw your attention on this 2geom commit: http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/10385 Should this be pushed to 2geom? I'm not too happy with the solution I chose for fixing part c) of this bug: https://bugs.launchpad.net/inkscape/+bug/640985
How does a little instability cause a crash? Weird results, okay, but a crash? As far as I'm concerned your change is a bit dubious, but pragmatic and commonplace, but it should not solve a crash. So can you explain what causes the crash?
All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2d-c2 _______________________________________________ Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel
Hi Jasper,
Connector-avoid is set on a circle. The circle is approximated by a set of points where two points are exactly at the same location (first one & last one). This seems to be because a circle is something like four 90deg arcs plus a straight line of length 0 to connect them. Now, when you move the circle around depending on the result of the transformation applied, the first and last points can be exactly at the same location or at slightly (epsilon) different locations. This causes the number of point making the convex for the circle to be variable of +/-1 point. The libavoid code does not accept this change.
Ah, ah. I should not have said crash, more the emission of an assertion by the libavoid code, reacting to this number of point change.
Hoping, I make sense...
Maybe making libavoid accept such a change would be a better solution.
-Julien
On 30/06/2011, at 5:25 PM, Gellule Xg wrote:
Connector-avoid is set on a circle. The circle is approximated by a set of points where two points are exactly at the same location (first one & last one). This seems to be because a circle is something like four 90deg arcs plus a straight line of length 0 to connect them. Now, when you move the circle around depending on the result of the transformation applied, the first and last points can be exactly at the same location or at slightly (epsilon) different locations. This causes the number of point making the convex for the circle to be variable of +/-1 point. The libavoid code does not accept this change.
Ah, ah. I should not have said crash, more the emission of an assertion by the libavoid code, reacting to this number of point change.
Hoping, I make sense...
Maybe making libavoid accept such a change would be a better solution.
The libavoid shape movement/resize code probably shouldn't have required the polygon passed to setNewPoly to have the same number of points. This was just an assumption when I wrote the original code. In the new interface, this method doesn't exist anymore and you can move (or resize) a polygon by passing the x and y distance moved, or just giving an updated (and possibly complete new) polygon. Hence this assertion problem will go away when I update the libavoid version in Inkscape (this was waiting on some code that was not available in the new version of libavoid, but now is, but still will require a few changes to the way Inkscape calls libavoid).
Gellule, if this is still not fixed with your other changes, and it's needed for the upcoming point release, then I could rewrite setNewPoly to accept a different number of points. It only has that assertion now because it reused the same memory and rewrites the points, rather than recreating the shape.
Cheers, Michael
On 6/30/11 8:04 PM, Michael Wybrow wrote:
On 30/06/2011, at 5:25 PM, Gellule Xg wrote:
Connector-avoid is set on a circle. The circle is approximated by a set of points where two points are exactly at the same location (first one& last one). This seems to be because a circle is something like four 90deg arcs plus a straight line of length 0 to connect them. Now, when you move the circle around depending on the result of the transformation applied, the first and last points can be exactly at the same location or at slightly (epsilon) different locations. This causes the number of point making the convex for the circle to be variable of +/-1 point. The libavoid code does not accept this change.
Ah, ah. I should not have said crash, more the emission of an assertion by the libavoid code, reacting to this number of point change.
Hoping, I make sense...
Maybe making libavoid accept such a change would be a better solution.
The libavoid shape movement/resize code probably shouldn't have required the polygon passed to setNewPoly to have the same number of points. This was just an assumption when I wrote the original code. In the new interface, this method doesn't exist anymore and you can move (or resize) a polygon by passing the x and y distance moved, or just giving an updated (and possibly complete new) polygon. Hence this assertion problem will go away when I update the libavoid version in Inkscape (this was waiting on some code that was not available in the new version of libavoid, but now is, but still will require a few changes to the way Inkscape calls libavoid).
Gellule, if this is still not fixed with your other changes, and it's needed for the upcoming point release, then I could rewrite setNewPoly to accept a different number of points. It only has that assertion now because it reused the same memory and rewrites the points, rather than recreating the shape.
Cheers, Michael
Most of the issues I was looking into are now addressed. Both the copy/paste, and the move issues are related to this setNewPoly limitation, therefore the best thing to do would be to try reverting my fixes as soon as your new version is in place. No pressure for 0.48.2. :) -Julien/Gellule
---- Gellule Xg <gellule.xg@...400...> wrote:
I would like to draw your attention on this 2geom commit: http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/10385 Should this be pushed to 2geom?
If you make changes to our 2geom copy, they will most probably be lost on the next 2geom update. So in general, yes you should push your changes to 2geom's repository too.
Specifically about your change; on quick inspection, the " == 0" seems wrong. In 2geom we use "are_near" to check whether floating point values are the same. I remember that the are_near methods use 2geom's default 'nearness' range of 1e-8.
Ciao, Johan
2011/6/30 Gellule Xg <gellule.xg@...400...>:
I would like to draw your attention on this 2geom commit: http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/10385 Should this be pushed to 2geom? I'm not too happy with the solution I chose for fixing part c) of this bug: https://bugs.launchpad.net/inkscape/+bug/640985
-Julien
This code should use the predicate Geom::are_near instead of a new arbitrary constant. The epsilon should be exposed in the interface, with a default of Geom::EPSILON
Regards, Krzysztof
participants (5)
-
unknown＠example.com -
Gellule Xg -
Jasper van de Gronde -
Krzysztof Kosiński -
Michael Wybrow