From 8dcece0cba7a39b7d2f8a84f5e3055c44f453c25 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.34.1 From ac9c86857184245036b2842a81743aea07a32da4 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.34.1 From 392f651d69e4a5c7a91719b4e8061665eadb0e47 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.34.1