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

Constraints with Clusters #318

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Brains
Copy link

@Brains Brains commented May 28, 2022

Explored this during #317. Decided to save the progress for future despite don't know how to complete this PR.

Messing with SugiyamaLayoutSettings.Transformation was kinda overcomplicated for such trivial goal in #317. A better solution would be to fix LayerConstraints to make them work with Nodes inside Clusters (and maybe even Clusters themselves).

Exploration has shown next sequence of usage

SugiyamaLayoutSettings
    🡇
InitialLayoutByCluster
    🡇
InitialLayoutByCluster.GetComponents
// Creates a shallow copy of the root cluster and divides into GeometryGraphs
// each of which is a connected component with respect to edges internal to root.
    🡇
InitialLayoutByCluster.ShallowNodeCopyDictionary
// Copy the cluster's child Nodes and Clusters as nodes and return a mapping of original to copy.
// The reverse mapping (copy to original) is available via the copy's UserData

GetComponents wraps each node into new Node(). This is the reason of the bug since constraints are added against original Nodes

graph.LayerConstraints.AddSameLayerNeighbors(
    graph.FindNode("47"),
    graph.FindNode("58"));

then used by

void MapNodesToToIntegers(int[] yLayers) {
LeftRightIntNeibs = new Set<Tuple<int, int>>(from p in LeftRightNeighbors
let left = NodeIndex(p.Item1)
where left != -1
let right = NodeIndex(p.Item2)
where right != -1
select new Tuple<int, int>(left, right));

so the method
int NodeIndex(Node node) {
int index;
if (nodeIdToIndex.TryGetValue(node, out index))
return index;
return -1;

always returns -1 since node is an original Node while nodeIdToIndex was built against wrapped Nodes (in GetComponents). As result, lists of constraints are always empty.

@Brains
Copy link
Author

Brains commented May 28, 2022

After the fix 5ee1a3f, method NodeIndex finds required node and returns its index.
Of course the fix is just to demonstrate the error since it does not handle non wrapped Nodes at all.

Unfortunatelly, an error occurs later in

void AddVertToLayers(int i, int[] runningLayerCounts, bool[] alreadyInLayers) {
if (alreadyInLayers[i])
return;
int layerIndex = LayerArrays.Y[i];
int xIndex = runningLayerCounts[layerIndex];
var layer = LayerArrays.Layers[layerIndex];
layer[xIndex++] = i;
alreadyInLayers[i] = true;
List<int> block;
if (horizontalConstraints.BlockRootToBlock.TryGetValue(i, out block))
foreach (var v in block) {
if (alreadyInLayers[v]) continue;
layer[xIndex++] = v;
alreadyInLayers[v] = true;
}
runningLayerCounts[layerIndex] = xIndex;
}

image

Tried to figure out why but no luck so far. Layers are too complicated, a lot of different indexes, arrays, counters and lookups.
Maybe you know how to this can be resolved.

@Brains
Copy link
Author

Brains commented May 28, 2022

Also noticed that playing with Subgraphs configuration in WpfApplicationSample often leads to same IndexOutOfRangeException (before committed 5ee1a3f)

@Brains Brains marked this pull request as draft May 28, 2022 18:39
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.

1 participant