Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 495707

Summary:

Compartment layout problem when addition of semantic elements involved a refresh of a diagram

Product:

[Modeling] Sirius

Reporter:

Laurent Redor <laurent.redor>

Component:

Core

Assignee:

Laurent Redor <laurent.redor>

Status:

CLOSED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

julien.dupont

Version:

3.1.0

Keywords:

triaged

Target Milestone:

4.1.0

Hardware:

All

OS:

All

See Also:

Whiteboard:

backport

Bug Depends on:

Bug Blocks:

492607, 497398, 499831, 501082

Attachments:

Description	Flags
CompartmentsLayoutProblem.zip	none
setBoundsStacksForClassicalContainer.txt	none
setBoundsStacksForClassicalContainer-abstract.txt	none
setBoundsStacksForRegionsContainer.txt	none
setBoundsStacksForRegionsContainer-abstract.txt	none
setBoundsStacksForClassicalContainer-abstract.txt	none
setBoundsStacksForRegionsContainer-abstract.txt	none
CompartmentsLayoutProblem2.zip	none

Description Laurent Redor

2016-06-08 11:51:06 EDT

Created attachment 262317 [details]
CompartmentsLayoutProblem.zip

There is a compartment layout problem when addition of semantic elements involved a refresh of a diagram.

Steps to reproduce:
* Import the project CompartimentsLayoutProblem from attached archive file
* Open the diagram "HStackDiag"
* Open the file My.ecore with the "Sample Ecore Model Editor"
* Copy the package "P1" in package "root" and rename it to "P2"
* Save the ecore file
* The diagram "HStackDiag" is refreshed:
** The new package P2 is created: OK
** The existing package P1 is auto-sized: KO. There is also a bug in the size of the container "P1" or its region "Left_class1" (the bug 495046 corresponds to this other problem).

If you do the same with the diagram "HStackDiagPinned" there is no problem. The difference between the 2 diagrams is that the package P1 is pinned in "HStackDiagPinned".

Comment 1 Julien Dupont

2016-06-09 06:41:02 EDT

I must click on refresh toolbar icon to refresh the diagram representation.

Comment 2 Laurent Redor

2016-06-13 04:48:36 EDT

Created attachment 262389 [details]
setBoundsStacksForClassicalContainer.txt

This file is the first of a serie of fourth to facilitate the analysis of the difference between a classical container and a regions container:
* setBoundsStacksForClassicalContainer.txt: Stacks corresponding to creation or execution of a SetBoundsCommand for a classical container
* setBoundsStacksForClassicalContainer-abstract.txt: Abstract of the above file
* setBoundsStacksForRegionsContainer.txt: Stacks corresponding to creation or execution of a SetBoundsCommand for a regions container
* setBoundsStacksForRegionsContainer-abstract.txt: Abstract of the above file

Comment 3 Laurent Redor

2016-06-13 04:48:54 EDT

Created attachment 262390 [details]
setBoundsStacksForClassicalContainer-abstract.txt

Comment 4 Laurent Redor

2016-06-13 04:49:13 EDT

Created attachment 262391 [details]
setBoundsStacksForRegionsContainer.txt

Comment 5 Laurent Redor

2016-06-13 04:49:28 EDT

Created attachment 262392 [details]
setBoundsStacksForRegionsContainer-abstract.txt

Comment 6 Laurent Redor

2016-06-13 05:18:32 EDT

Created attachment 262394 [details]
setBoundsStacksForClassicalContainer-abstract.txt

Comment 7 Laurent Redor

2016-06-13 05:19:03 EDT

Created attachment 262395 [details]
setBoundsStacksForRegionsContainer-abstract.txt

Comment 8 Laurent Redor

2016-06-13 05:28:10 EDT

By comparing the two abstract files, we can see that:
* The auto size is not handled by the same way (it is probably expected).
* The request REQ_MOVE is not send to NodeList C1 for ArrangeAllWithAutoSize.createSubCommands as the condition in line 308 returns false. This is caused by the margin that is set to 0 for Region in DiagramLayoutCustomization.getNodePadding(GraphicalEditPart). This seems also OK.
* The CommandWrapper returned by the PinnedElementsLayoutProvider is unexecutable for NodeList C1 (and so for all commands). So for the case of regions container, the pinned elements (or those to be considered as such, like C1 and P1) are not restored with their initial location and size. I think this point is the cause of the problem.

