Round commit graph connections

TESTING.md

@@ -196,8 +196,9 @@ xcrun simctl launch booted de.rfc1437.gotcha
       All in the navigation bar, and includes commits from multiple branches.
 - [ ] Open the branch menu; All is checked. Select a branch and verify the label,
       checkmark, commits, and graph update, then return to All.
-- [ ] Commit graph lanes and merge connections align with their rows while
+- [ ] Commit graph lanes align with their rows while scrolling; branch-out and
-      scrolling.
+      merge-back connections use smooth, rounded curves that meet the correct
+      lane and commit node without angular horizontal bars or overlaps.
 - [ ] Open a commit and verify Changed Files paths and statuses.
 - [ ] Open a changed file and verify the diff title, old/new line numbers,
       monospaced text, addition/removal/hunk colors, vertical scrolling, and

crates/app/src/presentation.rs

@@ -177,7 +177,8 @@ pub struct CommitRow {
     pub top_lanes: Vec<u32>,
     pub bottom_lanes: Vec<u32>,
     pub node_lane: Option<u32>,
-    pub connections: Vec<u32>,
+    pub top_connections: Vec<u32>,
+    pub bottom_connections: Vec<u32>,
 }
 
 #[derive(Clone, uniffi::Record)]
@@ -477,7 +478,8 @@ pub fn commit_page(
                     .top_lanes
                     .iter()
                     .chain(commit.bottom_lanes.iter())
-                    .chain(commit.connections.iter())
+                    .chain(commit.top_connections.iter())
+                    .chain(commit.bottom_connections.iter())
                     .chain(commit.node_lane.iter())
             })
             .max()
@@ -525,7 +527,8 @@ pub fn commit_page(
                 top_lanes: indices(&row.top_lanes),
                 bottom_lanes: indices(&row.bottom_lanes),
                 node_lane: row.node_lane.map(|lane| lane as u32),
-                connections: indices(&row.connections),
+                top_connections: indices(&row.top_connections),
+                bottom_connections: indices(&row.bottom_connections),
             })
         })
         .collect();

crates/gitea/src/domain.rs

@@ -156,7 +156,8 @@ pub struct HistoryCommit {
     pub top_lanes: Vec<usize>,
     pub bottom_lanes: Vec<usize>,
     pub node_lane: Option<usize>,
-    pub connections: Vec<usize>,
+    pub top_connections: Vec<usize>,
+    pub bottom_connections: Vec<usize>,
     pub refs: Vec<String>,
 }
 

crates/gitea/src/repositories.rs

@@ -144,7 +144,8 @@ impl Client {
                         top_lanes: Vec::new(),
                         bottom_lanes: Vec::new(),
                         node_lane: None,
-                        connections: Vec::new(),
+                        top_connections: Vec::new(),
+                        bottom_connections: Vec::new(),
                         refs: Vec::new(),
                     })
                     .collect(),
@@ -346,7 +347,8 @@ fn build_graph(
             top_lanes: Vec::new(),
             bottom_lanes: Vec::new(),
             node_lane: None,
-            connections: Vec::new(),
+            top_connections: Vec::new(),
+            bottom_connections: Vec::new(),
             refs: refs.remove(&sha).unwrap_or_default(),
         });
     }
