Adds ability to dynamically adjust seconds-per-day. #252
Conversation
|
Addresses #251 |
| idle_time = 0.0; | ||
| } | ||
|
|
||
| if(gGourceSettings.transitions.size() > 0 && commit.timestamp > gGourceSettings.transitions.front()) { |
There was a problem hiding this comment.
Because Gource can be used interactively and you could seek the timeline back to before the transition, this should really iterate over the transitions to find the relevant transition rather than modify the setting.
| printf(" for a number of seconds (default: 3)\n"); | ||
| printf(" --disable-auto-skip Disable auto skip\n"); | ||
| printf(" -s, --seconds-per-day SECONDS Speed in seconds per day (default: 10)\n"); | ||
| printf(" --transition TIMESTAMP Timestamp for transitions\n"); |
There was a problem hiding this comment.
Perhaps Timestamp for seconds per day transition
There was a problem hiding this comment.
Will also need a bit of README / man page documentation for how to use this feature.
| if(gource_settings->getEntry("seconds-per-day") == 0 || ( | ||
| gource_settings->getEntries("seconds-per-day")->size() != | ||
| timestamps->size() + 1)) { | ||
| conffile.entryException(entry, "transition requires exactly 1 more seconds-per-day specified"); |
There was a problem hiding this comment.
Maybe seconds-per-day should be specified 1 more times than transition would be easier to understand.
| return 0; | ||
| } | ||
|
|
||
| gGourceSettings.importGourceSettings(*conf, *gource_settings); |
There was a problem hiding this comment.
Gource supports config file having mutiple [gource] sections, so you can have multiple configurations in one file and it will cycle to the next one when the current one concludes. So it still needs to handle that, we'll just need to come up with another way to avoid it creating duplicate multi value entries.
There was a problem hiding this comment.
The overwriting behaviour is kind of nice as it allows for instance having a config file, but also having command line parameters in addition to the config file settings. Appending to existing multi value lists probably isnt useful though.
There was a problem hiding this comment.
You could avoid dealing with this issue if you use comma separators (e.g. like the --hide option) rather than multi value fields, which would also be easier to specify. e.g. --seconds-per-day 1,2,3 --transition 2020-01-01,2020-01-02
|
Thanks for the pull request, just need to find a different work around for the calling |
With this change, a user can:
Things to note:
Future ideas to consider:
Figuring out what values to set for
--seconds-per-dayis complex. The system will invert the value and create a days per second calculation. I eventually found the following formula works well and comes super close to what I want:The coefficient
was tuned after trial and error. Getting to it is probably dependent on the performance of the machine and the complexity of the graph being rendered. Even though the denominators are the same in this example, the 127 in the fudging coefficient remains constant when specifying other lengths of time.
I think a better user experience would be to specify ranges of days and how many seconds they should run in. I would need to study the code more to figure out the ticks / frames aspects, but this may be hard to guarantee with accuracy depending on the system rendering and graph complexity.
For now, this patch provides a powerful way to dynamically dilate the time across different time ranges. If there are ideas around the better experience and getting the timing right for all level of complexities and machine performance, we can work on that instead.