editor.diagram: Expose more ELK options

[alexanderpann]

Support for 10 new layout algorithms was added. Most of the options of the ELK layouter (150 options) can be customized through style class items. All options can also be set in the inspector of the chosen layout algorithm in the diagram cell. For our unstable layouting, the option interactive might be useful to not get so drastic changes when running the layouter again. I tested all the wiring of the options to the layouters and the sandbox and checked that all options have the correct default values. From a review perspective, just playing around with the editor and checking the non-option-related code changes should be enough. We have to check that it is impossible that an error is thrown in the editor and that the editor breaks.

Reference:

• https://eclipse.dev/elk/reference/algorithms.html

The new features can be tested in the model de.itemis.mps.editor.diagram.demo.elk.sandbox:

[ratiud]

@alexanderpann thank you for this extension ! I am playing with the examples and I get the following exception if direction is "undefined"

java.lang.RuntimeException: Unknown direction: UNDEFINED at de.itemis.mps.editor.diagram.runtime.jgraph.ElkLayouter.createDummyPort(ElkLayouter.java:496) at de.itemis.mps.editor.diagram.runtime.jgraph.ElkLayouter.buildElement(ElkLayouter.java:241) at de.itemis.mps.editor.diagram.runtime.jgraph.ElkLayouter.buildElementIfRequired(ElkLayouter.java:213) at de.itemis.mps.editor.diagram.runtime.jgraph.ElkLayouter.buildKGraph(ElkLayouter.java:286) at de.itemis.mps.editor.diagram.runtime.jgraph.ElkLayouter.layoutOnce(ElkLayouter.java:187) at de.itemis.mps.editor.diagram.runtime.jgraph.ElkLayouter.layout(ElkLayouter.java:150) at de.itemis.mps.editor.diagram.runtime.jgraph.BaseDiagramDCell$1.run(BaseDiagramDCell.java:139) at com.intellij.openapi.progress.impl.CoreProgressManager.startTask(CoreProgressManager.java:442) at com.intellij.openapi.progress.impl.ProgressManagerImpl.startTask(ProgressManagerImpl.java:114) at com.intellij.openapi.progress.impl.CoreProgressManager.lambda$runProcessWithProgressAsynchronously$5(CoreProgressManager.java:493) at com.intellij.openapi.progress.impl.ProgressRunner.lambda$submit$3(ProgressRunner.java:252) at com.intellij.openapi.progress.impl.CoreProgressManager.lambda$runProcess$2(CoreProgressManager.java:188) at com.intellij.openapi.progress.impl.CoreProgressManager.lambda$executeProcessUnderProgress$12(CoreProgressManager.java:608) at com.intellij.openapi.progress.impl.CoreProgressManager.registerIndicatorAndRun(CoreProgressManager.java:683) at com.intellij.openapi.progress.impl.CoreProgressManager.computeUnderProgress(CoreProgressManager.java:639) at com.intellij.openapi.progress.impl.CoreProgressManager.executeProcessUnderProgress(CoreProgressManager.java:607) at com.intellij.openapi.progress.impl.ProgressManagerImpl.executeProcessUnderProgress(ProgressManagerImpl.java:60) at com.intellij.openapi.progress.impl.CoreProgressManager.runProcess(CoreProgressManager.java:175) at com.intellij.openapi.progress.impl.ProgressRunner.lambda$submit$4(ProgressRunner.java:252) at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1768) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) at java.base/java.util.concurrent.Executors$PrivilegedThreadFactory$1$1.run(Executors.java:702) at java.base/java.util.concurrent.Executors$PrivilegedThreadFactory$1$1.run(Executors.java:699) at java.base/java.security.AccessController.doPrivileged(AccessController.java:399)

[alexanderpann]

I am not done yet with the PR but yes you are right, thanks. I'll fix it.

[ratiud]

I tried to instantiate this editor on fasten GSN diagrams - there I have the following issues:

1) many fields are red (the values are not set, we should if possible provide some defaults) 2) I get a generation error (picture below)

[alexanderpann]

Did you run the migration? The constructor of the layered configuration is probably not called, so the properties are not initialized.

[ratiud]

Did you run the migration? The constructor of the layered configuration is probably not called, so the properties are not initialized.

no - I had not ran them ... thank you for pointing out!

[slisson]

This adds lots of new style attributes and for some of them (e.g. core-padding) it's not clear that they are diagram specific. It makes the code completion menu difficult to read. I would add a common prefix (maybe "diagram-layout-") to all of them.

[alexanderpann]

@slisson anything else? Do you have a better idea for the implementation of ElkLayouter#getNodeConnectionPosition?  It works but I am not quite happy with the layout results. I can't find a better option though.

[slisson]

Do you have a better idea for the implementation of ElkLayouter#getNodeConnectionPosition?  It works but I am not quite happy with the layout results. I can't find a better option though.

I don't understand well enough what the effect is to be able to suggest an alternative.

[slisson]

One things that we maybe can improve is to reduce the code duplication. I see that all the subclasses of ElkLayouter basically just copy all the properties for the layout algorithm. The is the LayoutMetaDataService that provides information about the available algorithms and their properties. Maybe we can make use of it and have a generic implementation. This would also have the benefit, that new properties in future version of ELK are then automatically available.

[alexanderpann]

The idea was that with the dummy ports, edges from the same (dummy) ports get merged into one line (hyperedge) which might not always be the preferred. With the new diagram option "don't use dummy ports", the boxes are connected directly. We still need the right side of the box for the connecton similar to the approach with the ports, so I just set the start and end location of the edge section e.g.:

KVector position = getNodeConnectionPosition(fromKNode, true); 
edgeSection.setStartLocation(position.x, position.y);

The result looks slightly odd for some reason.

[alexanderpann]

I'll create followup tickets in case we need to improve those points.