Bug 141789 - [Layout] Arrange All should be called in generation code to initialize the diagram
Summary: [Layout] Arrange All should be called in generation code to initialize the di...
Status: NEW
Alias: None
Product: GMF-Runtime
Classification: Modeling
Component: General (show other bugs)
Version: 1.0   Edit
Hardware: PC Windows 2000
: P3 normal
Target Milestone: 2.1   Edit
Assignee: Anthony Hunter CLA
QA Contact:
URL:
Whiteboard:
Keywords: polish
Depends on:
Blocks:
 
Reported: 2006-05-15 11:54 EDT by Li Ding CLA
Modified: 2014-02-12 04:58 EST (History)
5 users (show)

See Also:


Attachments
possible fix #2 (14.31 KB, patch)
2006-08-12 14:22 EDT, Alex Boyko CLA
no flags Details | Diff
Layout Command fix (2.66 KB, patch)
2006-08-15 10:35 EDT, Alex Boyko CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Li Ding CLA 2006-05-15 11:54:22 EDT
When using ecore example editor to visualize ecore model, the diagram does not look nice when editor is up. The connections pass through the figures and very hard to read.  This feature is important to people using GMF to create a viewer instead of editor.

One way to do this by sending an ArrangeRequest to diagram editpart in editor's initializeGraphicalViewer(). I created a helper doAutoLayout() and called it from initializeGraphicalViewer():

/**
 *  Do automatically layout to initialize the diagram. 
 */
 protected void doAutoLayout() {
        
     // Create an ArrangeRequest
     ArrangeRequest arrangeRequest = new ArrangeRequest(ActionIds.ACTION_ARRANGE_ALL);
     ApplicationModelEditPart appModelEditPart = (ApplicationModelEditPart)getDiagramEditPart();
     List ep = new ArrayList();
     ep.add(appModelEditPart);
     arrangeRequest.setPartsToArrange(ep);
     // Get command for arrange all
     Command command = appModelEditPart.getCommand(arrangeRequest);
     getCommandStack().execute(command);
		
}

The ApplicationModelEditPart is my DiagramEditPart.
Comment 1 Alex Shatalin CLA 2006-06-08 05:35:06 EDT
Generated code initialize diagram using generated subclass of CanonicalConnectionEditPolicy, so CanonicalConnectionEditPolicy.refreshSemantic() method will be called. This method contains DeferredLayoutCommand execution for all the newly created EditParts (in case of initializing the diagram - all editparts located on diagram). The problem AFAIU is in implementation layout command - result of this command execution is not as good as result of ArrangeRequest execution on diagram opening.

DeferredLayoutCommand should be modified to produce the same result as "Arrange All" command if all the diagram elements was passed as a parameters to this request.

As a temporary workaround – suggested code snipped could be manually introduced into the generated code.
Comment 2 Anthony Hunter CLA 2006-08-02 16:19:27 EDT
I am bumping the priority of this defect up and moving back to 1.0.1.
Comment 3 Alex Boyko CLA 2006-08-11 16:27:35 EDT
The problem is in the generated EPackageCanonicalEditPolicy. ConnectionViews are not created before the DiferredLayoutCommand is executed, hence the diagram nodes are being layed out without considering connections between them. The solution for this bug would be the creation of connection views before the DifferedLayoutCommand is executed (see CanonicalConnectionEditPolicy). The CanonicalConnectionEditPolicy refreshSemantic method calls #refreshSemanticChildren which results in creating views for the nodes and #refreshSemanticConnections that should be creating views for connections. The #refreshSemanticConnections function calls #getSemanticConnectionsList function that should be implemented by clients and returning the list of connections' semantic elements. This method is implemented by EPackageCanonicalEditPolicy, but returns an empty list, hence views for connections are not created. (EPackageCanonicalEditPolicy creates them later on after the layout command is executed from #refreshSemantic method)

There are 2 ways to fix this:

1. Don't override #semanticRefresh, but override #refreshSemanticConnections. 
This function may do everything that #refreshConnections function from EPackagaCanonicalEditPolicy does, but returns the list of ConnectionViewDescriptors. (I've tried that and it works. I can attach a patch if needed)

2. Don't override #semanticRefresh, but override #getSemanticConnectionsList function that currently returns empty list. This will provide the super class (CanonicalConnectionEditPolicy) with connections' semantic elements for which the super class (CanonicalConnectionEditPolicy) will create the views. However, I'm not sure if it will work, since haven't tried it.

Comment 4 Alex Boyko CLA 2006-08-12 14:22:59 EDT
Created attachment 47815 [details]
possible fix #2

Here is the possible fix for the problem, i.e. what I think may be generated for the EPackageCanonicalEditPolicy. The fix tries to use the API suggested by the CanonicalConnectionEditPolicy as much as possible. This is the fix with overriding  #getSemanticConnectionsList. The #getSourceElement and #getTargetElement are also overriden, since super calss needs them to obtain source and target edit parts to create connection views. Even if super class is not refreshing connections optimally I think the generated code should be using the API provided by the super class as much as possible and we should optimizing the super class which is not being generated.
Comment 5 Alex Shatalin CLA 2006-08-14 11:05:03 EDT
You are right - this problem could be caused by calling layout command before creation of connections. Unfortunately generated code can not use existing CanonicalConnectionEditPolicy API due to a number of problems:

https://bugs.eclipse.org/bugs/show_bug.cgi?id=133505
https://bugs.eclipse.org/bugs/show_bug.cgi?id=150408

In any case, reassigning this request back – there should be a possibility to call layout command after creation of all the links. ;-)
Comment 6 Alex Boyko CLA 2006-08-15 10:35:28 EDT
Created attachment 47910 [details]
Layout Command fix

Aside the fix for the creation of connection views that needs to be done, there indeed was a difference between the execution of the deffered layout command and the "Arrange All" action. The difference was that with the layout command connection figures and their children were not revalidated, hence for a fresh diagram (that hasn't been layed out yet) labels had 0 dimensions and as a result connections were not spaced out nicely as for "Arrange All". The fix is a revalidation of connection figures during the execution of the deferred layout command. So with this patch and the fix of the connection views creation we'll match the "Arrange All" action's results.
Comment 7 Alex Shatalin CLA 2006-08-17 13:09:23 EDT
Generator was corrected to call layout command after link creation in both CVS branches (1.0.1 and HEAD).

Looks like a patch for DeferredLayoutCommand should be reviewed and commited by somabody fromthe runtime team. 

Changing component once more.
Comment 8 Alex Boyko CLA 2006-08-18 14:53:35 EDT
The patch for the layout command cannot be commited, since it drops the performance of visualizing by 20-25%. The difference seems to be caused by the layout routine performance when considering non-zero dimension connection labels and zero dimesnion connection labels or no labels at all. This problem will be solved when the layout routine will be reworked, most likely for 2.0 milestone.
Since a sufficient fix is already provided for this problem from the generator side, I think the target milestone for the defect can be changed to 2.0 as well as the title may become "layout routine needs improvement".
Comment 9 Anthony Hunter CLA 2006-08-31 10:52:19 EDT
Since a sufficient fix is already provided for this problem from the generator
side, I think the target milestone for the defect can be changed to 2.0 as well
as the title may become "layout routine needs improvement".
Comment 10 Anthony Hunter CLA 2007-06-19 11:43:38 EDT
Moving to the next release, GMF 2.1. 
Comment 11 Eclipse Webmaster CLA 2010-07-19 21:58:59 EDT
[GMF Restructure] Bug 319140 : product GMF and component
Runtime was the original product and component for this bug