diff --git a/docs/design/sysml2-tools-core/layout/internal/general-view-layout-strategy.md b/docs/design/sysml2-tools-core/layout/internal/general-view-layout-strategy.md index 1fb8966..9b01478 100644 --- a/docs/design/sysml2-tools-core/layout/internal/general-view-layout-strategy.md +++ b/docs/design/sysml2-tools-core/layout/internal/general-view-layout-strategy.md @@ -14,11 +14,13 @@ membership, attribute-typing, redefinition, subsetting, connect, allocate, depen binding edges orthogonally between the boxes. The whole diagram — package folders, definition-as-container boxes, and plain definitions alike — is expressed as a single `DemaConsulting.Rendering` `LayoutGraph` -and placed with one `HierarchicalLayoutAlgorithm.Apply` call: the root scope packs package folders -and top-level definitions by reading order (`ContainmentLayoutAlgorithm`), while each container's -own contents (a folder's or a nested-owning-definition's) are ordered by their intra-container -edges with the bundled layered algorithm -(`LayeredLayoutAlgorithm`). All box sizing (title bands, compartment rows) remains this strategy's +and placed with one `LayoutEngine.Layout` call via the `"auto"` meta-algorithm +(`AutoLayoutAlgorithm`): the root scope classifies package folders and top-level definitions by +connectivity, routing connected groups through the bundled layered algorithm +(`LayeredLayoutAlgorithm`) and packing disconnected/singleton nodes by reading order +(`ContainmentLayoutAlgorithm`), while each container's own contents (a folder's or a +nested-owning-definition's) are always ordered by their intra-container edges with the layered +algorithm. All box sizing (title bands, compartment rows) remains this strategy's responsibility, since the layout stage in `DemaConsulting.Rendering.Layout` is theme-agnostic; box title and folder-tab geometry come from `BoxMetrics` in `DemaConsulting.Rendering.Abstractions`. @@ -92,11 +94,12 @@ connect/allocate/dependency/binding relationships are resolved into qualified-name edges (plus any dropped-edge diagnostics) with `BuildModelEdges`, the single input `LayoutGraph` is built with `BuildGraph` (passed `childrenByParent` and whether the view has a resolved scope), and the -whole graph is placed with one -`HierarchicalLayoutAlgorithm().Apply(graph, LayoutOptions.ForAlgorithm("containment"))` call — -passing the desired root-scope leaf algorithm through the options parameter (not -`graph.Set(CoreOptions.Algorithm, …)`) so a caller going through `LayoutEngine.Layout(graph)` later -is never misled into skipping the hierarchical engine. When any package folder or +whole graph is placed with one `LayoutEngine.Layout(graph)` call after setting +`graph.Set(CoreOptions.Algorithm, AutoLayoutAlgorithm.AlgorithmId)` — delegating root-scope +algorithm selection to `"auto"`, which classifies the root's own folders/top-level definitions by +connectivity (routing connected groups through the layered algorithm and packing +disconnected/singleton nodes via containment) rather than pinning every root-scope node to a single +fixed algorithm regardless of its own connectivity. When any package folder or nested-definition-containing definition was depth-truncated, `DecorateTruncated` stamps each truncated container's "+N more…" ellipsis label onto its placed box. Finally, the returned tree's `Warnings` concatenates `LayoutWarnings.ForUnevaluatedFilter` (only when standalone filter parsing/evaluation failed), @@ -481,9 +484,9 @@ produces valid geometry, so no crossing warnings are emitted. inputs. - `LayoutGraph`, `LayoutGraphNode`, `LayoutGraphEdge`, and `CoreOptions` (`DemaConsulting.Rendering`) — the input graph model and its well-known cascading options. -- `HierarchicalLayoutAlgorithm`, `ContainmentLayoutAlgorithm`, `LayeredLayoutAlgorithm`, and - `LayoutOptions` (`DemaConsulting.Rendering.Layout`) — the layout engine and the bundled leaf - algorithms it delegates to per scope. +- `AutoLayoutAlgorithm`, `ContainmentLayoutAlgorithm`, `LayeredLayoutAlgorithm`, and `LayoutEngine` + (`DemaConsulting.Rendering.Layout`) — the layout engine and the bundled leaf algorithms it + delegates to per scope. - `BoxMetrics` (`DemaConsulting.Rendering.Abstractions`) — box title-area and folder-tab geometry. - `StdlibFilter` (Rendering Internal subsystem) — standard-library exclusion. - `SysmlWorkspace`, `SysmlDefinitionNode`, `SysmlFeatureNode` (Semantic subsystem) — model input; diff --git a/docs/design/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md b/docs/design/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md index df1f5a1..23f8c62 100644 --- a/docs/design/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md +++ b/docs/design/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md @@ -24,8 +24,12 @@ container size and content produced by laying out one definition's interior). Entry point. Resolves the view's `expose` scope via `ExposeScopeResolver.ResolveExposedScope`, selects the root part definition via `FindRoot(workspace, scope)`, builds the container-definition -index via `BuildDefinitionIndex`, lays out the root's interior via `LayOutInterior` (threading -`scope` through every recursive call), and assembles the root container box with the interior +index via `BuildDefinitionIndex`, lays out the root's interior via `LayOutInterior` (applying +`scope`'s namespace-prefix filter only at the root's own direct children, depth 0; every deeper +recursive call passes `scope: null` so a nested container's own interior always shows its full +composition structure regardless of which namespace it — or the view's exposed subject — happens to +be declared in, since composition structure and namespace/file organization are independent in +SysML v2), and assembles the root container box with the interior content nested as that box's own `Children` (mirroring the nesting `MakePartBox` already uses for a container part, so the root box is never a bare sibling of its own content) into the `LayoutTree`. Returns a minimal 200×100 empty `LayoutTree` when no root or no parts are found. diff --git a/docs/gallery/png/BatterySubsystemView.png b/docs/gallery/png/BatterySubsystemView.png index 2bbafc9..b05e4c2 100644 Binary files a/docs/gallery/png/BatterySubsystemView.png and b/docs/gallery/png/BatterySubsystemView.png differ diff --git a/docs/gallery/png/DroneGeneralView.png b/docs/gallery/png/DroneGeneralView.png index b23628c..91303fd 100644 Binary files a/docs/gallery/png/DroneGeneralView.png and b/docs/gallery/png/DroneGeneralView.png differ diff --git a/docs/gallery/png/GroundSegmentRecursiveView.png b/docs/gallery/png/GroundSegmentRecursiveView.png index 99e1213..c464183 100644 Binary files a/docs/gallery/png/GroundSegmentRecursiveView.png and b/docs/gallery/png/GroundSegmentRecursiveView.png differ diff --git a/docs/gallery/png/MissionControlGeneralView.png b/docs/gallery/png/MissionControlGeneralView.png index 384b38a..20b658d 100644 Binary files a/docs/gallery/png/MissionControlGeneralView.png and b/docs/gallery/png/MissionControlGeneralView.png differ diff --git a/docs/gallery/svg/BatterySubsystemView.svg b/docs/gallery/svg/BatterySubsystemView.svg index 4ae5279..a86a61a 100644 --- a/docs/gallery/svg/BatterySubsystemView.svg +++ b/docs/gallery/svg/BatterySubsystemView.svg @@ -1,4 +1,4 @@ - + @@ -30,19 +30,19 @@ - - «part def» - Battery - - attributes - capacity : Voltage - - ports - output : PowerPort - - - Rechargeable lithium-polymer battery pack supplying the drone's - * flight controller, motors, and sensor bus via the PowerPort - * interface. Demonstrates the BoxShape.Note annotation rendering. - + + «part def» + Battery + + attributes + capacity : Voltage + + ports + output : PowerPort + + + Rechargeable lithium-polymer battery pack supplying the drone's + * flight controller, motors, and sensor bus via the PowerPort + * interface. Demonstrates the BoxShape.Note annotation rendering. + diff --git a/docs/gallery/svg/DroneGeneralView.svg b/docs/gallery/svg/DroneGeneralView.svg index e470895..48da7ec 100644 --- a/docs/gallery/svg/DroneGeneralView.svg +++ b/docs/gallery/svg/DroneGeneralView.svg @@ -1,4 +1,4 @@ - + @@ -30,166 +30,166 @@ - + «package» QuadcopterDrone - - «interface def» - PowerBus - - «interface def» - DataBus - «port def» - PowerPort - - «port def» - TelemetryPort - - «port def» - MotorControlPort - - «port def» - SensorPort - - «attribute def» - Mass - - «attribute def» - Voltage - - «enum def» - FlightMode - - enum values - idle - manual - autonomous - - «part def» - Battery - - attributes - capacity : Voltage - - ports - output : PowerPort - - - Rechargeable lithium-polymer battery pack supplying the drone's - * flight controller, motors, and sensor bus via the PowerPort - * interface. Demonstrates the BoxShape.Note annotation rendering. - - «part def» - FlightController - - attributes - mode : FlightMode - - ports - power : PowerPort - telemetry : TelemetryPort - motors : MotorControlPort - sensors : SensorPort - - «part def» - Motor - - attributes - maxThrust : Mass - - ports - control : MotorControlPort - - «part def» - Propeller - - «part def» - ImuSensor - - ports - data : SensorPort - - «part def» - GpsSensor - - ports - data : SensorPort - - «part def» - Frame - - «part def» - RacingMotor - - attributes - maxThrust : Mass - - «part def» - EnduranceBattery - - «part def» - Drone - - attributes - totalMass : Mass - - parts - airframe : Frame - battery : Battery - controller : FlightController - motors : Motor [4] - propellers : Propeller [4] - imu : ImuSensor - gps : GpsSensor - - «part def» - RacingDrone - - parts - frontMotors : Motor [2] - - «requirement def» - FlightTimeRequirement - - «subject» - drone : Drone - - attributes - minFlightTimeMinutes : Mass - - «require constraint» - {drone.totalMass<=2500} - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - «allocate» + «interface def» + PowerBus + + «interface def» + DataBus + + «port def» + PowerPort + + «port def» + TelemetryPort + + «port def» + MotorControlPort + + «port def» + SensorPort + + «attribute def» + Mass + + «attribute def» + Voltage + + «enum def» + FlightMode + + enum values + idle + manual + autonomous + + «part def» + Battery + + attributes + capacity : Voltage + + ports + output : PowerPort + + + Rechargeable lithium-polymer battery pack supplying the drone's + * flight controller, motors, and sensor bus via the PowerPort + * interface. Demonstrates the BoxShape.Note annotation rendering. + + «part def» + FlightController + + attributes + mode : FlightMode + + ports + power : PowerPort + telemetry : TelemetryPort + motors : MotorControlPort + sensors : SensorPort + + «part def» + Motor + + attributes + maxThrust : Mass + + ports + control : MotorControlPort + + «part def» + Propeller + + «part def» + ImuSensor + + ports + data : SensorPort + + «part def» + GpsSensor + + ports + data : SensorPort + + «part def» + Frame + + «part def» + RacingMotor + + attributes + maxThrust : Mass + + «part def» + EnduranceBattery + + «part def» + Drone + + attributes + totalMass : Mass + + parts + airframe : Frame + battery : Battery + controller : FlightController + motors : Motor [4] + propellers : Propeller [4] + imu : ImuSensor + gps : GpsSensor + + «part def» + RacingDrone + + parts + frontMotors : Motor [2] + + «requirement def» + FlightTimeRequirement + + «subject» + drone : Drone + + attributes + minFlightTimeMinutes : Mass + + «require constraint» + {drone.totalMass<=2500} + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + «allocate» diff --git a/docs/gallery/svg/GroundSegmentRecursiveView.svg b/docs/gallery/svg/GroundSegmentRecursiveView.svg index 137a461..914f011 100644 --- a/docs/gallery/svg/GroundSegmentRecursiveView.svg +++ b/docs/gallery/svg/GroundSegmentRecursiveView.svg @@ -1,4 +1,4 @@ - + @@ -30,22 +30,22 @@ - - «part def» - Uplink - - «part def» - Downlink - - «part def» - OperatorConsole - - «part def» - DisplayPanel - - «part def» - CommsHandset - - «part def» - ThermalRegulator + + «part def» + OperatorConsole + + «part def» + DisplayPanel + + «part def» + CommsHandset + + «part def» + Uplink + + «part def» + Downlink + + «part def» + ThermalRegulator diff --git a/docs/gallery/svg/MissionControlGeneralView.svg b/docs/gallery/svg/MissionControlGeneralView.svg index 4a3b279..57bf26f 100644 --- a/docs/gallery/svg/MissionControlGeneralView.svg +++ b/docs/gallery/svg/MissionControlGeneralView.svg @@ -1,4 +1,4 @@ - + @@ -30,34 +30,34 @@ - - «package» - RadioNetwork - - «part def» - Uplink - - «part def» - Downlink - - «package» - GroundSegment - - «part def» - OperatorConsole - - «part def» - DisplayPanel - - «part def» - CommsHandset - - «part def» - ThermalRegulator - - «package» - SpaceSegment - - «part def» - Payload + + «package» + RadioNetwork + + «part def» + Uplink + + «part def» + Downlink + + «package» + GroundSegment + + «part def» + OperatorConsole + + «part def» + DisplayPanel + + «part def» + CommsHandset + + «part def» + ThermalRegulator + + «package» + SpaceSegment + + «part def» + Payload diff --git a/docs/reqstream/sysml2-tools-core/layout/internal/general-view-layout-strategy.yaml b/docs/reqstream/sysml2-tools-core/layout/internal/general-view-layout-strategy.yaml index d24fd46..6ad3b19 100644 --- a/docs/reqstream/sysml2-tools-core/layout/internal/general-view-layout-strategy.yaml +++ b/docs/reqstream/sysml2-tools-core/layout/internal/general-view-layout-strategy.yaml @@ -258,11 +258,13 @@ sections: boxes overlap and every package folder stays clear of the others. justification: | The whole General View is expressed as a single input LayoutGraph — package folders as - container nodes, definitions as leaves — and placed with one - HierarchicalLayoutAlgorithm.Apply call: the root scope packs folders and top-level - definitions by reading order (ContainmentLayoutAlgorithm), while each folder's own - contents are ordered by their intra-package edges with the bundled layered algorithm - (LayeredLayoutAlgorithm). Delegating the whole placement, including cross-container edge + container nodes, definitions as leaves — and placed with one LayoutEngine.Layout call + via the "auto" meta-algorithm (AutoLayoutAlgorithm): the root scope classifies folders + and top-level definitions by connectivity, routing connected groups through the bundled + layered algorithm (LayeredLayoutAlgorithm) and packing disconnected/singleton nodes by + reading order (ContainmentLayoutAlgorithm), while each folder's own contents are always + ordered by their intra-package edges with the layered algorithm. Delegating the whole + placement, including cross-container edge anchoring, to Rendering's own engine gives the General View ELK-style box placement and orthogonal edge routing without SysML2Tools duplicating any layout logic. The observable guarantee that matters to readers is that boxes and folders never overlap, regardless of diff --git a/src/DemaConsulting.SysML2Tools.Core/DemaConsulting.SysML2Tools.Core.csproj b/src/DemaConsulting.SysML2Tools.Core/DemaConsulting.SysML2Tools.Core.csproj index 1dacb42..3dfbfdd 100644 --- a/src/DemaConsulting.SysML2Tools.Core/DemaConsulting.SysML2Tools.Core.csproj +++ b/src/DemaConsulting.SysML2Tools.Core/DemaConsulting.SysML2Tools.Core.csproj @@ -56,7 +56,7 @@ - + diff --git a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/ActionFlowViewLayoutStrategy.cs b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/ActionFlowViewLayoutStrategy.cs index 43c034a..e31ccfa 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/ActionFlowViewLayoutStrategy.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/ActionFlowViewLayoutStrategy.cs @@ -85,11 +85,15 @@ public LayoutTree BuildLayout(ViewContext context, RenderOptions options) // Place action boxes with the layered algorithm flowing top-to-bottom (down). Each action // becomes a node and each succession a directed edge; the algorithm's cycle-breaking stage // makes the (possibly cyclic) flow graph acyclic, so it tolerates back edges. Self-loops are - // already excluded by ResolveSuccessions (it keeps only from != to). + // already excluded by ResolveSuccessions (it keeps only from != to). MergeParallelEdges is + // disabled so AddSuccessionEdges' 1:1 edgePolylines[e] indexing (see its own remarks) stays + // valid: with merging enabled, a back edge sharing a node pair with a forward edge (a cycle) + // collapses onto one shared line, leaving fewer routed lines than input successions. var placed = LayeredPlacement.Place( actions.Select(a => (a.Width, a.Height)).ToList(), edges, - LayoutFlowDirection.Down); + LayoutFlowDirection.Down, + mergeParallelEdges: false); // Compute the top-left of the content bounding box over the real action nodes and the screen // offset that normalizes it into the canvas, reserving a marker band at the top (start) and diff --git a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/GeneralViewLayoutStrategy.cs b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/GeneralViewLayoutStrategy.cs index 55d538a..6bdb916 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/GeneralViewLayoutStrategy.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/GeneralViewLayoutStrategy.cs @@ -23,10 +23,12 @@ namespace DemaConsulting.SysML2Tools.Layout.Internal; /// Every definition and package folder is expressed as a single — packages /// as -shaped container nodes, definitions as leaf nodes carrying their /// and — and the -/// whole graph is placed with a single call: the root -/// scope packs package folders and top-level definitions by reading order -/// (), while each folder's own contents are ordered by their -/// intra-package edges with the bundled layered algorithm (). All +/// whole graph is placed with a single call via +/// : root-scope folders/top-level definitions are classified by +/// connectivity, with connected groups routed through the bundled layered algorithm +/// () and disconnected/singleton nodes packed by reading order +/// (), while each folder's own contents are always ordered by +/// their intra-package edges with the layered algorithm. All /// box sizing (title bands, compartment rows) remains this strategy's responsibility, since the /// layout stage is theme-agnostic. Standard-library declarations are excluded via /// . caps nesting depth @@ -323,12 +325,14 @@ public LayoutTree BuildLayout(ViewContext context, RenderOptions options) // Build the single input graph: package folders as containers, definitions as leaves. var (graph, truncated) = BuildGraph(groups, childrenByParent, modelEdges, theme, options.DepthLimit, scope is not null); - // Lay out the whole graph in one call: the root scope packs folders/top-level definitions by - // reading order (containment), while each folder's own contents are ordered by their - // intra-package edges with the layered algorithm — selected per folder node, per the - // established per-container-algorithm convention. - var rootOptions = LayoutOptions.ForAlgorithm(ContainmentLayoutAlgorithm.AlgorithmId); - var tree = new HierarchicalLayoutAlgorithm().Apply(graph, rootOptions); + // Lay out the whole graph in one call, delegating root-scope algorithm selection to "auto": + // it classifies the root's own folders/top-level definitions by connectivity, routing + // connected groups through the layered algorithm and packing disconnected/singleton nodes by + // reading order (containment) — the same per-container-algorithm convention this strategy + // already applies per folder, just resolved automatically at the root instead of being pinned + // to a single fixed algorithm for every root-scope node regardless of its own connectivity. + graph.Set(CoreOptions.Algorithm, AutoLayoutAlgorithm.AlgorithmId); + var tree = LayoutEngine.Layout(graph); // Stamp the "+N more…" ellipsis label onto each truncated folder's or truncated // definition's placed box. The leaf algorithm emits one box per node in Nodes order at @@ -1359,7 +1363,7 @@ private static LayoutGraphNode MakeDefNode(LayoutGraph scope, DefBox def) /// pattern manipulates raw Rect/LayoutNode coordinates after a simpler, /// non-graph-based layout pass specific to state diagrams, which does not exist in this /// strategy's architecture (a single handed once to - /// ). Emitting the note as an ordinary graph + /// ). Emitting the note as an ordinary graph /// node/edge is consistent with how every other box/edge in this file is already produced, and /// lower-risk than hand-computing satellite coordinates in a graph-based layout model that was /// not designed for post-hoc absolute placement. Does nothing when the definition has no diff --git a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/InterconnectionViewLayoutStrategy.cs b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/InterconnectionViewLayoutStrategy.cs index 23143bd..2a4449e 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/InterconnectionViewLayoutStrategy.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/InterconnectionViewLayoutStrategy.cs @@ -4,6 +4,7 @@ using DemaConsulting.Rendering; using DemaConsulting.Rendering.Abstractions; +using DemaConsulting.Rendering.Layout; using DemaConsulting.SysML2Tools.Rendering; using DemaConsulting.SysML2Tools.Rendering.Internal; using DemaConsulting.SysML2Tools.Semantic; @@ -93,8 +94,13 @@ public LayoutTree BuildLayout(ViewContext context, RenderOptions options) var scope = ExposeScopeResolver.ResolveExposedScope(context.Workspace, context.ViewNode); + // Index of candidate container definitions (non-stdlib part defs with at least one part child). + // Built before FindRoot so root-selection can resolve which candidates are themselves used as + // another candidate's nested part type (see FindRoot's composition-graph-root preference). + var defsByName = BuildDefinitionIndex(context.Workspace); + // Choose the part definition whose internals to show. - var root = FindRoot(context.Workspace, scope); + var root = FindRoot(context.Workspace, scope, defsByName); if (root is null) { return new LayoutTree(200.0, 100.0, []); @@ -107,9 +113,6 @@ public LayoutTree BuildLayout(ViewContext context, RenderOptions options) return new LayoutTree(200.0, 100.0, []); } - // Index of candidate container definitions (non-stdlib part defs with at least one part child). - var defsByName = BuildDefinitionIndex(context.Workspace); - // Lay out the root's interior, recursing into any container parts. var visited = new HashSet(StringComparer.Ordinal); if (root.QualifiedName is { Length: > 0 }) @@ -140,9 +143,11 @@ public LayoutTree BuildLayout(ViewContext context, RenderOptions options) /// /// Lays out the interior of one definition: collects its parts (recursing into container - /// parts), places them with the bundled layered algorithm, and emits one rounded - /// box per part plus a port pair and connector line per connection — all positioned relative to - /// the container's own top-left origin (0, 0). + /// parts), places them with the "auto" layout algorithm — which classifies parts by connectivity + /// and packs disconnected/singleton parts via the containment algorithm while routing connected + /// groups through the bundled layered algorithm — and emits one rounded box per part plus a port + /// pair and connector line per connection, all positioned relative to the container's own + /// top-left origin (0, 0). /// /// The definition whose interior to lay out. /// The active rendering theme. @@ -165,6 +170,19 @@ private static InteriorLayout LayOutInterior( var partIndex = BuildPartIndex(parts); var pairs = ResolveConnections(def, partIndex); + return LayOutInteriorWithConnections(parts, pairs, theme, depth); + } + + /// + /// Lays out a definition's parts when at least one connection exists between them, delegating + /// placement and orthogonal edge routing to the bundled layered algorithm. + /// + private static InteriorLayout LayOutInteriorWithConnections( + IReadOnlyList parts, + IReadOnlyList pairs, + Theme theme, + int depth) + { var nodeSizes = parts.Select(p => (p.Width, p.Height, HasLabel: true, HasKeyword: true)).ToList(); var portEdges = pairs @@ -267,27 +285,35 @@ private static InteriorLayout LayOutInterior( } /// - /// Finds the part definition whose interior to render: the non-stdlib part def - /// with the most connections, falling back to the one with the most part usages. When + /// Finds the part definition whose interior to render: among scope-relevant candidates, prefers + /// the composition-graph root — a candidate that is not itself referenced as another candidate's + /// own nested part feature type — falling back to the non-stdlib part def with the + /// most connections (then most part usages) when no single such root exists. When /// is non-null (the view's resolved expose containment-subtree /// scope), candidates are first restricted to those relevant to the scope via /// — the candidate itself is an exposed /// subject, lies within an exposed subject's subtree, or an exposed subject lies within the - /// candidate's own subtree; because a nested definition and its ancestor can both be - /// scope-relevant, ties among relevant candidates are then broken by specificity (deepest/longest - /// qualified name wins) via , with the - /// connections/parts heuristic used only to break ties between equally specific candidates. When - /// no candidate is scope-relevant, no root is chosen (an empty canvas results, matching the - /// existing null-root path). When is , selection is - /// the plain connections/parts heuristic, unchanged. + /// candidate's own subtree. A broad expose (e.g. an entire namespace recursively) makes every + /// definition in it scope-relevant, so preferring the one candidate nothing else composes is what + /// actually identifies "the" root — a plain specificity/qualified-name-depth comparison cannot, + /// since sibling leaf definitions are equally specific to each other regardless of which one + /// happens to be the true top of the tree. When one or more candidates qualify as composition-graph + /// roots — including the case of several disjoint composition trees all independently exposed at + /// once — selection narrows to just that set, and ties among them (or among the full candidate + /// set, when none qualify — e.g. a cyclic composition graph where every candidate is someone's + /// child) are broken by specificity (deepest/longest qualified name wins) via + /// , with the connections/parts heuristic + /// used only to break ties between equally specific candidates. When no candidate is + /// scope-relevant, no root is chosen (an empty canvas results, matching the existing null-root + /// path). When is , selection is the plain + /// connections/parts heuristic, unchanged. /// - private static SysmlDefinitionNode? FindRoot(SysmlWorkspace workspace, ExposedScope? scope) + private static SysmlDefinitionNode? FindRoot( + SysmlWorkspace workspace, + ExposedScope? scope, + IReadOnlyDictionary defsByName) { - SysmlDefinitionNode? best = null; - string? bestQualifiedName = null; - var bestConnections = -1; - var bestParts = -1; - + var candidates = new List<(string QualifiedName, SysmlDefinitionNode Def, int Connections, int Parts)>(); foreach (var (qualifiedName, node) in workspace.Declarations) { if (node is not SysmlDefinitionNode def || def.DefinitionKeyword != "part def") @@ -307,6 +333,47 @@ private static InteriorLayout LayOutInterior( var connections = def.Children.OfType().Count(); var partCount = def.Children.OfType().Count(f => f.FeatureKeyword == "part"); + candidates.Add((qualifiedName, def, connections, partCount)); + } + + if (candidates.Count == 0) + { + return null; + } + + // Every candidate that some other candidate's own nested part feature resolves to as its + // type — i.e. every candidate that is a child in the composition graph, not its top. + var usedAsChildType = new HashSet(StringComparer.Ordinal); + foreach (var (_, def, _, _) in candidates) + { + foreach (var feature in def.Children.OfType()) + { + if (feature.FeatureKeyword != "part") + { + continue; + } + + if (ResolveByTyping(feature.FeatureTyping, defsByName)?.QualifiedName is { Length: > 0 } usedName) + { + usedAsChildType.Add(usedName); + } + } + } + + // A true composition-graph root must itself compose something (Parts > 0); otherwise an + // unrelated orphan leaf definition (composes nothing, is composed by nothing) would also + // qualify as "unused," and could then win the specificity tie-break below purely by having + // a longer qualified name, even though it has no interior worth rendering at all. + var topCandidates = candidates.Where(c => c.Parts > 0 && !usedAsChildType.Contains(c.QualifiedName)).ToList(); + var pool = topCandidates.Count > 0 ? topCandidates : candidates; + + SysmlDefinitionNode? best = null; + string? bestQualifiedName = null; + var bestConnections = -1; + var bestParts = -1; + + foreach (var (qualifiedName, def, connections, partCount) in pool) + { var scoreBetter = connections > bestConnections || (connections == bestConnections && partCount > bestParts); var isBetter = scope is not null ? ExposeScopeResolver.IsMoreSpecificCandidate(qualifiedName, bestQualifiedName, scoreBetter) @@ -328,11 +395,18 @@ private static InteriorLayout LayOutInterior( /// Collects the nested part usages of a definition, sized for rendering. A part whose type /// resolves to a container definition (a non-stdlib part def with its own internal parts, /// not already on the recursion path) is laid out recursively and sized to fit its interior; - /// every other part is sized intrinsically as a leaf. When is non-null, - /// a part feature whose own QualifiedName fails - /// is skipped; the same absolute - /// list is passed unchanged into recursive container calls, since - /// subject qualified names are absolute and need no re-resolution at deeper nesting levels. + /// every other part is sized intrinsically as a leaf. When is non-null + /// and is 0 (the root definition's own direct part usages), a + /// part feature whose own QualifiedName fails + /// is skipped — this lets a narrow expose (e.g. one specific subsystem, not the whole + /// system) select which of the root's own branches to draw. Scope is not re-applied to + /// any deeper recursive call ( > 0): per the InterconnectionView + /// definition (SysML v2 spec §9.2.20.2.6), an interconnection diagram presents "exposed features + /// as nodes, nested features as nested nodes" — once a part has been included as a node, its own + /// composed structure must always be shown recursively, regardless of whether that structure + /// happens to be declared in a different namespace than the one named in the view's expose + /// statement (namespace location and composition structure are independent in SysML v2 — a part + /// def's own nested parts are not themselves separate members of the exposed namespace). /// private static IReadOnlyList CollectParts( SysmlDefinitionNode root, @@ -350,7 +424,7 @@ private static IReadOnlyList CollectParts( continue; } - if (scope is not null && feature.QualifiedName is { Length: > 0 } fqn && + if (depth == 0 && scope is not null && feature.QualifiedName is { Length: > 0 } fqn && !ExposeScopeResolver.IsInSubjectScope(fqn, scope)) { continue; @@ -361,8 +435,11 @@ private static IReadOnlyList CollectParts( if (TryResolveContainer(feature.FeatureTyping, defsByName, visited, out var childDef)) { // Container part: lay out its interior bottom-up and treat it as an atomic node. + // Scope is intentionally not carried into this recursive call — see the remarks + // above on why nested composition structure is always shown once its owning part + // has been included, regardless of the exposed namespace scope. var childVisited = new HashSet(visited, StringComparer.Ordinal) { childDef.QualifiedName! }; - var inner = LayOutInterior(childDef, theme, depth + 1, defsByName, childVisited, scope); + var inner = LayOutInterior(childDef, theme, depth + 1, defsByName, childVisited, scope: null); result.Add(new PartItem(name, "part", feature.FeatureTyping, inner.Width, inner.Height, inner.Content)); } else @@ -425,28 +502,38 @@ private static bool TryResolveContainer( out SysmlDefinitionNode childDef) { childDef = null!; - if (string.IsNullOrEmpty(typing)) + var def = ResolveByTyping(typing, defsByName); + if (def?.QualifiedName is null || visited.Contains(def.QualifiedName)) { return false; } - if (!defsByName.TryGetValue(typing, out var def)) + childDef = def; + return true; + } + + /// + /// Resolves a feature typing reference to its definition by qualified name, falling back to + /// simple (last-segment) name if no qualified match exists. Returns if + /// neither form resolves. + /// + private static SysmlDefinitionNode? ResolveByTyping( + string? typing, + IReadOnlyDictionary defsByName) + { + if (string.IsNullOrEmpty(typing)) { - var sep = typing.LastIndexOf("::", StringComparison.Ordinal); - var simple = sep >= 0 ? typing[(sep + 2)..] : typing; - if (!defsByName.TryGetValue(simple, out def)) - { - return false; - } + return null; } - if (def.QualifiedName is null || visited.Contains(def.QualifiedName)) + if (defsByName.TryGetValue(typing, out var def)) { - return false; + return def; } - childDef = def; - return true; + var sep = typing.LastIndexOf("::", StringComparison.Ordinal); + var simple = sep >= 0 ? typing[(sep + 2)..] : typing; + return defsByName.TryGetValue(simple, out def) ? def : null; } /// Builds a name → index lookup for the nested parts. diff --git a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/LayeredPlacement.cs b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/LayeredPlacement.cs index c4def3e..524fd7b 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/LayeredPlacement.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/LayeredPlacement.cs @@ -98,13 +98,16 @@ internal static class LayeredPlacement /// Primary flow direction for the layered layout. /// /// Whether multiple edges between the same pair of nodes are merged into a single routed - /// connector. Defaults to (the library's own default and this method's - /// original, unconditional behavior), which keeps every pre-existing call site — including - /// ActionFlowViewLayoutStrategy and StateTransitionViewLayoutStrategy — byte-for-byte - /// unchanged. Pass to have every parallel edge preserved as its own - /// independently-routed connector (see ), which - /// InterconnectionViewLayoutStrategy requests so distinct SysML connections between the same - /// two parts never collapse onto one shared route. + /// connector. Defaults to (the library's own default). Pass + /// to have every parallel edge preserved as its own independently-routed + /// connector (see ). Every current caller — + /// ActionFlowViewLayoutStrategy, StateTransitionViewLayoutStrategy, and + /// InterconnectionViewLayoutStrategy — passes : each treats + /// as 1:1 with by index, which + /// only holds when no two edges are collapsed onto one shared line. Leaving the parameter's + /// default at for a caller whose edges can be cyclic or parallel (e.g. a + /// back edge sharing a node pair with a forward edge) will corrupt that indexing — collapsing + /// yields fewer entries in than . /// /// The placed rectangles, routed polylines, and overall content size. /// @@ -256,6 +259,7 @@ public static PlacedPortLayout PlaceWithPorts( graph.Set(CoreOptions.Direction, direction); graph.Set(CoreOptions.MergeParallelEdges, false); + graph.Set(CoreOptions.Algorithm, AutoLayoutAlgorithm.AlgorithmId); var tree = LayoutEngine.Layout(graph); diff --git a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/StateTransitionViewLayoutStrategy.cs b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/StateTransitionViewLayoutStrategy.cs index a86576b..9707d2c 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/StateTransitionViewLayoutStrategy.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/StateTransitionViewLayoutStrategy.cs @@ -76,12 +76,16 @@ public LayoutTree BuildLayout(ViewContext context, RenderOptions options) // Place state boxes with the layered algorithm flowing top-to-bottom (down). Non-self // transitions become directed edges; the algorithm's cycle-breaking stage makes the (cyclic) - // state graph acyclic, so it tolerates back edges and loops. + // state graph acyclic, so it tolerates back edges and loops. MergeParallelEdges is disabled so + // AddTransitions' 1:1 edgePolylines[e] indexing (see its own remarks) stays valid: with merging + // enabled, a back edge sharing a node pair with a forward edge (a cycle) collapses onto one + // shared line, leaving fewer routed lines than input transitions. var flowTransitions = transitions.Where(t => t.Source != t.Target).ToList(); var placed = LayeredPlacement.Place( states.Select(s => (s.Width, s.Height)).ToList(), flowTransitions.Select(t => (t.Source, t.Target)).ToList(), - LayoutFlowDirection.Down); + LayoutFlowDirection.Down, + mergeParallelEdges: false); // Compute the top-left of the content bounding box over the real state nodes and the screen // offset that normalizes it into the canvas (leaving room at the top for the initial marker). diff --git a/src/DemaConsulting.SysML2Tools.Tool/DemaConsulting.SysML2Tools.Tool.csproj b/src/DemaConsulting.SysML2Tools.Tool/DemaConsulting.SysML2Tools.Tool.csproj index 17c2e0a..dc18605 100644 --- a/src/DemaConsulting.SysML2Tools.Tool/DemaConsulting.SysML2Tools.Tool.csproj +++ b/src/DemaConsulting.SysML2Tools.Tool/DemaConsulting.SysML2Tools.Tool.csproj @@ -98,8 +98,8 @@ - - + + diff --git a/test/DemaConsulting.SysML2Tools.Tests/DemaConsulting.SysML2Tools.Tests.csproj b/test/DemaConsulting.SysML2Tools.Tests/DemaConsulting.SysML2Tools.Tests.csproj index 9381b76..3f6744a 100644 --- a/test/DemaConsulting.SysML2Tools.Tests/DemaConsulting.SysML2Tools.Tests.csproj +++ b/test/DemaConsulting.SysML2Tools.Tests/DemaConsulting.SysML2Tools.Tests.csproj @@ -56,8 +56,8 @@ - - + + diff --git a/test/DemaConsulting.SysML2Tools.Tests/Layout/InterconnectionViewLayoutStrategyTests.cs b/test/DemaConsulting.SysML2Tools.Tests/Layout/InterconnectionViewLayoutStrategyTests.cs index fbf6a48..0e60053 100644 --- a/test/DemaConsulting.SysML2Tools.Tests/Layout/InterconnectionViewLayoutStrategyTests.cs +++ b/test/DemaConsulting.SysML2Tools.Tests/Layout/InterconnectionViewLayoutStrategyTests.cs @@ -1190,4 +1190,216 @@ public void InterconnectionView_BuildLayout_ExposedUsage_ResolvesThroughTypingTo var container = CollectBoxes(layout.Nodes).First(b => b.Keyword == "part def"); Assert.Equal("SysB", container.Label); } + + /// + /// Builds a workspace where the root system definition (Root::NsA::System) composes a + /// subsystem (Root::NsB::Sub) via a typed part feature, and that subsystem in + /// turn composes two of its own nested units — but the subsystem lives in a different + /// namespace than the root, so the subsystem's own part features' qualified names + /// (Root::NsB::Sub::unit1/unit2) fall outside the Root::NsA::System:: + /// prefix. This is the exact shape that reproduced the VersionMark bug: composition structure + /// and namespace/file organization are independent in SysML v2, so a re-applied expose-scope + /// namespace-prefix check at recursion depth > 0 incorrectly hid a genuinely nested part's + /// own interior. + /// + private static SysmlWorkspace BuildCrossNamespaceCompositionWorkspace() + { + var subsystem = new SysmlDefinitionNode + { + Name = "Sub", + QualifiedName = "Root::NsB::Sub", + DefinitionKeyword = "part def", + Children = + [ + new SysmlFeatureNode { Name = "unit1", QualifiedName = "Root::NsB::Sub::unit1", FeatureKeyword = "part", FeatureTyping = "Unit" }, + new SysmlFeatureNode { Name = "unit2", QualifiedName = "Root::NsB::Sub::unit2", FeatureKeyword = "part", FeatureTyping = "Unit" } + ] + }; + var system = new SysmlDefinitionNode + { + Name = "System", + QualifiedName = "Root::NsA::System", + DefinitionKeyword = "part def", + Children = + [ + new SysmlFeatureNode { Name = "sub", QualifiedName = "Root::NsA::System::sub", FeatureKeyword = "part", FeatureTyping = "Sub" } + ] + }; + return new SysmlWorkspace + { + Declarations = new Dictionary + { + ["Root::NsA::System"] = system, + ["Root::NsB::Sub"] = subsystem + } + }; + } + + /// + /// Regression test (SysML v2 spec §9.2.20.2.6, "nested features as nested nodes"): when the + /// view exposes only the root definition recursively (expose Root::NsA::System::**;), + /// the root's nested subsystem part still renders its own nested units, even though the + /// subsystem is declared in a different namespace than the root's exposed subject and its own + /// part features' qualified names fall outside the root subject's namespace prefix. Before + /// the fix, the same expose scope was re-applied unchanged at every recursion depth, so the + /// subsystem's own parts were incorrectly filtered out and rendered as an empty box. + /// + [Fact] + public void InterconnectionView_BuildLayout_ExposeRootOnly_NestedSubsystemInDifferentNamespace_RendersItsOwnUnits() + { + var strategy = new InterconnectionViewLayoutStrategy(); + var workspace = BuildCrossNamespaceCompositionWorkspace(); + var viewNode = new SysmlViewNode + { + Name = "V", + QualifiedName = "Root::V", + ExposeMembers = [new ExposeMember("Root::NsA::System", null, ExposeRecursionKind.MembershipRecursive)], + ResolvedEdges = [new SysmlEdge("Root::V", "Root::NsA::System", SysmlEdgeKind.Expose)] + }.WithResolvedExposeMembers(); + var context = new ViewContext("v", workspace, viewNode); + var options = new RenderOptions(Themes.Light); + + var layout = strategy.BuildLayout(context, options); + + var container = CollectBoxes(layout.Nodes).First(b => b.Keyword == "part def"); + Assert.Equal("System", container.Label); + + // The subsystem's own nested units render as nested part boxes, not an empty container. + var subBox = FindPartBox(layout, "sub : Sub"); + var nestedLabels = CollectBoxes(subBox.Children).Select(b => b.Label).ToList(); + Assert.Contains(nestedLabels, l => l is not null && l.Contains("unit1", StringComparison.Ordinal)); + Assert.Contains(nestedLabels, l => l is not null && l.Contains("unit2", StringComparison.Ordinal)); + } + + /// + /// Builds a workspace with a genuine two-tier composition root (Root::NsA::System + /// composes Root::NsB::Sub via a typed part feature) alongside an unrelated + /// orphan definition (Root::NsC::Deeply::Nested::OrphanLeaf) that has a deeper + /// qualified name than System but composes nothing and is composed by nothing. + /// + private static SysmlWorkspace BuildBroadNamespaceWorkspace() + { + var system = new SysmlDefinitionNode + { + Name = "System", + QualifiedName = "Root::NsA::System", + DefinitionKeyword = "part def", + Children = + [ + new SysmlFeatureNode { Name = "sub", QualifiedName = "Root::NsA::System::sub", FeatureKeyword = "part", FeatureTyping = "Sub" } + ] + }; + var sub = new SysmlDefinitionNode { Name = "Sub", QualifiedName = "Root::NsB::Sub", DefinitionKeyword = "part def" }; + var orphanLeaf = new SysmlDefinitionNode + { + Name = "OrphanLeaf", + QualifiedName = "Root::NsC::Deeply::Nested::OrphanLeaf", + DefinitionKeyword = "part def" + }; + return new SysmlWorkspace + { + Declarations = new Dictionary + { + ["Root::NsA::System"] = system, + ["Root::NsB::Sub"] = sub, + ["Root::NsC::Deeply::Nested::OrphanLeaf"] = orphanLeaf + } + }; + } + + /// + /// Regression test: exposing an entire namespace recursively (expose Root::**;) makes + /// every definition it contains scope-relevant, including an unrelated orphan leaf definition + /// with a deeper qualified name than the genuine composition root. FindRoot must select + /// System — the one candidate nothing else composes and that itself composes + /// something — rather than OrphanLeaf, which the old pure qualified-name-depth + /// specificity tie-break would have picked purely for having more "::" segments, + /// despite composing and being composed by nothing (the exact "arbitrary root" bug diagnosed + /// against the real VersionMark model). + /// + [Fact] + public void InterconnectionView_BuildLayout_ExposeWholeNamespace_SelectsGenuineCompositionRootNotDeepestOrphan() + { + var strategy = new InterconnectionViewLayoutStrategy(); + var workspace = BuildBroadNamespaceWorkspace(); + var viewNode = new SysmlViewNode + { + Name = "V", + QualifiedName = "Root::V", + ExposeMembers = [new ExposeMember("Root", null, ExposeRecursionKind.NamespaceRecursive)], + ResolvedEdges = [new SysmlEdge("Root::V", "Root", SysmlEdgeKind.Expose)] + }.WithResolvedExposeMembers(); + var context = new ViewContext("v", workspace, viewNode); + var options = new RenderOptions(Themes.Light); + + var layout = strategy.BuildLayout(context, options); + + var container = CollectBoxes(layout.Nodes).First(b => b.Keyword == "part def"); + Assert.Equal("System", container.Label); + } + + /// + /// Regression test: a part def whose parts have no connections between them (a common case + /// for a top-level system composed purely of independent subsystems) must be packed into a + /// compact multi-row/column grid via the containment-packing algorithm, not stacked into a + /// single tall vertical column. Reproduces the shape of the real VersionMark model that + /// surfaced the bug: nine unconnected peer parts. (Fewer than about nine same-sized minimum- + /// width parts is not actually enough for the packer's area-based width budget to clear the + /// two-column threshold — this part count is the smallest that reliably demonstrates the + /// fix rather than an artifact of the packer's own sizing heuristic.) A degenerate single- + /// column layout would place every box at a distinct X (or Y) coordinate; the containment- + /// packed layout instead reuses coordinates across rows/columns. + /// + [Fact] + public void InterconnectionView_BuildLayout_UnconnectedPeerParts_PacksIntoCompactGridNotSingleColumn() + { + // Arrange: a System part def with nine unconnected parts (no SysmlConnectionNode children). + var strategy = new InterconnectionViewLayoutStrategy(); + var system = new SysmlDefinitionNode + { + Name = "System", + QualifiedName = "M::System", + DefinitionKeyword = "part def", + Children = + [ + new SysmlFeatureNode { Name = "p1", QualifiedName = "M::System::p1", FeatureKeyword = "part", FeatureTyping = "P1" }, + new SysmlFeatureNode { Name = "p2", QualifiedName = "M::System::p2", FeatureKeyword = "part", FeatureTyping = "P2" }, + new SysmlFeatureNode { Name = "p3", QualifiedName = "M::System::p3", FeatureKeyword = "part", FeatureTyping = "P3" }, + new SysmlFeatureNode { Name = "p4", QualifiedName = "M::System::p4", FeatureKeyword = "part", FeatureTyping = "P4" }, + new SysmlFeatureNode { Name = "p5", QualifiedName = "M::System::p5", FeatureKeyword = "part", FeatureTyping = "P5" }, + new SysmlFeatureNode { Name = "p6", QualifiedName = "M::System::p6", FeatureKeyword = "part", FeatureTyping = "P6" }, + new SysmlFeatureNode { Name = "p7", QualifiedName = "M::System::p7", FeatureKeyword = "part", FeatureTyping = "P7" }, + new SysmlFeatureNode { Name = "p8", QualifiedName = "M::System::p8", FeatureKeyword = "part", FeatureTyping = "P8" }, + new SysmlFeatureNode { Name = "p9", QualifiedName = "M::System::p9", FeatureKeyword = "part", FeatureTyping = "P9" } + ] + }; + var workspace = new SysmlWorkspace + { + Declarations = new Dictionary { ["M::System"] = system } + }; + var context = new ViewContext("SystemInterconnectionView", workspace); + var options = new RenderOptions(Themes.Light); + + // Act + var layout = strategy.BuildLayout(context, options); + + // Assert: nine non-overlapping part boxes, no connector lines (nothing to route). + var parts = CollectBoxes(layout.Nodes).Where(b => b.Shape == BoxShape.RoundedRectangle).ToList(); + Assert.Equal(9, parts.Count); + Assert.Empty(CollectLines(layout.Nodes)); + for (var i = 0; i < parts.Count; i++) + { + for (var j = i + 1; j < parts.Count; j++) + { + Assert.False(Overlaps(parts[i], parts[j]), $"{parts[i].Label} overlaps {parts[j].Label}"); + } + } + + // Assert: packed into more than one row and more than one column — a degenerate single + // vertical column would have six distinct Y values and a single shared X value. + var distinctX = parts.Select(p => p.X).Distinct().Count(); + var distinctY = parts.Select(p => p.Y).Distinct().Count(); + Assert.True(distinctX > 1, "Expected parts to span more than one column, not a single vertical column."); + Assert.True(distinctY < parts.Count, "Expected some parts to share a row, not one row per part."); + } }