@@ -373,11 +375,12 @@ fn layout_graph(commits: &mut [HistoryCommit]) {
             .filter_map(|(index, lane)| lane.as_ref().map(|_| index))
             .filter(|index| existing_node.is_some() || *index != node)
             .collect();
-        let mut connections = BTreeSet::new();
+        let mut top_connections = BTreeSet::new();
         for duplicate in matching_lanes.into_iter().skip(1) {
             lanes[duplicate] = None;
-            connect(node, duplicate, &mut connections);
+            top_connections.insert(duplicate);
         }
+        let mut bottom_connections = BTreeSet::new();
         let parents: Vec<_> = commit_parents(&row.commit).map(str::to_string).collect();
         lanes[node] = None;
         for (index, parent) in parents.into_iter().enumerate() {
@@ -387,14 +390,17 @@ fn layout_graph(commits: &mut [HistoryCommit]) {
                 .iter()
                 .position(|next| next.as_deref() == Some(&parent))
             {
-                connect(node, existing, &mut connections);
+                if node != existing {
+                    bottom_connections.insert(existing);
+                }
             } else {
                 let branch = allocate_lane(&mut lanes, &parent);
-                connect(node, branch, &mut connections);
+                bottom_connections.insert(branch);
             }
         }
         row.node_lane = Some(node);
-        row.connections = connections.into_iter().collect();
+        row.top_connections = top_connections.into_iter().collect();
+        row.bottom_connections = bottom_connections.into_iter().collect();
         row.bottom_lanes = lanes
             .iter()
             .enumerate()
@@ -413,12 +419,6 @@ fn allocate_lane(lanes: &mut Vec<Option<String>>, sha: &str) -> usize {
     }
 }
 
-fn connect(left: usize, right: usize, connections: &mut BTreeSet<usize>) {
-    for lane in left.min(right) + 1..=left.max(right) {
-        connections.insert(lane);
-    }
-}
-
 fn commit_parents(commit: &models::Commit) -> impl Iterator<Item = &str> {
     commit
         .parents
@@ -464,15 +464,39 @@ mod tests {
                 (
                     0,
                     "main".into(),
-                    vec![commit("merge", &["left", "right"]), commit("left", &[])],
+                    vec![
+                        commit("merge", &["left", "right"]),
+                        commit("left", &["base"]),
+                        commit("base", &[]),
+                    ],
+                ),
+                (
+                    1,
+                    "feature".into(),
+                    vec![commit("right", &["base"]), commit("base", &[])],
                 ),
-                (1, "feature".into(), vec![commit("right", &[])]),
             ],
             HashMap::new(),
         );
-        assert_eq!(rows.len(), 3);
+        assert_eq!(rows.len(), 4);
         assert!(rows.iter().any(|row| row.refs == ["main"]));
         assert!(rows.iter().any(|row| row.refs == ["feature"]));
-        assert!(rows.iter().any(|row| !row.connections.is_empty()));
+        let merge = rows
+            .iter()
+            .find(|row| row.commit.sha.as_deref() == Some("merge"))
+            .unwrap();
+        assert_eq!(merge.node_lane, Some(0));
+        assert_eq!(merge.bottom_connections, [1]);
+        let right = rows
+            .iter()
+            .find(|row| row.commit.sha.as_deref() == Some("right"))
+            .unwrap();
+        assert_eq!(right.node_lane, Some(1));
+        let base = rows
+            .iter()
+            .find(|row| row.commit.sha.as_deref() == Some("base"))
+            .unwrap();
+        assert_eq!(base.node_lane, Some(0));
+        assert_eq!(base.top_connections, [1]);
     }
 }

ios/Generated/gotcha_core.swift

@@ -1487,18 +1487,20 @@ public struct CommitRow: Equatable, Hashable {
     public var topLanes: [UInt32]
     public var bottomLanes: [UInt32]
     public var nodeLane: UInt32?
-    public var connections: [UInt32]
+    public var topConnections: [UInt32]
+    public var bottomConnections: [UInt32]
 
     // Default memberwise initializers are never public by default, so we
     // declare one manually.
-    public init(sha: String, title: String, detail: String, topLanes: [UInt32], bottomLanes: [UInt32], nodeLane: UInt32?, connections: [UInt32]) {
+    public init(sha: String, title: String, detail: String, topLanes: [UInt32], bottomLanes: [UInt32], nodeLane: UInt32?, topConnections: [UInt32], bottomConnections: [UInt32]) {
         self.sha = sha
         self.title = title
         self.detail = detail
         self.topLanes = topLanes
         self.bottomLanes = bottomLanes
         self.nodeLane = nodeLane
-        self.connections = connections
+        self.topConnections = topConnections
+        self.bottomConnections = bottomConnections
     }
 
 
@@ -1523,7 +1525,8 @@ public struct FfiConverterTypeCommitRow: FfiConverterRustBuffer {
                 topLanes: FfiConverterSequenceUInt32.read(from: &buf),
                 bottomLanes: FfiConverterSequenceUInt32.read(from: &buf),
                 nodeLane: FfiConverterOptionUInt32.read(from: &buf),
-                connections: FfiConverterSequenceUInt32.read(from: &buf)
+                topConnections: FfiConverterSequenceUInt32.read(from: &buf),
+                bottomConnections: FfiConverterSequenceUInt32.read(from: &buf)
         )
     }
 
@@ -1534,7 +1537,8 @@ public struct FfiConverterTypeCommitRow: FfiConverterRustBuffer {
         FfiConverterSequenceUInt32.write(value.topLanes, into: &buf)
         FfiConverterSequenceUInt32.write(value.bottomLanes, into: &buf)
         FfiConverterOptionUInt32.write(value.nodeLane, into: &buf)
-        FfiConverterSequenceUInt32.write(value.connections, into: &buf)
+        FfiConverterSequenceUInt32.write(value.topConnections, into: &buf)
+        FfiConverterSequenceUInt32.write(value.bottomConnections, into: &buf)
     }
 }
 

ios/Sources/ContentScreens.swift

