Bug 509944

Comment 1 Matthias Wienand 2017-01-13 11:57:13 EST

While playing around with the bug, I noticed that you need to select the right handle shape first after creating the situation where the orthogonal connection has three segments in order to reproduce the bug. However, I am still searching for the cause. I can verify that the anchor computation is triggered, but it yields the incorrect point that does not lie on the outline of the right handle shape.

Comment 2 Matthias Wienand 2017-01-14 11:57:28 EST

The reason for the problem is that the position does not change within the local coordinate system of the anchored (the Connection), but it does change within the coordinate system of the content-layer (where it is drawn). So, there is a conceptual problem here: position-change-listeners are not notified although the position might have changed in the context where it is needed. Maybe the "context-node" should be supplied when registering a position-change-listener for an anchor-key. Until the problem is fixed, you can use the following (ugly) workaround that issues a fake update when the position did change in scene coordinates but not in local coordinates:

public class SceneRelativeUpdatingDynamicAnchor extends DynamicAnchor {
@Override
protected void updatePosition(AnchorKey key) {
  if (!isAttached(key)) return;
  Point oldPosition = getPosition(key);
  Point newPosition = computePosition(key);
  boolean changedInScene = false;
  if (oldPosition != null && oldPosition.equals(newPosition)) {
    // check if scene coordinates changed
    Point2D newScenePoint = key.getAnchored()
        .localToScene(Geometry2FX.toFXPoint(newPosition));
    changedInScene = oldScenePoint.containsKey(key)
        && !oldScenePoint.get(key).equals(newScenePoint);
    oldScenePoint.put(key, newScenePoint);
  }
  if (oldPosition == null || changedInScene
      || !oldPosition.equals(newPosition)) {
    if (newPosition != null && !Double.isNaN(newPosition.x)
        && !Double.isInfinite(newPosition.x)
        && !Double.isNaN(newPosition.y)
        && !Double.isInfinite(newPosition.y)) {
      if (changedInScene) {
        positions.put(key, new Point(newPosition.x + 0.5,
            newPosition.y + 0.5));
      }
      positions.put(key, newPosition);
}}} }

Comment 3 Matthias Wienand 2017-01-16 06:51:11 EST

I found another solution that is compatible with the current anchor implementation. The Connection uses anchors to determine its points. However, the anchor positions are transformed (using curve node's local-to-parent-transform) before they are written to the Connection's #pointsProperty(). The problem uccurs when this transformation changes, but the anchor positions remain the same. I implemented a fix within Connection that transforms the anchor positions when the curve node's local-to-parent-transform changes. The code is published on the master branch, therefore, I resolve this ticket as fixed for 5.0.0 M5.

Comment 4 Matthias Wienand 2017-01-16 08:53:17 EST

It seems like I introduced a regression by transforming the points when the curve-to-connection transform changes. I will try to fix this regression today, but O will revert my changes if I am unable to do so, and develop the fix locally until all the tests are passing. Sorry for the inconvenience.

Comment 5 Matthias Wienand 2017-01-19 13:20:49 EST

Pushed the following changes to origin/master:
 - Properly guard Connection#refresh().
     - PCLs are disabled during refresh.
     - Points are manually updated after routing.
     - Routing is done twice to ensure that parameters are set.
 - Ensure VisualChangeListener notifies about all relative changes.
     - Transform changes need to be observed for both nodes up-to the common parent.
     - Feedback and handles need to disable their visual change listeners during refreshing of visuals.
 - Prevent DynamicAnchor from swallowing exceptions.
     - When parameters are initialized only certain exceptions need to be handled by DynamicAnchor. The rest falls through to user code.
 - Prevent CME when updating anchor positions.
     - The set of anchor keys is copied for the iteration so that addition and removal of anchor keys is possible in response to anchor position changes.
 - Add test for erroneous behavior.
 - Guard against iterating empty list.

With these changes in place, I can no longer observe the erroneous behavior. Therefore, I resolve this ticket as fixed for 5.0.0 M5.