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

Fix off-by-1 error for computing step sizes (#184) #185

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jpatokal
Copy link

@jpatokal jpatokal commented May 6, 2015

Likely fix for issue #184.

@javisantana
Copy link
Contributor

not sure this is the fix we need, basically step here is not about the number of frames, is about the size of each step. I feel like the problem is in animator (

time: function(_) {
) or maybe in the tileslider component. Actually none of the use step variable

There is also a problem when getSteps returns 1

@jpatokal
Copy link
Author

jpatokal commented May 6, 2015

As described in #184, the number of steps is correct, the problem is the size of the steps. For the original case, (end-start)/steps = 360/7 = 51 months (wrong); while (end-start)/(steps-1) = 360/6 = 60 months (correct).

Good point re: getSteps returning 1 though, I presume step should be set to 0 in that case?

@fdansv
Copy link
Contributor

fdansv commented May 7, 2015

This is more or less how the timeline is. Each change in the timeslider is represented by a numbered vertical line. The duration of every step is the longitude of the horizontal segments. Probably the position/value should reflect the midpoint of the step instead of its beginning, like it's doing now.

0     1     2     3     4
|_____|_____|_____|_____|_____

With the current layout, we're not displaying the end of the timeline properly. For long steps, this was addressed the other day by specifying ranges instead of exact dates, so it looks a bit like this:

  0-1   1-2   2-3   3-4   4-5
|_____|_____|_____|_____|_____

@jpatokal
Copy link
Author

jpatokal commented May 8, 2015

@fdansv, I'm not following you here. If there are 5 frames, there should only be 4 steps:

 Frames
 0     1     2     3     4
 |_____|_____|_____|_____|
    1     2     3     4 
    Steps

So if you simply lengthen the steps a little, it will cover the entire data set. Or am I missing something?

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

Successfully merging this pull request may close these issues.

3 participants