Make "point intersection curves" tools define lengths

Hi,

I wanted this for the same reason as in this post: Segment curve from intersection of two curves . Using the tool “point intersection curves” does define the point but you can’t use it in any formulas. You can work around this by using the “point intersect curve and axis” tool on the created point to split the other curve but it’s tedious and counter-intuitive. And you have to do it twice to split both curves.

Looking at the code I see that in vtoolcurveintersectaxis.cpp there is the function InitSegments() which appears to do the magic and vtoolpointofintersectioncurves.cpp has no such magic. But the code I think would be nearly the same in both cases I think.

Is this a wanted feature? Is the above the correct idea for implementing this?

Thanks in advance.

2 Likes

@Douglas or @slspencer could probably answer better than I can, but I do know that you are welcome to fix the issue in your own personal git fork, & push the fix for acceptance into the main branch. You definitely have at least a little more understanding of the dev side of things than I do!

1 Like

Well, so I tried what I described and it, umm, appeared to work first time. Which I didn’t expect at all.

I don’t quite understand how it all works and the following code makes me a bit nervous:

arc1.setId(p->id() + 1);
arc2.setId(p->id() + 2);

At least during testing p->id() appears to always be zero, I don’t immediately see why though. I’ll look at it again this morning, I don’t do a lot of C++ coding.

One thing I’m not sure about: should I make an issue on github to assign the pull request to, or do I refer to this thread ?

2 Likes

As I understand it, you’re supposed to make an issue on github which refers to this (& any other pertinent) thread as proof that the issue has been discussed.

1 Like

Why would you think that?

With vtoolcurveintersectaxis you end up with 1 curve with 2 segments.

With vtoolpointofintersectioncurves there are 2 curves, each with 2 segments. Are you creating both sets of spline segments?

The vtoolpointofintersectioncurves is producing spline (curve) segments, not arc segments. Semantics.

That should be a clue right there… IF p->id() is returning zero, that means there is NO point Id, No point Id means NO name. No name - no way to build 2 (4) segment names.

I think the idea is good, but probably needs some more discussion here. Like @Pneumarian said you could make changes on your fork and I could take a look, but there’s probably more to it than you think.

2 Likes

Hi @rode_kater! Super, you’ve already started work on this new tool!

If you haven’t already, please create a new issue for it on github and we’ll move code discussion over to the github issue comments.

2 Likes

Github issue: New Feature: Make "point intersection curves" tools define lengths · Issue #317 · FashionFreedom/Seamly2D · GitHub

I’ve also pushed a patch you can look at.

3 Likes

Please test this build here:

https://github.com/FashionFreedom/Seamly2D/releases/tag/test-issue-%23317

2 Likes

I just tested it for a particular edge case where the two curves share an endpoint. A3 is the intersection point of Spl_A2_A1 and Spl_A2_A.

Should we improve the definition of ‘intersection’ to exclude the endpoints if a non-endpoint intersection is found?

1 Like

Another edge case where there are multiple points of intersection.

Should we ask the user to choose ‘1st intersection point’, ‘2nd intersection point’, ‘3rd intersection point’,… ‘nth intersection point’?

image

1 Like

Yay! Looks good with my experiments!

I think that would be ideal, yes.

As opposed to the current “Vertical correction”, “Horizontal correction” Which could conceivably make the created point switch which intersection it applies to under certain, perhaps edge as well, cases? Yes please!

ETA: Actually, we’d have to figure out what would happen if one of the intersections were to wander off.

1 Like

Yeah there’s no way to select the other intersection point:

2 Likes

Like this…?

arc1 arc2

In this case by moving the arcs away the intersection point A2 is no longer an intersection point and the origin is used as the place holder so the pattern doesn’t crash. Since the arcs no longer intersect there are no arc segment lengths in the variables table->curve lengths.

Edit:

One thing I did discover is that the arc segments will become ZERO as the intersection point is removed… and if you were using the curve lengths in a formula as a divisor you’ll get an error.

2 Likes

Just checking - I assume this build was BEFORE you fixed the unicode issue or from the user’s fork?

2 Likes

That too. Actually, I was thinking more of when there would be multiple crossings, & it’s the furthest intersection which is pointed, but then it ends up getting straightened out to fewer intersections. Does it yoink to zero-point, or does it still take the furthest intersection even though it’s num1 instead of num3?

1 Like

The default is to yoink :slight_smile: to the first intersection even if its an endpoint. Which is actually what it should do. The user should then be able to go into options and select from a list of the 1st, 2nd, …nth intersection points. :slight_smile: Currently this tool’s Options allow selection of Highest, Lowest , Rightmost, and Leftmost intersection which leaves out those pesky middle points.

2 Likes

wait I was looking at Intersect Curves tool. So theres a separate issue with the Intersecting Arc tool when an arc changes and there’s no longer an intersection point? Not sure there’s a fix for this one. It may be the best option to use the origin point. Other tools default to work with the origin point when there’s no longer an intersection or proper result .

2 Likes

Just a reminder… once you start changing the option of which point the tool uses outside those names which are currently provided in the tool - you might get into the issue of fundamentally changing the tool. Which could require more extensive work on the schema, reading / writing the pattern DomDocument, maintaining compatibility with existing patterns that use that tool and/or conversion, tool dialog changes, updates to the property browser (dock), and possible geometry changes.

Currently I believe this tool uses 2 of the possible “point” schema element tags “vCrossPoint” and “hCrossPoint”… just changing those and you could be looking at a can of worms. Trust me, I know what it was like just changing “passmark” to “notch” in the schema. It’s why I’ve come to understand Roman’s skepticism anytime someone with a little bit of C / C++ experience thinks they can just add a new or change a tool.

Food for thought.

2 Likes

Didn’t look at that… was using the point of intersection of curves tool using arcs vs using simple or bezier curves.

But yes… in the example I posted simply moving the arcs away to a point where they didn’t intersect, the point is replaced with the origin… and in one case the arc segments disappeared in the variables table… other times they were there, but the length value was zero.

Need to investigate more.

Edit: Since the point intersection of curves works with arcs as well there can only be 2 points of intersection… there can’t be an n’th point past 2.

1 Like

This is why I said this…

In testing this some more I found another issue with it… There is a discrepancy between the Table of variables Dialog and the Edit Formula dialog. Oops.

.

There’s an even greater discrepancy the more intersect points you create on a given curve…

2 Likes