#738: Add route data to ComputedBoundsAction #201
Conversation
packages/client/src/features/bounds/glsp-hidden-bounds-updater.ts
Outdated
Show resolved
Hide resolved
packages/client/src/features/bounds/glsp-hidden-bounds-updater.ts
Outdated
Show resolved
Hide resolved
|
If I'm not mistaken we could also refactor the This should no longer be necessary as the updated routing points will be sent with the "ComputedBoundsAction` anyways |
martin-fleck-at
force-pushed
the
issue-738
branch
from
4a1de2e
to
55e572a
Compare
|
Thank you very much for your feedback, Tobias! I updated my code accordingly. Following the discussion in eclipse-glsp/glsp#767, I also added the source and target point to the arguments of the edge and renamed the field from 'routingPoints' to 'route' which is much more accurate now.
You are right, I removed that call. |
martin-fleck-at
changed the title
martin-fleck-at
force-pushed
the
issue-738
branch
from
55e572a
to
2a9f55e
Compare
|
Thanks for the update. In general the changes look good to me. During testing, I encountered a weird bug. The manhattan routing now seems to be broken in certain cases i.e. the visual feedback on the client side and the final router after the server update are different: This occurs for instance when you delete route segments by moving the target element (backtracking on the route). |
|
@martin-fleck-at what is your opinion about the finding from @tortmayr ? I think one issue with the new implementation is, that the server part must now also deal with this scenario. This is different as you usually would do it. Just to give you an impression about my current thoughts: When I load an external BPMN model into my GLSP-OpenBPMN modeler, I now do remove the first and last 'way-point' from the source model information when I transfer the model into GLSP. This is to align the behavior to the GLSP Client. If you want to make me an early Christmas present, then we could do a web conference about this topic? Just to clarify what we 3 really expect from this change. What do you think? |
martin-fleck-at
force-pushed
the
issue-738
branch
from
2a9f55e
to
0f2e031
Compare
martin-fleck-at
force-pushed
the
issue-738
branch
from
0f2e031
to
25600af
Compare
|
@tortmayr I pushed an update that should fix the issue that you were seeing. In short: The change bounds operation caused a request bounds from the server with the "old" (i.e., updated bounds but no routing points) model resulting in a wrong layout on the server. I kept the code that updates the routing points with the change bounds operation and then everything works as expected again: Now you receive the full route on any computed bounds action (i.e., every operation) and can do custom layouting on it. @rsoika Could you please elaborate on the problem that you foresee with this implementation? This is how I see it: If you have a custom layouter registered as a layout engine, it should receive the up to date route information as soon as it is available, i.e., that is the routing points on the edge as well as the start and end position as edge arguments. The start and end point are determined on the client and cannot be set directly from outside but depend on your routing strategy and the source and target element that is set. From what I understand, you would like to set the start and end point directly from the server, is that it? I think in that case you would need to provide a custom router. At the moment, the Manhattan router creates the default anchors (positions) for source and target based on the given source and target element and a given side (right, left, top, bottom). If there are still some unclear points, we can have a quick discussion online, sure. |
|
@martin-fleck-at Thanks for your update. I think we are fine with the over all concept as you explained it. When I load a external BPMN file I need to remove the start and endpoint when building the GLSP Edge/Routing Point information. This is fine and I have already implemented this part.
Of course I will test it as soon as possible and I will provide you my feedback (and a short demo). Should I build the GLSP client from sources or can I take a nightly build (which would be more save to work on the correct code base)? |
|
@rsoika I'm going to review this PR tomorrow. Each change on master automatically triggers a new nightly build so once its merged you can test it by consuming the corresponding next/snapshot version from npm/maven. |
|
@rsoika Yes, it should work exactly the way that you describe. There will be a few utility methods to support you in dealing with routes on the server:
|
|
@martin-fleck-at I try to use the new glsp client version But I run into some compiler issues which I can't solve. For example a code like this (from the workflow example): @injectable()
export class IconView extends ShapeView {
render(element: Icon, context: RenderingContext): VNode | undefined {
let scaleFactor = 1;
let translateX = 0;
let translateY = 0;
if (!this.isVisible(element, context)) {
return undefined;
}
....
}Gets no longer compiled with the following error message: My dependencies look like this: my open-pmn-model and open-bpmn-properties works with 1.1.0-RC02. (also switching @eclipse-glsp/config to latest 1.1.0-next.fe535db.107 did not solve this issue) |
|
Hi Ralph, |
|
@tortmayr yes, after adding the following dependency int my module, it compiles fine again. glsp-client/packages/client/package.json Line 39 in 196fab7 strange.... |
The problem is that that sprotty-theia (which is indirectly used by the theia-integration) depends on a different version of sprotty (https://github.com/eclipse/sprotty-theia/blob/efd153a410267d81af555e70c219e9efa3e3b197/package.json#L44). Which means we end up with two different resolved versions and as a consequence two (only partly initialized) DI containers. As I mentioned we already have a solution for this and are just waiting for the upstream fix in Theia so that we can implement it. |
|
ok great thanks. I am really sorry but I run into a new problem now. It seems like I run into a version problem between my client and my server part. When I try to open a diagram the following exception is thrown on the server side: I tried the org.eclipse.glsp.server versions Does this mean my client is using the wrong/old version? |
Yes, it looks like the client version did not get properly updated. I just double checked and the RC02 Version indeed references the client version "next" instead of "1.1.0-RC02". (https://github.com/eclipse-glsp/glsp-theia-integration/blob/1b73a55849801eb3f1cfd4ecd6bdeb29938de5f7/packages/theia-integration/package.json#L38%5D(https://github.com/eclipse-glsp/glsp-theia-integration/blob/1b73a55849801eb3f1cfd4ecd6bdeb29938de5f7/packages/theia-integration/package.json#L38)) Sorry for the inconveniences, we are still working on improving our automated release candidates so feature versions are hopefully more stable and less error prone 😉 |
|
Thanks, I tried to fix it, but did not succeed. By the way: I know you taught me I should not touch the yarn.lock file in my git repo. But in the past I run too often into situations where the build was not working after I changed dependency versions. Now I changed my clean-up script and now I also remove the |
|
The Release Candidate 03 is now available. |
|
@tortmayr I upgraded to The server code depends on Build process of frontend and backend is fine. It seems that there is still something wrong with my project setup. I also switched to theia 1.27.0 and How can I figure out from where the old protocol version 0.9.0 is coming from ? |
|
I found out, that deep in my node_modules I still have a code setting the protocolVersion to 0.9.0
I guess this is the root of the problem and I should use a different theia-integration verision than |
|
ok... we are near.... I changed the But now my import { createBPMNDiagramContainer } from '@open-bpmn/open-bpmn-glsp';
import {
configureDiagramServer,
GLSPDiagramConfiguration,
GLSPTheiaDiagramServer,
TheiaDiagramServer
} from '@eclipse-glsp/theia-integration/lib/browser';
import { Container, injectable } from 'inversify';
import 'sprotty-theia/css/theia-sprotty.css';
import { BPMNLanguage } from '../../common/bpmn-language';
@injectable()
export class BPMNDiagramConfiguration extends GLSPDiagramConfiguration {
diagramType: string = BPMNLanguage.diagramType;
doCreateContainer(widgetId: string): Container {
const container = createBPMNDiagramContainer(widgetId);
configureDiagramServer(container, GLSPTheiaDiagramServer);
container.bind(TheiaDiagramServer).toService(GLSPTheiaDiagramServer);
return container;
}
}I get now this compiler error: Can you point me to a workflow-example branch where I can verify my general setup. I guess I have to update my code |
Yes, I guess so to. You should be able to use the
You can use the yarn why command for this. It lists a bunch of information to identify why a particular package (version) has been installed. So in your case, you could use |
At first glance, this looks like the sprotty version conflict you already encountered previously. Please make sure that sprotty is part of your resolutions block to enforce the resolution of the required version. |
|
@tortmayr Thanks! adding |
|
@martin-fleck-at @tortmayr I have now implemented one custom The public class BPMNComputedBoundsActionHandler extends AbstractActionHandler<ComputedBoundsAction> {
@Inject
protected BPMNGModelState modelState;
@Override
protected List<Action> executeAction(final ComputedBoundsAction actualAction) {
BPMNProcess context = modelState.getBpmnModel().openDefaultProcess();
List<ElementAndRoutingPoints> routings = actualAction.getRoutes();
try {
for (ElementAndRoutingPoints routingInfo : routings) {
String id = routingInfo.getElementId();
BPMNSequenceFlow bpmnSequenceFlow = context.findBPMNSequenceFlowById(id);
if (bpmnSequenceFlow != null) {
List<GPoint> newGLSPRoutingPoints = routingInfo.getNewRoutingPoints();
// update the BPMN Waypoint.
System.out.println("===== Updating " + newGLSPRoutingPoints.size() + " BPMN WayPoints =======");
bpmnSequenceFlow.clearWayPoints();
// add the new routing points
for (GPoint point : newGLSPRoutingPoints) {
BPMNPoint bpmnPoint = new BPMNPoint(point.getX(), point.getY());
bpmnSequenceFlow.addWayPoint(bpmnPoint);
}
}
}
} catch (Exception e) {
e.printStackTrace();
}
// no more action - the GModel is now up to date
return none();
}
}The public class BPMNChangeRoutingPointsOperationHandler extends AbstractOperationHandler<ChangeRoutingPointsOperation> {
@Inject
protected BPMNGModelState modelState;
@Override
public void executeOperation(final ChangeRoutingPointsOperation operation) {
List<ElementAndRoutingPoints> routingPoints = operation.getNewRoutingPoints();
try {
for (ElementAndRoutingPoints routingPoint : routingPoints) {
String id = routingPoint.getElementId();
List<GPoint> newGLSPRoutingPoints = routingPoint.getNewRoutingPoints();
// update the GModel.
Optional<GEdge> _edge = modelState.getIndex().findElementByClass(id, GEdge.class);
if (_edge.isPresent()) {
GEdge edge = _edge.get();
edge.getRoutingPoints().clear();
edge.getRoutingPoints().addAll(newGLSPRoutingPoints);
}
}
} catch (Exception e) {
e.printStackTrace();
}
// no more action - the GModel is now up to date
}
}Both implementations are necessary. As a result, the BPMN source Model information and the GModel in the diagram plane is now always exactly what I expected. So I don't want any more changes. I am absolutely happy! Can you only answer me one last question in brief: How would you describe the function of a |
Part of eclipse-glsp/glsp#738