tesis-dynaware / graph-editor

Eclipse Public License 1.0
132 stars 42 forks source link

Alignment of programmatically created GConnection #17

Closed eckig closed 9 years ago

eckig commented 9 years ago

If I programmatically create and add a GConnection (not with the help of mouse-gestures, but directly through EMF Commands) the GJoint(s) are not correctly displayed:

autp-create-1

If I now select the GNode the GJoint(s) suddenly jump to their correct positions:

autp-create-2

rmfisher commented 9 years ago

For the light-grey connection, it looks like during the first 'draw connections' pass, the node skin's getConnectorPosition method is returning a y-value of zero. You could debug this method and see why.

For the dark-grey connection, it looks like the y-value is a bit too small.

This can be tricky if the connector-skin or node-skin JavaFX nodes have not been layed-out correctly before the connections are drawn.

eckig commented 9 years ago

Yep, that is the issue, but at this point I do not have any chance of giving a correct coordinate, since I only got this from a non rendered Node?

RectBounds { minX:0.0, minY:0.0, maxX:-1.0, maxY:-1.0} (w:-1.0, h:-1.0)

rmfisher commented 9 years ago

So your node skin has zero size the first time the connections are layed-out? It's tricky for me to debug this but perhaps you could provide a simplified example of a custom GNodeSkin that has this problem and I'll take a look into it.

eckig commented 9 years ago

You already pointed me in the right direction. I added a manual layout pass and it seems to work now.

But this seems rather unnecessary (GeometryUtils.getConnectionPosition()):

final double connectorX = nodeSkin.getConnectorPosition(connectorSkin).getX();
final double connectorY = nodeSkin.getConnectorPosition(connectorSkin).getY();

Wouldn't this be better?

final Point2D connectorPosition = nodeSkin.getConnectorPosition(connectorSkin);
final double connectorX = connectorPosition.getX();
final double connectorY = connectorPosition.getY();
rmfisher commented 9 years ago

Yes, that's a better idea.

rmfisher commented 9 years ago

Done. Do you use the Maven Central artifacts? If not I will wait for more changes before I deploy a new version.

eckig commented 9 years ago

No, not at the moment. I will wait for the next version - its not a blocker ;-)