Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Readability revision graph #5782

Closed
spdr870 opened this issue Nov 15, 2018 · 55 comments · Fixed by #9028, #9050, #10637 or #11059
Closed

Readability revision graph #5782

spdr870 opened this issue Nov 15, 2018 · 55 comments · Fixed by #9028, #9050, #10637 or #11059

Comments

@spdr870
Copy link
Member

spdr870 commented Nov 15, 2018

Do you want to request a feature or report a bug?
Discussion/feature

What is the current behavior?
The revision graph is very curly. I think the readability could be improved easily. I propose to make the change as visible in this screenprint (left is new):
image

POC: #5783

Environment you encounter the issue:

  • Git Extensions 3.00.rc2
  • d6496b8 (Dirty)
  • Git 2.19.1.windows.1
  • Microsoft Windows NT 10.0.17134.0
  • .NET Framework 4.7.3110.0
  • DPI X:100,00% Y:100,00%
@Noguai
Copy link
Contributor

Noguai commented Nov 15, 2018

I actually also wanted to suggest a similar thing, eventhough I think the graph could be even more straight than your suggestion.

I just returned a few months ago to using GitExtensions exclusively from about 1,5 years of first only SourceTree and later for the most part SourceTree and GitExtensions only for complex stuff as SourceTree with it's bugs and overall slowness just got on my nerves.

But in my opinion one of the things that SourceTree did and still does a million times better than GitExtensions is it's commit graph: It is so much straighter, which makes it much easier to follow a string of related commits and - from what I've noticed from colleagues that are new to git - much easier to understand.

I also prefer, if diagonal lines like the ones at the commit Revision graph distorted #5673 in your image could be completely straight.

I would even go as far and propose a mode for the commit graph in which a line strictly stays in the column it began in, so that you can put your mouse / finger / pen on it and scroll without ever loosing track what the line belongs to you are looking at.

A friend and I wrote a prototype for a Git GUI in WPF which did exactly that and I quite liked the graph layout in cases a normal layout would get quite wavy:
straight-commit-graph

@spdr870
Copy link
Member Author

spdr870 commented Nov 15, 2018

@Noguai Completely straight lines could like nice. But I wonder, what if there are many branches? Wouldn't it be to wide?

@drewnoakes
Copy link
Member

image

You could go further (see right). You can go further than this still, but there's a limit to my Gimp skills.

  • More straightening
  • One (ugly) example of a straight line when extending beyond two rows on an angle

@spdr870
Copy link
Member Author

spdr870 commented Nov 15, 2018

Getting the lines more straight is easy. Since I do not have Gimp skills at all, I build some POC's to experiment.

Small repo (gitextensions)
image

Bigger repo. Problem is that its hard to find a good balance between the total width and waviness (is that a word?):
image

Same big repo, further down in history. It takes a lot width because there where lanes occupying the space before. Bad example:
image

@ghost
Copy link

ghost commented Nov 15, 2018

Thank you for the visual samples.
BTW: https://en.wikipedia.org/wiki/Waviness

@Noguai
Copy link
Contributor

Noguai commented Nov 15, 2018

Yes, with many or even more so long living branches (in relation to commit frequency) the graph can get quite wide, which is why i explicitly spoke of a mode. I should have made that clear, that I meant something that could be toggled on and off.

@spdr870
Copy link
Member Author

spdr870 commented Nov 15, 2018

Just did some more POCing, Its easy to detect 'long lived branches' and I applied more 'gravity' to those branches. The long lived branches keep left more. The short lived branches try to keep their lane.

Still, I think we can only do this if we also get rid of some curly lines. All lines should go directly to their destination, or it will become less readable. Not sure how to handle it. Without curves it would be easy, but with it is harder. See example below.

image

Still. The original proposal is easy to do!

@Noguai About the option; I think we already have too much options. I would like to find a solution that is an improvement for all users. Maybe optimistic, but lets at least try. Options require more testing. Worse even; I will build it for others and not for myself....:) I work with large repo's daily at work.

@spdr870
Copy link
Member Author

spdr870 commented Nov 15, 2018

Another example, just for fun. I don't like the spikes of the left screenshot. Would be nice to see the left layout, but without the sharp corners. The straight lines are cleaner in my opinion. Just the corners are ugly.

image

@vbjay
Copy link
Contributor

vbjay commented Nov 15, 2018

Makes it a lot easier to follow. I like it.

@drewnoakes
Copy link
Member

gitk crosses lines when plotting it's graphs. It might provide some inspiration.

