Skip to content

Commit

Permalink
[426] DRAFT - Fix Handle border nodes more precisely
Browse files Browse the repository at this point in the history
The previous commit was wrong. This commit is just here to see the
result.
Analysis: It seems that before, the code with comment "manage location
of bordered node with closest side" is called only in specific
circumstances before. Now it is called systematically but it should not.

Bug: #426
  • Loading branch information
lredor committed Jul 30, 2024
1 parent 8e57c60 commit 44700cb
Show file tree
Hide file tree
Showing 11 changed files with 37 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public Range getVerticalRange() {
result = CacheHelper.getViewToRangeCache().get(node);
}
if (result == null) {
Rectangle absoluteBounds = GMFHelper.getAbsoluteBounds(node);
Rectangle absoluteBounds = GMFHelper.getAbsoluteBounds(node, false);
int y = absoluteBounds.y;
int height = absoluteBounds.height;
// GMFHelper.getAbsoluteBounds() use default
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ private void updateEdge(DEdge dEdge) {

private void computeEdgeDataWithoutEdgeLayoutData() {
Point firstClick = new Point(0, 0);
Rectangle sourceBounds = GMFHelper.getAbsoluteBounds((Node) source);
Rectangle sourceBounds = GMFHelper.getAbsoluteBounds((Node) source, false);
if (source.getElement() instanceof AbstractDNode) {
AbstractDNode sourceDNode = (AbstractDNode) source.getElement();
if (sourceDNode.eContainer() instanceof AbstractDNode) {
Expand Down Expand Up @@ -181,7 +181,7 @@ private void computeEdgeDataWithoutEdgeLayoutData() {
sourceTerminal = "(" + sourceRelativeLocation.preciseX() + "," + sourceRelativeLocation.preciseY() + ")"; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$

Point secondClick = new Point(0, 0);
Rectangle targetBounds = GMFHelper.getAbsoluteBounds((Node) target);
Rectangle targetBounds = GMFHelper.getAbsoluteBounds((Node) target, false);

if (target.getElement() instanceof AbstractDNode) {
AbstractDNode targetDNode = (AbstractDNode) target.getElement();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ private void avoidOverlaps(Diagram diagram) {
Location location = (Location) layoutConstraint;
constraint = new Rectangle(location.getX(), location.getY(), -1, -1);
}
Point parentAbsoluteLocation = GMFHelper.getAbsoluteLocation(parentNode);
Point parentAbsoluteLocation = GMFHelper.getAbsoluteLocation(parentNode, false, true);
constraint.translate(parentAbsoluteLocation.x, parentAbsoluteLocation.y);
final Point realLocation = borderItemLocator.getValidLocation(constraint, node, otherBorderedNodesToIgnore);
final Dimension d = realLocation.getDifference(parentAbsoluteLocation);
Expand Down Expand Up @@ -215,7 +215,7 @@ private void migrateGraphicalFiltersAndGMFBounds(Diagram diagram) {
LayoutConstraint layoutConstraint = node.getLayoutConstraint();
if (layoutConstraint instanceof Bounds) {
Bounds bounds = (Bounds) layoutConstraint;
Rectangle rectBounds = GMFHelper.getAbsoluteBounds(node);
Rectangle rectBounds = GMFHelper.getAbsoluteBounds(node, false);
// The GMF node size must be stored in collapse
// filter (to can set this size
// when this node is expanded).
Expand Down Expand Up @@ -287,6 +287,7 @@ private void migrateChildrenOfCollapsedNode(Diagram diagram) {
* </UL>
*/
private static class IsStandardDiagramPredicate implements Predicate<Diagram> {
@Override
public boolean apply(Diagram input) {
boolean apply = false;
if (input.getElement() instanceof DDiagram) {
Expand All @@ -311,6 +312,7 @@ public boolean apply(Diagram input) {
*/
private static class IsBorderedNodePredicate implements Predicate<Node> {

@Override
public boolean apply(Node input) {
// Is this node the main view of a DNode and a border
// node ?
Expand All @@ -327,6 +329,7 @@ public boolean apply(Node input) {
*/

private static class IsDirectlyCollapsedNodePredicate implements Predicate<Node> {
@Override
public boolean apply(Node input) {
boolean apply = false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@ private boolean isOverlapped(Node node, StraightenToCommandData data) {
// the move. It should be the same as the one stored in GMF but
// sometimes, probably caused by some bugs, it could be wrong in
// GMF.
Point parentAbsoluteLocation = GMFHelper.getAbsoluteLocation(parentNode);
Point parentAbsoluteLocation = GMFHelper.getAbsoluteLocation(parentNode, true);
Rectangle initialAsboluteConstraint = initialRelativeConstraint.getTranslated(parentAbsoluteLocation);
Point validInitialAbsoluteLocation = borderItemLocator.getValidLocation(initialAsboluteConstraint, node, new ArrayList<Node>(Arrays.asList(node)));
Rectangle validInitialAbsoluteConstraint = initialAsboluteConstraint.getCopy();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ private LayoutDataResult updateAbstractDNode_ownedBorderedNodes_Bounds(View crea
locator.setConstraint(constraint);
dummyFigure.setVisible(true);
final Rectangle rect = new Rectangle(constraint);
Point parentAbsoluteLocation = GMFHelper.getAbsoluteLocation(parentNode, true);
Point parentAbsoluteLocation = GMFHelper.getAbsoluteLocation(parentNode, true, true);
rect.translate(parentAbsoluteLocation.x, parentAbsoluteLocation.y);
dummyFigure.setBounds(rect);
final Point realLocation = locator.getValidLocation(rect, createdNode, new ArrayList<Node>(Arrays.asList(createdNode)));
Expand Down Expand Up @@ -666,7 +666,7 @@ private LayoutDataResult updateAbstractDNode_ownedBorderedNodes_Bounds(View crea

// CanonicalDBorderItemLocator works with absolute GMF parent
// location so we need to translate BorderedNode absolute location.
final org.eclipse.draw2d.geometry.Point parentAbsoluteLocation = GMFHelper.getAbsoluteBounds(parentNode).getTopLeft();
final org.eclipse.draw2d.geometry.Point parentAbsoluteLocation = GMFHelper.getAbsoluteBounds(parentNode, true).getTopLeft();
final Point realLocation = locator.getValidLocation(new Rectangle(location.getTranslated(parentAbsoluteLocation), size), createdNode, Collections.singleton(createdNode));

// Compute the new relative position to the parent
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,8 @@ private GMFHelper() {
*
* @return the absolute location of the node relative to the origin (Diagram)
*/
public static Point getAbsoluteLocation(Node node) {
return getAbsoluteLocation(node, false);
public static Point getAbsoluteLocation(Node node, boolean absoluteLocationForChild) {
return getAbsoluteLocation(node, false, absoluteLocationForChild);
}

/**
Expand All @@ -160,7 +160,7 @@ public static Point getAbsoluteLocation(Node node) {
*
* @return the absolute location of the node relative to the origin (Diagram)
*/
public static Point getAbsoluteLocation(Node node, boolean insetsAware) {
public static Point getAbsoluteLocation(Node node, boolean insetsAware, boolean absoluteLocationForChild) {
// TODO: Location of title "DNode*NameEditPart", x coordinate, can be wrong according to label alignment. This
// problem is not yet handled.

Expand All @@ -173,7 +173,7 @@ public static Point getAbsoluteLocation(Node node, boolean insetsAware) {
location.setY(gmfBounds.getY());
}
ViewQuery viewQuery = new ViewQuery(node);
if (viewQuery.isBorderedNode() && layoutConstraint instanceof Bounds gmfBounds) {
if (!absoluteLocationForChild && viewQuery.isBorderedNode() && layoutConstraint instanceof Bounds gmfBounds) {
// manage location of bordered node with closest side
if (node.getElement() instanceof DNode && node.getElement().eContainer() instanceof AbstractDNode) {
DNode dNode = (DNode) node.getElement();
Expand All @@ -187,7 +187,7 @@ public static Point getAbsoluteLocation(Node node, boolean insetsAware) {
new Rectangle(parentBounds.getX(), parentBounds.getY(), parentBounds.getWidth(), parentBounds.getHeight()));
updateLocation(location, position, parentBounds, gmfBounds);
}
System.out.println("location=" + location); //$NON-NLS-1$
// System.out.println("location=" + location); //$NON-NLS-1$
}
}
}
Expand All @@ -201,7 +201,7 @@ public static Point getAbsoluteLocation(Node node, boolean insetsAware) {
} else if (viewQuery.isListItem()) {
if (node.eContainer() instanceof Node container) {
if (container.getChildren().get(0) == node) {
Point parentNodeLocation = getAbsoluteLocation(container, insetsAware);
Point parentNodeLocation = getAbsoluteLocation(container, insetsAware, true);
location.translate(parentNodeLocation);
if (insetsAware) {
translateWithInsets(location, node);
Expand All @@ -222,7 +222,7 @@ public static Point getAbsoluteLocation(Node node, boolean insetsAware) {
} else if (viewQuery.isVerticalRegion()) {
if (node.eContainer() instanceof Node container) {
if (container.getChildren().get(0) == node) {
Point parentNodeLocation = getAbsoluteLocation(container, insetsAware);
Point parentNodeLocation = getAbsoluteLocation(container, insetsAware, true);
location.translate(parentNodeLocation);
if (insetsAware) {
translateWithInsets(location, node);
Expand All @@ -236,7 +236,7 @@ public static Point getAbsoluteLocation(Node node, boolean insetsAware) {
} else if (viewQuery.isHorizontalRegion()) {
if (node.eContainer() instanceof Node container) {
if (container.getChildren().get(0) == node) {
Point parentNodeLocation = getAbsoluteLocation(container, insetsAware);
Point parentNodeLocation = getAbsoluteLocation(container, insetsAware, true);
location.translate(parentNodeLocation);
if (insetsAware) {
translateWithInsets(location, node);
Expand All @@ -248,7 +248,7 @@ public static Point getAbsoluteLocation(Node node, boolean insetsAware) {
}
}
} else if (node.eContainer() instanceof Node container) {
Point parentNodeLocation = getAbsoluteLocation(container, insetsAware);
Point parentNodeLocation = getAbsoluteLocation(container, insetsAware, true);
location.translate(parentNodeLocation);
if (insetsAware) {
translateWithInsets(location, node);
Expand Down Expand Up @@ -539,7 +539,7 @@ private static void translateWithInsets(Rectangle boundsToTranslate, Node curren
*/
@Deprecated
public static Point getLocation(Node node) {
return getAbsoluteLocation(node, true);
return getAbsoluteLocation(node, true, false);
}

private static void updateLocation(Point location, int position, Bounds parentBounds, Bounds gmfBounds) {
Expand Down Expand Up @@ -569,8 +569,8 @@ private static void updateLocation(Point location, int position, Bounds parentBo
*
* @return the absolute bounds of the node relative to the origin (Diagram)
*/
public static Rectangle getAbsoluteBounds(Node node) {
return getAbsoluteBounds(node, false);
public static Rectangle getAbsoluteBounds(Node node, boolean absoluteLocationForChild) {
return getAbsoluteBounds(node, false, absoluteLocationForChild);
}

/**
Expand All @@ -584,8 +584,8 @@ public static Rectangle getAbsoluteBounds(Node node) {
*
* @return the absolute bounds of the node relative to the origin (Diagram)
*/
public static Rectangle getAbsoluteBounds(Node node, boolean insetsAware) {
return getAbsoluteBounds(node, insetsAware, false, false);
public static Rectangle getAbsoluteBounds(Node node, boolean insetsAware, boolean absoluteLocationForChild) {
return getAbsoluteBounds(node, insetsAware, false, false, absoluteLocationForChild);
}

/**
Expand All @@ -601,8 +601,8 @@ public static Rectangle getAbsoluteBounds(Node node, boolean insetsAware) {
* otherwise
* @return the absolute bounds of the node relative to the origin (Diagram)
*/
public static Rectangle getAbsoluteBounds(Node node, boolean insetsAware, boolean boxForConnection) {
return getAbsoluteBounds(node, insetsAware, boxForConnection, false);
public static Rectangle getAbsoluteBounds(Node node, boolean insetsAware, boolean boxForConnection, boolean absoluteLocationForChild) {
return getAbsoluteBounds(node, insetsAware, boxForConnection, false, absoluteLocationForChild);
}

/**
Expand All @@ -620,15 +620,15 @@ public static Rectangle getAbsoluteBounds(Node node, boolean insetsAware, boolea
* true if this method is called from a parent "getBounds" call, false otherwise.
* @return the absolute bounds of the node relative to the origin (Diagram)
*/
public static Rectangle getAbsoluteBounds(Node node, boolean insetsAware, boolean boxForConnection, boolean recursiveGetBounds) {
public static Rectangle getAbsoluteBounds(Node node, boolean insetsAware, boolean boxForConnection, boolean recursiveGetBounds, boolean absoluteLocationForChild) {
if (!recursiveGetBounds) {
boundsCache.clear();
}
Rectangle result;
if (boundsCache.containsKey(node)) {
result = boundsCache.get(node);
} else {
Point location = getAbsoluteLocation(node, insetsAware);
Point location = getAbsoluteLocation(node, insetsAware, absoluteLocationForChild);
Rectangle bounds = getBounds(node, false, false, boxForConnection, recursiveGetBounds, location);
result = new PrecisionRectangle(location.preciseX(), location.preciseY(), bounds.preciseWidth(), bounds.preciseHeight());
if (!recursiveGetBounds) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ private void handleNodeAndContainer(IAbstractDiagramNodeEditPart newEditPart, Co
figure.translateToAbsolute(figureBounds);

// We compute the new GMF Absolute Location. The SnapToHelper works with absolute coordinates.
Point newGMFAbsoluteLocation = GMFHelper.getAbsoluteLocation((Node) newEditPart.getModel(), true);
Point newGMFAbsoluteLocation = GMFHelper.getAbsoluteLocation((Node) newEditPart.getModel(), true, false);

// We convert in screen coordinate as it is expected by the SnapToHelper
GraphicalHelper.logical2screen(newGMFAbsoluteLocation, newEditPart);
Expand Down Expand Up @@ -224,7 +224,7 @@ private void handleBorderNode(IDiagramBorderNodeEditPart newEditPart, CompositeT
Point newValidLocation = itemLocator.getValidLocation(newGMFAbsoluteBounds, node, Collections.singleton(node));

// We compute the new relative coordinates
Point parentAbsoluteLocation = GMFHelper.getAbsoluteLocation(parentNode, true);
Point parentAbsoluteLocation = GMFHelper.getAbsoluteLocation(parentNode, true, true);
Point newValidRelativeLocation = newValidLocation.getTranslated(parentAbsoluteLocation.getNegated());

// We compute the move delta by calculating the difference between the current figure relative location and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public boolean apply(EObject arg0) {
Node node = (Node) iterator.next();
CanonicalDBorderItemLocator locator = new CanonicalDBorderItemLocator((Node) node.eContainer(), PositionConstants.NSEW);
locator.setBorderItemOffset(IBorderItemOffsets.DEFAULT_OFFSET);
Rectangle expectedBounds = GMFHelper.getAbsoluteBounds(node);
Rectangle expectedBounds = GMFHelper.getAbsoluteBounds(node, false);
Point location = locator.getValidLocation(expectedBounds, node, Collections.singletonList(node));
assertEquals("The border node should not be in conflict", expectedBounds.getLocation(), location);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ protected void assertBorderedNodeAtLocation(String borderedNodeLabel, Point expe
errorMessage = "the BorderedNode has been created at wrong location (for near collapsed bordered node case).";
}
// Check GMF
Point gmfNodeLocation = GMFHelper.getAbsoluteLocation((Node) borderNodePart.getModel(), true);
Point gmfNodeLocation = GMFHelper.getAbsoluteLocation((Node) borderNodePart.getModel(), true, false);
assertSameLocation("For GMF, " + errorMessage, expectedLocation, gmfNodeLocation, parentLocation, creationLocation, parentPart);
// Check Draw2d
assertSameLocation("For Draw2d, " + errorMessage, expectedLocation, nodeLocation, parentLocation, creationLocation, parentPart);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ public void testCreateTwoBorderNodesAtSameLocation() {

assertFalse("Second border node should not overlap the first one (draw2d).", firstNewNodeAbsoluteBounds.intersects(secondNewNodeAbsoluteLocation));

Rectangle firstNewNodeGMFBounds = GMFHelper.getAbsoluteBounds((Node) firstNewNode.part().getModel());
Rectangle secondNewNodeGMFBounds = GMFHelper.getAbsoluteBounds((Node) secondNewNode.part().getModel());
Rectangle firstNewNodeGMFBounds = GMFHelper.getAbsoluteBounds((Node) firstNewNode.part().getModel(), false);
Rectangle secondNewNodeGMFBounds = GMFHelper.getAbsoluteBounds((Node) secondNewNode.part().getModel(), false);

assertFalse("Second border node should not overlap the first one (GMF).", firstNewNodeGMFBounds.intersects(secondNewNodeGMFBounds));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,14 @@ public String getFailureMessage() {
Point draw2DSourceBNLocation = GraphicalHelper.getAbsoluteBoundsIn100Percent(source, true).getTopLeft();
assertTrue("For starting point (" + draw2DSourceBNLocation.preciseX() + ", " + draw2DSourceBNLocation.preciseY() + "), the x or y coordinate should be on the grid (step=" + gridStep
+ ") or at least one should be the same as parent: " + parentSourceBounds + ".", checkLocation(draw2DSourceBNLocation, parentSourceBounds));
Point gmfSourceBNLocation = GMFHelper.getAbsoluteLocation((Node) source.getModel(), true);
Point gmfSourceBNLocation = GMFHelper.getAbsoluteLocation((Node) source.getModel(), true, false);
assertEquals("The computing starting point from GMF data should be the same than draw2D", draw2DSourceBNLocation, gmfSourceBNLocation);

Rectangle parentTargetBounds = GraphicalHelper.getAbsoluteBoundsIn100Percent(target, true);
Point draw2DTargetBNLocation = GraphicalHelper.getAbsoluteBoundsIn100Percent(target, true).getTopLeft();
assertTrue("For ending point (" + draw2DTargetBNLocation.preciseX() + ", " + draw2DTargetBNLocation.preciseY() + "), the x or y coordinate should be on the grid (step=" + gridStep
+ ") or at least one should be the same as parent: " + parentTargetBounds + ".", checkLocation(draw2DTargetBNLocation, parentTargetBounds));
Point gmfTargetBNLocation = GMFHelper.getAbsoluteLocation((Node) target.getModel(), true);
Point gmfTargetBNLocation = GMFHelper.getAbsoluteLocation((Node) target.getModel(), true, false);
assertEquals("The computing starting point from GMF data should be the same than draw2D", draw2DTargetBNLocation, gmfTargetBNLocation);
// Contrary to super class, the checkSide are not done because
// snapToGrid is enabled and has effect on the side.
Expand Down

0 comments on commit 44700cb

Please sign in to comment.