eclipse / elk

Eclipse Layout Kernel - Automatic layout for Java applications.
https://www.eclipse.org/elk/
Other
246 stars 82 forks source link

Self-loop not layered properly with ELK Layered algorithm #469

Closed vrichard12 closed 4 years ago

vrichard12 commented 4 years ago

Here is attached an example of an elkg file to reproduce the issue. The middle node has 3 self-loops crossing other edges and with their labels overlaping each others.

Self-loop-elk-layered-issue-example

Self-loop-elk-layered-issue-example.elkg.zip

le-cds commented 4 years ago

@vrichard12 I tested your example with our new self loop implementation:

469

I'd count that as an expected result. I do remember you saying that you tried out the current master. However, we found out just a few days ago that the master did not build successfully since I merged the new self loop code -- without the server yelling at me. If I remember correctly, could you please try again with the current nightly builds?

lredor commented 4 years ago

@le-cds Hello, I've tested the layout of the diagram from Self-loop-elk-layered-issue-example.elkg.zip. And the result is always the same (same than the initial picture). Tested on:

Are you sure that you have changed nothing in the layout option from diagram in Self-loop-elk-layered-issue-example.elkg.zip? I see no reason to have different result than yours.

le-cds commented 4 years ago

@lredor That is massively strange. I changed nothing (even downloaded the file again just now to be sure), and my result was what I posted. I'm not sure what's wrong here.

Regarding the update site: it contained version 0.7.0 because of a misconfiguration on my part: the release branch build checked out the master branch by mistake, which already moved to 0.7.0...

lredor commented 4 years ago

@le-cds With a new install of Eclipse 201909 and https://build.eclipse.org/modeling/elk/updates/0.6.0/ the result is always the initial result. Even in ELKLive, I have the same result. What can I do to try to explain the difference with your result?

lredor commented 4 years ago

OK, I have the same result than yours. But for that I have to modified the initial elkg file. Indeed, the 3 self loop edges are not "declared" in the correct node. They are children of node "Mvvfxvmnghdvo upjocli OdhkDxtbd" and not of the diagram. Here is the "fixed" version of the elg file: Self-loop-elk-layered-issue-example.elkg-2.zip.

We can considered this issue as fixed. Thx

le-cds commented 4 years ago

Ah, edge containment is an important point. I'll revert my milestone change, which was completely unnecessary...