From 65fa9ab276d9bd7bb59d2187fd04ba6f69f82db5 Mon Sep 17 00:00:00 2001
From: "sorokin.oleg.edu" <sorokin.oleg.edu@gmail.com>
Date: Sat, 5 Sep 2026 21:05:21 +0300
Subject: [PATCH] Avoid automatic diagram label collisions with stable bounded
 placement

---
 crates/oa-core/src/routing.rs            | 117 ++++++++++++++++---
 crates/oa-core/tests/routing_geometry.rs | 138 +++++++++++++++++++++++
 2 files changed, 240 insertions(+), 15 deletions(-)

diff --git a/crates/oa-core/src/routing.rs b/crates/oa-core/src/routing.rs
index 7ca0a4f8..87fda464 100644
--- a/crates/oa-core/src/routing.rs
+++ b/crates/oa-core/src/routing.rs
@@ -345,6 +345,7 @@ fn route_diagram_impl(
     ordered.sort_by_key(|e| e.id);
     let mut edges = BTreeMap::new();
     let mut previous = Vec::new();
+    let mut previous_labels = Vec::new();
     let mut budget = VIEW_SEARCH_BUDGET;
     let mut bounds = BoundsBuilder::default();
     for node in &view.nodes {
@@ -370,6 +371,7 @@ fn route_diagram_impl(
             }
             if !cached.label.text.is_empty() {
                 bounds.rect(cached.label.bounds.position, cached.label.bounds.size);
+                previous_labels.push(cached.label.bounds);
             }
             record_segments(&mut previous, &cached.points);
             edges.insert(edge.id, cached.clone());
@@ -415,12 +417,14 @@ fn route_diagram_impl(
                     &updated.points,
                     route.and_then(|r| r.label.as_ref()),
                     &obstacles,
+                    &previous_labels,
                 );
                 for &point in &updated.points {
                     bounds.point(point);
                 }
                 if !updated.label.text.is_empty() {
                     bounds.rect(updated.label.bounds.position, updated.label.bounds.size);
+                    previous_labels.push(updated.label.bounds);
                 }
                 record_segments(&mut previous, &updated.points);
                 edges.insert(edge.id, updated);
@@ -464,12 +468,14 @@ fn route_diagram_impl(
             &points,
             route.and_then(|r| r.label.as_ref()),
             &obstacles,
+            &previous_labels,
         );
         for &point in &points {
             bounds.point(point);
         }
         if !label.text.is_empty() {
             bounds.rect(label.bounds.position, label.bounds.size);
+            previous_labels.push(label.bounds);
         }
         record_segments(&mut previous, &points);
         edges.insert(
@@ -1021,6 +1027,7 @@ fn make_label(
     points: &[Position],
     authored: Option<&crate::EdgeLabel>,
     obstacles: &[Rect],
+    previous_labels: &[RoutingBounds],
 ) -> EdgeLabelGeometry {
     let text = label_text(project, edge);
     let size = Size {
@@ -1031,8 +1038,8 @@ fn make_label(
         },
         height: if text.is_empty() { 0.0 } else { 24.0 },
     };
-    let offset = authored.map_or(pos(0.0, -18.0), |l| l.offset);
-    let geometry = |fraction| {
+    let mut offset = authored.map_or(pos(0.0, -18.0), |l| l.offset);
+    let geometry = |fraction, offset: Position| {
         let anchor = point_along_polyline(points, fraction).unwrap_or(pos(0.0, 0.0));
         let center = pos(anchor.x + offset.x, anchor.y + offset.y);
         (
@@ -1045,21 +1052,19 @@ fn make_label(
         )
     };
     let mut fraction = authored.map_or(0.5, |l| l.fraction);
-    // Automatic labels prefer the middle, then clear segment centres. An
-    // explicitly positioned label is exact, even if its author overlaps a card.
-    if authored.is_none()
-        && !text.is_empty()
-        && obstacles.iter().any(|r| r.overlaps(geometry(fraction).2))
-    {
+    // Keep authored placement exact. Automatic labels share the stable-ID
+    // prefix used for route scoring, so previews and settled solves agree.
+    if authored.is_none() && !text.is_empty() {
         let total = total_length(points);
         let mut traversed = 0.0;
-        let mut alternatives = Vec::new();
+        let mut alternatives = vec![0.5];
         if total > 0.0 {
             for pair in points.windows(2) {
                 let len = length(pair[0], pair[1]);
                 alternatives.push((traversed + len / 2.0) / total);
                 traversed += len;
             }
+            alternatives.extend((1..20).map(|i| i as f32 / 20.0));
         }
         alternatives.sort_by(|a, b| {
             (a - 0.5)
@@ -1067,14 +1072,50 @@ fn make_label(
                 .total_cmp(&(b - 0.5).abs())
                 .then_with(|| a.total_cmp(b))
         });
-        if let Some(clear) = alternatives
-            .into_iter()
-            .find(|f| !obstacles.iter().any(|r| r.overlaps(geometry(*f).2)))
-        {
-            fraction = clear;
+        alternatives.dedup();
+        alternatives.truncate(32);
+        // Prefer another position on the existing path at the normal offset.
+        // Only then try a small bounded set of offsets, never move the route.
+        let offsets = [
+            offset,
+            pos(0.0, 18.0),
+            pos(0.0, -46.0),
+            pos(0.0, 46.0),
+            pos(0.0, -74.0),
+            pos(0.0, 74.0),
+            pos(size.width / 2.0 + 18.0, 0.0),
+            pos(-size.width / 2.0 - 18.0, 0.0),
+        ];
+        let mut best = None;
+        // Bound work independently per label, including pathological coincident
+        // edges. A preview sees the same stable prefix and the same budget.
+        let mut comparisons = 8_192;
+        'candidate: for candidate_offset in offsets {
+            for &candidate_fraction in &alternatives {
+                let candidate = geometry(candidate_fraction, candidate_offset).2;
+                let Some(score) = label_collisions(
+                    candidate,
+                    obstacles,
+                    previous_labels,
+                    best.as_ref().map(|(score, _, _)| *score),
+                    &mut comparisons,
+                ) else {
+                    break 'candidate;
+                };
+                if best.as_ref().is_none_or(|(old, _, _)| score < *old) {
+                    best = Some((score, candidate_fraction, candidate_offset));
+                }
+                if score == (0, 0) {
+                    break 'candidate;
+                }
+            }
+        }
+        if let Some((_, chosen_fraction, chosen_offset)) = best {
+            fraction = chosen_fraction;
+            offset = chosen_offset;
         }
     }
-    let (anchor, center, bounds) = geometry(fraction);
+    let (anchor, center, bounds) = geometry(fraction, offset);
     EdgeLabelGeometry {
         text,
         fraction,
@@ -1084,6 +1125,52 @@ fn make_label(
     }
 }
 
+fn label_collisions(
+    bounds: RoutingBounds,
+    obstacles: &[Rect],
+    labels: &[RoutingBounds],
+    best: Option<(usize, usize)>,
+    comparisons: &mut usize,
+) -> Option<(usize, usize)> {
+    let mut nodes = 0;
+    for obstacle in obstacles {
+        if *comparisons == 0 {
+            return None;
+        }
+        *comparisons -= 1;
+        if obstacle.overlaps(bounds) {
+            nodes += 1;
+        }
+        if best.is_some_and(|score| nodes > score.0) {
+            return Some((nodes, usize::MAX));
+        }
+    }
+    // Once a candidate is worse than the current best, additional collisions
+    // cannot change the decision. This bounds dense coincident-label work.
+    let label_limit = best
+        .filter(|score| nodes == score.0)
+        .map_or(usize::MAX, |score| score.1.saturating_add(1));
+    let mut collisions = 0;
+    for other in labels {
+        if *comparisons == 0 {
+            return None;
+        }
+        *comparisons -= 1;
+        // Four diagram units between text boxes also keeps hit regions legible.
+        if bounds.position.x < other.position.x + other.size.width + 4.0
+            && bounds.position.x + bounds.size.width > other.position.x - 4.0
+            && bounds.position.y < other.position.y + other.size.height + 4.0
+            && bounds.position.y + bounds.size.height > other.position.y - 4.0
+        {
+            collisions += 1;
+            if collisions >= label_limit {
+                break;
+            }
+        }
+    }
+    Some((nodes, collisions))
+}
+
 #[derive(Default)]
 struct BoundsBuilder {
     extents: Option<(f32, f32, f32, f32)>,
diff --git a/crates/oa-core/tests/routing_geometry.rs b/crates/oa-core/tests/routing_geometry.rs
index 98cbe93b..a4422deb 100644
--- a/crates/oa-core/tests/routing_geometry.rs
+++ b/crates/oa-core/tests/routing_geometry.rs
@@ -282,6 +282,144 @@ fn automatic_label_fraction_survives_its_first_authored_drag_without_a_jump() {
         point(label.center.x + 15., label.center.y - 10.)
     );
 }
+
+fn label_overlap(a: RoutingBounds, b: RoutingBounds) -> bool {
+    a.position.x < b.position.x + b.size.width
+        && a.position.x + a.size.width > b.position.x
+        && a.position.y < b.position.y + b.size.height
+        && a.position.y + a.size.height > b.position.y
+}
+
+#[test]
+fn workshop_automatic_labels_are_readable_without_mutating_authored_routes() {
+    let p = language::parse(include_str!("../../../examples/routing-workshop.arcadia")).unwrap();
+    let d = p
+        .diagrams
+        .values()
+        .find(|d| d.alias == "sa_system_boundary_and_functions")
+        .unwrap();
+    let view = project_diagram(&p, d);
+    let routed = route_diagram(&p, d, &view);
+    assert_eq!(routed.edges.len(), 12);
+    let labels: Vec<_> = routed
+        .edges
+        .values()
+        .filter(|e| !e.label.text.is_empty())
+        .collect();
+    let mut overlaps = Vec::new();
+    for (i, a) in labels.iter().enumerate() {
+        for b in labels.iter().skip(i + 1) {
+            if label_overlap(a.label.bounds, b.label.bounds) {
+                overlaps.push(format!(
+                    "{} at {:?} / {} at {:?}",
+                    a.label.text, a.label.center, b.label.text, b.label.center
+                ));
+            }
+        }
+    }
+    assert!(
+        overlaps.is_empty(),
+        "Overlapping automatic workshop labels: {overlaps:#?}"
+    );
+    for edge in &view.edges {
+        let label = &routed.edges[&edge.id].label;
+        let mut ancestors = std::collections::BTreeSet::new();
+        for endpoint in [edge.source, edge.target] {
+            let mut parent = view
+                .nodes
+                .iter()
+                .find(|n| n.id == endpoint)
+                .unwrap()
+                .visual_parent;
+            while let Some(id) = parent {
+                ancestors.insert(id);
+                parent = view
+                    .nodes
+                    .iter()
+                    .find(|n| n.id == id)
+                    .and_then(|n| n.visual_parent);
+            }
+        }
+        for node in &view.nodes {
+            let size = if ancestors.contains(&node.id) {
+                Size {
+                    height: node.size.height.min(64.),
+                    ..node.size
+                }
+            } else {
+                node.size
+            };
+            assert!(
+                !label_overlap(
+                    label.bounds,
+                    RoutingBounds {
+                        position: node.position,
+                        size
+                    }
+                ),
+                "{} overlaps {}",
+                label.text,
+                p.named(node.element)
+            );
+        }
+    }
+    let offset_count = labels
+        .iter()
+        .filter(|e| e.label.center != point(e.label.anchor.x, e.label.anchor.y - 18.))
+        .count();
+    eprintln!("Workshop labels: 12 clear labels; {offset_count} bounded offset placements");
+
+    // Explicit placement stays exact even when deliberately overlapping another
+    // label. Preview updates only the selected label; its path cannot jump.
+    let selected = *routed.edges.keys().last().unwrap();
+    let baseline = &routed.edges[&selected];
+    let destination = labels
+        .iter()
+        .find(|e| e.id != selected)
+        .unwrap()
+        .label
+        .center;
+    let mut edited = p.clone();
+    set_route(
+        &mut edited,
+        d.id,
+        selected,
+        vec![],
+        Some(EdgeLabel {
+            fraction: baseline.label.fraction,
+            offset: point(
+                destination.x - baseline.label.anchor.x,
+                destination.y - baseline.label.anchor.y,
+            ),
+        }),
+    );
+    let preview = route_edge_preview(&edited, &edited.diagrams[&d.id], &view, &routed, selected);
+    assert_eq!(preview.edges[&selected].label.center, destination);
+    assert_eq!(preview.edges[&selected].points, baseline.points);
+    assert_eq!(
+        preview.edges[&selected],
+        route_diagram(&edited, &edited.diagrams[&d.id], &view).edges[&selected]
+    );
+    for (&id, edge) in &routed.edges {
+        if id != selected {
+            assert_eq!(preview.edges[&id], *edge);
+        }
+    }
+    // Resetting a label uses the same earlier-label occupancy as a settled solve.
+    edited.diagrams.get_mut(&d.id).unwrap().routes[0].label = None;
+    let reset = route_edge_preview(&edited, &edited.diagrams[&d.id], &view, &preview, selected);
+    assert_eq!(reset.edges[&selected], *baseline);
+    // An actual bend edit also places the automatic label against that prefix.
+    edited.diagrams.get_mut(&d.id).unwrap().routes[0]
+        .waypoints
+        .push(point(1100., 720.));
+    let pin_preview =
+        route_edge_preview(&edited, &edited.diagrams[&d.id], &view, &routed, selected);
+    assert_eq!(
+        pin_preview.edges[&selected],
+        route_diagram(&edited, &edited.diagrams[&d.id], &view).edges[&selected]
+    );
+}
 fn distance(a: Position, b: Position) -> f32 {
     (a.x - b.x).hypot(a.y - b.y)
 }