From 33984a542248bb6e20fe547932cded706dab4568 Mon Sep 17 00:00:00 2001 From: Malcolm Nixon Date: Wed, 8 Jul 2026 17:37:53 -0400 Subject: [PATCH] fix: route same-scope named-port edges through hierarchical layout; throw on boundary-crossing port edges - HierarchicalLayoutAlgorithm.ClassifyEdges previously silently dropped any edge with a LayoutGraphPort endpoint, even when the edge did not cross a container boundary, whenever the scope contained any container node elsewhere. This broke named-port support (Phase 1) as soon as any containment existed anywhere in the graph. - Add portOwner map and TryResolveOwner helper to resolve port endpoints to their owning node for scope classification. - Same-scope port edges (both endpoints direct members of current scope) are now routed normally through to LayeredLayoutAlgorithm, which already supports LayoutGraphPort endpoints via TryResolveEndpoint. - Genuinely boundary-crossing port edges now throw a clear NotSupportedException instead of being silently dropped. - BuildSizedView/TryMapConnectable carry Ports onto view nodes and remap leaf edge endpoints; LayoutScope extracts LayoutPort nodes from leaf output; Translate gained a LayoutPort case for correct nested-scope translation. - Add 3 unit tests covering same-scope port routing, boundary-crossing exception, and nested container port translation. - Update ROADMAP.md, design doc, verification doc, and reqstream yaml. --- ROADMAP.md | 15 ++ .../hierarchical-layout-algorithm.md | 41 ++- .../hierarchical-layout-algorithm.yaml | 30 +++ .../hierarchical-layout-algorithm.md | 24 +- .../HierarchicalLayoutAlgorithm.cs | 248 ++++++++++++++---- .../HierarchicalLayoutAlgorithmTests.cs | 126 +++++++++ 6 files changed, 420 insertions(+), 64 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 1c935db..06014c5 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -52,6 +52,21 @@ downward/upward flow). The gallery therefore ships two companion diagrams, `ContentInsetLeft` case) and `ports-showcase-vertical` (downward flow; top/bottom ports), which together exercise all four sides. +**`HierarchicalLayoutAlgorithm` bug fix (same-scope port edges, distinct from Phase 2 below).** +`HierarchicalLayoutAlgorithm` (the compound/recursive engine, not the flat `layered` algorithm +itself) previously dropped *every* edge touching a named `LayoutGraphPort` the instant its scope +contained any container node at all, even when the port edge's own endpoints were both literal, +non-nested members of that scope (for example two root-level siblings, with some unrelated +container elsewhere in the same scope) — silently, with no diagnostic. This has been fixed: a +same-scope port edge now reaches the selected leaf algorithm and is routed exactly as it would be in +a flat (container-free) graph. A port edge that genuinely crosses a container boundary — one +endpoint's owning node nested inside a container relative to the scope while the other is not, or an +edge otherwise sharing a box with such an edge — still has no anchoring/routing design and now fails +loudly with `NotSupportedException` instead of being silently dropped, pending the +`HierarchyHandling.Recursive`/boundary-port work described below and in "Phase 2 (hierarchy, once +`HierarchyHandling.Recursive` exists)" further down this file. Implementing actual +boundary-crossing port routing/anchoring remains out of scope for this fix. + The bundled `layered` algorithm's layout pipeline currently treats the input graph as simple: when two or more edges share the same directed `(source, target)` node pair, only one is retained and routed, and every input edge sharing that pair resolves to the same single routed diff --git a/docs/design/rendering-layout/hierarchical-layout-algorithm.md b/docs/design/rendering-layout/hierarchical-layout-algorithm.md index 8ba38e8..32f3343 100644 --- a/docs/design/rendering-layout/hierarchical-layout-algorithm.md +++ b/docs/design/rendering-layout/hierarchical-layout-algorithm.md @@ -51,26 +51,36 @@ overrides win over the supplied options, exactly like every nested scope), and c records both the sub-layout and the container's effective size — the sub-layout size grown by `ContainerPadding` on every side plus a title band when the container is labelled. 3. **Sized view.** The engine builds an internal, side-effect-free *view* graph with the same nodes in - the same order (container nodes carrying their effective size, leaves their own size, labels copied), - only the edges whose endpoints are both direct members of this scope. Every `CoreOptions` property - this scope needs is already present in `effective`, so the view carries no options of its own — the - leaf algorithm resolves them from `effective`, not from the view graph. The caller's input graph is - never mutated. + the same order (container nodes carrying their effective size, leaves their own size, labels copied, + and each direct member's named ports copied onto its view counterpart), only the edges whose direct- + member endpoints are both under this scope and neither nested inside a container relative to it — + including an edge whose endpoint is a named `LayoutGraphPort` rather than the node itself, so a + same-scope port edge is routed by the leaf algorithm exactly as it would be in a flat (container-free) + graph, regardless of whether the scope also contains an unrelated container elsewhere. Every + `CoreOptions` property this scope needs is already present in `effective`, so the view carries no + options of its own — the leaf algorithm resolves them from `effective`, not from the view graph. The + caller's input graph is never mutated. 4. **Placement.** The resolved leaf algorithm places the sized view against `effective`, emitting one box - per node in input order followed by routed lines for the in-scope edges. + per node in input order, followed by routed lines and any named-port anchors (`LayoutPort`) for the + in-scope edges. 5. **Composition.** Each container's placed box receives its recursively laid-out children, translated from their local origin to the box's padded (and title-offset) interior via a recursive `Translate` - that shifts nested boxes and line waypoints (local-to-absolute translation, following the - `ComponentPacker` precedent). + that shifts nested boxes, line waypoints, and port anchors (local-to-absolute translation, following + the `ComponentPacker` precedent). 6. **Cross-container (LCA) routing.** Edges whose endpoints resolve to different direct-member containers of this scope — mapped from any descendant endpoint up to its owning top-level box — are routed at this level with `ConnectorRouter.Route`, steering around the sibling boxes; the `EdgeRouting` style is read from this scope's own cascaded `effective` snapshot, so an override set on the owning scope's graph is honored rather than falling back to the root options. Edges already routed by the leaf algorithm (both endpoints direct) or belonging to a lower scope (both endpoints - under one container) are skipped. + under one container) are skipped. A genuine cross-container edge whose endpoint is a named + `LayoutGraphPort` — this scope's router is built on the box-only `ConnectorRouter`/`Connection` and + has no port concept — throws `NotSupportedException` instead of being routed or silently dropped; + full boundary-crossing port support (anchoring, routing) is a separate, not-yet-designed Phase 2 + effort (see ROADMAP.md). 7. **Assembly.** The engine returns a `LayoutTree` with the leaf algorithm's canvas size for this level - and the composed boxes followed by the leaf-routed lines and the cross-container lines. + and the composed boxes followed by the leaf-routed lines, any leaf-emitted port anchors, and the + cross-container lines. Every `CoreOptions` property (`Algorithm`, `Direction`, `EdgeRouting`, `HierarchyHandling`, `NodeSpacing`, `LayerSpacing`) cascades through this same generalized mechanism, built once on @@ -99,8 +109,15 @@ input graph and ignores the supplied options would silently break cascading for Null `graph`, `options`, or (injecting constructor) `registry` throw `ArgumentNullException`. A scope that selects an algorithm identifier absent from the registry surfaces the registry's -`KeyNotFoundException`. Edges whose endpoints are not under the current scope are skipped rather than -treated as errors. +`KeyNotFoundException`. An edge whose endpoint resolves to no node at all under the current scope (an +out-of-scope reference) is skipped rather than treated as an error. A port edge that genuinely crosses +a container boundary — one endpoint's owning node nested inside a container relative to this scope +while the other is not, or an edge otherwise promoted into this scope's router because it shares a box +with such an edge — throws `NotSupportedException` with a message identifying named ports crossing a +container boundary as not yet supported; this scope's router has no port concept and full +boundary-crossing port support remains a separate Phase 2 effort. A same-scope port edge (neither +endpoint nested relative to this scope) is not an error case: it is routed locally by the leaf +algorithm exactly as it would be in a flat graph. ### HierarchicalLayoutAlgorithm Dependencies diff --git a/docs/reqstream/rendering-layout/hierarchical-layout-algorithm.yaml b/docs/reqstream/rendering-layout/hierarchical-layout-algorithm.yaml index d9e20ea..585f224 100644 --- a/docs/reqstream/rendering-layout/hierarchical-layout-algorithm.yaml +++ b/docs/reqstream/rendering-layout/hierarchical-layout-algorithm.yaml @@ -105,6 +105,36 @@ sections: tests: - Apply_CrossContainerEdge_HonorsScopeEdgeRoutingOverride + - id: Rendering-Layout-HierarchicalLayout-SameScopePortEdges + title: >- + HierarchicalLayoutAlgorithm shall route a same-scope edge whose endpoint is a named + LayoutGraphPort through the selected leaf algorithm exactly as a flat (container-free) graph + would, even when the scope also contains an unrelated container node. + justification: | + Previously ClassifyEdges unconditionally skipped (silently dropped) any edge touching a + LayoutGraphPort the instant the scope contained any container node at all — a + previously-undocumented defect, now fixed — regardless of whether the port edge actually + crossed a container boundary. A port edge whose direct-member endpoints are both literal, + non-nested members of the scope does not cross any boundary and must reach the leaf algorithm, + which already resolves a LayoutGraphPort endpoint correctly. + tests: + - Apply_SameScopePortEdge_WithUnrelatedContainerElsewhere_RoutesLikePortEdge + - Apply_NestedContainerPortEdge_TranslatesPortIntoAncestorCoordinates + + - id: Rendering-Layout-HierarchicalLayout-BoundaryPortEdgeThrows + title: >- + HierarchicalLayoutAlgorithm shall throw NotSupportedException, rather than silently dropping + the edge, when a named-port edge genuinely crosses a container boundary. + justification: | + This scope's cross-container router is built on the box-only ConnectorRouter/Connection and + has no port concept, so it cannot anchor or route a port endpoint. Failing loudly with a clear, + actionable message is safer than silently dropping the edge (the previous, previously- + undocumented defect behavior) or anchoring it incorrectly as a plain box connector. Full + boundary-crossing port support (anchoring, routing) remains a separate, not-yet-designed Phase + 2 effort (see ROADMAP.md). + tests: + - Apply_PortEdgeCrossingContainerBoundary_Throws + - id: Rendering-Layout-HierarchicalLayout-ValidatesGraph title: >- HierarchicalLayoutAlgorithm shall reject a null graph argument with an argument-null error. diff --git a/docs/verification/rendering-layout/hierarchical-layout-algorithm.md b/docs/verification/rendering-layout/hierarchical-layout-algorithm.md index 125903a..1c74fa8 100644 --- a/docs/verification/rendering-layout/hierarchical-layout-algorithm.md +++ b/docs/verification/rendering-layout/hierarchical-layout-algorithm.md @@ -31,8 +31,9 @@ A verification run passes when every named scenario below asserts without unexpe the referenced tests cover each `Rendering-Layout-HierarchicalLayout-*` requirement. Any drift in the stable identifier (`"hierarchical"`), in the flat-graph byte-equivalence guarantee, in container sizing (padding, title band), in per-scope algorithm resolution, in cross-container -LCA edge routing, or in the argument-null validation behavior constitutes a failure. Mutation of -input node sizes also constitutes a failure. +LCA edge routing, in same-scope named-port edge routing (or its ancestor-coordinate translation), in +the boundary-crossing port `NotSupportedException` behavior, or in the argument-null validation +behavior constitutes a failure. Mutation of input node sizes also constitutes a failure. ### Test Scenarios @@ -80,6 +81,20 @@ input node sizes also constitutes a failure. `Apply_CrossContainerEdge_HonorsScopeEdgeRoutingOverride` confirms a cross-container edge is routed using the owning scope's own cascaded `CoreOptions.EdgeRouting` override rather than the root options. +- **Same-scope port edges** (`Rendering-Layout-HierarchicalLayout-SameScopePortEdges`): + `Apply_SameScopePortEdge_WithUnrelatedContainerElsewhere_RoutesLikePortEdge` confirms a port-to-node + edge between two root-level siblings is routed by the leaf algorithm — emitting exactly one + `LayoutPort` carrying the port's external label, anchored on the source box's boundary, and exactly + one connecting `LayoutLine` — even though the scope also contains an unrelated container node + elsewhere with no edges of its own. + `Apply_NestedContainerPortEdge_TranslatesPortIntoAncestorCoordinates` confirms a `LayoutPort` emitted + by a nested container's own leaf pass is correctly translated into the ancestor's absolute + coordinates when composed, landing within the composed container box's bounds. +- **Boundary port edge throws** (`Rendering-Layout-HierarchicalLayout-BoundaryPortEdgeThrows`): + `Apply_PortEdgeCrossingContainerBoundary_Throws` confirms an edge from a root-level named port + directly to a node nested inside a separate container — a genuine boundary-crossing port edge — + throws `NotSupportedException` with a message identifying named ports crossing a container boundary + as not yet supported, rather than the edge being silently dropped. - **Validation** (`Rendering-Layout-HierarchicalLayout-ValidatesGraph`, `Rendering-Layout-HierarchicalLayout-ValidatesOptions`, `Rendering-Layout-HierarchicalLayout-ValidatesRegistry`): `Apply_NullGraph_Throws`, @@ -107,6 +122,11 @@ input node sizes also constitutes a failure. Apply_ThreeLevelEdgeRoutingCascade_ReachesEveryLeafAlgorithmCall - **`Rendering-Layout-HierarchicalLayout-HonorsScopeEdgeRouting`**: Apply_CrossContainerEdge_HonorsScopeEdgeRoutingOverride +- **`Rendering-Layout-HierarchicalLayout-SameScopePortEdges`**: + Apply_SameScopePortEdge_WithUnrelatedContainerElsewhere_RoutesLikePortEdge, + Apply_NestedContainerPortEdge_TranslatesPortIntoAncestorCoordinates +- **`Rendering-Layout-HierarchicalLayout-BoundaryPortEdgeThrows`**: + Apply_PortEdgeCrossingContainerBoundary_Throws - **`Rendering-Layout-HierarchicalLayout-ValidatesGraph`**: Apply_NullGraph_Throws - **`Rendering-Layout-HierarchicalLayout-ValidatesOptions`**: diff --git a/src/DemaConsulting.Rendering.Layout/HierarchicalLayoutAlgorithm.cs b/src/DemaConsulting.Rendering.Layout/HierarchicalLayoutAlgorithm.cs index bb1cee8..ce2c337 100644 --- a/src/DemaConsulting.Rendering.Layout/HierarchicalLayoutAlgorithm.cs +++ b/src/DemaConsulting.Rendering.Layout/HierarchicalLayoutAlgorithm.cs @@ -244,16 +244,19 @@ private LayoutTree LayoutScope(LayoutGraph graph, LayoutOptions effective) var effectiveSize = new Dictionary(); LayoutContainerChildren(graph, effective, subLayouts, effectiveSize); - // Map every descendant node to the direct member of this scope that contains it (needed before - // placement so edge classification below can decide, purely from graph structure, which edges - // the leaf algorithm should route locally versus which the router must handle for this scope). + // Map every descendant node to the direct member of this scope that contains it, and every + // descendant's named port to the node that owns it (needed before placement so edge + // classification below can decide, purely from graph structure, which edges the leaf algorithm + // should route locally versus which the router must handle for this scope, resolving a port + // endpoint to its owning node the same way as a plain node endpoint). var descendantToDirect = new Dictionary(); + var portOwner = new Dictionary(); foreach (var direct in graph.Nodes) { - MapDescendants(direct, direct, descendantToDirect); + MapDescendants(direct, direct, descendantToDirect, portOwner); } - var (leafEdges, routedEdges) = ClassifyEdges(graph, descendantToDirect); + var (leafEdges, routedEdges) = ClassifyEdges(graph, descendantToDirect, portOwner); var (view, _) = BuildSizedView(graph, effectiveSize, leafEdges); @@ -262,16 +265,19 @@ private LayoutTree LayoutScope(LayoutGraph graph, LayoutOptions effective) var placed = algo.Apply(view, effective); var placedBoxes = placed.Nodes.OfType().ToList(); var placedLines = placed.Nodes.OfType().ToList(); + var placedPorts = placed.Nodes.OfType().ToList(); var (composed, indexOf) = ComposeBoxes(graph, placedBoxes, subLayouts); var crossLines = RouteCrossContainerEdges(routedEdges, composed, indexOf, descendantToDirect, effective); - // Assemble: composed boxes, then the leaf algorithm's routed lines, then the cross-container - // lines. The canvas dimensions are the leaf algorithm's for this (sized) level. - var nodes = new List(composed.Length + placedLines.Count + crossLines.Count); + // Assemble: composed boxes, then the leaf algorithm's routed lines and any LayoutPort it + // emitted for a leaf-routed port edge, then the cross-container lines. The canvas dimensions + // are the leaf algorithm's for this (sized) level. + var nodes = new List(composed.Length + placedLines.Count + placedPorts.Count + crossLines.Count); nodes.AddRange(composed); nodes.AddRange(placedLines); + nodes.AddRange(placedPorts); nodes.AddRange(crossLines); return new LayoutTree(placed.Width, placed.Height, nodes); } @@ -320,15 +326,17 @@ private void LayoutContainerChildren( /// /// Builds an internal, side-effect-free sized view of : the same nodes in - /// the same order (container nodes carrying their effective size, leaves their own size), only the - /// edges in — those between two distinct direct members that neither - /// touch a box also involved in a cross-container edge, per . Every - /// other edge (genuine cross-container edges, and any direct-member edge that shares a box with - /// one) is routed by this scope's router instead, so a box that receives both kinds of connector - /// has every one of its anchors allocated by a single coordinated pass. The scope's own cascaded - /// options are carried separately as the caller's effective snapshot rather than propagated - /// onto this view, since every leaf algorithm resolves such properties from the options it is - /// invoked with. + /// the same order (container nodes carrying their effective size, leaves their own size, and each + /// direct member's named copied onto its view counterpart), + /// only the edges in — those between two distinct direct members that + /// neither touch a box also involved in a cross-container edge, per , + /// including any such edge whose endpoint is a named rather than the + /// node itself. Every other edge (genuine cross-container edges, and any direct-member edge that + /// shares a box with one) is routed by this scope's router instead, so a box that receives both + /// kinds of connector has every one of its anchors allocated by a single coordinated pass. The + /// scope's own cascaded options are carried separately as the caller's effective snapshot + /// rather than propagated onto this view, since every leaf algorithm resolves such properties from + /// the options it is invoked with. /// /// The caller's scope, which is never mutated. /// Effective sizes for the container nodes of this scope. @@ -342,6 +350,7 @@ private static (LayoutGraph View, Dictionary V var view = new LayoutGraph(); var viewOf = new Dictionary(graph.Nodes.Count); + var viewPortOf = new Dictionary(); foreach (var node in graph.Nodes) { var (width, height) = effectiveSize.TryGetValue(node, out var size) @@ -356,6 +365,23 @@ private static (LayoutGraph View, Dictionary V viewNode.FolderTabWidth = node.FolderTabWidth; viewNode.FolderTabHeight = node.FolderTabHeight; viewOf[node] = viewNode; + + // Copy named ports too, so a same-scope port edge classified into leafEdges (per + // ClassifyEdges — a port edge can only reach leafEdges when its owning node is itself a + // direct member of this scope) can be faithfully reproduced on the view graph and reach + // the leaf algorithm's own port-aware endpoint resolution unchanged. + if (!node.HasPorts) + { + continue; + } + + foreach (var port in node.Ports.Ports) + { + var viewPort = viewNode.Ports.AddPort(port.Id); + viewPort.ExternalLabel = port.ExternalLabel; + viewPort.InternalLabel = port.InternalLabel; + viewPortOf[port] = viewPort; + } } foreach (var edge in graph.Edges) @@ -368,8 +394,8 @@ private static (LayoutGraph View, Dictionary V continue; } - if (edge.Source is LayoutGraphNode edgeSourceNode && viewOf.TryGetValue(edgeSourceNode, out var viewSource) && - edge.Target is LayoutGraphNode edgeTargetNode && viewOf.TryGetValue(edgeTargetNode, out var viewTarget)) + if (TryMapConnectable(edge.Source, viewOf, viewPortOf, out var viewSource) && + TryMapConnectable(edge.Target, viewOf, viewPortOf, out var viewTarget)) { var viewEdge = view.AddEdge(edge.Id, viewSource, viewTarget); viewEdge.TargetEnd = edge.TargetEnd; @@ -381,6 +407,37 @@ private static (LayoutGraph View, Dictionary V return (view, viewOf); } + /// + /// Resolves an original-graph edge endpoint (a node or one of its named ports) to its counterpart + /// on the sized view graph built by . + /// + /// The original edge endpoint to resolve. + /// Map from each original direct-member node to its view counterpart. + /// Map from each original port to its copied view counterpart. + /// The resolved view endpoint, when resolution succeeds. + /// when the endpoint resolves to a view node or view port. + private static bool TryMapConnectable( + ILayoutConnectable connectable, + Dictionary viewOf, + Dictionary viewPortOf, + out ILayoutConnectable mapped) + { + switch (connectable) + { + case LayoutGraphNode node when viewOf.TryGetValue(node, out var viewNode): + mapped = viewNode; + return true; + + case LayoutGraphPort port when viewPortOf.TryGetValue(port, out var viewPort): + mapped = viewPort; + return true; + + default: + mapped = null!; + return false; + } + } + /// /// Composes the placed boxes for this level: each container box receives its recursively laid-out /// children, translated from their local origin into the box's padded (and, when labelled, @@ -456,8 +513,10 @@ private static List RouteCrossContainerEdges( var connections = new List(routedEdges.Count); foreach (var edge in routedEdges) { - // ClassifyEdges only ever adds edges with LayoutGraphNode endpoints to routedEdges/leafEdges - // (Phase 1 scope excludes port endpoints from this hierarchical algorithm). + // ClassifyEdges only ever adds edges with LayoutGraphNode endpoints to routedEdges — a + // genuine cross-container edge whose endpoint is a named LayoutGraphPort is rejected with + // a NotSupportedException before it can reach this router, since Connection/ConnectorRouter + // are box-only and have no port concept. var sourceNode = (LayoutGraphNode)edge.Source; var targetNode = (LayoutGraphNode)edge.Target; var from = composed[indexOf[descendantToDirect[sourceNode]]]; @@ -474,36 +533,63 @@ private static List RouteCrossContainerEdges( /// instead. /// /// - /// An edge whose direct-member endpoints (per ) resolve to the - /// same node belongs to a lower scope entirely and is skipped here. An edge between two distinct - /// direct members, with at least one endpoint actually nested inside a container, is a genuine - /// cross-container edge routed by this scope's router. An edge between two distinct direct members - /// that are both literally direct (neither endpoint nested) would normally be routed - /// locally by the leaf algorithm — but when either of those direct members is also an endpoint of a - /// genuine cross-container edge, the edge is promoted to this scope's router too, so every connector - /// converging on that box's face is anchored by one coordinated pass instead of two independent, - /// mutually unaware ones (the leaf algorithm's own internal routing and this scope's router). + /// + /// An edge whose direct-member endpoints (per ) resolve to + /// the same node belongs to a lower scope entirely and is skipped here. An edge between two + /// distinct direct members, with at least one endpoint actually nested inside a container, is a + /// genuine cross-container edge routed by this scope's router. An edge between two distinct + /// direct members that are both literally direct (neither endpoint nested) would + /// normally be routed locally by the leaf algorithm — but when either of those direct members is + /// also an endpoint of a genuine cross-container edge, the edge is promoted to this scope's + /// router too, so every connector converging on that box's face is anchored by one coordinated + /// pass instead of two independent, mutually unaware ones (the leaf algorithm's own internal + /// routing and this scope's router). + /// + /// + /// A named endpoint is resolved to its owning node (per + /// ) for the purpose of this scope-membership classification, then + /// treated identically to a plain node endpoint: a same-scope port edge (both direct members + /// literally direct — neither endpoint nested relative to this scope) is routed locally by the + /// leaf algorithm exactly like a flat, container-free graph would route it. This scope's own + /// router (, built on the box-only + /// /) has no port concept, so a port edge + /// that would otherwise be classified as a genuine cross-container edge — or promoted into the + /// router's batch because it shares a box with one — instead throws + /// : full boundary-crossing port support (anchoring, routing) + /// is a separate, not-yet-designed Phase 2 effort (see ROADMAP.md), so this deliberately fails + /// loudly rather than silently dropping the edge or routing it incorrectly as a plain box + /// connector. + /// /// /// The scope whose edges are classified. /// Map from every descendant node to the direct member that owns it. + /// Map from every descendant's named port to the node that owns it. /// The edges to route locally, and the edges this scope's router must handle. + /// + /// Thrown when a named-port edge would cross a container boundary (a genuine cross-container edge, + /// or a same-scope edge promoted into the router's batch because it shares a box with one) — this + /// hierarchical engine does not yet support routing or anchoring a port across a container boundary. + /// private static (HashSet LeafEdges, List RoutedEdges) ClassifyEdges( LayoutGraph graph, - Dictionary descendantToDirect) + Dictionary descendantToDirect, + Dictionary portOwner) { - var directDirect = new List(); + const string boundaryPortMessage = + "Named ports on edges crossing a container boundary are not yet supported; see ROADMAP.md Phase 2."; + + var directDirect = new List<(LayoutGraphEdge Edge, LayoutGraphNode SourceDirect, LayoutGraphNode TargetDirect)>(); var routedEdges = new List(); var conflicted = new HashSet(); foreach (var edge in graph.Edges) { - // Skip edges whose endpoints are not both under this scope. Named-port endpoints are not - // yet supported by this hierarchical algorithm (Phase 1 scope is the flat/layered - // algorithm only), so an edge touching a port is skipped here too. - if (edge.Source is not LayoutGraphNode sourceNode || - edge.Target is not LayoutGraphNode targetNode || - !descendantToDirect.TryGetValue(sourceNode, out var sourceDirect) || - !descendantToDirect.TryGetValue(targetNode, out var targetDirect)) + // Skip edges whose endpoints are not both under this scope (a reference to a node or port + // outside this scope's descendant tree entirely). + if (!TryResolveOwner(edge.Source, portOwner, out var sourceOwner) || + !TryResolveOwner(edge.Target, portOwner, out var targetOwner) || + !descendantToDirect.TryGetValue(sourceOwner, out var sourceDirect) || + !descendantToDirect.TryGetValue(targetOwner, out var targetDirect)) { continue; } @@ -514,15 +600,24 @@ edge.Target is not LayoutGraphNode targetNode || continue; } - var bothDirect = ReferenceEquals(sourceDirect, sourceNode) && ReferenceEquals(targetDirect, targetNode); + var bothDirect = ReferenceEquals(sourceDirect, sourceOwner) && ReferenceEquals(targetDirect, targetOwner); if (bothDirect) { // Provisionally a leaf-routed edge; may still be promoted below if either endpoint is // also touched by a genuine cross-container edge. - directDirect.Add(edge); + directDirect.Add((edge, sourceDirect, targetDirect)); } else { + // A genuine cross-container edge (at least one endpoint is nested relative to this + // scope). This scope's router is box-only, so a port endpoint here cannot be routed + // and instead fails loudly rather than being silently dropped or routed incorrectly + // as a plain box connector. + if (edge.Source is LayoutGraphPort || edge.Target is LayoutGraphPort) + { + throw new NotSupportedException(boundaryPortMessage); + } + routedEdges.Add(edge); conflicted.Add(sourceDirect); conflicted.Add(targetDirect); @@ -530,11 +625,17 @@ edge.Target is not LayoutGraphNode targetNode || } var leafEdges = new HashSet(); - foreach (var edge in directDirect) + foreach (var (edge, sourceDirect, targetDirect) in directDirect) { - if ((edge.Source is LayoutGraphNode sourceNode && conflicted.Contains(sourceNode)) || - (edge.Target is LayoutGraphNode targetNode && conflicted.Contains(targetNode))) + if (conflicted.Contains(sourceDirect) || conflicted.Contains(targetDirect)) { + // Promoted into the router's batch because it shares a box with a genuine + // cross-container edge; the same box-only router limitation applies here too. + if (edge.Source is LayoutGraphPort || edge.Target is LayoutGraphPort) + { + throw new NotSupportedException(boundaryPortMessage); + } + routedEdges.Add(edge); } else @@ -546,6 +647,36 @@ edge.Target is not LayoutGraphNode targetNode || return (leafEdges, routedEdges); } + /// + /// Resolves an edge endpoint (a node or one of its named ports) to the node that owns it: a plain + /// node endpoint resolves to itself, while a endpoint resolves to its + /// owning node via . + /// + /// The edge endpoint to resolve. + /// Map from every descendant's named port to the node that owns it. + /// The resolved owning node, when resolution succeeds. + /// when the endpoint resolves to a node in this scope's graph. + private static bool TryResolveOwner( + ILayoutConnectable connectable, + Dictionary portOwner, + out LayoutGraphNode owner) + { + switch (connectable) + { + case LayoutGraphNode node: + owner = node; + return true; + + case LayoutGraphPort port when portOwner.TryGetValue(port, out var ownerNode): + owner = ownerNode; + return true; + + default: + owner = null!; + return false; + } + } + /// /// Resolves the leaf-algorithm identifier a scope is placed with from its already-cascaded effective /// options. @@ -568,31 +699,45 @@ private static string ResolveLeafAlgorithmId(LayoutOptions effective) /// /// Records that and all of its descendants belong to the direct-member /// container , so a cross-container edge referencing any descendant can be - /// anchored to the top-level box representing that container. + /// anchored to the top-level box representing that container. Also records, for every named port + /// owned by or any of its descendants, the node that owns it, so an edge + /// endpoint that is a can be resolved to its owning node the same way + /// a plain node endpoint resolves to itself. /// /// The node currently being mapped (initially the direct member itself). /// The direct member of the scope that owns this subtree. /// The descendant-to-direct-member map being populated. + /// The port-to-owning-node map being populated. private static void MapDescendants( LayoutGraphNode node, LayoutGraphNode direct, - Dictionary map) + Dictionary map, + Dictionary portOwner) { map[node] = direct; + if (node.HasPorts) + { + foreach (var port in node.Ports.Ports) + { + portOwner[port] = node; + } + } + if (node.HasChildren) { foreach (var child in node.Children.Nodes) { - MapDescendants(child, direct, map); + MapDescendants(child, direct, map, portOwner); } } } /// - /// Translates a placed layout node by a fixed offset, recursively shifting a box's nested children - /// and a line's waypoints so a locally-placed sub-layout can be composed into an absolute position. + /// Translates a placed layout node by a fixed offset, recursively shifting a box's nested children, + /// a line's waypoints, and a port's anchor centre, so a locally-placed sub-layout can be composed + /// into an absolute position. /// - /// The placed node to translate; only boxes and lines are produced by the bundled leaf algorithms. + /// The placed node to translate; boxes, lines, and ports are produced by the bundled leaf algorithms. /// Offset added to every X coordinate, in logical pixels. /// Offset added to every Y coordinate, in logical pixels. /// A translated copy of , or the node unchanged for unknown subtypes. @@ -618,6 +763,9 @@ private static LayoutNode Translate(LayoutNode node, double deltaX, double delta return line with { Waypoints = waypoints }; + case LayoutPort port: + return port with { CentreX = port.CentreX + deltaX, CentreY = port.CentreY + deltaY }; + default: // Forward compatibility: an unknown node subtype is left untranslated rather than dropped. return node; diff --git a/test/DemaConsulting.Rendering.Layout.Tests/HierarchicalLayoutAlgorithmTests.cs b/test/DemaConsulting.Rendering.Layout.Tests/HierarchicalLayoutAlgorithmTests.cs index e88592e..1e23edd 100644 --- a/test/DemaConsulting.Rendering.Layout.Tests/HierarchicalLayoutAlgorithmTests.cs +++ b/test/DemaConsulting.Rendering.Layout.Tests/HierarchicalLayoutAlgorithmTests.cs @@ -584,6 +584,132 @@ public void Apply_BoxWithBothLeafAndCrossContainerEdges_SpreadsSharedFaceAnchors Assert.NotEqual(lines[0].Waypoints[^1], lines[1].Waypoints[^1]); } + /// + /// Proves that a same-scope port edge — one whose direct-member endpoints are both literal, + /// non-nested members of the scope — is routed by the leaf algorithm exactly as it would be in + /// a flat graph, even though the scope also contains an unrelated container node elsewhere. + /// Regression: the algorithm's edge classification used to unconditionally skip (silently + /// drop) any edge touching a the instant the scope contained any + /// container node at all, regardless of whether the port edge actually crossed a container + /// boundary. + /// + [Fact] + public void Apply_SameScopePortEdge_WithUnrelatedContainerElsewhere_RoutesLikePortEdge() + { + // Arrange: two root-level siblings joined by a port-to-node edge, plus an unrelated container + // elsewhere in the scope with its own child and no edges of its own. + var graph = new LayoutGraph(); + var source = graph.AddNode("source", 80, 40); + source.Label = "Source"; + var target = graph.AddNode("target", 80, 40); + target.Label = "Target"; + var port = source.Ports.AddPort("out1"); + port.ExternalLabel = "output"; + graph.AddEdge("e1", port, target); + + var unrelated = graph.AddNode("unrelated", 10, 10); + unrelated.Label = "Unrelated"; + unrelated.Children.AddNode("unrelated-child", 60, 40); + + // Act + var tree = new HierarchicalLayoutAlgorithm().Apply(graph, new LayoutOptions()); + + // Assert: exactly one LayoutPort is emitted, carrying the port's external label, and exactly + // one LayoutLine connects the two boxes — the connector is routed, not dropped, even though + // the scope contains an (unrelated) container. + var port1 = Assert.Single(tree.Nodes.OfType()); + Assert.Equal("output", port1.Label); + Assert.Single(tree.Nodes.OfType()); + + // The port anchor lies exactly on the source box's boundary, matching how a flat (no + // container) graph with the same port edge would be routed by the leaf algorithm directly. + var sourceBox = tree.Nodes.OfType().Single(box => box.Label == "Source"); + Assert.True( + OnBoxBoundary(port1.CentreX, port1.CentreY, sourceBox), + "The port anchor does not lie on the source box's boundary."); + } + + /// + /// Proves that a port edge which genuinely crosses a container boundary — one endpoint's owning + /// node nested inside a container relative to the scope, the other a plain direct member — + /// throws a clear instead of being silently dropped or + /// routed incorrectly as a plain box connector. Full boundary-crossing port support (anchoring, + /// routing) remains a separate, not-yet-designed Phase 2 effort; this proves the interim + /// behavior fails loudly. + /// + [Fact] + public void Apply_PortEdgeCrossingContainerBoundary_Throws() + { + // Arrange: a root-level node with a named port, and a separate container with a nested child. + // An edge from the outer port directly to the nested child is a genuine boundary-crossing + // port edge, added at the root (their lowest common ancestor). + var graph = new LayoutGraph(); + var outer = graph.AddNode("outer", 80, 40); + var port = outer.Ports.AddPort("out1"); + + var container = graph.AddNode("container", 10, 10); + var nested = container.Children.AddNode("nested", 60, 40); + + graph.AddEdge("cross", port, nested); + + // Act / Assert + var ex = Assert.Throws( + () => new HierarchicalLayoutAlgorithm().Apply(graph, new LayoutOptions())); + Assert.Contains("container boundary", ex.Message, StringComparison.Ordinal); + } + + /// + /// Proves that a emitted by a nested scope's own leaf pass is + /// correctly translated into the ancestor container's absolute coordinates when composed, + /// rather than left at its local (pre-translation) position. Closes a latent gap in the + /// algorithm's node-translation helper, which previously had no case for + /// and left it untranslated. + /// + [Fact] + public void Apply_NestedContainerPortEdge_TranslatesPortIntoAncestorCoordinates() + { + // Arrange: a container whose own children graph has a port-to-node edge (exercised via the + // flat fast path one level down), composed into a root that also has an unrelated sibling + // container. + var graph = new LayoutGraph(); + var group = graph.AddNode("group", 10, 10); + group.Label = "Group"; + var c1 = group.Children.AddNode("c1", 80, 40); + var c2 = group.Children.AddNode("c2", 80, 40); + var port = c1.Ports.AddPort("out1"); + port.ExternalLabel = "internal"; + group.Children.AddEdge("c1-c2", port, c2); + + var sibling = graph.AddNode("sibling", 10, 10); + sibling.Label = "Sibling"; + sibling.Children.AddNode("sibling-child", 60, 40); + + // Act + var tree = new HierarchicalLayoutAlgorithm().Apply(graph, LayoutOptions.ForAlgorithm("layered")); + + // Assert: the "group" container box holds the emitted LayoutPort among its (translated) + // children, positioned within the composed container box's absolute bounds. + var groupBox = tree.Nodes.OfType().Single(box => box.Label == "Group"); + var port1 = Assert.Single(groupBox.Children.OfType()); + Assert.InRange(port1.CentreX, groupBox.X, groupBox.X + groupBox.Width); + Assert.InRange(port1.CentreY, groupBox.Y, groupBox.Y + groupBox.Height); + } + + /// + /// Returns true when the point lies (within a small tolerance) on the boundary of the box. + /// + private static bool OnBoxBoundary(double x, double y, LayoutBox box) + { + const double tolerance = 1e-6; + var onVerticalEdge = + (Math.Abs(x - box.X) < tolerance || Math.Abs(x - (box.X + box.Width)) < tolerance) && + y >= box.Y - tolerance && y <= box.Y + box.Height + tolerance; + var onHorizontalEdge = + (Math.Abs(y - box.Y) < tolerance || Math.Abs(y - (box.Y + box.Height)) < tolerance) && + x >= box.X - tolerance && x <= box.X + box.Width + tolerance; + return onVerticalEdge || onHorizontalEdge; + } + /// /// Proves that a container node's , /// , and folder-geometry hints, and a nested leaf's