Routingpoints missing in the ChangeRoutingPointsOperation

[rsoika]

I am hunting a bug for some days now, regarding RoutingPoint information tranferred from the client to the server.

Specifically, I miss the anchor points of the source and target node. Apparently, only the routing points between the start and end point are transmitted. I use the ManhattanRouter. While debugging this router I noticed that all routing points are calculated correctly in sprotty.

Now I came across this GLSP client code (@tortmayr pointed me to this detail some weeks ago #730)

The strange thing is that there is a method call calcElementAndRoutingPoints line 239. And in this line there is the following comment:

filter duplicate points (same x,y coordinates) and only keep actual routing points (no start or target, no volatile points)

So apparently exactly here is done what has surprised me so much for days. Is it true that you remove the start and end point from the routing list?
On the other hand, this seems to be completely new code that I don't even have in version 1.0.0 (????)

I would like to ask what the desired behavior from your point of view is. Do you really want to remove the start and endpoints from the routing information? As I said, I am extremely confused because I have been analyzing exactly this behaviour for days that seems to be changing right now...

Short message: The start and end point is extremely important to me.

[tortmayr]

HI @rsoika

So apparently exactly here is done what has surprised me so much for days. Is it true that you remove the start and end point from the routing list?

Yes that is true and is also intended. Sprotty (and as a consequence also GLSP) makes a clear distinction between routing points and anchor points (start & end points). Anchor computation is completely separated from the routing computation and the "routingPoints". Sprotty expects that the "routingPoints" property only contains the actual routing points an no anchors.

Do you really want to remove the start and endpoints from the routing information? As I said, I am extremely confused because I have been analyzing exactly this behaviour for days that seems to be changing right now...

Due to a bug we previously also sent the anchorpoints to the server. However, this resulted in duplicated routing points on the client (Start and endpoint are already part of the routing points but are are also recomputed and added during the client side routing).
This was never the intended behavior and only went unnoticed for so long because in most cases the sprotty routers are smart enough to remove duplicated routing points.

From the top of my head I'm not sure what the best approach would be to retrieve the start and endpoints on the server side. @planger WDYT?

[planger]

From the top of my head I'm not sure what the best approach would be to retrieve the start and endpoints on the server side. @planger WDYT?

Hm, I tend to just add them to the ComputedBoundsAction (or a separate action according to your comment) introduced in eclipse-glsp/glsp-server#181, but mark the anchor points differently to the routing points?

[rsoika]

ok, I will try to follow your thoughts but this no not so easy. You say:

Sprotty (and as a consequence also GLSP) makes a clear distinction between routing points and anchor points (start & end points).

I see this distinction between the computing of AnchorPoints and the route between them, but not in the resulting Sprotty property edge.routingPoints. If this is the case, a Sprotty Edge object should contain not only the edge.routingPoints property, but also corresponding Start- and Endpoint properties. But this is not the case - or at least I can't find this in the code.

So this would mean, that the Sprotty ManhattanEdgeRouter does not follow this concept and is implemented wrong. Take a look at the current code:

    route(edge: SRoutableElement): RoutedPoint[] {
        if (!edge.source || !edge.target)
            return [];
        const routedCorners = this.createRoutedCorners(edge);
        const sourceRefPoint = routedCorners[0]
            || translatePoint(Bounds.center(edge.target.bounds), edge.target.parent, edge.parent);
        const sourceAnchor = this.getTranslatedAnchor(edge.source, sourceRefPoint, edge.parent, edge, edge.sourceAnchorCorrection);
        const targetRefPoint = routedCorners[routedCorners.length - 1]
            || translatePoint(Bounds.center(edge.source.bounds), edge.source.parent, edge.parent);
        const targetAnchor = this.getTranslatedAnchor(edge.target, targetRefPoint, edge.parent, edge, edge.targetAnchorCorrection);
        if (!sourceAnchor || !targetAnchor)
            return [];
        const routedPoints: RoutedPoint[] = [];
        routedPoints.push({ kind: 'source', ...sourceAnchor});
        routedCorners.forEach(corner => routedPoints.push(corner));
        routedPoints.push({ kind: 'target', ...targetAnchor});
        return routedPoints;
    }

I agree with you, there is a distinction between computing the Anchors and the Route in Sprotty (which is clever), but not in the resulting routing property!
The router simply adds the AnchorPoint to the list of routing points. At least I can't find a piece of code that stores this information anywhere else within Sprotty. (I do not know the behavior of the PolylineEdgeRouter and BezierEdgeRouter but I assume it is similar).

If I understand you correctly, then you generally want to pack the first and last point into a new property that does not exist yet?
In a consequence this would mean the GEdge needs a new implementation. This is a immense change I think.

Maybe you're trying to solve a problem that doesn't exist.

Due to a bug we previously also sent the anchorpoints to the server. However, this resulted in duplicated routing points on the client (Start and endpoint are already part of the routing points but are are also recomputed and added during the client side routing).

I would say, this means the Sprotty router implementation was simply wrong. And to be honest, I haven't yet understood why the Sprotty router is constantly recalculating the routers. Could it be, that the problem lies somewhere else entirely?

I really don't want to stop you in such a complex topic, but it seems to me, that you are trying to solve a problem that has nothing to do with GLSP?

To make this clear again: A route must start with the start point and end with the end point. That is the essential concept of a route.

[tortmayr]

To make this clear again: A route must start with the start point and end with the end point. That is the essential concept of a route.

I agree the computed route must have a start and endpoint. However, the routingPoints property of a sprotty edgei is not necessarily equal to the final computed route.
For example if we have a look at the sprotty class diagram example (no GLSP involved, plain sprotty):

You can see that the "routingPoints" property only has the three "segement" routing points (i.e. no start or end). The start and end points are only added to the final route computed by the corresponding edgeRouter.

So this would mean, that the Sprotty ManhattanEdgeRouter does not follow this concept and is implemented wrong. Take a look at the current code:

No that's not true the sprotty edgeRouters behave exactly as I described (see also the example above). The "routingPoints" property does not contain the start and end point instead this points are computed dynamically based on the defined anchors. (This happens in https://github.com/eclipse/sprotty/blob/b0e9d60e9c9ab7d302f84f55eef227828ba7d68c/packages/sprotty/src/features/routing/manhattan-edge-router.ts#L63)

To sum up the problem: The routingPoints property of a sprotty edge does not provide the final computed route, it rather serves as an input for the edgeRouter. The final route is currently not persisted in the model.

Typically this is not a problem because routing and anchor computation is either done completely on the client side or completely on server side. What you described is a mixed case where you basically want to compute the anchors on the client side and then use them to compute the route on the server side. This is afaik currently not supported hence you would need a workaround for that.

[rsoika]

@tortmayr based on your answer and the example code: You differentiate between RoutingPoints and the Route. I was then a bit inaccurate. Is there any way to get to the route? Is the route somewhere available in the GLSP code?

... routing and anchor computation is either done completely on the client side or completely on server side. What you described is a mixed case where you basically want to compute the anchors on the client side and then use them to compute the route on the server side. This is afaik currently not supported hence you would need a workaround for that.

Can you please explain that briefly. How is it possible to implement a router on the server side and is there an impact regarding performance? If I can implement my requirements in this way, I'll be happy to do so.

Do you mean I should simply compute the route during the CreateEdgeOperation? But is the moving of routingPoints on the client still supported? I am really sorry that I have to ask such fundamental questions again.

[martin-fleck-at]

@rsoika The route can be calculated using the edge router registry, as done in the calcElementAndRoutingPoints.

I general, I do not know why routing points do not consider source point and target points in sprotty but I believe if we want to change that, it should be adapted in sprotty so we do not lose compatibility.

However, to provide the information for GLSP, I opened a PR which should address the issue that you are describing. It extends the ComputedBoundsAction with the route information of an edge and applies that information on the server side as routing points with source point and target point as additional arguments on the edge:

• #738: Add route data to ComputedBoundsAction glsp-client#201
• #738: Add route data to ComputedBoundsAction glsp-server#181

Using that information, a server-side LayoutEngine can read the routing points, source point and target point from the GEdge to consider them in their own layout.

[rsoika]

@martin-fleck-at , yes I recognized already the method calcElementAndRoutingPoints.

And what was disturbing me here is the following comment and the filter implementation:

  // filter duplicate points (same x,y coordinates) and only keep actual routing points (no start or target, no volatile points)
  const clientRoutingPoints = router?.route(element)
  .....

From as far as I was able to debug the code, the method call router?.route(element) returns the correct route information :

start-point +  end-point  + all routingPoints

But the GLSP filter code is removing the start and endpoint information again. And as far as I now understand, this is only because you distinct between:

• startpoint = anchor start point in the diagram
• endpoint = anchor end point in the diagram
• routingPoints = waypoints between the start and endpoint
• route = startpoint + routingPoints + endpoint

And the event ChangeRoutingPointsOperation should (form your understanding) only send the routingPoints (sorry I was not aware of this distinction between a Route and the RoutingPoints)

And now I hope you are also accept my problem: I need the start and endpoint for my BPMN model - the complete route computed by the router. @tortmayr says, I do mixing client and server logic here. But I don't think I am doing this. I just expected that GLSP provides an event or a way to send me the current route computed by the client. But this is not the case.

Proposal:

Why not introduce a second Event for the route infomation and sending two different GLSP Events:

 if  newRoutingPoints.length > 0
          [ChangeRoutingPointsOperation.create(newRoutingPoints)];
          [ChangeRoutingOperation.create(fullRoute)];
....

Then the adopter can decide which event is from interest.....

[martin-fleck-at]

@rsoika Yes, I completely agree with you. The calcElementAndRoutingPoints was working for the routing point case but was too restrictive for the route case.

As for the overall solution to the problem: We considered creating a new ChangeRoutingOperation but we would have the problem that initially the routing information is not available on the server as the ChangeRoutingPointsOperation is only triggered when the routing points are changed by the user. So what we do instead in the PR is that we consider the route as part of the client-side layout information (ComputedBoundsAction), similar to the size of the elements (bounds) or the alignment of elements. Adding the information to the ComputedBoundsAction has the advantage that we can ensure that the information is always correctly provided based on the client-side rendering without having to worry who might under some circumstances do a change that can affect the routing (e.g., moving the start or end element also would affect the route, so we would also add a custom operation to that).

As always, if the server does not care about this information, it can simply ignore it or customize the ComputedBoundsActionHandler. If the information is not needed on the client side and we want to avoid the additional data sent, we can the post processor that sends the ComputedBoundsAction. However, from a framework point of view, providing the information to the server by default seems a good strategy.

[rsoika]

Ok, that sounds really good to me! Then I will shut up now, and wait for the new implementation ;-)