The internal method GMFHelper.getAbsoluteBounds(Node) can be improved for containers with auto-size #426

lredor · 2024-06-26T16:58:50Z

As a reminder, this method attempts to calculate, based on the GMF data, a dimension as close as possible to the dimension that the corresponding Draw2D figure will have.

In several cases, the result is false. This method is used, for example, in the Debug view.

Here are some samples. They should probably be completed with list containers and, Vertical or Horizontal Stack containers.

lredor · 2024-06-26T17:00:52Z

The above examples can be used as starting point to check the results. The corresponding project is here: Issue426-ProjectSample.zip.

Several points are handled in this commit. It should maybe split in several commit to facilitate reviewing... * 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. * Other cases to details Bug: #426

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: #426

Bug: #426

* Add method getBottomRightInsets to use it in auto-size computation * Consider shadow border * Consider border nodes when computing children bottom-right corner. Bug: #426

This compartment is an intermediate GMF Node and should not have the same behavior when computing auto-size: - Minimal size defined by ResizableCompartmentFigure.MIN_CLIENT_DP - No shadow border Bug: #426

Before this commit, this kind of container was not correctly handled. Bug: #426

Bug: #426

Before this commit, this kind of container was not correctly handled. Bug: #426

Bug: #426

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: #426

Bug: #426

* Add method getBottomRightInsets to use it in auto-size computation * Consider shadow border * Consider border nodes when computing children bottom-right corner. Bug: #426

This compartment is an intermediate GMF Node and should not have the same behavior when computing auto-size: - Minimal size defined by ResizableCompartmentFigure.MIN_CLIENT_DP - No shadow border Bug: #426

Before this commit, this kind of container was not correctly handled. Bug: #426

Bug: #426

Before this commit, this kind of container was not correctly handled. Bug: #426

Bug: #426

Before this commit, this kind of container was not correctly handled. Bug: #426

Bug: #426

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: #426

Bug: #426

* Add method getBottomRightInsets to use it in auto-size computation * Consider shadow border * Consider border nodes when computing children bottom-right corner. Bug: #426

This compartment is an intermediate GMF Node and should not have the same behavior when computing auto-size: - Minimal size defined by ResizableCompartmentFigure.MIN_CLIENT_DP - No shadow border Bug: #426

Before this commit, this kind of container was not correctly handled. Bug: #426

The code will be more logic with this refactoring and facilitate the following changes. Bug: #426

This compartment is an intermediate GMF Node and should not have the same behavior when computing auto-size: - Minimal size defined by ResizableCompartmentFigure.MIN_CLIENT_DP - No shadow border Bug: #426

Before this commit, this kind of container was not correctly handled. Bug: #426

Bug: #426

This test was based on some wrong bounds. The edge has been slightly moved to retrieve the previous edge bendpoints number after collapse. Bug: #426

The bounds for a node can be computed several times for a parent or a child bounds calculation. This commit adds a cache to avoid computing it again. Bug: #426

Analysis: It seems that before, the code with comment "manage location of bordered node with closest side", in GMFHelper.getAbsoluteLocation, is called only in specific circumstances. Since recent changes, it was called systematically, but that shouldn't be the case. This commit also reduces the number of getAbsoluBounds methods to ease the impact analysis. Bug: #426

The bounds for a node can be computed several times for a parent or a child bounds calculation. This commit adds a cache to avoid computing it again. Bug: #426

Analysis: It seems that before, the code with comment "manage location of bordered node with closest side", in GMFHelper.getAbsoluteLocation, is called only in specific circumstances. Since recent changes, it was called systematically, but that shouldn't be the case. This commit also reduces the number of getAbsoluBounds methods to ease the impact analysis. Bug: #426

lredor · 2024-09-12T08:37:29Z

After several checks, the PR #427 is now OK to be reviewed.
To validate this issue, you can check the initial description and the initial project. You can also validate the 24 tests added in org.eclipse.sirius.tests.swtbot.layout.GMFHelperTest.
All tests have also been launched locally to detect potential regressions (in progress).

Analysis: It seems that before, the code with comment "manage location of bordered node with closest side", in GMFHelper.getAbsoluteLocation, is called only in specific circumstances. Since recent changes, it was called systematically, but that shouldn't be the case. This commit also reduces the number of getAbsoluBounds methods to ease the impact analysis. Bug: #426