Comment 9 Laurent Redor

2016-06-13 07:04:58 EDT

Indeed, the problem is arround the PinnedElementsLayoutProvider and the MOVE request ignored by RegionResizableEditPolicy. The MOVE request is ignored by the RegionResizableEditPolicy as it is forbidden to move a region. But this is a specific case of MOVE request. The goal of this request is to reset bounds (location AND size) of the edit part after the arrange selection of new elements (an arrange all is launched and then there is a "revert" of the not concerned edit part).

The solution is to authorized the MOVE request for Region only in some circumstances (reset from PinnedElementsLayoutProvider for example).

Comment 10 Eclipse Genie

2016-06-13 08:10:18 EDT

New Gerrit change created: https://git.eclipse.org/r/75147

Comment 11 Eclipse Genie

2016-06-13 08:10:22 EDT

New Gerrit change created: https://git.eclipse.org/r/75146

Comment 12 Eclipse Genie

2016-06-14 15:55:26 EDT

New Gerrit change created: https://git.eclipse.org/r/75276

Comment 13 Eclipse Genie

2016-06-16 02:15:14 EDT

Gerrit change https://git.eclipse.org/r/75147 was merged to [master].
Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=0e6c68281db6a9c0e2495acdc669aea0d306f846

Comment 14 Eclipse Genie

2016-06-16 02:15:22 EDT

Gerrit change https://git.eclipse.org/r/75146 was merged to [master].
Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=4c22f9b8234cafc16a93dea18dfe491194571342

Comment 15 Eclipse Genie

2016-06-16 02:15:28 EDT

Gerrit change https://git.eclipse.org/r/75276 was merged to [master].
Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=974473d9916f48b95252193319c9e7302f0c8e3e

Comment 16 Laurent Redor

2016-06-16 10:34:32 EDT

The previous commits reduce the number of call to RegionContainerUpdateLayoutOperation. But at least one is wrongly removed.

Steps to reproduce:
* Import the project CompartimentsLayoutProblem from attached archive file (CompartimentsLayoutProblem2.zip)
* Open the diagram "VStackDiag"
* Open the diagram "HStackDiag"
* Delete "Left_class1" from "HStackDiag"
* Set the focus on "VStackDiag": KO, the layout of the 2 remaining regions are not done. There is a blank space at the location of "Left_class1".

Comment 17 Laurent Redor

2016-06-16 10:35:08 EDT

Created attachment 262496 [details]
CompartmentsLayoutProblem2.zip

Comment 18 Eclipse Genie

2016-06-16 12:06:17 EDT

New Gerrit change created: https://git.eclipse.org/r/75413

Comment 19 Laurent Redor

2016-06-16 12:08:44 EDT

The gerrit https://git.eclipse.org/r/#/c/75413/1 adds tests that reveal the problem.

Comment 20 Eclipse Genie

2016-06-16 12:33:25 EDT

New Gerrit change created: https://git.eclipse.org/r/75418

Comment 21 Eclipse Genie

2016-06-17 11:45:02 EDT

New Gerrit change created: https://git.eclipse.org/r/75476

Comment 22 Eclipse Genie

2016-06-20 08:52:44 EDT

Gerrit change https://git.eclipse.org/r/75476 was merged to [master].
Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=ec188e6debb715e47313ae5d045d4de1457b4a06

Comment 23 Eclipse Genie

2016-06-20 08:52:58 EDT

Gerrit change https://git.eclipse.org/r/75413 was merged to [master].
Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=3f878ffc2cc51edc98e3192f1c4d677ccafdc19a

Comment 24 Eclipse Genie

2016-06-20 08:53:00 EDT

Gerrit change https://git.eclipse.org/r/75418 was merged to [master].
Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=b943ce8c07aac76207f83438859d6d1314fcd310

Comment 25 Laurent Redor

2016-06-20 08:53:57 EDT

The 3 last commits fix the regression of comment 16.

Comment 26 Pierre-Charles David

2016-10-18 11:08:42 EDT

Available in Sirius 4.1.0, see https://wiki.eclipse.org/Sirius/4.1.0 for details.