From 7106dd644f840320dbe7373cfc8a6807e471ad85 Mon Sep 17 00:00:00 2001 From: Laurent Redor Date: Wed, 17 Jul 2024 15:47:42 +0200 Subject: [PATCH 01/17] [426] Add tests to reveal the problems Bug: https://github.com/eclipse-sirius/sirius-desktop/issues/426 --- .../data/unit/layout/gmfHelper/My.ecore | 31 + .../data/unit/layout/gmfHelper/My.odesign | 123 ++ .../layout/gmfHelper/representations.aird | 1484 +++++++++++++++++ .../tests/swtbot/layout/GMFHelperTest.java | 286 ++++ .../tests/swtbot/suite/AllTestSuite.java | 2 + 5 files changed, 1926 insertions(+) create mode 100644 plugins/org.eclipse.sirius.tests.swtbot/data/unit/layout/gmfHelper/My.ecore create mode 100644 plugins/org.eclipse.sirius.tests.swtbot/data/unit/layout/gmfHelper/My.odesign create mode 100644 plugins/org.eclipse.sirius.tests.swtbot/data/unit/layout/gmfHelper/representations.aird create mode 100644 plugins/org.eclipse.sirius.tests.swtbot/src/org/eclipse/sirius/tests/swtbot/layout/GMFHelperTest.java diff --git a/plugins/org.eclipse.sirius.tests.swtbot/data/unit/layout/gmfHelper/My.ecore b/plugins/org.eclipse.sirius.tests.swtbot/data/unit/layout/gmfHelper/My.ecore new file mode 100644 index 0000000000..b55cbd640a --- /dev/null +++ b/plugins/org.eclipse.sirius.tests.swtbot/data/unit/layout/gmfHelper/My.ecore @@ -0,0 +1,31 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/plugins/org.eclipse.sirius.tests.swtbot/data/unit/layout/gmfHelper/My.odesign b/plugins/org.eclipse.sirius.tests.swtbot/data/unit/layout/gmfHelper/My.odesign new file mode 100644 index 0000000000..3d7affcc59 --- /dev/null +++ b/plugins/org.eclipse.sirius.tests.swtbot/data/unit/layout/gmfHelper/My.odesign @@ -0,0 +1,123 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/plugins/org.eclipse.sirius.tests.swtbot/data/unit/layout/gmfHelper/representations.aird b/plugins/org.eclipse.sirius.tests.swtbot/data/unit/layout/gmfHelper/representations.aird new file mode 100644 index 0000000000..025b7d3e47 --- /dev/null +++ b/plugins/org.eclipse.sirius.tests.swtbot/data/unit/layout/gmfHelper/representations.aird @@ -0,0 +1,1484 @@ + + + + My.ecore + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/plugins/org.eclipse.sirius.tests.swtbot/src/org/eclipse/sirius/tests/swtbot/layout/GMFHelperTest.java b/plugins/org.eclipse.sirius.tests.swtbot/src/org/eclipse/sirius/tests/swtbot/layout/GMFHelperTest.java new file mode 100644 index 0000000000..822067eb92 --- /dev/null +++ b/plugins/org.eclipse.sirius.tests.swtbot/src/org/eclipse/sirius/tests/swtbot/layout/GMFHelperTest.java @@ -0,0 +1,286 @@ +/******************************************************************************* + * Copyright (c) 2024 Obeo. + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Obeo - initial API and implementation + *******************************************************************************/ +package org.eclipse.sirius.tests.swtbot.layout; + +import org.eclipse.draw2d.geometry.Rectangle; +import org.eclipse.gef.EditPart; +import org.eclipse.gef.GraphicalEditPart; +import org.eclipse.gmf.runtime.notation.Node; +import org.eclipse.sirius.diagram.DDiagram; +import org.eclipse.sirius.diagram.ui.internal.edit.parts.DNodeContainer2EditPart; +import org.eclipse.sirius.diagram.ui.internal.edit.parts.DNodeContainerEditPart; +import org.eclipse.sirius.diagram.ui.internal.edit.parts.DNodeListEditPart; +import org.eclipse.sirius.diagram.ui.internal.edit.parts.DNodeListElementEditPart; +import org.eclipse.sirius.diagram.ui.internal.refresh.GMFHelper; +import org.eclipse.sirius.tests.swtbot.Activator; +import org.eclipse.sirius.tests.swtbot.support.api.AbstractSiriusSwtBotGefTestCase; +import org.eclipse.sirius.tests.swtbot.support.api.business.UIDiagramRepresentation.ZoomLevel; +import org.eclipse.sirius.tests.swtbot.support.api.business.UIResource; +import org.eclipse.sirius.tests.swtbot.support.api.editor.SWTBotSiriusDiagramEditor; +import org.eclipse.sirius.tests.swtbot.support.api.view.DesignerViews; +import org.eclipse.sirius.tests.swtbot.support.utils.SWTBotUtils; +import org.eclipse.swtbot.eclipse.gef.finder.widgets.SWTBotGefEditPart; + +/** + * Tests defined to ensure that GMF Helper returns correct absolute GMF bounds. + * + * TODO: - Add tests for border nodes bounds - Add tests for nodes bounds (inside container) + * + * @author lredor + */ +public class GMFHelperTest extends AbstractSiriusSwtBotGefTestCase { + + private static final String FREE_FORM_CONTAINER_REPRESENTATION_NAME = "DiagramWithFreeFormContainers"; + + private static final String LIST_CONTAINER_REPRESENTATION_NAME = "DiagramWithListContainers"; + + private static final String HSTACK_CONTAINER_REPRESENTATION_NAME = "DiagramWithHStackContainers"; + + private static final String VSTACK_CONTAINER_REPRESENTATION_NAME = "DiagramWithVStackContainers"; + + private static final String MODEL = "My.ecore"; + + private static final String SESSION_FILE = "representations.aird"; + + private static final String ODESIGN_FILE = "My.odesign"; + + private static final String DATA_UNIT_DIR = "data/unit/layout/gmfHelper/"; + + private static final String FILE_DIR = "/"; + + /** + * The name of a container that is empty. + */ + private static final String EMPTY_CONTAINER_NAME = "p1"; + + /** + * The name of a container with children. + */ + private static final String CONTAINER_WITH_CHILDREN_NAME = "p2"; + + /** + * The name of a container that is empty and in another container + */ + private static final String EMPTY_CONTAINER_INSIDE_CONTAINER_NAME = "p22"; + + + /** + * The name of an empty container with border node. + */ + private static final String EMPTY_CONTAINER_WITH_BORDER_NODE_NAME = "p3"; + + /** + * The name of a container with children having border node. + */ + private static final String CONTAINER_WITH_2_CHILDREN_HAVING_BORDER_NODE_NAME = "p4"; + + /** + * The name of an empty container with border node, inside a container. + */ + private static final String EMPTY_CONTAINER_WITH_BORDER_NODE_INSIDE_CONTAINER_2_NAME = "p41"; + + /** + * The name of a container with children having border node. + */ + private static final String CONTAINER_WITH_3_CHILDREN_HAVING_BORDER_NODE_NAME = "p5"; + + /** + * The name of an empty container with border node, inside a container. + */ + private static final String EMPTY_CONTAINER_WITH_BORDER_NODE_INSIDE_CONTAINER_3_NAME = "p53"; + + /** + * {@inheritDoc} + */ + @Override + protected void onSetUpBeforeClosingWelcomePage() throws Exception { + copyFileToTestProject(Activator.PLUGIN_ID, DATA_UNIT_DIR, MODEL, SESSION_FILE, ODESIGN_FILE); + } + + /** + * {@inheritDoc} + */ + @Override + protected void onSetUpAfterOpeningDesignerPerspective() throws Exception { + sessionAirdResource = new UIResource(designerProject, FILE_DIR, SESSION_FILE); + localSession = designerPerspective.openSessionFromFile(sessionAirdResource); + closeOutline(); + } + + /** + * Method to open an editor. It should be called at the beginning of each test. + * + * @param representationName + * The name of the representation to be opened. + */ + private void openEditor(String representationName) { + editor = (SWTBotSiriusDiagramEditor) openRepresentation(localSession.getOpenedSession(), representationName, representationName, DDiagram.class, true); + } + + /** + * {@inheritDoc} + */ + @Override + protected void tearDown() throws Exception { + // Go to the origin to avoid scroll bar + editor.scrollTo(0, 0); + // Restore the default zoom level + editor.click(1, 1); // Set the focus on the diagram + editor.zoom(ZoomLevel.ZOOM_100); + // Go to the origin to avoid scroll bar + editor.scrollTo(0, 0); + // Reopen outline + new DesignerViews(bot).openOutlineView(); + SWTBotUtils.waitAllUiEvents(); + super.tearDown(); + } + + public void testFreeFormContainerBounds_1() { + testFreeFormContainerBounds(EMPTY_CONTAINER_NAME, DNodeContainerEditPart.class); + } + + public void testFreeFormContainerBounds_2() { + testFreeFormContainerBounds(CONTAINER_WITH_CHILDREN_NAME, DNodeContainerEditPart.class); + } + + public void testFreeFormContainerBounds_3() { + testFreeFormContainerBounds(EMPTY_CONTAINER_INSIDE_CONTAINER_NAME, DNodeContainer2EditPart.class); + } + + public void testFreeFormContainerBounds_4() { + testFreeFormContainerBounds(EMPTY_CONTAINER_WITH_BORDER_NODE_NAME, DNodeContainerEditPart.class); + } + + public void testFreeFormContainerBounds_5() { + testFreeFormContainerBounds(CONTAINER_WITH_2_CHILDREN_HAVING_BORDER_NODE_NAME, DNodeContainerEditPart.class); + testFreeFormContainerBounds(CONTAINER_WITH_3_CHILDREN_HAVING_BORDER_NODE_NAME, DNodeContainerEditPart.class); + } + + public void testFreeFormContainerBounds_6() { + testFreeFormContainerBounds(EMPTY_CONTAINER_WITH_BORDER_NODE_INSIDE_CONTAINER_2_NAME, DNodeContainer2EditPart.class); + testFreeFormContainerBounds(EMPTY_CONTAINER_WITH_BORDER_NODE_INSIDE_CONTAINER_3_NAME, DNodeContainerEditPart.class); + } + + + public void testListContainerBounds_1() { + testListContainerBounds(EMPTY_CONTAINER_NAME, DNodeListEditPart.class); + } + + public void testListContainerBounds_2() { + testListContainerBounds(CONTAINER_WITH_CHILDREN_NAME, DNodeListEditPart.class); + } + + public void testListContainerBounds_3() { + testListContainerBounds(EMPTY_CONTAINER_INSIDE_CONTAINER_NAME, DNodeListElementEditPart.class); + } + + public void testListContainerBounds_4() { + testListContainerBounds(EMPTY_CONTAINER_WITH_BORDER_NODE_NAME, DNodeListEditPart.class); + } + + public void testListContainerBounds_5() { + testListContainerBounds(CONTAINER_WITH_2_CHILDREN_HAVING_BORDER_NODE_NAME, DNodeListEditPart.class); + testListContainerBounds(CONTAINER_WITH_3_CHILDREN_HAVING_BORDER_NODE_NAME, DNodeListEditPart.class); + } + + public void testListContainerBounds_6() { + testListContainerBounds(EMPTY_CONTAINER_WITH_BORDER_NODE_INSIDE_CONTAINER_2_NAME, DNodeListElementEditPart.class); + testListContainerBounds(EMPTY_CONTAINER_WITH_BORDER_NODE_INSIDE_CONTAINER_3_NAME, DNodeListElementEditPart.class); + } + + public void testHStackContainerBounds_1() { + testHStackContainerBounds(EMPTY_CONTAINER_NAME, DNodeContainerEditPart.class); + } + + public void testHStackContainerBounds_2() { + testHStackContainerBounds(CONTAINER_WITH_CHILDREN_NAME, DNodeContainerEditPart.class); + } + + public void testHStackContainerBounds_3() { + testHStackContainerBounds(EMPTY_CONTAINER_INSIDE_CONTAINER_NAME, DNodeContainer2EditPart.class); + } + + public void testHStackContainerBounds_4() { + testHStackContainerBounds(EMPTY_CONTAINER_WITH_BORDER_NODE_NAME, DNodeContainerEditPart.class); + } + + public void testHStackContainerBounds_5() { + testHStackContainerBounds(CONTAINER_WITH_2_CHILDREN_HAVING_BORDER_NODE_NAME, DNodeContainerEditPart.class); + testHStackContainerBounds(CONTAINER_WITH_3_CHILDREN_HAVING_BORDER_NODE_NAME, DNodeContainerEditPart.class); + } + + public void testHStackContainerBounds_6() { + testHStackContainerBounds(EMPTY_CONTAINER_WITH_BORDER_NODE_INSIDE_CONTAINER_2_NAME, DNodeContainer2EditPart.class); + testHStackContainerBounds(EMPTY_CONTAINER_WITH_BORDER_NODE_INSIDE_CONTAINER_3_NAME, DNodeContainer2EditPart.class); + } + + public void testVStackContainerBounds_1() { + testVStackContainerBounds(EMPTY_CONTAINER_NAME, DNodeContainerEditPart.class); + } + + public void testVStackContainerBounds_2() { + testVStackContainerBounds(CONTAINER_WITH_CHILDREN_NAME, DNodeContainerEditPart.class); + } + + public void testVStackContainerBounds_3() { + testVStackContainerBounds(EMPTY_CONTAINER_INSIDE_CONTAINER_NAME, DNodeContainer2EditPart.class); + } + + public void testVStackContainerBounds_4() { + testVStackContainerBounds(EMPTY_CONTAINER_WITH_BORDER_NODE_NAME, DNodeContainerEditPart.class); + } + + public void testVStackContainerBounds_5() { + testVStackContainerBounds(CONTAINER_WITH_2_CHILDREN_HAVING_BORDER_NODE_NAME, DNodeContainerEditPart.class); + testVStackContainerBounds(CONTAINER_WITH_3_CHILDREN_HAVING_BORDER_NODE_NAME, DNodeContainerEditPart.class); + } + + public void testVStackContainerBounds_6() { + testVStackContainerBounds(EMPTY_CONTAINER_WITH_BORDER_NODE_INSIDE_CONTAINER_2_NAME, DNodeContainer2EditPart.class); + testVStackContainerBounds(EMPTY_CONTAINER_WITH_BORDER_NODE_INSIDE_CONTAINER_3_NAME, DNodeContainer2EditPart.class); + } + + private void testFreeFormContainerBounds(String elementToCheckName, Class expectedEditPartType) { + testContainerBounds(FREE_FORM_CONTAINER_REPRESENTATION_NAME, elementToCheckName, expectedEditPartType); + } + + private void testListContainerBounds(String elementToCheckName, Class expectedEditPartType) { + testContainerBounds(LIST_CONTAINER_REPRESENTATION_NAME, elementToCheckName, expectedEditPartType); + } + + private void testHStackContainerBounds(String elementToCheckName, Class expectedEditPartType) { + testContainerBounds(HSTACK_CONTAINER_REPRESENTATION_NAME, elementToCheckName, expectedEditPartType); + } + + private void testVStackContainerBounds(String elementToCheckName, Class expectedEditPartType) { + testContainerBounds(VSTACK_CONTAINER_REPRESENTATION_NAME, elementToCheckName, expectedEditPartType); + } + + private void testContainerBounds(String representationName, String elementToCheckName, Class expectedEditPartType) { + openEditor(representationName); + SWTBotGefEditPart swtBotEditPart = editor.getEditPart(elementToCheckName, expectedEditPartType); + Rectangle draw2DAbsoluteBounds = editor.getAbsoluteBounds(swtBotEditPart); + // It's strange, the above method does not return absolute bounds. To check in another commit. And the method + // GraphicalHelper.getAbsoluteBoundsIn100Percent consider handleBounds and this is not what is expected here. + ((GraphicalEditPart) swtBotEditPart.part()).getFigure().translateToAbsolute(draw2DAbsoluteBounds); + Rectangle gmfAbsoluteBounds = GMFHelper.getAbsoluteBounds((Node) swtBotEditPart.part().getModel(), true); + assertEquals("The GMF width of " + elementToCheckName + " in " + representationName + " does not correspond to the Draw2D one.", draw2DAbsoluteBounds.preciseWidth(), + gmfAbsoluteBounds.preciseWidth()); + assertEquals("The GMF height of " + elementToCheckName + " in " + representationName + " does not correspond to the Draw2D one.", draw2DAbsoluteBounds.preciseHeight(), + gmfAbsoluteBounds.preciseHeight()); + assertEquals("The GMF x coordinate of " + elementToCheckName + " in " + representationName + " does not correspond to the Draw2D one.", draw2DAbsoluteBounds.preciseX(), + gmfAbsoluteBounds.preciseX()); + assertEquals("The GMF y coordinate of " + elementToCheckName + " in " + representationName + " does not correspond to the Draw2D one.", draw2DAbsoluteBounds.preciseY(), + gmfAbsoluteBounds.preciseY()); + } +} \ No newline at end of file diff --git a/plugins/org.eclipse.sirius.tests.swtbot/src/org/eclipse/sirius/tests/swtbot/suite/AllTestSuite.java b/plugins/org.eclipse.sirius.tests.swtbot/src/org/eclipse/sirius/tests/swtbot/suite/AllTestSuite.java index d6333738dc..62ab3555c4 100644 --- a/plugins/org.eclipse.sirius.tests.swtbot/src/org/eclipse/sirius/tests/swtbot/suite/AllTestSuite.java +++ b/plugins/org.eclipse.sirius.tests.swtbot/src/org/eclipse/sirius/tests/swtbot/suite/AllTestSuite.java @@ -56,6 +56,7 @@ import org.eclipse.sirius.tests.swtbot.layout.EdgeLabelsAlignAndDistributeTests; import org.eclipse.sirius.tests.swtbot.layout.EdgeLayoutStabilityWithToolWizardTest; import org.eclipse.sirius.tests.swtbot.layout.EdgeStabilityOnCopyPasteLayoutTest; +import org.eclipse.sirius.tests.swtbot.layout.GMFHelperTest; import org.eclipse.sirius.tests.swtbot.layout.LayoutStabilityOnManualRefreshTest; import org.eclipse.sirius.tests.swtbot.layout.ModifyEdgeLayoutAfterRefreshTest; import org.eclipse.sirius.tests.swtbot.layout.PackageLayoutStabilityOnManyViewsCreationToolTest; @@ -442,6 +443,7 @@ public static void addPart2(TestSuite suite) { suite.addTestSuite(PackageLayoutStabilityOnManyViewsCreationToolTest.class); suite.addTestSuite(ResetOriginTest.class); suite.addTestSuite(LayoutStabilityOnManualRefreshTest.class); + suite.addTestSuite(GMFHelperTest.class); suite.addTestSuite(EdgeAndPortStabilityOnSemanticChangeTest.class); suite.addTestSuite(SessionSaveableTest.class); suite.addTest(new JUnit4TestAdapter(DragAndDropFromControlledResourceTest.class)); From 00e44cdacce6deaada5397d4014cb326420ad514 Mon Sep 17 00:00:00 2001 From: Laurent Redor Date: Wed, 29 May 2024 16:53:21 +0200 Subject: [PATCH 02/17] [426] Border nodes are wrongly considered in GMFHelper.getBottomRight Because of typo, the border nodes are considered in org.eclipse.sirius.diagram.ui.internal.refresh.GMFHelper.getBottomRight(Node) when they should be ignored. Bug: https://github.com/eclipse-sirius/sirius-desktop/issues/426 --- .../sirius/diagram/ui/internal/refresh/GMFHelper.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/GMFHelper.java b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/GMFHelper.java index 7287d9d61b..8872b2523f 100644 --- a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/GMFHelper.java +++ b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/GMFHelper.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2011, 2023 THALES GLOBAL SERVICES and others. + * Copyright (c) 2011, 2024 THALES GLOBAL SERVICES and others. * This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 * which accompanies this distribution, and is available at @@ -274,7 +274,7 @@ public static Dimension getBorderSize(DDiagramElementContainer ddec) { */ private static void translateWithInsets(Point locationToTranslate, Node currentNode) { NodeQuery nodeQuery = new NodeQuery(currentNode); - // bordered node are not concerned by those insets. + // Border nodes are not concerned by those insets. if (!nodeQuery.isBorderedNode()) { locationToTranslate.translate(getContainerTopLeftInsets(currentNode, false)); } @@ -789,8 +789,8 @@ public static Point getBottomRight(Node node) { int bottom = 0; for (Iterator children = Iterators.filter(node.getChildren().iterator(), Node.class); children.hasNext(); /* */) { Node child = children.next(); - // The bordered nodes is ignored - if (!(new NodeQuery(node).isBorderedNode())) { + // The border nodes are ignored + if (!(new NodeQuery(child).isBorderedNode())) { Rectangle bounds = getBounds(child); Point bottomRight = bounds.getBottomRight(); if (bottomRight.x > right) { From 192ccfcb5d1b29f11d17d1cdae4a651798acc32a Mon Sep 17 00:00:00 2001 From: Laurent Redor Date: Tue, 9 Jul 2024 15:29:56 +0200 Subject: [PATCH 03/17] [426] Refactor getContainerTopLeftInsets without change Bug: https://github.com/eclipse-sirius/sirius-desktop/issues/426 --- .../ui/internal/refresh/GMFHelper.java | 49 +++++++++++++------ 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/GMFHelper.java b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/GMFHelper.java index 8872b2523f..f7956bed68 100644 --- a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/GMFHelper.java +++ b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/GMFHelper.java @@ -162,6 +162,38 @@ public static Point getAbsoluteLocation(Node node, boolean insetsAware) { return absoluteNodeLocation; } + /** + * Return the top-left insets of this container. The insets also considers its border. + * + * @param container + * The container for which we wish to have the insets. This {@link Node} must correspond to a container, + * otherwise, {0,0} is returned + * @return the top-left insets of this container + */ + public static Dimension getTopLeftInsets(Node container) { + Dimension result = new Dimension(0, 0); + NodeQuery nodeQuery = new NodeQuery(container); + if (nodeQuery.isContainer()) { + EObject element = container.getElement(); + if (element instanceof DDiagramElementContainer) { + DDiagramElementContainer ddec = (DDiagramElementContainer) element; + // RegionContainer do not have containers insets + if (ddec instanceof DNodeContainer) { + if (new DNodeContainerExperimentalQuery((DNodeContainer) ddec).isRegionContainer() || hasFullLabelBorder(ddec)) { + result.setHeight(CONTAINER_INSETS.y + getLabelSize(container) + AbstractDiagramElementContainerEditPart.DEFAULT_SPACING); + } else { + result.setWidth(CONTAINER_INSETS.x); + result.setHeight(CONTAINER_INSETS.y); + } + } + Dimension borderSize = getBorderSize(ddec); + result.setWidth(result.width() + borderSize.width()); + result.setHeight(result.height() + borderSize.height()); + } + } + return result; + } + /** * Return the top-left insets of the container of this node. The insets also considers its border. * @@ -179,22 +211,7 @@ public static Dimension getContainerTopLeftInsets(Node node, boolean searchFirst Node parentNode = (Node) nodeContainer; NodeQuery nodeQuery = new NodeQuery(parentNode); if (nodeQuery.isContainer()) { - EObject element = parentNode.getElement(); - if (element instanceof DDiagramElementContainer) { - DDiagramElementContainer ddec = (DDiagramElementContainer) element; - // RegionContainer do not have containers insets - if (ddec instanceof DNodeContainer) { - if (new DNodeContainerExperimentalQuery((DNodeContainer) ddec).isRegionContainer() || hasFullLabelBorder(ddec)) { - result.setHeight(CONTAINER_INSETS.y + getLabelSize(parentNode) + AbstractDiagramElementContainerEditPart.DEFAULT_SPACING); - } else { - result.setWidth(CONTAINER_INSETS.x); - result.setHeight(CONTAINER_INSETS.y); - } - } - Dimension borderSize = getBorderSize(ddec); - result.setWidth(result.width() + borderSize.width()); - result.setHeight(result.height() + borderSize.height()); - } + result = getTopLeftInsets(parentNode); } else if (searchFirstParentContainer) { result = getContainerTopLeftInsets(parentNode, searchFirstParentContainer); } From 4d3df75a653b416ad68881e2d44d85fd3151ac3f Mon Sep 17 00:00:00 2001 From: Laurent Redor Date: Tue, 9 Jul 2024 15:36:19 +0200 Subject: [PATCH 04/17] [426] Consider insets and border nodes around children for auto-size * Add method getBottomRightInsets to use it in auto-size computation * Consider shadow border * Consider border nodes when computing children bottom-right corner. Bug: https://github.com/eclipse-sirius/sirius-desktop/issues/426 --- .../ui/internal/refresh/GMFHelper.java | 89 +++++++++++++++---- 1 file changed, 70 insertions(+), 19 deletions(-) diff --git a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/GMFHelper.java b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/GMFHelper.java index f7956bed68..ced3b79311 100644 --- a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/GMFHelper.java +++ b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/GMFHelper.java @@ -27,6 +27,7 @@ import org.eclipse.draw2d.IFigure; import org.eclipse.draw2d.PositionConstants; import org.eclipse.draw2d.geometry.Dimension; +import org.eclipse.draw2d.geometry.Insets; import org.eclipse.draw2d.geometry.Point; import org.eclipse.draw2d.geometry.PrecisionRectangle; import org.eclipse.draw2d.geometry.Rectangle; @@ -111,9 +112,10 @@ public final class GMFHelper { /** * see org.eclipse.sirius.diagram.ui.internal.edit.parts. AbstractDNodeContainerCompartmentEditPart.DEFAULT_MARGIN - * the Y value is the DEFAULT_MARGIN + the InvisibleResizableCompartmentFigure top Inset (1px) + * the top value is the DEFAULT_MARGIN + the InvisibleResizableCompartmentFigure top Inset (1px) */ - private static Point CONTAINER_INSETS = new Point(AbstractDNodeContainerCompartmentEditPart.DEFAULT_MARGIN, IContainerLabelOffsets.LABEL_OFFSET); + private static Insets CONTAINER_INSETS = new Insets(IContainerLabelOffsets.LABEL_OFFSET, AbstractDNodeContainerCompartmentEditPart.DEFAULT_MARGIN, + AbstractDNodeContainerCompartmentEditPart.DEFAULT_MARGIN, AbstractDNodeContainerCompartmentEditPart.DEFAULT_MARGIN); /** * The gap in pixels between the Label's icon and its text @@ -180,10 +182,10 @@ public static Dimension getTopLeftInsets(Node container) { // RegionContainer do not have containers insets if (ddec instanceof DNodeContainer) { if (new DNodeContainerExperimentalQuery((DNodeContainer) ddec).isRegionContainer() || hasFullLabelBorder(ddec)) { - result.setHeight(CONTAINER_INSETS.y + getLabelSize(container) + AbstractDiagramElementContainerEditPart.DEFAULT_SPACING); + result.setHeight(CONTAINER_INSETS.top + getLabelSize(container) + AbstractDiagramElementContainerEditPart.DEFAULT_SPACING); } else { - result.setWidth(CONTAINER_INSETS.x); - result.setHeight(CONTAINER_INSETS.y); + result.setWidth(CONTAINER_INSETS.left); + result.setHeight(CONTAINER_INSETS.top); } } Dimension borderSize = getBorderSize(ddec); @@ -219,6 +221,40 @@ public static Dimension getContainerTopLeftInsets(Node node, boolean searchFirst return result; } + /** + * Return the bottom-right insets of the container of this node. The insets also considers its border. + * + * @param container + * The container for which we wish to have the insets. This {@link Node} must correspond to a container, + * otherwise, {0,0} is returned + * @return the bottom-right insets of this container + */ + private static Dimension getBottomRightInsets(Node container) { + Dimension result = new Dimension(0, 0); + NodeQuery nodeQuery = new NodeQuery(container); + if (nodeQuery.isContainer()) { + EObject element = container.getElement(); + if (element instanceof DDiagramElementContainer) { + DDiagramElementContainer ddec = (DDiagramElementContainer) element; + // RegionContainer do not have containers insets + if (ddec instanceof DNodeContainer) { + if (new DNodeContainerExperimentalQuery((DNodeContainer) ddec).isRegionContainer() || hasFullLabelBorder(ddec)) { + // TODO : Not sure about that, to verify + result.setHeight(CONTAINER_INSETS.bottom); + } else { + result.setWidth(CONTAINER_INSETS.right); + result.setHeight(CONTAINER_INSETS.bottom); + } + } + Dimension borderSize = getBorderSize(ddec); + // Added twice as this insets is used to compute the "global" size including the border + result.setWidth(result.width() + (borderSize.width() * 2)); + result.setHeight(result.height() + (borderSize.height() * 2)); + } + } + return result; + } + /** * Return the top-left insets of the container of this node that is after the label. The insets also * considers its border. @@ -239,8 +275,8 @@ public static Dimension getContainerTopLeftInsetsAfterLabel(Node node, boolean s if (nodeQuery.isContainer()) { EObject element = parentNode.getElement(); if (element instanceof DDiagramElementContainer) { - result.setWidth(CONTAINER_INSETS.x); - result.setHeight(CONTAINER_INSETS.y); + result.setWidth(CONTAINER_INSETS.left); + result.setHeight(CONTAINER_INSETS.top); Dimension borderSize = getBorderSize((DDiagramElementContainer) element); result.setWidth(result.width() + borderSize.width()); @@ -436,7 +472,7 @@ public static Rectangle getAbsoluteBounds(Node node, boolean insetsAware) { */ public static Rectangle getAbsoluteBounds(Node node, boolean insetsAware, boolean boxForConnection) { Node currentNode = node; - Rectangle absoluteNodeBounds = getBounds(currentNode, false, false, boxForConnection); + Rectangle absoluteNodeBounds = getBounds(currentNode, false, false, boxForConnection, false); if (currentNode.eContainer() instanceof Node) { currentNode = (Node) currentNode.eContainer(); Point parentNodeLocation = getAbsoluteLocation(currentNode, insetsAware); @@ -572,7 +608,7 @@ public static Rectangle getBounds(Node node, boolean useFigureForAutoSizeConstra * @return the bounds of the node. */ public static Rectangle getBounds(Node node, boolean useFigureForAutoSizeConstraint, boolean forceFigureAutoSize) { - return getBounds(node, useFigureForAutoSizeConstraint, forceFigureAutoSize, false); + return getBounds(node, useFigureForAutoSizeConstraint, forceFigureAutoSize, false, false); } /** @@ -588,9 +624,12 @@ public static Rectangle getBounds(Node node, boolean useFigureForAutoSizeConstra * @param boxForConnection * true if we want to have the bounds used to compute connection anchor from source or target, false * otherwise + * @param recursiveGetBounds + * true if this method is called from a parent "getBounds" call, false otherwise. + * * @return the bounds of the node. */ - public static Rectangle getBounds(Node node, boolean useFigureForAutoSizeConstraint, boolean forceFigureAutoSize, boolean boxForConnection) { + public static Rectangle getBounds(Node node, boolean useFigureForAutoSizeConstraint, boolean forceFigureAutoSize, boolean boxForConnection, boolean recursiveGetBounds) { PrecisionRectangle bounds = new PrecisionRectangle(0, 0, 0, 0); LayoutConstraint layoutConstraint = node.getLayoutConstraint(); EObject element = node.getElement(); @@ -619,10 +658,10 @@ public static Rectangle getBounds(Node node, boolean useFigureForAutoSizeConstra } else { // Make a default size for label (this size is purely an average estimate) - replaceAutoSize(node, bounds, useFigureForAutoSizeConstraint, getLabelDimension(node, new Dimension(50, 20))); + replaceAutoSize(node, bounds, useFigureForAutoSizeConstraint, getLabelDimension(node, new Dimension(50, 20)), recursiveGetBounds); } } else { - replaceAutoSize(node, bounds, useFigureForAutoSizeConstraint, null); + replaceAutoSize(node, bounds, useFigureForAutoSizeConstraint, null, recursiveGetBounds); } if (boxForConnection) { @@ -684,8 +723,10 @@ public static boolean isShadowBorderNeeded(Node node) { * true to use draw2d figure to get size * @param providedDefaultSize * The size used for creation for this kind of node. It is the minimum size. + * @param recursive + * true if this method is called from a "parent" call, false otherwise. */ - private static void replaceAutoSize(Node node, Rectangle bounds, boolean useFigureForAutoSizeConstraint, Dimension providedDefaultSize) { + private static void replaceAutoSize(Node node, Rectangle bounds, boolean useFigureForAutoSizeConstraint, Dimension providedDefaultSize, boolean recursive) { if (bounds.width == -1 || bounds.height == -1) { Dimension defaultSize = providedDefaultSize; if (providedDefaultSize == null) { @@ -727,8 +768,15 @@ private static void replaceAutoSize(Node node, Rectangle bounds, boolean useFigu } else { // Compute the bounds of all children and use the lowest // one (y+height) for height and the rightmost one - // (x+width) for width. - Point bottomRight = getBottomRight(node); + // (x+width) for width plus the margin. + Point bottomRight = getBottomRight(node, recursive); + double shadowBorderSize = getShadowBorderSize(node); + Dimension topLeftInsets = getTopLeftInsets(node); + Dimension bottomRightInsets = getBottomRightInsets(node); + if (!recursive) { + bottomRight.setX(bottomRight.x + Double.valueOf(shadowBorderSize).intValue() + topLeftInsets.width() + bottomRightInsets.width()); + bottomRight.setY(bottomRight.y + Double.valueOf(shadowBorderSize).intValue() + topLeftInsets.height() + bottomRightInsets.height()); + } if (bounds.width == -1) { if (bottomRight.x > defaultSize.width) { bounds.setWidth(bottomRight.x); @@ -798,17 +846,20 @@ private static void lookForNextRegionLocation(Rectangle bounds, Node node) { * * @param node * the node whose bottom right corner is to compute. + * @param considerBorderNode + * true to consider border nodes when computing the bottom right corner point, false otherwise. * * @return Point at the bottom right of the rectangle */ - public static Point getBottomRight(Node node) { + private static Point getBottomRight(Node node, boolean considerBorderNodes) { int right = 0; int bottom = 0; for (Iterator children = Iterators.filter(node.getChildren().iterator(), Node.class); children.hasNext(); /* */) { Node child = children.next(); - // The border nodes are ignored - if (!(new NodeQuery(child).isBorderedNode())) { - Rectangle bounds = getBounds(child); + // The border nodes are ignored, except if it is expected to consider it (auto-size of a container with + // children having border nodes) + if (considerBorderNodes || !(new NodeQuery(child).isBorderedNode())) { + Rectangle bounds = getBounds(child, false, false, false, true); Point bottomRight = bounds.getBottomRight(); if (bottomRight.x > right) { right = bottomRight.x; From 35e8f23b38786966487b379e5b7d82f38fc0fc90 Mon Sep 17 00:00:00 2001 From: Laurent Redor Date: Mon, 29 Jul 2024 11:01:44 +0200 Subject: [PATCH 05/17] [426] Use the font defined in the GMF node to compute label dimension Bug: https://github.com/eclipse-sirius/sirius-desktop/issues/426 --- .../ui/internal/refresh/GMFHelper.java | 51 ++++++++++++------- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/GMFHelper.java b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/GMFHelper.java index ced3b79311..dd34ec9baf 100644 --- a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/GMFHelper.java +++ b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/GMFHelper.java @@ -19,6 +19,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.stream.Stream; import org.eclipse.core.runtime.Path; @@ -44,12 +45,14 @@ import org.eclipse.gmf.runtime.notation.Bounds; import org.eclipse.gmf.runtime.notation.Diagram; import org.eclipse.gmf.runtime.notation.Edge; +import org.eclipse.gmf.runtime.notation.FontStyle; import org.eclipse.gmf.runtime.notation.LayoutConstraint; import org.eclipse.gmf.runtime.notation.Location; import org.eclipse.gmf.runtime.notation.Node; import org.eclipse.gmf.runtime.notation.NotationPackage; import org.eclipse.gmf.runtime.notation.RelativeBendpoints; import org.eclipse.gmf.runtime.notation.Size; +import org.eclipse.gmf.runtime.notation.Style; import org.eclipse.gmf.runtime.notation.View; import org.eclipse.gmf.runtime.notation.datatype.RelativeBendpoint; import org.eclipse.jface.resource.ImageDescriptor; @@ -73,6 +76,7 @@ import org.eclipse.sirius.diagram.ui.business.internal.query.DNodeContainerQuery; import org.eclipse.sirius.diagram.ui.business.internal.query.DNodeQuery; import org.eclipse.sirius.diagram.ui.edit.api.part.AbstractDiagramElementContainerEditPart; +import org.eclipse.sirius.diagram.ui.edit.api.part.IDiagramNameEditPart; import org.eclipse.sirius.diagram.ui.internal.edit.parts.AbstractDNodeContainerCompartmentEditPart; import org.eclipse.sirius.diagram.ui.internal.edit.parts.DNodeContainer2EditPart; import org.eclipse.sirius.diagram.ui.internal.edit.parts.DNodeList2EditPart; @@ -182,7 +186,7 @@ public static Dimension getTopLeftInsets(Node container) { // RegionContainer do not have containers insets if (ddec instanceof DNodeContainer) { if (new DNodeContainerExperimentalQuery((DNodeContainer) ddec).isRegionContainer() || hasFullLabelBorder(ddec)) { - result.setHeight(CONTAINER_INSETS.top + getLabelSize(container) + AbstractDiagramElementContainerEditPart.DEFAULT_SPACING); + result.setHeight(CONTAINER_INSETS.top + getLabelDimension(container, new Dimension(50, 20)).width() + AbstractDiagramElementContainerEditPart.DEFAULT_SPACING); } else { result.setWidth(CONTAINER_INSETS.left); result.setHeight(CONTAINER_INSETS.top); @@ -338,23 +342,6 @@ private static boolean hasFullLabelBorder(DDiagramElementContainer ddec) { return labelBorderStyle.some() && LabelBorderStyleIds.LABEL_FULL_BORDER_STYLE_FOR_CONTAINER_ID.equals(labelBorderStyle.get().getId()); } - private static int getLabelSize(Node parentNode) { - int labelSize = 0; - for (Node child : Iterables.filter(parentNode.getVisibleChildren(), Node.class)) { - if (new ViewQuery(child).isForNameEditPart()) { - // TODO Compute the real label height - // It depends on the font size - // It might require to set the layout constraint of the label - // GMF node which will not be used by the - // ConstrainedToolbarLayout to locate the label but might be - // usefull to store the value in the model. - labelSize = 16; - break; - } - } - return labelSize; - } - private static boolean isFirstRegion(DDiagramElementContainer ddec) { EObject potentialRegionContainer = ddec.eContainer(); if (potentialRegionContainer instanceof DNodeContainer) { @@ -1012,7 +999,16 @@ public static Dimension getLabelDimension(Node node, Dimension defaultDimension) if (!new DDiagramElementQuery(dDiagramElement).isLabelHidden()) { if (siriusStyle instanceof BasicLabelStyle) { BasicLabelStyle bls = (BasicLabelStyle) siriusStyle; - Font defaultFont = VisualBindingManager.getDefault().getFontFromLabelStyle(bls, (String) viewQuery.getDefaultValue(NotationPackage.Literals.FONT_STYLE__FONT_NAME)); + String fontName = (String) viewQuery.getDefaultValue(NotationPackage.Literals.FONT_STYLE__FONT_NAME); + Optional