Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changes/unreleased/drawio-unrouted-edges.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- The `drawio` rendering form no longer falls back silently when the router finds no path for an edge: the file carries a notice naming the edge and its cells, and an end at a port still leaves through the port's face. Two causes of such edges are gone: a flush pin at a quarter, half or three quarters of a side (a container's header pin above a centred member) coincided with one of libavoid's generic pins and attached to neither, in the diagram panel's router as well as in the export, and a member drawn outside its container was routed inside the container's bounds. Routed waypoints and anchors are written to a thousandth of a pixel instead of with float noise, while a stated `Route` keeps its exact points, and an edge that is a declaration of its own (a connection, flow, succession, transition, dependency and the like) carries its qualified name as `fqn` on its `<object>`, as the nodes do.
19 changes: 14 additions & 5 deletions docs/project/view-rendering-forms.md
Original file line number Diff line number Diff line change
Expand Up @@ -1419,9 +1419,10 @@ one model write byte-identical files. No draw.io is run to write it, and none is
Every semantic cell — a node, a port, an edge — is wrapped in an `<object>` carrying the
element's handle beside draw.io's own `label`: `kind`, the node's keyword (`part def`, `state`,
`initial`, …) or the edge's kind; `view`, the view rendered; `link`, the declaration's
`file:line` when the element has one; and, on a node or a port, `fqn`, the declaration's
qualified name. An edge carries no `fqn` — a relationship has no name of its own in most
models — and is matched by its `link` and the `source` and `target` cells it joins.
`file:line` when the element has one; and `fqn`, the declaration's qualified name, on a node, a
port, and an edge that is a declaration of its own (a connection, flow, succession, transition,
dependency and the like). An edge drawn from a member's declaration — a composition, a typing —
carries no `fqn` and is matched by its `link` and the `source` and `target` cells it joins.
Compartment rows and edge end labels are plain `mxCell`s and carry none of these. The `fqn` is
exact and the same for every cell of one element, so a tool reading an edited file back can
match each cell to its declaration; a later round trip of positions edited in draw.io builds on
Expand Down Expand Up @@ -1461,8 +1462,16 @@ of one container share stay inside that container, with its header and its other
obstacles and the ancestors an end lies in reduced to their headers; an edge from a container to
its own member leaves the header's underside above the member. Containers are padded by more than
twice the clearance so a path fits between neighbours. A stated `Route` is never re-routed, and
an edge the router finds no path for (two boxes nearer than twice the clearance) is written
without waypoints under `edgeStyle=orthogonalEdgeStyle`, for draw.io to route as before. The `js` and `wasip1` builds of `sysml`
an edge the router finds no path for (two boxes nearer than twice the clearance plus their
port squares, as the default layout's 40 px between siblings can be) is written without
waypoints under `edgeStyle=orthogonalEdgeStyle`, for draw.io to route as before, under a notice
naming the edge and its cells; an end at a port keeps its face as the anchor, the inner face when
the edge leads into the port's owner. A member drawn outside its container is routed in the
nearest container holding both ends. Routed waypoints are written to a thousandth of a pixel; a
stated `Route` keeps its points. An edge that is a declaration of its own — a connection, flow,
succession, transition, dependency and the like — carries its qualified name as `fqn` on its
`<object>`, as the nodes do; an edge drawn from a member's declaration, such as a composition or a
typing, carries none. The `js` and `wasip1` builds of `sysml`
carry no router (a WebAssembly runtime inside WebAssembly would route several times slower and
add to the download): they write every unrouted edge that way, under a notice, and draw.io routes
them when the file opens.
Expand Down
8 changes: 8 additions & 0 deletions editors/vscode/src/webview/avoid.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -834,6 +834,14 @@ test("avoidRoutes keeps randomized port endpoints on their faces with perpendicu
}
});

