Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Wrong layout for self-loop edge #425

Closed
lredor opened this issue May 23, 2019 · 5 comments
Closed

Wrong layout for self-loop edge #425

lredor opened this issue May 23, 2019 · 5 comments
Assignees
@le-cds
Labels
alg-layered Affects the ELK Layered algorithm. invalid This is not in fact an issue.
Milestone
Release 0.6.0

Comments

@lredor
Copy link
Contributor

lredor commented May 23, 2019

The layout of self loop edge is wrong when using org.eclipse.elk.layered algoriythm. It seems shifted with the coordinates of its container.

Steps to reproduce:

  • Open SampleWithEdgeInsideSelfLoop.elkg in the Layout Graph view (from SampleWithEdgeInsideSelfLoop.zip). The initial graph, without layout, is like
    SampleWithEdgeInsideSelfLoop-initial
  • The layout result is like this graph
    SampleWithEdgeInsideSelfLoop-result
    The coordinates of the edge is
  <sections startX="27.0" startY="47.0" endX="49.0" endY="47.0">
    <bendPoints x="26.999999999999996" y="37.0"/>
    <bendPoints x="49.0" y="37.0"/>
  </sections>
  • But the expected result is like this graph
    SampleWithEdgeInsideSelfLoop-expected
    The coordinates should be shifted with its container coordinates {5.0, 47.0}.
  <sections startX="22.0" startY="0.0" endX="44.0" endY="0.0">
    <bendPoints x="21.999999999999996" y="37.0"/>
    <bendPoints x="44.0" y="37.0"/>
  </sections>

This issue is maybe linked to ELK#420.
@lredor lredor mentioned this issue May 23, 2019
Closed
@le-cds le-cds self-assigned this Jul 30, 2019
@le-cds le-cds added alg-layered Affects the ELK Layered algorithm. bug Erroneous behaviour. labels Jul 30, 2019
@le-cds le-cds mentioned this issue Aug 8, 2019
Closed
@le-cds
Copy link
Contributor

le-cds commented Oct 23, 2019

Fixed as soon as #446 is on master.

@le-cds le-cds closed this as completed Oct 23, 2019
@le-cds le-cds added this to the Release 0.6.0 milestone Nov 21, 2019
@lredor lredor mentioned this issue Dec 19, 2019
Closed
@lredor
Copy link
Contributor Author

lredor commented Dec 20, 2019

This issue is not resolved. The result is different but is not OK:
CurrentResultOnMaster

@le-cds
Copy link
Contributor

le-cds commented Feb 7, 2020

@lredor This is strange. At least with what's currently on the master, I'm getting this result:

result

Please verify the results you're getting. I'm reopening the issue until I have confirmation that the bug doesn't exist anymore, or that I need to take another look.

@le-cds le-cds reopened this Feb 7, 2020
@lredor
Copy link
Contributor Author

lredor commented Feb 7, 2020

I verified and the problem is always here. I searched to understand why you have not the same result. With the elkg file in the zip file of description, the result is:
originalUseCase

In this file, the edge is contained in the Node1. That seems normal as both source and target are "in" the node. I updated the sample to have the edge contained in the Container1. And with this version, I have the same result as you:
updatedUseCase

My question: Does an inside loop edge "must" be contained in the container of the node and not directly in the node? If the answer is "Yes", you can close this issue, the problem is in the structure of the graph. If the answer is "No", there is probably a bug in the code for this case.

The 2 versions of the elkg is in this file SampleWithEdgeInsideSelfLoop-2versions.zip:
SampleWithEdgeInsideSelfLoop-2versions.zip

@le-cds le-cds added invalid This is not in fact an issue. and removed bug Erroneous behaviour. labels Feb 11, 2020
@le-cds
Copy link
Contributor

le-cds commented Feb 11, 2020

That is the problem indeed: self loops must be contained in the parent of the node the self loops loops around. ElkGraphUtil.updateContainment(...) can help find the proper containment for edges. I've updated the documentation to include a paragraph on where self loops are contained.

@le-cds le-cds closed this as completed Feb 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@le-cds le-cds

Labels
alg-layered Affects the ELK Layered algorithm. invalid This is not in fact an issue.
Projects
None yet
Milestone
Release 0.6.0
Development

No branches or pull requests
2 participants
@lredor @le-cds