Bug: #426

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: #426

Bug: #426

* Add method getBottomRightInsets to use it in auto-size computation * Consider shadow border * Consider border nodes when computing children bottom-right corner. Bug: #426

Bug: #426

The code will be more logic with this refactoring and facilitate the following changes. Bug: #426

This compartment is an intermediate GMF Node and should not have the same behavior when computing auto-size: - Minimal size defined by ResizableCompartmentFigure.MIN_CLIENT_DP - No shadow border Bug: #426

Before this commit, this kind of container was not correctly handled. Bug: #426

Bug: #426

This test was based on some wrong bounds. The edge has been slightly moved to retrieve the previous edge bendpoints number after collapse. Bug: #426

The bounds for a node can be computed several times for a parent or a child bounds calculation. This commit adds a cache to avoid computing it again. Bug: #426

Analysis: It seems that before, the code with comment "manage location of bordered node with closest side", in GMFHelper.getAbsoluteLocation, is called only in specific circumstances. Since recent changes, it was called systematically, but that shouldn't be the case. This commit also reduces the number of getAbsoluBounds methods to ease the impact analysis. Bug: #426

lredor changed the title ~~The internal method GMFHelper.getAbsoluteBounds(Node, boolean) can be improved for containers with auto-size~~ The internal method GMFHelper.getAbsoluteBounds(Node) can be improved for containers with auto-size Jun 26, 2024

lredor linked a pull request Jun 26, 2024 that will close this issue

[426] Improve GMFHelper.getAbsoluteBounds method #427

Open

lredor added a commit that referenced this issue Jul 17, 2024

[426] Refactor getContainerTopLeftInsets without change

c27f246

Bug: #426

lredor added a commit that referenced this issue Jul 17, 2024

[426] Handle List container and list items in GMFHelper bounds computing

0eb5b9b

Before this commit, this kind of container was not correctly handled. Bug: #426

lredor added a commit that referenced this issue Jul 17, 2024

[426] Use the font defined in the GMF node to compute label size

f3ec948

Bug: #426

lredor added a commit that referenced this issue Jul 17, 2024

[426] Handle vertical stack container

7d5ef49

Before this commit, this kind of container was not correctly handled. Bug: #426

lredor added a commit that referenced this issue Jul 17, 2024

[426] Add tests

1f6282d

Bug: #426

lredor self-assigned this Jul 17, 2024

lredor added a commit that referenced this issue Jul 26, 2024

[426] Add tests to reveal the problems

e460632

Bug: #426

lredor added a commit that referenced this issue Jul 26, 2024

[426] Refactor getContainerTopLeftInsets without change

cfdc3ce

Bug: #426

lredor added a commit that referenced this issue Jul 26, 2024

[426] Handle List container and list items in GMFHelper bounds computing

1d877b0

Before this commit, this kind of container was not correctly handled. Bug: #426

lredor added a commit that referenced this issue Jul 26, 2024

[426] Use the font defined in the GMF node to compute label size

ba99866

Bug: #426

lredor added a commit that referenced this issue Jul 26, 2024

[426] Handle vertical stack container

cea7cf9

Before this commit, this kind of container was not correctly handled. Bug: #426

lredor added a commit that referenced this issue Jul 26, 2024

[426] Refactor to focus on 2 main ideas: locations and bounds

6b1a6ce

Bug: #426

lredor added a commit that referenced this issue Jul 26, 2024

[426] Handle horizontal stack container

f83cf18

Before this commit, this kind of container was not correctly handled. Bug: #426

lredor added a commit that referenced this issue Jul 29, 2024

[426] Add tests to reveal the problems

795c94f

Bug: #426

lredor added a commit that referenced this issue Jul 29, 2024

[426] Refactor getContainerTopLeftInsets without change

0cf0236

Bug: #426

lredor added a commit that referenced this issue Jul 29, 2024

[426] Handle List container and list items in GMFHelper bounds computing

26ddad8

Before this commit, this kind of container was not correctly handled. Bug: #426

lredor added a commit that referenced this issue Sep 10, 2024