test("routingPinPositions drops a generic pin under a flush one so libavoid attaches to the pin", () => {
const box = { x: 0, y: 0, width: 200, height: 40 };
const pins = routingPinPositions({ box, ports: [{ id: "hdr", side: "south", offset: 0.5, flush: true }] });
assert.equal(pins.filter((pin) => pin.side === "south" && pin.y === 1 && pin.x === 0.5).length, 1);
assert.equal(pins.filter((pin) => pin.id === undefined).length, 11);
assert.equal(routingPinPositions({ box, ports: [{ id: "p", side: "south", offset: 0.5 }] }).filter((pin) => pin.id === undefined).length, 12);
});

test("routingPinPositions pins a flush port on the border itself and grows the routing box for none", () => {
const box = { x: 0, y: 0, width: 100, height: 50 };
const pins = routingPinPositions({ box, ports: [{ id: "a/3", side: "south", offset: 0.3, flush: true }] });
Expand Down
7 changes: 6 additions & 1 deletion editors/vscode/src/webview/avoid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,12 @@ export function routingPinPositions(shapeData: AvoidShape): RoutingPinPosition[]
return { id, side, x: 0, y: portion((y - routingBox.y) / routingBox.height) };
}
});
return [...genericPins, ...namedPins];
// libavoid attaches to neither of two pins at one point: a generic pin
// under a named one (a flush pin at a quarter, half or three quarters) goes.
const uncovered = genericPins.filter(
(pin) => !namedPins.some((named) => named.side === pin.side && named.x === pin.x && named.y === pin.y),
);
return [...uncovered, ...namedPins];
}

// addShape is a box as a routing obstacle, with twelve proportional pins spread
Expand Down
116 changes: 99 additions & 17 deletions internal/ir/view/drawio.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/Open-MBEE/OpenSysML/internal/ir/view/route"
"math"
"os"
"slices"
"strconv"
"strings"

