Bug 495707 - Compartment layout problem when addition of semantic elements involved a refresh of a diagram
Summary: Compartment layout problem when addition of semantic elements involved a refr...
Status: CLOSED FIXED
Alias: None
Product: Sirius
Classification: Modeling
Component: Core (show other bugs)
Version: 3.1.0   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 4.1.0   Edit
Assignee: Laurent Redor CLA
QA Contact:
URL:
Whiteboard: backport
Keywords: triaged
Depends on:
Blocks: 492607 497398 499831 501082
  Show dependency tree
 
Reported: 2016-06-08 11:51 EDT by Laurent Redor CLA
Modified: 2016-10-18 11:08 EDT (History)
1 user (show)

See Also:


Attachments
CompartmentsLayoutProblem.zip (5.34 KB, application/x-zip-compressed)
2016-06-08 11:51 EDT, Laurent Redor CLA
no flags Details
setBoundsStacksForClassicalContainer.txt (68.30 KB, text/plain)
2016-06-13 04:48 EDT, Laurent Redor CLA
no flags Details
setBoundsStacksForClassicalContainer-abstract.txt (2.20 KB, text/plain)
2016-06-13 04:48 EDT, Laurent Redor CLA
no flags Details
setBoundsStacksForRegionsContainer.txt (37.59 KB, text/plain)
2016-06-13 04:49 EDT, Laurent Redor CLA
no flags Details
setBoundsStacksForRegionsContainer-abstract.txt (1.29 KB, text/plain)
2016-06-13 04:49 EDT, Laurent Redor CLA
no flags Details
setBoundsStacksForClassicalContainer-abstract.txt (2.32 KB, text/plain)
2016-06-13 05:18 EDT, Laurent Redor CLA
no flags Details
setBoundsStacksForRegionsContainer-abstract.txt (1.78 KB, text/plain)
2016-06-13 05:19 EDT, Laurent Redor CLA
no flags Details
CompartmentsLayoutProblem2.zip (6.19 KB, application/x-zip-compressed)
2016-06-16 10:35 EDT, Laurent Redor CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Laurent Redor CLA 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 CLA 2016-06-09 06:41:02 EDT
I must click on refresh toolbar icon to refresh the diagram representation.
Comment 2 Laurent Redor CLA 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 CLA 2016-06-13 04:48:54 EDT
Created attachment 262390 [details]
setBoundsStacksForClassicalContainer-abstract.txt
Comment 4 Laurent Redor CLA 2016-06-13 04:49:13 EDT
Created attachment 262391 [details]
setBoundsStacksForRegionsContainer.txt
Comment 5 Laurent Redor CLA 2016-06-13 04:49:28 EDT
Created attachment 262392 [details]
setBoundsStacksForRegionsContainer-abstract.txt
Comment 6 Laurent Redor CLA 2016-06-13 05:18:32 EDT
Created attachment 262394 [details]
setBoundsStacksForClassicalContainer-abstract.txt
Comment 7 Laurent Redor CLA 2016-06-13 05:19:03 EDT
Created attachment 262395 [details]
setBoundsStacksForRegionsContainer-abstract.txt
Comment 8 Laurent Redor CLA 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 CLA 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 CLA 2016-06-13 08:10:18 EDT
New Gerrit change created: https://git.eclipse.org/r/75147
Comment 11 Eclipse Genie CLA 2016-06-13 08:10:22 EDT
New Gerrit change created: https://git.eclipse.org/r/75146
Comment 12 Eclipse Genie CLA 2016-06-14 15:55:26 EDT
New Gerrit change created: https://git.eclipse.org/r/75276
Comment 16 Laurent Redor CLA 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 CLA 2016-06-16 10:35:08 EDT
Created attachment 262496 [details]
CompartmentsLayoutProblem2.zip
Comment 18 Eclipse Genie CLA 2016-06-16 12:06:17 EDT
New Gerrit change created: https://git.eclipse.org/r/75413
Comment 19 Laurent Redor CLA 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 CLA 2016-06-16 12:33:25 EDT
New Gerrit change created: https://git.eclipse.org/r/75418
Comment 21 Eclipse Genie CLA 2016-06-17 11:45:02 EDT
New Gerrit change created: https://git.eclipse.org/r/75476
Comment 25 Laurent Redor CLA 2016-06-20 08:53:57 EDT
The 3 last commits fix the regression of comment 16.
Comment 26 Pierre-Charles David CLA 2016-10-18 11:08:42 EDT
Available in Sirius 4.1.0, see https://wiki.eclipse.org/Sirius/4.1.0 for details.