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

update slider positioning code #129

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alifeee
Copy link

@alifeee alifeee commented Mar 19, 2024

I spent a long time looking at slider positioning logic in an attempt to fix #128

I could not fix #128, but I did make the slider code more readable (imo), and changed it to use save tick time instead of name. This fixes some issues with saves that are the same hour as other saves, which are then named "2-1" (for the 2nd save of 2hrs, after "2")

The slider code had two states:

Here I provide a before/after example of both. The saves used are those in #128, which are at times 00:05, 00:30, 01:20, and 02:31.

Normal state

Before

image

After

image

Dense state

Before

image

After

image

Conclusion

As you can see, the "1-1" save (the 00:05 h) save was wrongly positioned before, but now is placed correctly.

I hope you find my changes acceptable. Thanks for continuing to maintain this great tool.

use save tick time instead of name
@L0laapk3
Copy link
Owner

L0laapk3 commented Mar 20, 2024

Hey, thanks for your contribution efforts. I have a couple comments:

Normally, "1-1h" is definitely supposed to come after "1h". If the timestamp says otherwise, then it is likely that you generated the timeline with the saves out of order. Out of order capturing will never work, this is by design due to how the differential image saving works. I should really add a safeguard for this sometime :p (#130)

As for the actual slider itself, I gave it a bit of thought and I am leaning towards just removing the current code with the two modes and writing something new and simpler that just works for all cases, to hopefully finally fix this issue for once and for all.

For this, while the height of the slider could be changed, at the end of the day it is limited by the screen size, so the labels should be able to placed optimally for any given slider height.
I was thinking of an approach like this:

  • use a fixed "minimum desired" height offset between every label, just enough to keep the text from overlapping.
  • subtract that from the slider height. If there is remaining height, it should then all be distributed in relation to the timestamp of the labels.

Let me know if you're interested in implementing something like this. If not, I will have a look at it myself soon™️.

@alifeee
Copy link
Author

alifeee commented Mar 20, 2024

Normally, "1-1h" is definitely supposed to come after "1h".

Interesting. As I sort by tick in the above code, the name does not matter, saves with more save-time will be higher on the slider. Notably, these saves are also from #128 where I think order-behaviour is screwy anyway.

If the timestamp says otherwise, then it is likely that you generated the timeline with the saves out of order.

Please see #128 (comment) and #128 (comment). Interestingly, the same result comes from either order.

As for the actual slider itself, I gave it a bit of thought and I am leaning towards just removing the current code with the two modes and writing something new and simpler that just works for all cases, to hopefully finally fix this issue for once and for all.

For this, while the height of the slider could be changed, at the end of the day it is limited by the screen size, so the labels should be able to placed optimally for any given slider height.
I was thinking of an approach like this:

  • use a fixed "minimum desired" height offset between every label, just enough to keep the text from overlapping.
  • subtract that from the slider height. If there is remaining height, it should then all be distributed in relation to the timestamp of the labels.

I may be interested.

Would you want to increase the slider length to better fit tick-ordering? i.e., would you rather have something like

1

image

or 2

image

I have not fully thought out the logic, but there is a situation where we could start with 1 (maximum height, spaced), and 'squish' the slider (keeping relative separation between saves) until the minimum distance between any 2 labels is at a minimum.

Or, we could start like 2, with minimum separation, and then 'stretch' the slider, stretching the distances that represent a larger time difference more than the others.

Let me know if you're interested in implementing something like this. If not, I will have a look at it myself soon™️.

Anyway, I would be interested, but my effort in this repository would go towards fixing #128 before this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants