Skip to content

Commit

Permalink
Fix bogus calculation of graph node location
Browse files Browse the repository at this point in the history
The location of the graph node should match whatever has been calculated
in the layout algorithm and should not include the node size.

To make the getLayout() method of the Graph callable within the tests,
it now returns an object of type NodeLayout, rather than the
package-private InternalNodeLayout.
  • Loading branch information
ptziegler committed Sep 19, 2024
1 parent 7102481 commit 4d71e35
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1214,7 +1214,7 @@ void removeNode(GraphNode node) {
this.getSelection().remove(node);
}
figure2ItemMap.remove(figure);
node.getLayout().dispose();
node.internalGetLayout().dispose();
}

void addConnection(GraphConnection connection, boolean addToEdgeLayer) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.eclipse.zest.core.widgets.internal.ZestRootLayer;
import org.eclipse.zest.layouts.LayoutEntity;
import org.eclipse.zest.layouts.constraints.LayoutConstraint;
import org.eclipse.zest.layouts.interfaces.NodeLayout;

import org.eclipse.draw2d.Animation;
import org.eclipse.draw2d.ColorConstants;
Expand Down Expand Up @@ -340,7 +341,7 @@ public void setLocation(double x, double y) {
currentLocation.setPreciseY(y);
refreshLocation();
if (getGraphModel().isDynamicLayoutEnabled() && parent instanceof IContainer2 parent2) {
((InternalLayoutContext) parent2.getLayoutContext()).fireNodeMovedEvent(this.getLayout());
((InternalLayoutContext) parent2.getLayoutContext()).fireNodeMovedEvent(internalGetLayout());
}
}
}
Expand Down Expand Up @@ -912,12 +913,12 @@ public void figureMoved(IFigure source) {
}
currentLocation = new PrecisionPoint(nodeFigure.getBounds().getTopLeft());
if (parent instanceof IContainer2 parent2) {
((InternalLayoutContext) parent2.getLayoutContext()).fireNodeResizedEvent(getLayout());
((InternalLayoutContext) parent2.getLayoutContext()).fireNodeResizedEvent(internalGetLayout());
}
} else if (currentLocation.x != newBounds.x || currentLocation.y != newBounds.y) {
currentLocation = new PrecisionPoint(nodeFigure.getBounds().getTopLeft());
if (parent instanceof IContainer2 parent2) {
((InternalLayoutContext) parent2.getLayoutContext()).fireNodeMovedEvent(getLayout());
((InternalLayoutContext) parent2.getLayoutContext()).fireNodeMovedEvent(internalGetLayout());
}
}
}
Expand Down Expand Up @@ -1135,13 +1136,17 @@ void paint() {
/**
* @noreference This method is not intended to be referenced by clients.
*/
public InternalNodeLayout getLayout() {
public final NodeLayout getLayout() {
if (layout == null) {
layout = new InternalNodeLayout(this);
}
return layout;
}

InternalNodeLayout internalGetLayout() {
return (InternalNodeLayout) getLayout();
}

void applyLayoutChanges() {
if (layout != null) {
layout.applyLayout();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public EntityLayout[] getEntities() {
List<EntityLayout> result = new ArrayList<>();
for (GraphNode node : this.container.getNodes()) {
if (!isLayoutItemFiltered(node)) {
InternalNodeLayout nodeLayout = node.getLayout();
InternalNodeLayout nodeLayout = node.internalGetLayout();
if (!nodeLayout.isPruned()) {
result.add(nodeLayout);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ void applyLayout() {
} else {
node.setSize(-1, -1);
if (location != null) {
node.setLocation(location.x - getSize().width / 2, location.y - size.height / 2);
node.setLocation(location.x, location.y);
}
if (size != null) {
Dimension currentSize = node.getSize();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,14 @@ public void testCustomLayout() {
assertEquals(node1.getLocation().y, node3.getLocation().y);

waitEventLoop(5);

for (GraphNode node : graph.getNodes()) {
Point p = node.getLocation();
Rectangle bounds = graph.getBounds();
assertTrue("Node outside of bounds", p.x >= 0 && p.x <= bounds.width);
assertTrue("Node outside of bounds", p.y >= 0 && p.y <= bounds.height);
}

assertNoOverlap(graph);
}

Expand Down

0 comments on commit 4d71e35

Please sign in to comment.