#514 Refactor GlspElkLayoutEngine #5
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tortmayr
changed the title
- Refactor the `GlspElkLayoutEngine` and related concepts to properly support layout computation for ports. In this process we also opted out of trying to reuse the existing `SprottyElkLayoutEngine` and instead implement a new layout engine specifically for the GLSP graphmodel. This implementation is more in line with the Java ELK-Layout Engine and implements similar features like automatic detection of common ancestors of edge source/targets to correctly transform them in the ELK graph. - Removes the no longer needed `BasicTypeMapper` Also: - Provide utility functions for querying parent elements of a given element that match a predicate. (gmodel-util.ts) - Only rerexport the the subset of types that are really needed from "sprotty-elk" to avoid unncessary naming clashes. - Add jsdoc for `GModelSerializer` - Fix implementation of `getAllEdges` in `GModelIndex` - Remove unused (and unset) `source` and `target` references in `GEdge` Contributed on behalf of STMicroelectronics
sgraband
reviewed
Feb 3, 2022
Comment on lines
+147
to
+148
| elkGraph.edges = []; | ||
| this.elkEdges.push(...(this.findChildren(graph, GEdge).map(child => this.transformToElk(child)) as ElkEdge[])); |
There was a problem hiding this comment.
Should these edges not also be pushed into the elkGraph.edges? Otherwise the array is always empty, right?
There was a problem hiding this comment.
No, edges need special handling. ELK expects that edges are always set in their respective parent container (either the graph or another node in case of nested diagrams). Otherwise edges want be layouted correctly. However, the same assumption is not true for the graphical model. If we have an edge between A and B the edge doesn't have to be a child of the container of A and B. It could be placed anywhere in the graph (e.g. as child of A).
Hence I implemented the same mechanism that is used in the Java LayoutEngine. (Store the edges in a temporary map and then in a postprocess step find the most suitable parent container). (
)
Comment on lines
+163
to
+164
| elkNode.edges = []; | ||
| this.elkEdges.push(...(this.findChildren(node, GEdge).map(child => this.transformToElk(child)) as ElkEdge[])); |
tortmayr
added a commit
that referenced
this pull request
Feb 3, 2022
* #514 Refactor GlspElkLayoutEngine - Refactor the `GlspElkLayoutEngine` and related concepts to properly support layout computation for ports. In this process we also opted out of trying to reuse the existing `SprottyElkLayoutEngine` and instead implement a new layout engine specifically for the GLSP graphmodel. This implementation is more in line with the Java ELK-Layout Engine and implements similar features like automatic detection of common ancestors of edge source/targets to correctly transform them in the ELK graph. - Removes the no longer needed `BasicTypeMapper` Also: - Provide utility functions for querying parent elements of a given element that match a predicate. (gmodel-util.ts) - Only rerexport the the subset of types that are really needed from "sprotty-elk" to avoid unncessary naming clashes. - Add jsdoc for `GModelSerializer` - Fix implementation of `getAllEdges` in `GModelIndex` - Remove unused (and unset) `source` and `target` references in `GEdge` Contributed on behalf of STMicroelectronics
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor the
GlspElkLayoutEngineand related concepts to properly support layout computation for ports. In this process we also opted out of trying to reuse the existingSprottyElkLayoutEngineand instead implement a new layout engine specifically for the GLSP graphmodel. This implementation is more in line with the Java ELK-Layout Engine and implements similar features like automatic detection of common ancestors of edge source/targets to correctly transform them in the ELK graph.Removes the no longer needed
BasicTypeMapperAlso:
GModelSerializergetAllEdgesinGModelIndexsourceandtargetreferences inGEdgeContributed on behalf of STMicroelectronics