[426] Refactor to focus on 2 main ideas: absolute locations and bounds

4ccb5fc

The code will be more logic with this refactoring and facilitate the following changes. Bug: #426

lredor added a commit that referenced this issue Sep 10, 2024

[426] Handle List container and list items

9f7b3ff

Before this commit, this kind of container was not correctly handled. Bug: #426

lredor added a commit that referenced this issue Sep 10, 2024

[426] Handle vertical stack container

f65a8fa

Before this commit, this kind of container was not correctly handled. Bug: #426

lredor added a commit that referenced this issue Sep 10, 2024

[426] Handle horizontal stack container

01a8883

Before this commit, this kind of container was not correctly handled. Bug: #426

lredor added a commit that referenced this issue Sep 10, 2024

[426] Handle border nodes more precisely

f0e5c0d

Bug: #426

lredor added a commit that referenced this issue Sep 10, 2024

[426] Fix EdgeStabilityOnPortCollapsingTest

10fcebf

This test was based on some wrong bounds. The edge has been slightly moved to retrieve the previous edge bendpoints number after collapse. Bug: #426

lredor added a commit that referenced this issue Sep 10, 2024

[426] Add cache for bounds

0b5b4e8

The bounds for a node can be computed several times for a parent or a child bounds calculation. This commit adds a cache to avoid computing it again. Bug: #426

lredor added a commit that referenced this issue Sep 11, 2024

[426] Add cache for bounds

f002c92

The bounds for a node can be computed several times for a parent or a child bounds calculation. This commit adds a cache to avoid computing it again. Bug: #426

lredor mentioned this issue Sep 12, 2024

PR to fix the wrong border node location creation problem #404

Closed

pcdavid added this to the v8.0.0 milestone Sep 12, 2024

lredor added a commit that referenced this issue Sep 17, 2024

[426] Add tests to reveal the problems

319df2a

Bug: #426

lredor added a commit that referenced this issue Sep 17, 2024

[426] Refactor getContainerTopLeftInsets without change

8eb52ab

Bug: #426

lredor added a commit that referenced this issue Sep 17, 2024

[426] Use the font defined in the GMF node to compute label dimension

ae9308d

Bug: #426

lredor added a commit that referenced this issue Sep 17, 2024

[426] Refactor to focus on 2 main ideas: absolute locations and bounds

9926d61

The code will be more logic with this refactoring and facilitate the following changes. Bug: #426

lredor added a commit that referenced this issue Sep 17, 2024

[426] Handle List container and list items

83171d3

Before this commit, this kind of container was not correctly handled. Bug: #426

lredor added a commit that referenced this issue Sep 17, 2024

[426] Handle vertical stack container

4b9a350

Before this commit, this kind of container was not correctly handled. Bug: #426

lredor added a commit that referenced this issue Sep 17, 2024

[426] Handle horizontal stack container

e16d426

Before this commit, this kind of container was not correctly handled. Bug: #426

lredor added a commit that referenced this issue Sep 17, 2024

[426] Handle border nodes more precisely

f323cac

Bug: #426

lredor added a commit that referenced this issue Sep 17, 2024

[426] Fix EdgeStabilityOnPortCollapsingTest

918e7ca

This test was based on some wrong bounds. The edge has been slightly moved to retrieve the previous edge bendpoints number after collapse. Bug: #426

lredor added a commit that referenced this issue Sep 17, 2024

[426] Add cache for bounds

45c05a9

The bounds for a node can be computed several times for a parent or a child bounds calculation. This commit adds a cache to avoid computing it again. Bug: #426

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

The internal method GMFHelper.getAbsoluteBounds(Node) can be improved for containers with auto-size #426

The internal method GMFHelper.getAbsoluteBounds(Node) can be improved for containers with auto-size #426

lredor commented Jun 26, 2024

lredor commented Jun 26, 2024

lredor commented Sep 12, 2024

The internal method GMFHelper.getAbsoluteBounds(Node) can be improved for containers with auto-size #426

The internal method GMFHelper.getAbsoluteBounds(Node) can be improved for containers with auto-size #426

Comments

lredor commented Jun 26, 2024

lredor commented Jun 26, 2024

lredor commented Sep 12, 2024