Bugzilla – Full Text Bug Listing
| Summary: | Repeated Points in Connection | ||
|---|---|---|---|
| Product: | [Modeling] Graphiti | Reporter: | Hernan Gonzalez <hjg.com.ar> |
| Component: | Core | Assignee: | Project Inbox <graphiti-inbox> |
| Status: | CLOSED WONTFIX | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | matthias.gorning, michael.wenz |
| Version: | 0.8.0 | ||
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
I've implemented this ugly workaround:
@Override
public boolean canAddBendpoint(IAddBendpointContext context) {
// check that we are not trying to insert the same point
FreeFormConnection freeFormConnection = context.getConnection();
List<Point> bendpoints = freeFormConnection.getBendpoints();
int index = context.getBendpointIndex();
if(index >=0 && index < bendpoints.size() && bendpoints.get(index).getX() == context.getX() && bendpoints.get(index).getY() != context.getY() ) {
System.err.println("duplicated bendpoint");
return false;
}
index--;
if(index >=0 && index < bendpoints.size() && bendpoints.get(index).getX() == context.getX() && bendpoints.get(index).getY() != context.getY() ) {
System.err.println("duplicated bendpoint");
return false;
}
return true;
}
Seems to avoid the insertion, but now another different issue (should I fill another bug?) arises:
When I'm manipulatin the bendpoints, if canAddBendpoint() returns false, the bendopoint is nonetheless shown in the selected connection (dashed orange line), and the line is shaped as it had been inserted; only when I click outside the connection, to deselect it, the line returns to its previous shape. Obviously, this is very confusing for the user.
A stupid error fixed:
@Override
public boolean canAddBendpoint(IAddBendpointContext context) {
// check that we are not trying to insert the same point
FreeFormConnection freeFormConnection = context.getConnection();
List<Point> bendpoints = freeFormConnection.getBendpoints();
int index = context.getBendpointIndex();
if(index >=0 && index < bendpoints.size()
&& bendpoints.get(index).getX() == context.getX()
&& bendpoints.get(index).getY() == context.getY() ) {
System.err.println("duplicated bendpoint");
return false;
}
index--;
if(index >=0 && index < bendpoints.size()
&& bendpoints.get(index).getX() == context.getX()
&& bendpoints.get(index).getY() == context.getY() ) {
System.err.println("duplicated bendpoint");
return false;
}
return true;
}
I tried to avoid the duplicated bendpoint creation by refusing to do it in the addBendpoint() method (instead of denegating in canAdd() method) and returning silently, but then the next requests come with wrong getBendpointIndex(), I guess that we are losing sync with the underlying GEF objects. Then, I guess that GEF is to blame here and we must resign to accept that a connection can have duplicated bendpoints... Thanks for the detailed description, this surely deserves a closer look. Please, can you describe the steps how I can get duplicated bendpoints in the tutorial? (In reply to comment #6) > Please, can you describe the steps how I can get duplicated bendpoints in the > tutorial? Just create two bendpoints in a connection, and drag the second over the first one. So you expect that bendpoints get merged if you drag one above the other? In general, i think it is good that we can have several bendpoints with the same coordinates. This enables for instance, that we can model connections which cross through themselves. And regarding your use-case: other users may not want to merge bendpoints when dragged on top. I would propose that -- unless other developers want to have such a feature as well -- we leave it as is. I tend to agree that, because GEF creates/accepts repeated bendpoints, Graphiti should follow this behavior. But I still I'm unconfortable with the fact that Graphiti design seems to assume that repetead bendopints will NOT happen - they eventually work... "by mere luck". This could break something in the future or introduce some bugs, see my comment in the first post about graphiti.ecore, with FreeFormConnection.bendpoints speficiyng "Unique=true". Perhaps changing this to false, and/or adding some comment/warning somewhere... I think unique=true is perfectly alright. No duplicates of identical (==) objects should be allowed. (Otherwise deleting one bendpoint in the UI would delete/invalidate other bendpoint as well etc...) You might also look at http://www.eclipse.org/forums/index.php/mv/msg/155702/491137/#msg_491137 for a discussion of the ambiguity of EMF's "unique" attribute.... Can you close the bug if you agree? |
I've noticed that in some cases, when manipulating the bendpoints of a the FreeFormConnection, repeated bendpoints are created. Below goes an example, created from the tutorial. I believe there are two related issues here: First, the logic inside the FreeFormConnection should no attempt to create a bendpoint already existent. Second, the bendpoints list should not accept it. Indeeed, I detected this problem when I tried to store the bendpoints also as attributes of my model, and a duplicated insertion triggered an exception. And I see inside graphiti.ecore that FreeFormConnection.bendpoints specifies "Unique=true". However, the EMF implementation uses a EObjectContainmentEList, which (because a Point is an "Object" instead of a DataType) uses == instead of equal, and hence two structurally identical Points are considered as different objects. I don't think this is desirable, two points with identical x,y values should be considered the same point. I'm a novice in EMF, but I think something should be done here. ======================= <connections xsi:type="pi:FreeFormConnection" visible="true" active="true" start="/0/@children.0/@anchors.1" end="/0/@children.1/@anchors.0"> <graphicsAlgorithm xsi:type="al:Polyline" lineWidth="1" filled="false" transparency="0.0" style="/0/@styles.0"/> <link businessObjects="/1/new%20EReference"/> <connectionDecorators visible="true" locationRelative="true" location="1.0"> <graphicsAlgorithm xsi:type="al:Polyline" lineWidth="1" filled="false" transparency="0.0" style="/0/@styles.0"> <points x="-15" y="10"/> <points/> <points x="-15" y="-10"/> </graphicsAlgorithm> </connectionDecorators> <bendpoints x="201" y="113"/> <bendpoints x="201" y="113"/> <bendpoints x="201" y="113"/> <bendpoints x="281" y="138"/> </connections>