jeremyckahn / shifty

The fastest TypeScript animation engine on the web
https://jeremyckahn.github.io/shifty/doc/
MIT License
1.54k stars 88 forks source link

getInterpolatedValues() does not take "delay" into account. #69

Closed ghost closed 9 years ago

ghost commented 9 years ago

It seems that getInterpolatedValues() method from shifty.interpolate.js module does not take "delay" factor into account, thus, we are actually left with a fake frame (ahead by "delay" milliseconds). Should we get delay into account?

jeremyckahn commented 9 years ago

Hi @adrianvoica, thanks for reporting. I don't think that a delay value should be taken into account for the shifty.interpolate.js extension. That extension provides a low-level utility that isn't really applicable to a normal time-based tween. For instance, interpolate is the heart of Rekapi, rather than Tweenable#tween.

However, I may be misunderstanding you. Do you mean to say that something isn't working as expected for you? The delay functionality was added very recently due to #65, and I didn't notice any broken tests or functionality after adding it, but it's possible that I may have broken shifty.interpolate.js in the process. Can you describe the use case for taking delay into account for interpolate calls?

ghost commented 9 years ago

Hi @jeremyckahn. Well, the lookup table optimization that I'm doing is based on interpolate. For instance, I have a tweenable object with start value X = 10, end value X = 20, delay of 500ms and duration of 1000ms. I am expecting that after 500ms from start, X is 10, but without taking delay into account, I get a totally different value (X is 15), which is not quite correct. I hope this example is clear enough of a use case. Cheers.

jeremyckahn commented 9 years ago

Ok, I think I understand now. In that case, interpolate should account for the delay. Did you want to try to fix the bug, or should I take a stab at it?

ghost commented 9 years ago

I didn't try to fix it. At most, I would have adapted my code in the lookup optimization code, but the optimal place to fix it, I think, would be in the shifty.interpolate.js file. If you could fix it, it would be super great.

jeremyckahn commented 9 years ago

Ok. I'll try to fix it this weekend.

jeremyckahn commented 9 years ago

@adrianvoica, I'm having some trouble implementing this. I have some tests for how the API might work, but I can't seem to get them to pass. I'm missing something mathematically that's probably pretty simple, can you take a look at my delay-interpolate branch and see if you can get the tests to pass? Or even make sure that the tests are correct?

ghost commented 9 years ago

Yes, I think it can be wrapped in a new release. But on the other hand, I think it would be a good time to halt development of new features and focus completely on performance optimisations, even if code readability has to suffer a bit. I think there is huge room for improvement in speedups and efficiency, and Shifty being at the core of other libraries/tools, it should perform at its highest. I am committed to helping out in this regard (I am also directly interested in performance since I have some good stuff of my own in development based on Shifty core).

jeremyckahn commented 9 years ago

That seems like a good idea. Rather than package up a release to master, I just created a develop branch to hold stable commits that haven't been released (built and tagged). This is the workflow I use in my job, and it scales well for a team. I have merged the feature/delay-interpolate into develop (and subsequently deleted feature/delay-interpolate) so you can keep working on your performance improvements without worrying about master releases. From this point forward, please branch off of develop and make a Pull Request back into it. Does this approach work for you?

Full disclosure: I am not actively working on this project right now, as it meets the needs I built it for. However, I encourage you to continue on with your improvements to Shifty. I'll do my best to work with you to get your work integrated into the project and help out where I can, quickly and effectively.