From 06c99f537a8a23ff4a96697f6faa7ddb323020c2 Mon Sep 17 00:00:00 2001 From: murin Date: Tue, 20 May 2025 12:09:59 +0300 Subject: Fix revision graph canvas width calculation on repository page When multiple branches point to the same subset of commits, the width of the revision graph canvas increases unnecessarily. Solution: Avoid indexing branches that reference commits which already have the "space" attribute assigned. --- app/helpers/repositories_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/repositories_helper.rb b/app/helpers/repositories_helper.rb index c72816367..67b19c2dd 100644 --- a/app/helpers/repositories_helper.rb +++ b/app/helpers/repositories_helper.rb @@ -300,7 +300,7 @@ module RepositoriesHelper heads.sort_by!(&:to_s) space = nil heads.each do |head| - if commits_by_scmid.include? head.scmid + if commits_by_scmid.include?(head.scmid) && commits_by_scmid[head.scmid][:space].nil? space = index_head((space || -1) + 1, head, commits_by_scmid) end end -- 2.25.1 From ed4602261fa379ebb42dc967c1ef5e87b9b7fcd5 Mon Sep 17 00:00:00 2001 From: murin Date: Tue, 20 May 2025 12:53:08 +0300 Subject: Change the way commits are indexed for revision graph The original approach resulted in orphan commits on the page being displayed on the first branch, and in unnecessary offsets. Solution: Always begin indexing from the latest commit, then index the heads present on the page, and finally index the orphan commits present on the page. Partially relates to #10954#note-5 --- app/helpers/repositories_helper.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/app/helpers/repositories_helper.rb b/app/helpers/repositories_helper.rb index 67b19c2dd..6ed17c692 100644 --- a/app/helpers/repositories_helper.rb +++ b/app/helpers/repositories_helper.rb @@ -298,14 +298,18 @@ module RepositoriesHelper } end heads.sort_by!(&:to_s) - space = nil + # Process commits starting from the latest + space = index_head(0, commits.first, commits_by_scmid) + # Process commits from heads heads.each do |head| if commits_by_scmid.include?(head.scmid) && commits_by_scmid[head.scmid][:space].nil? - space = index_head((space || -1) + 1, head, commits_by_scmid) + space = index_head(space + 1, head, commits_by_scmid) end end - # when no head matched anything use first commit - space ||= index_head(0, commits.first, commits_by_scmid) + # Process orphan commits + while (commit = commits.find { |commit| commits_by_scmid[commit.scmid][:space].nil? }) + space = index_head(space + 1, commit, commits_by_scmid) + end return commits_by_scmid, space end -- 2.25.1 From 4af029d9ec8e5760bedd4ecb8b0dea0627f25f98 Mon Sep 17 00:00:00 2001 From: murin Date: Tue, 20 May 2025 16:08:53 +0300 Subject: Change the shape of curves connecting commits across different branches in the revision graph The original curve design made it difficult to identify which commits were connected, when several commits existed between the connected ones on the branches. Now, connections between commits on different branches are more easily recognizable in typical scenarios. The new curves connect from the left or right side to commit that already have vertical connections, and from the top or bottom to the last or first commit on a branch, respectively. | | | | | * /-* * * *-\ | | | | | | | *-/ * *-/ * * | | | | | Related to #10954 --- app/assets/javascripts/revision_graph.js | 38 ++++++++++++++++++++---- app/helpers/repositories_helper.rb | 9 ++++++ 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/revision_graph.js b/app/assets/javascripts/revision_graph.js index faa872287..e54cf5750 100644 --- a/app/assets/javascripts/revision_graph.js +++ b/app/assets/javascripts/revision_graph.js @@ -63,26 +63,52 @@ function drawRevisionGraph(holder, commits_hash, graph_space) { fill: colors[commit.space], stroke: 'none' }).toFront(); - // paths to parents - $.each(commit.parent_scmids, function(index, parent_scmid) { - parent_commit = commits_by_scmid[parent_scmid]; + + // check for parents in the same column + let noVerticalParents = true; + $.each(commit.parent_scmids, function (index, parentScmid) { + parent_commit = commits_by_scmid[parentScmid]; if (parent_commit) { if (!parent_commit.hasOwnProperty("space")) parent_commit.space = 0; + // has parent in the same column on this page + if (parent_commit.space === commit.space) + noVerticalParents = false; + } else { + // has parent in the same column on the other page + noVerticalParents = false; + } + }); + + // paths to parents + $.each(commit.parent_scmids, function(index, parent_scmid) { + parent_commit = commits_by_scmid[parent_scmid]; + if (parent_commit) { parent_y = yForRow(max_rdmid - parent_commit.rdmid); parent_x = graph_x_offset + XSTEP / 2 + XSTEP * parent_commit.space; - if (parent_commit.space == commit.space) { + const controlPointDelta = (parent_y - y) / 8; + + if (parent_commit.space === commit.space) { // vertical path path = revisionGraph.path([ 'M', x, y, 'V', parent_y]); + } else if (noVerticalParents) { + // branch start (Bezier curve) + path = revisionGraph.path([ + 'M', x, y, + 'C', x, y + controlPointDelta, x, parent_y - controlPointDelta, parent_x, parent_y]); + } else if (!parent_commit.hasOwnProperty('vertical_children')) { + // branch end (Bezier curve) + path = revisionGraph.path([ + 'M', x, y, + 'C', parent_x, y + controlPointDelta, parent_x, parent_y, parent_x, parent_y]); } else { // path to a commit in a different branch (Bezier curve) path = revisionGraph.path([ 'M', x, y, - 'C', x, y, x, y + (parent_y - y) / 2, x + (parent_x - x) / 2, y + (parent_y - y) / 2, - 'C', x + (parent_x - x) / 2, y + (parent_y - y) / 2, parent_x, parent_y-(parent_y-y)/2, parent_x, parent_y]); + 'C', parent_x, y, x, parent_y, parent_x, parent_y]); } } else { // vertical path ending at the bottom of the revisionGraph diff --git a/app/helpers/repositories_helper.rb b/app/helpers/repositories_helper.rb index 6ed17c692..7cfbcdda0 100644 --- a/app/helpers/repositories_helper.rb +++ b/app/helpers/repositories_helper.rb @@ -310,6 +310,15 @@ module RepositoriesHelper while (commit = commits.find { |commit| commits_by_scmid[commit.scmid][:space].nil? }) space = index_head(space + 1, commit, commits_by_scmid) end + # Set vertical_children flag for commits that have children in the same column + # for S-style connections between commits + commits_by_scmid.each_value do |commit| + commit[:parent_scmids].each do |scmid| + if (parent = commits_by_scmid[scmid]) && parent[:space] == commit[:space] + parent[:vertical_children] = true + end + end + end return commits_by_scmid, space end -- 2.25.1