@spdr870
Copy link
Member Author

spdr870 commented Nov 15, 2018

@drewnoakes I looked at gitk, but I think this provides a clear overview of what is happening. I did implement the arrows for branches that span a long period, that could help a little and is easy to implement. But the branching/merging is very hard for me to understand in the gitk graph. In the screenshot below the same revisions are selected. The order in GitExtensions is almost date-order. The order in gitk is... I do not know... topo-order-per-branch? Also notice how the 'main branch' is jumping across lanes in gitk (noticible in commits 'Merge pull request 5586', 'Merge pull request 5585'.

image

@Noguai
Copy link
Contributor

Noguai commented Nov 15, 2018

About the option; I think we already have too much options. I would like to find a solution that is an improvement for all users. Maybe optimistic, but lets at least try. Options require more testing.

That is actually a good point. 👍

And in that context to be honest a combination of the two proof of concepts looks pretty close to ideal to me. I can see why someone would dislike the sharp spikes, but I actually don't mind them at all and rather take the overall cleanliness:
imageimage

But the branching/merging is very hard for me to understand in the gitk graph.

I have to agree on that.

@RussKie
Copy link
Member

RussKie commented Nov 15, 2018

Since I do not have Gimp skills at all, I build some POC's to experiment.

🤣 Legend!

I like straighter lines with curves, I'm not quite fond of pointy angles.
I don't particularly have a problem with the cross-overs as long as the colours are distinctly different.

@spdr870
Copy link
Member Author

spdr870 commented Nov 16, 2018

How about this? (disclaimer: this is a very quickly done and rough POC, just to give some ideas)

image

@RussKie
Copy link
Member

RussKie commented Nov 16, 2018

I have very mixed feelings about these.
I like straighter lines of the left one, but I'm not particularly fond of hooks, they look weird.
The right (I presume it is the current one) feels to busy compared to the left though.

I like your earlier example:
image

@spdr870
Copy link
Member Author

spdr870 commented Nov 16, 2018

I have very mixed feelings about these.
I like straighter lines of the left one, but I'm not particularly fond of hooks, they look weird.
The right (I presume it is the current one) feels to busy compared to the left though.

The hooks are an unwanted side effect. Just wanted the straight line but with round corners. But it was a rough implementation. No idea why the hooks appeared..:P

@spdr870
Copy link
Member Author

spdr870 commented Nov 16, 2018

How about this? Still rough, but just another example.

image
image

Some details compared:
image

@drewnoakes
Copy link
Member

The more parallel the lines can appear, the better. Irregular angles create cognitive load.

It's looking much cleaner though. Very nice to see it evolving too!

@RussKie
Copy link
Member

RussKie commented Nov 17, 2018

Good work, Henk!

The more parallel the lines can appear, the better. Irregular angles create cognitive load.

I had the same feeling but struggled to express it as clearly

@spdr870
Copy link
Member Author

spdr870 commented Nov 17, 2018

Could you give a concrete example where irregular angles should be removed? I tried to render with the least angles as possible. Also tried to do lane transitions subtle. There is little room to play with, since everything is rendered per cell.

@Noguai
Copy link
Contributor

Noguai commented Nov 17, 2018

Looks great!

But I found a point in the GitExtensions History, where a line of a starting branch crosses the point of a commit entry very closely. I guess that could be improved by moving the point where the line originates from to the center of the row instead of the bottom of the row:
cross
cross2

@Noguai
Copy link
Contributor

Noguai commented Nov 17, 2018

Also looking at the current implementation I got another idea: Would it be possible to do your version of straightening of lines with a lookahead of 2 or 3 rows instead of just one? What about straightening the start of branches? Don't know about the performance impact though...
straightening

@spdr870
Copy link
Member Author

spdr870 commented Nov 17, 2018

@Noguai Increasing the look ahead is not that expensive. Since I only do it for the visible rows. I just pushed it with a lookahead of 4 (spans 3 rows). I'm not sure if it is an improvement in all situations, Actually, I like it better without the extended straightening. Just try it out, browse through the history of GitExtensions and see for yourself. For some reason I find it harder to predict how the lines are flowing through history. Not sure how to explain. There are examples where it is an improvement, just not always.
Not yet changed the lanes crossing revisions points, this is harder. I need to think about this some more....

(your example)
image

(other example)
image

(other example)
image

(other example)
image

The number of rows it looks ahead is variable. I think it becomes better again if you set the number higher. At least 10 rows:
image

I think the problem we are trying to solve by straightening the lines more is very small. The extra complexity and extra weird situations increase. Not sure if its worth it.

@Noguai
Copy link
Contributor

Noguai commented Nov 18, 2018

I'm not sure if it is an improvement in all situations, Actually, I like it better without the extended straightening. [...] For some reason I find it harder to predict how the lines are flowing through history. Not sure how to explain.

I think the problem we are trying to solve by straightening the lines more is very small. The extra complexity and extra weird situations increase. Not sure if its worth it.

Thanks for the work you put into it 👍, I know I'm kinda getting on everyones nerves a bit. I can't tell you guys what to do, but in full honesty I think it's an improvement. You are completely right that it isn't always, but it feels like in the majority of cases it is.
I played around with the lookahead and looked at different sections of a commits, most of the time I liked 10, sometimes 5 the best. (Tried to post the images to an imgur album, but that doesn't seem to work currently)

The position I'm coming from is, that at my workplace there are, next to the intermediates, also a bunch of beginners in Git (mostly undergraduate students) the latter of which I have to introduce into Git and the concept of a VCS itself. And one thing that I noticed repeatedly is, that people loose track of what's going on in a commit graph, what the current situation is or what they have done, when lines get wavy and move around a lot. And I notice the same thing somewhat at a less extreme level for myself too, when I see myself confronted with a part of a repository where things have not been done as cleanly as they could have been. It takes just a little more effort to follow the lines around, and even more when you're not that comfortable and need a GUI the most.

The current improvement with none, or a small lookahead of 2 is a great one and one that I gladly take. The reason why I am so involved with the idea of straightening the lines is, that it could help said people even further. But if your experience tells you a different story, the GitExtensions team or most other users don't like it at all, or if I am making a problem bigger than it actually is, I'm fine without too. Just trying to get as much out of it as I can. 😉😄

@gerhardol
Copy link
Member

Tried to post the images to an imgur album, but that doesn't seem to work currently

Just paste images here

@spdr870
Copy link
Member Author

spdr870 commented Nov 18, 2018

Another try. Just for fun. This time I attempted to keep the lanes straight, without gaps. I imitated the order from another git client. @Noguai, try to guess which one! The graph is 100% equal to this other client. To complete the imitation I made the curves a lot sharper and disabled 'draw non relatives gray'. Again, its a quick prototype. I did't push it, I hacked it in.

The rules are simple: order the lanes by the order the branches are started. Its actually a lot more simple then the current code.

The result is that the lanes are more straight. The tradeoff is more crossings.

Left is new right is old:
image
Page 2:
image

@vbjay
Copy link
Contributor

vbjay commented Nov 21, 2018 via email

@RussKie
Copy link
Member

RussKie commented Nov 22, 2018

Just for fun, I opened the same commits in another git gui (fork). It really is the same commit that is selected!

I hope we're not planning to do the same... to me it looks awful

@mstv
Copy link
Member

mstv commented Nov 27, 2018

I pushed the 'keep your lane' branch if anyone wants to try it (feature/keepyourlane). I worked with it for a while, and in my work scenario there are more cons then pro's.

I have used this branch since you pushed it (yet without the branch laneinfo). I like the straight lines.
(Today, I tried once again to commit a refactoring in a way that Git does not break the history. It's Git's great weakness.)
When I checked the history of a renamed file (100% similarity), it threw an exception on opening the File History Window:
System.Drawing.Brush GetBrushForRevision(GitUI.UserControls.RevisionGrid.Graph.LaneInfo, GitUI.UserControls.RevisionGrid.Graph.RevisionGraphRevision, Boolean) @ GitUI.UserControls.RevisionGrid.Columns.RevisionGraphColumnProvider
grafik
The current master (Appveyor build 3.00.00.4310) works partially and shows a single revision although I activated Show Full History.

BTW I've also seen this red frame and cross (or something similarly ugly IMO) flashing up while editing the commit message (but don't know how to reproduce it).

@spdr870
Copy link
Member Author

spdr870 commented Nov 28, 2018

I pushed the 'keep your lane' branch if anyone wants to try it (feature/keepyourlane). I worked with it for a while, and in my work scenario there are more cons then pro's.

I have used this branch since you pushed it (yet without the branch laneinfo). I like the straight lines.
(Today, I tried once again to commit a refactoring in a way that Git does not break the history. It's Git's great weakness.)
When I checked the history of a renamed file (100% similarity), it threw an exception on opening the File History Window:
System.Drawing.Brush GetBrushForRevision(GitUI.UserControls.RevisionGrid.Graph.LaneInfo, GitUI.UserControls.RevisionGrid.Graph.RevisionGraphRevision, Boolean) @ GitUI.UserControls.RevisionGrid.Columns.RevisionGraphColumnProvider
grafik
The current master (Appveyor build 3.00.00.4310) works partially and shows a single revision although I activated Show Full History.

BTW I've also seen this red frame and cross (or something similarly ugly IMO) flashing up while editing the commit message (but don't know how to reproduce it).

Could you specify the commit you are working on? I would advise 4c30361 in my fork, if you want that specific graph layout. If the problem is there, could you send the call stack?

@mstv
Copy link
Member

mstv commented Nov 29, 2018

I worked on 3369613 in the GE repo (upstream/feature/keepyourlane).
The NRE can be reproduced with 4c30361, too, and even with 0e233aa "Improve LaneInfo".
Just double click the file which was renamed in c4ac995 of my branch https://github.com/mstv/gitextensions/tree/repro/nre_getbrushforrevision.
Callstack:

GitUI.dll!GitUI.UserControls.RevisionGrid.Columns.RevisionGraphColumnProvider.GetBrushForRevision(GitUI.UserControls.RevisionGrid.Graph.LaneInfo laneInfo, bool isRelative) Line 371
    at F:\Build\gitextensions\GitUI\UserControls\RevisionGrid\Columns\RevisionGraphColumnProvider.cs(371)
GitUI.dll!GitUI.UserControls.RevisionGrid.Columns.RevisionGraphColumnProvider.OnCellPainting.__DrawItem|14_3(System.Drawing.Graphics g, int index, ref GitUI.UserControls.RevisionGrid.Columns.RevisionGraphColumnProvider.<>c__DisplayClass14_0 value, ref GitUI.UserControls.RevisionGrid.Columns.RevisionGraphColumnProvider.<>c__DisplayClass14_1 value) Line 317
    at F:\Build\gitextensions\GitUI\UserControls\RevisionGrid\Columns\RevisionGraphColumnProvider.cs(317)
GitUI.dll!GitUI.UserControls.RevisionGrid.Columns.RevisionGraphColumnProvider.OnCellPainting.__DrawVisibleGraph|14_2(ref GitUI.UserControls.RevisionGrid.Columns.RevisionGraphColumnProvider.<>c__DisplayClass14_0 value, ref GitUI.UserControls.RevisionGrid.Columns.RevisionGraphColumnProvider.<>c__DisplayClass14_1 value) Line 201
    at F:\Build\gitextensions\GitUI\UserControls\RevisionGrid\Columns\RevisionGraphColumnProvider.cs(201)
GitUI.dll!GitUI.UserControls.RevisionGrid.Columns.RevisionGraphColumnProvider.OnCellPainting.__PaintGraphCell|14_0(int rowIndex, System.Drawing.Rectangle cellBounds, System.Drawing.Graphics graphics, ref GitUI.UserControls.RevisionGrid.Columns.RevisionGraphColumnProvider.<>c__DisplayClass14_0 value) Line 147
    at F:\Build\gitextensions\GitUI\UserControls\RevisionGrid\Columns\RevisionGraphColumnProvider.cs(147)
GitUI.dll!GitUI.UserControls.RevisionGrid.Columns.RevisionGraphColumnProvider.OnCellPainting(System.Windows.Forms.DataGridViewCellPaintingEventArgs e, GitCommands.GitRevision revision, int rowHeight, GitUI.UserControls.RevisionGrid.CellStyle style) Line 73
    at F:\Build\gitextensions\GitUI\UserControls\RevisionGrid\Columns\RevisionGraphColumnProvider.cs(73)
GitUI.dll!GitUI.UserControls.RevisionGrid.RevisionDataGridView.OnCellPainting(object sender, System.Windows.Forms.DataGridViewCellPaintingEventArgs e) Line 275
    at F:\Build\gitextensions\GitUI\UserControls\RevisionGrid\RevisionDataGridView.cs(275)
[External Code]
GitUI.dll!GitUI.GitUICommands.StartBrowseDialog(System.Windows.Forms.IWin32Window owner, string filter, GitUIPluginInterfaces.ObjectId selectedCommit) Line 1073
    at F:\Build\gitextensions\GitUI\GitUICommands.cs(1073)
GitUI.dll!GitUI.GitUICommands.RunBrowseCommand(System.Collections.Generic.IReadOnlyList<string> args) Line 1549
    at F:\Build\gitextensions\GitUI\GitUICommands.cs(1549)
GitUI.dll!GitUI.GitUICommands.RunCommandBasedOnArgument(System.Collections.Generic.IReadOnlyList<string> args, System.Collections.Generic.IReadOnlyDictionary<string, string> arguments) Line 1381
    at F:\Build\gitextensions\GitUI\GitUICommands.cs(1381)
GitUI.dll!GitUI.GitUICommands.RunCommand(System.Collections.Generic.IReadOnlyList<string> args) Line 1350
    at F:\Build\gitextensions\GitUI\GitUICommands.cs(1350)
GitExtensions.exe!GitExtensions.Program.RunApplication() Line 147
    at F:\Build\gitextensions\GitExtensions\Program.cs(147)
GitExtensions.exe!GitExtensions.Program.Main() Line 64
    at F:\Build\gitextensions\GitExtensions\Program.cs(64)

@spdr870
Copy link
Member Author

spdr870 commented Dec 4, 2018

@mstv I fixed the null reference exception and rebased the branches on master. Also pushed a few other variants on the keepyourlane concept, to visualize some options. I still didn't find a way of rendering the graph that is optimal for all cases.

For me, the branch spdr870/feature/keepyourlane_alittle_curves is the best. Even in larger repostories it is reasonable. Whats best about this branch; the change is only 2 lines of code. This means we can make this optional. We don't have to render the curves as smooth as I did, it has drawbacks. It renders some corners sharp and some really smooth. We could render the curves the same as now.

@mstv
Copy link
Member

mstv commented Dec 4, 2018

Thank you, the exception is not thrown anymore.

The branch spdr870/feature/keepyourlane_alittle_curves is my favorite, too. Though I would like a horizontal axis of symmetry very much (see the PR #5783 (comment) item 1), i.e. no direct line between nodes in adjacent rows. This looks much better in the branch spdr870/feature/keepyourlane_bendy in my opinion:
grafik instead of grafik and grafik instead of grafik

@spdr870
Copy link
Member Author

spdr870 commented Dec 5, 2018

@mstv Sorry for the branch naming, but this is probably better: feature/keepyourlane_alittle_bendy

@mstv
Copy link
Member

mstv commented Dec 5, 2018

Yes, it is. To make it nearly perfect for me: Could it be combined with the curves for branches crossing lanes without nodes in the affected rows?
combine grafik and grafik to grafik

@Noguai
Copy link
Contributor

Noguai commented Dec 6, 2018

Woah, that's a lot of new stuff to try out! I've used 4c30361 for some time now, and I really like it. I absolutely don't mind if edges are a little more or less rounded over, there' just one thing i find a little problematic which seems to have gotten a bit worse with rounding:
image
Nope, these aren't merge commits. You can see it, but you have to pay attention. (Disabling the gray out option, whatever it's original name was 😄, helps.)

But I did't have much time to try your new variants out yet. Right now at a quick glance I'm hard pressed to see a difference between feature/keepyourlane_curves and feature/keepyourlane_alittle_curves. I also didn't see holes in the graph with feature/keepyourlane_sharp_skipholes while quickly scrolling through the commit history yet. But I have to double check that I didn't make a mistake while building...

Great stuff!

@OJacot-Descombes
Copy link

OJacot-Descombes commented Feb 15, 2019

I think that a good compromise between straight lanes and a very compact but curvy look would be to differentiate between the curvature of the lanes (the branches) and the curvature related to branching and merging. The lanes would be barely bended and move only slowly to the right or left, while branching and merging would be quick. It would be configurable; how many commits it takes for a branch to move to another lane. Example: 1 = old compact look, 20 = balanced, 1000 = (almost) straight lanes. Like this, one single strategy would satisfy everybody.

This manually (quick & dirty) drawn graph shows the idea of locally almost stright lanes that are bending around structures on a higher scale.
gitextbendinglanes

@ghost ghost added the 🚧 status: in progress Issues which have associated PRs label Mar 21, 2021
@ghost ghost closed this as completed in #9028 Mar 31, 2021
@ghost ghost removed the 🚧 status: in progress Issues which have associated PRs label Aug 6, 2021
@iaswtw
Copy link

iaswtw commented Jan 17, 2022

Is this feature available in a released version of Git Extensions yet?

@gerhardol
Copy link
Member

Is this feature available in a released version of Git Extensions yet?

Some of the changes are in vNext, which means that they are included in master, not yet released.

@ghost ghost added the 🚧 status: in progress Issues which have associated PRs label Jan 14, 2023
@xnousnow
Copy link

Any progress?

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment