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

Bug 154380

Summary:

Subgraph doesn't obey Graph's direction if EAST

Product:

[Tools] GEF

Reporter:

Craig Laurent <craiglaurent>

Component:

GEF-Legacy Draw2d

Assignee:

Alexander Nyßen <nyssen>

Status:

RESOLVED FIXED

QA Contact:

Severity:

major

Priority:

CC:

aboyko, ahunter.eclipse, nyssen

Version:

unspecified

Target Milestone:

3.7.1 (Indigo SR1)

Hardware:

All

OS:

All

Whiteboard:

Attachments:

Description	Flags
CompoundTransposeMetrics to fix the problem	none
Patch for CompoundDirectedGraphLayout, using CompoundTransposeMetrics	none

Description Craig Laurent

2006-08-18 12:49:49 EDT

When creating a CompoundDirectedGraph with Subgraphs, if the direction is EAST (not the default), the graph changes direction, but the Subgraphs do not.
I am using the current HEAD from dev.eclipse.org CVS.

This is easy to test by adding the following to any of the CompoundDirectedGraph examples (say CompoundGraphTests.aaaapull()):
graph.setDirection(PositionConstants.EAST);

e.g.:
CompoundDirectedGraph graph = new CompoundDirectedGraph();
graph.nodes = nodes;
graph.edges = edges;
graph.setDirection(PositionConstants.EAST);
new CompoundDirectedGraphLayout().visit(graph);

* I put this in SWT, as I couldn't find the Draw2D Component/category.

Comment 1 Craig Laurent

2006-08-18 14:41:28 EDT

Created attachment 48195 [details]
CompoundTransposeMetrics to fix the problem

Fixing this is just a matter of changing the CompoundDirectedGraphLayout to use a custom version of the TransposeMetrics.  The new version should flip the subgraphs...as the current TransposeMetrics already does for the Nodes and Edges.

Attached is a working example that could be added to the project, and then reference it from the CDGL.java instead of the TransposeMetrics.

problem solved.
(now just need to get the subgraphs to be able to have a different orientation from the regular graph!!)

Comment 2 Craig Laurent

2006-08-18 14:42:31 EDT

When do you think this fix will make it into the HEAD, and then the released product?

Comment 3 Craig Laurent

2006-08-21 08:57:42 EDT

Additionally, why are all of the parts of this formatter class security "friendly", such that I can't use them or subclass them?  e.g. TransposeMetrics.  I had to put this directly in the draw2d framework package.
This makes it very difficult to extend the capabilitied for custom needs.

We will be forced to have a private build of the codebase until this can make its way to a release.  If we could have subclassed and used this, we could have made a custom layout that worked just like the Compound one, with the customization of the Transposer.  But we can't.

Comment 4 Alex Boyko

2008-01-23 17:05:27 EST

I looked at the attached CompoundTransposeMetrics class and have a few questions:

1) Shouldn't we transpose subgraphs on the visit as well?
2) Shouldn't we transpose the children of the subgraph too?

Comment 5 Alex Boyko

2008-01-28 10:10:09 EST

The answer to both questions is no. The list of nodes on the graph object is a flat list of all nodes that belong to the graph. The proposed fix seems to be valid - the layout algorithm moves subgraphs from list of nodes to the list of subgraphs.

Comment 6 Anthony Hunter

2009-03-06 11:33:27 EST

Comment on attachment 48195 [details]
CompoundTransposeMetrics to fix the problem

Patch does not fix the issue.

Comment 7 Alexander Nyßen

2011-03-07 17:54:27 EST

Updated DirectedGraphDemo and CompoundGraphDemo within org.eclipse.draw2d.examples.graph to allow selection of graph direction via combo box for all examples.

Comment 8 Alexander Nyßen

2011-03-07 18:00:21 EST

Created attachment 190608 [details]
Patch for CompoundDirectedGraphLayout, using CompoundTransposeMetrics

For me, the following seems to work. I tested it with all examples (I modified them so one can easily change direction) and they all look good to me. However, Anthony, as you have rejected the former patch, I am not going to commit this directly, but will leave it to you to confirm its validity first.

Comment 9 Alexander Nyßen

2011-03-25 13:27:57 EDT

Anthony, can we still put this in for M7, or does it have to be postponed to post-Indigo?

Comment 10 Alexander Nyßen

2011-04-13 15:45:20 EDT

Removing milestone. As we are past API-freeze, this will have to go into 3.8.

Comment 11 Alexander Nyßen

2011-07-20 16:44:23 EDT

Applied patch to R3_7_maintenance branch as well as HEAD. Resolving as fixed.