Skip to content
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

feat(routers.RightAngle): support user defined vertices #2224

Merged
merged 39 commits into from
Oct 20, 2023

Conversation

MartinKanera
Copy link
Contributor

@MartinKanera MartinKanera commented Jun 19, 2023

Description

Until now the RightAngle router didn't use the vertices that were provided. This change allows users to define vertices through which the link is supposed to go. The RightAngle router itself works the same way as it did, the only difference is that it takes in account the provided vertices and finds route between them.

Motivation and Context

A simple upgrade of the algorithm that enables you to use the RightAngle router in more ways.

Screenshots (if appropriate):

image

@MartinKanera MartinKanera self-assigned this Jun 19, 2023
@frnora
Copy link

frnora commented Jul 11, 2023

Hi Martin,

Here is a quick summary of the issues I found while using the new version of the router. All the screenshots are taken with shapes.standard.Rectangle with a custom Link inheriting from shapes.standard.Link.

Issues:

  1. The target marker's angle is incorrect when the link is vertical and the target is a point. Seems like the tangent of the line segment is not calculated correctly or something like that. I assume this also affects the source marker but as my link has none, I did not test.
    marker

  2. The link loops when it is horizontal and the target is a point and never un-loops, even if the anchors are perfectly vertically aligned. One would expect a straight line then.
    loops

  3. Sometimes the route-change is not rounded but angled, despite the fact that I am using rounded connector. Happens around a vertex.
    angled

  4. The link is crossing the element. (you can once again see a not-so-round route change where the vertex is)
    crossing

  5. This, I assume, happens in the orthogonal and the original rightAngle one as well but I thought I would mention it. What is the reason for these not fancy overlaps? The right side is the very same link but the vertex is moved 10 points to the left. The routing looks much better.
    route

The code for point 4 (I assume it will be reproducible with it on your setup):

let rect = new shapes.standard.Rectangle();
rect.resize(220, 60);
rect.position(400, 400);
rect.addTo(graph);

let rect2 = new shapes.standard.Rectangle();
rect2.resize(220, 60);
rect2.position(20, 200);
rect2.addTo(graph);

let link = new shapes.standard.Link();
link.vertices([{
    x: 530,
    y: 320
}, {
    x: 130,
    y: 320
}]);
link.source(rect);
link.target(rect2);
link.addTo(graph);

Thanks for the attention!

@frnora
Copy link

frnora commented Jul 12, 2023

An additional note: view.getTangentAtRatio(0) always returns NaN for start.x and start.y so something seems to be off there indeed. It works fine for every other router I tried (manhattan, orthogonal, current rightAngle).

Edit: I forgot to mention - only in case of a Point. Connected to an element it seems to be ok.

@kumilingus
Copy link
Contributor

This usually happens when you define a path with two coincident points, e.g. M 0 0 L 20 20 L 20 20.

@frnora
Copy link

frnora commented Jul 12, 2023

But that's not the case. I am just moving the arrowhead around. As soon as I place it to point, the tangent calc breaks. It happens only with this version.

This is what I mean:

Screen.Recording.2023-07-12.at.12.29.13.mov

Notice how plus-sign tool gets placed at 0, 0 instead of at-the-source. That's because getTangentAtRatio bugs out. Doesn't happen with any other router.

Actually, it gets placed at the source in one case (around 6 secs into the video) but it is in wrong position. It should be from the other side of the "arrowhead".
Screen Shot 2023-07-12 at 12 33 18

E.g. this is the current rightAngle. Works as expected:
https://github.com/clientIO/joint/assets/129097753/9d56a9f2-df1e-46ad-a722-c5e25df720bd

@kumilingus
Copy link
Contributor

What is the d attribute of the [joint-selector="line"] link node when this happens?

@frnora
Copy link

frnora commented Jul 12, 2023

E.g. in case of this one:
Screen Shot 2023-07-12 at 13 35 11

the path is: M 300 390 L 300 390 C 300 390 300 393.3333333333333 300 400 L 300 450 C 300 456.66666666666663 296.66666666666663 459.99999999999994 290 460 L 160 460

Complete outer html:
<path fill="none" pointer-events="none" joint-selector="line" id="v-50" stroke="#666666" stroke-width="2" stroke-linejoin="round" d="M 300 390 L 300 390 C 300 390 300 393.3333333333333 300 400 L 300 450 C 300 456.66666666666663 296.66666666666663 459.99999999999994 290 460 L 160 460" marker-end="url(#v-2325765029)"></path>

Edit: could it be a rounding problem? I hit one issue before when I was defining shapes and having too many decimal spaces broke the connection point or something like that (cannot remember anymore). I had to round it. Maybe something similar?

@kumilingus
Copy link
Contributor

kumilingus commented Jul 12, 2023

Right, the first segment of the path is not differentiable (has no tangent defined) M 300 390 L 300 390. That is something @MartinKanera has to fix.

@frnora
Copy link

frnora commented Jul 12, 2023

When you are right, you are right! Thanks for looking into this!

@MartinKanera
Copy link
Contributor Author

Hey @frnora,
Sorry for taking so long to reply, I updated the rightAngle router, could you try it out and share any possible issues?

I really appreciate any help you can provide.
Martin