Expand Down Expand Up @@ -49,6 +50,8 @@ func (r *Rendering) DrawioWith(options Options) (string, error) {
cells: map[string]bool{},
portBoxes: map[string]drawioBox{},
routed: map[int]bool{},
inward: map[int]drawioEnds{},
fqns: map[string]bool{},
}
if r.Kind == KindAction {
w.used = r.usedPorts(w.ports)
Expand All @@ -71,21 +74,24 @@ func (r *Rendering) DrawioWith(options Options) (string, error) {

// drawioWriter composes one draw.io file from a rendering.
type drawioWriter struct {
fills familyFills
labels labeller
ports portView
used map[string]map[string]bool
links Links
view string
boxes map[string]drawioBox // node ID -> its box in canvas coordinates
unrouted bool // this build has no router; draw.io routes on opening
cells map[string]bool // the cell IDs written, the ends an edge may attach to
portBoxes map[string]drawioBox // port cell ID -> its box in canvas coordinates
routed map[int]bool // the edges (by index) the router gave their route
edgeIDs map[[2]string]string // edge ends -> its cell ID, the anchor an edge note joins
grid int // nodes and notes laid on the grid for want of a position
extent drawioBox // the bounds of everything placed
b strings.Builder
fills familyFills
labels labeller
ports portView
used map[string]map[string]bool
links Links
view string
boxes map[string]drawioBox // node ID -> its box in canvas coordinates
unrouted bool // this build has no router; draw.io routes on opening
cells map[string]bool // the cell IDs written, the ends an edge may attach to
portBoxes map[string]drawioBox // port cell ID -> its box in canvas coordinates
routed map[int]bool // the edges (by index) the router gave their route
unroutable []int // the edges (by index) the router found no path for
inward map[int]drawioEnds // per edge, the ends that lead into the port's owner
fqns map[string]bool // the qualified names written on cells so far
edgeIDs map[[2]string]string // edge ends -> its cell ID, the anchor an edge note joins
grid int // nodes and notes laid on the grid for want of a position
extent drawioBox // the bounds of everything placed
b strings.Builder
}

// drawioBox is an axis-aligned box in canvas coordinates: the top-left corner
Expand All @@ -104,6 +110,11 @@ func (b drawioBox) relativeTo(parent drawioBox) drawioBox {
func (b drawioBox) right() float64 { return b.X + b.Width }
func (b drawioBox) bottom() float64 { return b.Y + b.Height }

// contains reports whether o lies within b.
func (b drawioBox) contains(o drawioBox) bool {
return o.X >= b.X && o.Y >= b.Y && o.right() <= b.right() && o.bottom() <= b.bottom()
}

// The spacing the grid lays unplaced nodes out with, and the padding a
// container keeps about its nested children, in draw.io pixels.
const (
Expand Down Expand Up @@ -449,6 +460,9 @@ func (w *drawioWriter) notices(r *Rendering, options Options) []string {
if w.unrouted {
notices = append(notices, "edges not routed: this build runs no router; draw.io routes them when the file opens")
}
for _, i := range w.unroutable {
notices = append(notices, fmt.Sprintf("edge %s not routed: the router found no path at this layout; draw.io routes it when the file opens", w.edgeName(r.Edges[i])))
}
if options.Style != "" && options.Style != StylePilot {
notices = append(notices, styleNotice(options.Style))
}
Expand Down Expand Up @@ -494,6 +508,7 @@ func (w *drawioWriter) openObject(indent string, o drawioObject) {
w.attr("label", o.label)
if o.fqn != "" {
w.attr("fqn", o.fqn)
w.fqns[o.fqn] = true
}
w.attr("kind", o.kind)
w.attr("view", w.view)
Expand Down Expand Up @@ -941,8 +956,11 @@ func (w *drawioWriter) writeEdge(i int, edge Edge) {
if w.routed[i] && len(points) >= 2 {
anchors = w.anchor("exit", source, points[0]) + w.anchor("entry", target, points[len(points)-1])
points = points[1 : len(points)-1]
} else if slices.Contains(w.unroutable, i) {
// No route, but an end at a port still leaves through the port's face.
anchors = w.faceAnchor("exit", source, w.inward[i]&drawioFrom != 0) + w.faceAnchor("entry", target, w.inward[i]&drawioTo != 0)
}
w.openObject(" ", drawioObject{id: id, label: edge.Label, kind: edge.Kind.String(), link: w.link(edge.Origin)})
w.openObject(" ", drawioObject{id: id, label: edge.Label, fqn: w.edgeFQN(edge), kind: edge.Kind.String(), link: w.link(edge.Origin)})
w.b.WriteString(" <mxCell")
w.attr("style", drawioEdgeFullStyle(edge, anchors))
w.b.WriteString(` edge="1"`)
Expand All @@ -963,6 +981,70 @@ func (w *drawioWriter) writeEdge(i int, edge Edge) {
w.endLabel(id+":to", id, edge.ToLabel, 0.8, edge.Style)
}

// edgeFQN is the qualified name of an edge that is a declaration of its own (a connection,
// flow, transition…); a member's edge (composition, typing) or a name a cell carries gives none.
func (w *drawioWriter) edgeFQN(edge Edge) string {
switch edge.Kind {
case EdgeConnection, EdgeBinding, EdgeAssociation, EdgeFlow, EdgeSuccession, EdgeSuccessionFlow, EdgeTransition,
EdgeRequire, EdgeAssume, EdgeAssert, EdgeFrame, EdgeEvent, EdgeImport, EdgeDependency, EdgeDerive, EdgeRefine,
EdgeExpose, EdgeSatisfy, EdgeVerify, EdgeAllocate, EdgeInclude, EdgePerform, EdgeExhibit:
default:
return ""
}
fqn := w.siteName(edge.Origin)
if w.fqns[fqn] {
return ""
}
return fqn
}

// drawioEnds is a set of edge ends.
type drawioEnds uint8

const (
drawioFrom drawioEnds = 1 << iota
drawioTo
)

// edgeName names an edge in a notice by its qualified name or label, when it has one, and its
// cells, which the file's reader can find and which tell apart a declaration drawn twice.
func (w *drawioWriter) edgeName(edge Edge) string {
ends := edge.From + " -> " + edge.To
if fqn := w.siteName(edge.Origin); fqn != "" {
return fqn + " (" + ends + ")"
}
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Resolved
if edge.Label != "" {
return edge.Label + " (" + ends + ")"
}
return ends
}

// faceAnchor fixes an edge end at the middle of a port cell's outer face, or of its inner
// face when the edge leads into the port's owner; a node end is left to draw.io.
func (w *drawioWriter) faceAnchor(end, cell string, inward bool) string {
owner, _, isPort := strings.Cut(cell, ":")
port, ok := w.portBoxes[cell]
box, known := w.boxes[owner]
if !isPort || !ok || !known {
return ""
}
cx, cy := port.X+port.Width/2, port.Y+port.Height/2
fx, fy, best := 0.5, 0.0, cy-box.Y
if d := box.right() - cx; d < best {
fx, fy, best = 1, 0.5, d
}
if d := box.bottom() - cy; d < best {
fx, fy, best = 0.5, 1, d
}
if d := cx - box.X; d < best {
fx, fy = 0, 0.5
}
if inward {
fx, fy = 1-fx, 1-fy
}
return fmt.Sprintf("%sX=%s;%sY=%s;%sPerimeter=0;", end, formatCoord(fx), end, formatCoord(fy), end)
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Resolved
}

// anchor fixes an edge end at p on its cell, as the fraction of the cell's box
// draw.io reads from exitX/exitY or entryX/entryY, taken exactly (no perimeter).
func (w *drawioWriter) anchor(end, cell string, p Point) string {
Expand All @@ -975,7 +1057,7 @@ func (w *drawioWriter) anchor(end, cell string, p Point) string {
if size > 0 {
f = min(1, max(0, (v-origin)/size))
}
return strconv.FormatFloat(f, 'f', -1, 64)
return strconv.FormatFloat(math.Round(f*10000)/10000, 'f', -1, 64)
}
return fmt.Sprintf("%sX=%s;%sY=%s;%sPerimeter=0;", end, fraction(p.X, box.X, box.Width), end, fraction(p.Y, box.Y, box.Height), end)
}
Expand Down
21 changes: 20 additions & 1 deletion internal/ir/view/drawio_route.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"errors"
"fmt"
"maps"
"math"
"slices"
"sort"

Expand All @@ -25,6 +26,10 @@ func (w *drawioWriter) routeEdges(r *Rendering) ([]Edge, error) {
continue
}
key := tree.commonContainer(edge.From, edge.To)
// A member drawn outside its container is not held by it.
for key != "" && !(w.boxes[key].contains(w.boxes[edge.From]) && w.boxes[key].contains(w.boxes[edge.To])) {
key = tree.parents[key]
}
groups[key] = append(groups[key], i)
}
keys := make([]string, 0, len(groups))
Expand Down Expand Up @@ -64,6 +69,11 @@ func (w *drawioWriter) routeEdges(r *Rendering) ([]Edge, error) {
// leaves it through its inner face, into the container.
if shape, ok := w.portShape(tree.nodes[key], w.boxes[key], port, true); ok {
portShapes[shape.ID] = shape
if edges[i].From == key {
w.inward[i] |= drawioFrom
} else {
w.inward[i] |= drawioTo
}
continue
}
pin := route.Port{ID: fmt.Sprintf("\x00member/%d", i), Side: route.South, Flush: true}
Expand Down Expand Up @@ -123,11 +133,17 @@ func (w *drawioWriter) routeEdges(r *Rendering) ([]Edge, error) {
}
for _, found := range routes {
i := group[found.Index]
// Routed points carry float noise a declared route never has.
for _, p := range found.Points {
edges[i].Route = append(edges[i].Route, Point{X: p.X, Y: p.Y})
edges[i].Route = append(edges[i].Route, Point{X: drawioRound(p.X), Y: drawioRound(p.Y)})
}
w.routed[i] = true
}
for _, i := range group {
if !w.routed[i] {
w.unroutable = append(w.unroutable, i)
}
}
}
return edges, nil
}
Expand Down Expand Up @@ -290,3 +306,6 @@ func (t drawioTree) commonContainer(a, b string) string {
}
return ""
}

// drawioRound trims a routed coordinate to a thousandth of a pixel.
func drawioRound(v float64) float64 { return math.Round(v*1000) / 1000 }
3 changes: 2 additions & 1 deletion internal/ir/view/drawio_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,7 @@ func TestDrawioSharesANoteAmongItsAnchors(t *testing.T) {
func TestDrawioRoutesUnroutedEdges(t *testing.T) {
for _, c := range []struct{ name, file, view string }{
{"interconnection-ports", "interconnection-ports.sysml", "ToasterViews::toasterView"},
{"interconnection-header-ports", "interconnection-header-ports.sysml", "PumpStationViews::stationView"},
{"tree-edges", "tree-edges.sysml", "FleetViews::structure"},
{"layout", "layout.sysml", "PlantViews::placedView"},
} {
Expand Down Expand Up @@ -479,7 +480,7 @@ func TestDrawioRoutesUnroutedEdges(t *testing.T) {
path := append(append([]Point{exit}, cell.points...), entry)
for j := 1; j < len(path); j++ {
a, b := path[j-1], path[j]
if math.Abs(a.X-b.X) > 1e-6 && math.Abs(a.Y-b.Y) > 1e-6 {
if math.Abs(a.X-b.X) > 0.5 && math.Abs(a.Y-b.Y) > 0.5 {
t.Errorf("%s: segment %v -> %v is not orthogonal", cell.id, a, b)
}
}
Expand Down
14 changes: 12 additions & 2 deletions internal/ir/view/route/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package route
import (
"errors"
"math"
"slices"
)

// Routes is the orthogonal route of each edge the router found a path for,
Expand Down Expand Up @@ -232,6 +233,7 @@ func RoutingPinPositions(shape *Shape) []PinPosition {
pins = append(pins, pin)
}
}
var named []PinPosition
for _, port := range shape.Ports {
point := portPoint(box, port)
pin := PinPosition{ID: port.ID, Named: true, Side: port.Side}
Expand All @@ -247,9 +249,17 @@ func RoutingPinPositions(shape *Shape) []PinPosition {
case West:
pin.Y = portion((point.Y - routingBox.Y) / routingBox.Height)
}
pins = append(pins, pin)
named = append(named, pin)
}
return pins
// libavoid attaches to neither of two pins at one point: a generic pin
// under a named one (a flush pin at a quarter, half or three quarters) goes.
kept := pins[:0]
for _, pin := range pins {
if !slices.ContainsFunc(named, func(n PinPosition) bool { return n.Side == pin.Side && n.X == pin.X && n.Y == pin.Y }) {
kept = append(kept, pin)
}
}
return append(kept, named...)
}

// snap rounds to the pixel grid the way JavaScript's Math.round does (halves go up).
Expand Down
15 changes: 15 additions & 0 deletions internal/ir/view/route/route_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,18 @@ func abs(v float64) float64 {
}
return v
}

func TestRoutesAFlushPinUnderAGenericOne(t *testing.T) {
owner := Shape{ID: "owner", Box: Box{X: 0, Y: 0, Width: 200, Height: 40}, Ports: []Port{{ID: "hdr", Side: South, Offset: 0.5, Flush: true}}}
member := Shape{ID: "member", Box: Box{X: 60, Y: 120, Width: 80, Height: 40}}
routes, err := Routes([]Shape{owner, member}, []Edge{{Index: 0, From: "owner", FromPort: "hdr", To: "member"}}, nil)
if err != nil {
t.Fatal(err)
}
if len(routes) != 1 {
t.Fatalf("routes = %v, want the flush pin at the middle of the header's underside routed", routes)
}
if first := routes[0].Points[0]; first != (Point{X: 100, Y: 40}) {
t.Errorf("route starts at %v, want the pin at (100, 40)", first)
}
}
Loading
Loading