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.

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

Do not set bend points on elk edge when transforming from Sprotty edge #436

Closed
wants to merge 1 commit into from

Conversation

jbicker
Copy link
Contributor

@jbicker jbicker commented Feb 21, 2024

Fixes #427

The bend points are now completely neglected in the edge transformation from SEdge to an ElkExtendedEdge. So elk sets completely new bend points.

Feels a bit like a naive fix but it works at least for the random graph example. I hope it does not break something else.

Copy link
Contributor

@spoenemann spoenemann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In some cases, an ELK layout algorithm might use the previous bend points in its computations, so we shouldn't remove them. Could you try to fix it in the other direction, making sure that bend points are erased in the SModel when ELK doesn't specify any?

when transforming from Sprotty edge

Signed-off-by: Jan Bicker <[email protected]>
@jbicker
Copy link
Contributor Author

jbicker commented Feb 21, 2024

@spoenemann, thanks for the hint. I did as you proposed.

@spoenemann
Copy link
Contributor

Hmmm, it seems to me that the root of the problem is somewhere else. The elkjs library uses this ELK Java/Xtend class method to transfer the layout algorithm results to the JSON data:
https://github.com/kieler/elkjs/blob/72b471e65f5c5d7b0e94450b14c14bdf12384f82/src/java/org/eclipse/elk/js/ElkJs.java#L206

I created eclipse-elk/elk#1001 to discuss and possibly fix this in ELK.

@jbicker
Copy link
Contributor Author

jbicker commented Feb 22, 2024

Could this PR then be closed?

@spoenemann
Copy link
Contributor

I think so yes. The current solution smells like a workaround for a problem that lies somewhere else.

@spoenemann spoenemann closed this Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sprotty-elk sometimes doesn't delete old edge bends on layout changes
2 participants