@@ -1080,6 +1080,9 @@ final class CommitsViewController: RefreshingTableViewController {
 }
 
 final class CommitCell: UITableViewCell {
+    private static let laneOrigin: CGFloat = 12
+    private static let laneSpacing: CGFloat = 12
+
     private var row: CommitRow?
     private var laneCount: UInt32 = 0
 
@@ -1096,7 +1099,9 @@ final class CommitCell: UITableViewCell {
         self.row = row
         self.laneCount = laneCount
         var content = defaultContentConfiguration()
-        content.directionalLayoutMargins.leading = laneCount == 0 ? 0 : min(72, CGFloat(laneCount) * 10 + 8)
+        content.directionalLayoutMargins.leading = laneCount == 0
+            ? 0
+            : CGFloat(laneCount) * Self.laneSpacing + 10
         content.text = row.title
         content.secondaryText = row.detail
         content.secondaryTextProperties.numberOfLines = 2
@@ -1107,33 +1112,90 @@ final class CommitCell: UITableViewCell {
     override func draw(_ rect: CGRect) {
         super.draw(rect)
         guard let row, laneCount > 0, let context = UIGraphicsGetCurrentContext() else { return }
-        let spacing: CGFloat = 10
         let centerY = bounds.midY
+        let nodeX = row.nodeLane.map(laneX)
+        context.setLineWidth(3)
+        context.setLineCap(.round)
+        context.setLineJoin(.round)
+
         for lane in 0..<laneCount {
-            let x = 10 + CGFloat(lane) * spacing
-            let color = laneColor(lane)
-            context.setStrokeColor(color.cgColor)
-            context.setLineWidth(3)
             if row.topLanes.contains(lane) {
-                context.move(to: CGPoint(x: x, y: 0))
+                let start = CGPoint(x: laneX(lane), y: 0)
-                context.addLine(to: CGPoint(x: x, y: centerY))
+                if row.topConnections.contains(lane), let nodeX {
-                context.strokePath()
+                    strokeCurve(
+                        context,
+                        from: start,
+                        to: CGPoint(x: nodeX, y: centerY),
+                        color: laneColor(lane)
+                    )
+                } else {
+                    strokeLine(
+                        context,
+                        from: start,
+                        to: CGPoint(x: start.x, y: centerY),
+                        color: laneColor(lane)
+                    )
+                }
             }
             if row.bottomLanes.contains(lane) {
-                context.move(to: CGPoint(x: x, y: centerY))
+                let end = CGPoint(x: laneX(lane), y: bounds.height)
-                context.addLine(to: CGPoint(x: x, y: bounds.height))
+                if !row.bottomConnections.contains(lane) || row.topLanes.contains(lane) {
-                context.strokePath()
+                    strokeLine(
-            }
+                        context,
-            if row.connections.contains(lane), lane > 0 {
+                        from: CGPoint(x: end.x, y: centerY),
-                context.move(to: CGPoint(x: x - spacing, y: centerY))
+                        to: end,
-                context.addLine(to: CGPoint(x: x, y: centerY))
+                        color: laneColor(lane)
-                context.strokePath()
+                    )
-            }
+                }
-            if row.nodeLane == lane {
+                if row.bottomConnections.contains(lane), let nodeX {
-                context.setFillColor(color.cgColor)
+                    strokeCurve(
-                context.fillEllipse(in: CGRect(x: x - 5, y: centerY - 5, width: 10, height: 10))
+                        context,
+                        from: CGPoint(x: nodeX, y: centerY),
+                        to: end,
+                        color: laneColor(lane)
+                    )
+                }
             }
         }
+        if let lane = row.nodeLane, let nodeX {
+            context.setFillColor(laneColor(lane).cgColor)
+            context.fillEllipse(
+                in: CGRect(x: nodeX - 5, y: centerY - 5, width: 10, height: 10)
+            )
+        }
+    }
+
+    private func laneX(_ lane: UInt32) -> CGFloat {
+        Self.laneOrigin + CGFloat(lane) * Self.laneSpacing
+    }
+
+    private func strokeLine(
+        _ context: CGContext,
+        from start: CGPoint,
+        to end: CGPoint,
+        color: UIColor
+    ) {
+        context.setStrokeColor(color.cgColor)
+        context.move(to: start)
+        context.addLine(to: end)
+        context.strokePath()
+    }
+
+    private func strokeCurve(
+        _ context: CGContext,
+        from start: CGPoint,
+        to end: CGPoint,
+        color: UIColor
+    ) {
+        let middleY = (start.y + end.y) / 2
+        context.setStrokeColor(color.cgColor)
+        context.move(to: start)
+        context.addCurve(
+            to: end,
+            control1: CGPoint(x: start.x, y: middleY),
+            control2: CGPoint(x: end.x, y: middleY)
+        )
+        context.strokePath()
     }
 
     private func laneColor(_ lane: UInt32) -> UIColor {