@frnora
Copy link

frnora commented Aug 2, 2023

Hi @MartinKanera,

Thanks a bunch for the update! I began using it an hour ago and it looks much better now, so far I haven't found any problem. I will keep using it and in case of any issues, I will drop a line here.

Btw, you bamboozled me real nice with hiding it behind an option, lol. Took me several minutes to figure out that I am really not using the old/current router (while having giant question mark above my head). I assume this was done to keep it consistent with the current version? Tbh, I cannot imagine who would not want to use vertices for this router... and if not, just don't use the tool.

@frnora
Copy link

frnora commented Aug 2, 2023

Update - 3 issues found so far:

  1. This is the above-mentioned issues with the connections not being rounded (there is a vertex at that point):
Screen Shot 2023-08-02 at 12 00 49
let rect = new shapes.standard.Rectangle();
rect.resize(220, 60);
rect.position(700, 700);
rect.addTo(graph);

let link = new shapes.standard.Link();
link.vertices([
    {x: 900, y: 630},
    {y: 630, x: 1040},
    {x: 1040, y: 670}
]);
link.source(rect);
link.target(rect);
link.addTo(graph);
  1. This one is not directly related to this new version, the previous version also demonstrates it:

When the source and anchor points are aligned in a way that the link's connection point to the element is the same, it creates a kind of an L-shaped link:
Screen Shot 2023-08-02 at 10 39 29

First of all, I think it looks a bit unusual, maybe it would be better to connect it to the second closest side in this case instead of the closest side to create a loop (similarly to orthogonal).

But the "real" issue is that when this happens something is not counted correctly (cos of the "same" anchor? I don't know). On my link I have another path that's connected with the link atConnectionRatio. When the above happens, the path is not displayed in the correct position because the matrix calculation breaks for it:
<path joint-selector="offsetConnector" id="v-108" d="M 0 0 0 -30" stroke="#666666" stroke-dasharray="4 4" stroke-width="2" transform="matrix(NaN,NaN,NaN,NaN,1060,825)"></path>
Screen Shot 2023-08-02 at 10 42 02
Moving the anchor a bit upper or lower, the path appears correctly:
Screen Shot 2023-08-02 at 10 44 04

EDIT:
3. Just found this one:
This path is wrong, tangent at source cannot be calculated, or so it seems. It also (once again) not keeping the connector rounded at the 2nd vertex:
M 900 1120 L 900 1120 C 900 1120 896.6666666666666 1120 890 1120 L 780 1120 C 773.3333333333333 1120 769.9999999999999 1123.3333333333333 770 1130 L 770 1145 C 769.9999999999999 1151.6666666666665 773.3333333333333 1155 780 1155 L 820 1155 C 826.6666666666665 1155 829.9999999999999 1158.3333333333333 830 1165 L 830 1180 C 829.9999999999999 1186.6666666666665 829.9999999999999 1190 830 1190 L 830 1190 C 829.9999999999999 1190 829.9999999999999 1190 830 1190 L 830 1190 C 829.9999999999999 1190 826.6666666666665 1190 820 1190 L 740 1190 C 733.3333333333333 1190 730 1193.3333333333333 730 1200 L 730 1215 C 730 1221.6666666666665 730 1225 730 1225 L 730 1225 C 730 1225 730 1228.3333333333333 730 1235 L 730 1260

source: {x: 900, y: 1120}
target: {x: 730, y: 1260}
vertices: [{x: 770, y: 1120}, {x: 830, y: 1190}, {x: 730, y: 1190}]
Screen Shot 2023-08-02 at 12 41 08
(no source plus-sign = no tangent, I assume; vertex no. 2 not rounded)

@MartinKanera
Copy link
Contributor Author

Thank you very much for your feedback. I will work on this as soon as possible!

Martin

@frnora
Copy link

frnora commented Aug 9, 2023

Great update, @MartinKanera !

It is almost there. Almost. :)

  1. For new links I have a wrong tangent calc for the marker when the end is over the source element. Quite possibly happening for any link that's dragged but the arrowhead most likely covers it (and I am too lazy to check, heh confirmed it happens with existing ones). I can reproduce it easily for links coming from left/right. Top/bottom is better but it still shows the problem when the X position of the dragged end is the same as the X of the not dragged end.
Screen Shot 2023-08-09 at 10 27 41
  1. I cannot reproduce the "angled" connector anymore so that's great but some direction changes are still kind of weird. This one has 3 vertices at the points it changes direction. Padding is 20 (I guess that's the default anyway?), all the rest is default. 2 of the radii are off, like they would be squashed.
Screen_Shot_2023-08-09_at_10_26_08
  1. Oh, I forgot - point 2 from me previous post is still valid, transform calc fails when entry and exit points are the same.

I think we can live with both of these things but I thought I would let you know. If you feel like looking at one of them, pick the 1st, it is more obvious. ;)

As always, if I find something else, I will report. Thank you for your hard work!

@MartinKanera MartinKanera marked this pull request as ready for review October 13, 2023 16:06
@kumilingus kumilingus closed this Oct 19, 2023
@kumilingus kumilingus reopened this Oct 19, 2023
@kumilingus kumilingus merged commit 7aca426 into clientIO:master Oct 20, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants