-
-
Notifications
You must be signed in to change notification settings - Fork 753
Scale nodes by file size #351
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
base: master
Are you sure you want to change the base?
Conversation
5ba8bc7 to
193af0e
Compare
|
Cool looking feature. Thanks for the detailed overview of the changes. |
193af0e to
813c3d8
Compare
813c3d8 to
da2c52a
Compare
|
I have now had some time to test the PR a bit more and I think the physics in particular needs some more tweaking before it could be deemed "stable". Examples:
That being said, it still works fine for most common projects I have tested it on. I have some ideas on how to stabilize it somewhat. Like adding adding an outward force or a more "spring like" gravity model. For now I made a few changes to the PR:
I will try to improve it and report back. |
| if (gGourceSettings.scale_by_file_size) { | ||
| for (RFile* f : files) { | ||
| if (!f->isHidden()) { | ||
| float r = f->getSize() / 2.0f; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this sets the radius to file size. It means the actual dot size will be proportional to file size squared. The area of the dots will not be proportional to file size.
| } | ||
|
|
||
| GitCommitLog::GitCommitLog(const std::string& logfile) : RCommitLog(logfile, 'u') { | ||
| GitCommitLog::GitCommitLog(const std::string& logfile) : RCommitLog(logfile, 'u'), m_repository_path(logfile) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe instead RCommitLog could keep a copy of logfile and then we can refer to it later with self.logfile
| //this isnt a commit we are parsing, abort | ||
| if(commit.timestamp == 0) return false; | ||
|
|
||
| if(!logf->getNextLine(line)) return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logfile could be an existing file generated with a previous version of Gource's log file command without the hash. It should still continue to work with the existing logfile, so if this line is a file line (starting with ':'), and they didn't request scaling by file size, it needs to recover from that. If they are using file scaling and there's no hash it could fail to parse at that point, ideally with a descriptive error.
|
|
||
| if (gGourceSettings.scale_by_file_size && status != "D") { | ||
| char cmd_buff[2048]; | ||
| int written = snprintf(cmd_buff, 2048, "git --git-dir=%s/.git --work-tree=%s cat-file -s %s", m_repository_path.c_str(), m_repository_path.c_str(), dst_blob.c_str()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to cause a performance issue doing this here as it blocks the UI while fetching the blob. Also this wont work if logfile is a file and not a directory. You can also see it block when you move the mouse cursor of the interactive timeline as it peeks at the part of the log under the cursor to get the timestamp.
I think for performance instead what would be good is we get all the file blob sizes up front (maybe just get every blob file size in the repo at once) right after we generate the git log, puts them into a data structure, and then we can look up into it here.
| } | ||
|
|
||
| void RFile::setFileSize(unsigned int new_file_size) { | ||
| file_size = new_file_size; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to animate the file size changing, so it interpolates between the old file size and the new file size, maybe over the same period as the action laser beam. So it would keep the old size but also store the new size, and then it could use another variable like file_size_change_time_elapsed to work out the current size.
Summary
This PR implements node scaling based on file size through
-S/--scale-by-file-size. The legacy spiral layout remains the default behavior. This also uses a new physics-based layout. Related issues: #91 #54 #147 #223Screencast.from.2025-09-30.15.22.31.webm
File Size Calculation
File size is calculated with
git cat-file -s <blob>, and the custom log format has been extended totimestamp|username|type|file|colour|file_size.Physics-Based Layout
A new physics-based layout is added to better position nodes of varying radii.
The physics can be tweaked with these CLI parameters:
--file-gravity FACTOR: Sets the gravity for file nodes (default: 0.01).--file-repulsion FACTOR: Sets the repulsion for file nodes (default: 0.1).The physics simulation is implemented in the
applyFilePhysicsfunction insrc/dirnode.cpp.Other parameters
Some more CLI parameters were added during debugging:
--file-scale FACTOR: Sets the scale factor for file nodes (default: 1.0).--dir-spacing FACTOR: Sets the spacing between node clusters (default: 1.0).--show-file-size-on-hover: Show file size in the tooltip when hovering over a node.Other changes
/src/dirnode.h:calcFileDest()had wrong parameter name.Minor cleanup in/src/gource_settings.cpp: removed extra backslashes in regex. Seems like they are not needed anmore and gave me compiler warnings.Discussion
I have tried it out moderately. Generally I will run something like
gource -S --show-file-size-on-hover. It works well but the physics can become a bit wonky when there are many large files in a single directory. Ideas can be to tweak--file-gravityand--file-repulsionfurther.Currenly all files are scaled logarithmically (
log(size + 1.0)). We might want to better show file size differences, however, we can't just translate filesize to radius directly since it would lead to giant nodes. At the very least they would need to be normalized.Some paremeters should maybe be hidden or exposed to debug/tweak more. For example, should
--show-file-size-on-hoveralways be enabled when--scale-by-file-sizeis enabled?Currently the scaling is done by file size, i.e. bytes. Other ideas/alternatives could be to use number of lines, as suggested by PR #136. This could be a possible future parameter.
If any changes are too intrusive or ambiguous, please let me know :)