First off, threads are expensive. That does not mean that this technique cannot work, just that it is likely to be inefficient.
In your particular case, you will have threads waking up each 10 milliseconds, and as many threads as animations you have. If you will have a thread that wakes each 10 milliseconds anyway, you could use one thread per core (with affinity set to each core) and balance the animation among those threads. That would be more efficient. We could then consider updating each animation a “task”, and voila you have a rudimentary “task” system.
Or at the very least recycle your threads. Notice that when you kill the animation, the thread becomes garbage that the system has to collect and you will be creating a new – expensive – thread for any new animation. Do not do that, do a thread pool.
What you are doing with the animation thread is a fancy clock. It should be possible to calculate what should be the current frame of the animation given the
currentTimeMillis and the delays.
If the delays are unpredictable, that means that to calculate the current frame you would: 1) get duration of the animation by adding the delays (value that be stored and reused). 2) Do a mod of the difference of the current time minus the start time by the duration of the animation. 3) Check the resulting reminder against the delays (complexity is O(n)). On the other hand, if the delays are follow a mathematical pattern, then it can probably done much more efficiently. For example, if each frame has the same delay, you do
currentFrameNumber = ((currentTime - startTime) % (delay * frames.length)) / delay; (complexity is O(1)).
For your threading solution to be worth doing, it should be more efficient that doing that. Moreover, guess what, you can opt to do that only when you need to show one of the sprites, meaning that you would only have to do it for the sprites on screen. Meanwhile your threads will be working for every animation in the scene… behind the scenes. Or wasting CPU time each 10 milliseconds.
I guess there is some edge case where you will have too many animation, of which, all of them are always on the screen, and having some animation lag in exchange of responsiveness is acceptable... then I would agree that threading the animation is a good idea... but on a thread pool, please.
By the way, you should be able to use the
delay to sleep... you say:
the thread is interrupted every time we try and get the image
That should not be the case! I do not know why it would be… I hope you are not pausing the thread to read it. If it is an artifact
synchronized (it should not be), then I am going to tell you to get rid of that anyway.
Second, the presented code, I presume is incomplete, otherwise, it will never work because
init will always be false.
WHY it takes so much to animate in this method.
It takes forever if
init is false.
If I were you I would not have
init and I would not have a default constructor that sets fields to
null. If you need to do some work before starting the animation, you should be able to work with the
frames arrays before calling the constructor. In fact, I would suggest creating an animation class that has those arrays as members… but that is off-topic.
Third, what will be a problem – even for a singular correctly initialized animation – is thread visibility. The thread is updating the field
currentFrame, but the CPU could be keeping the value of
currentFrame in the local cache of the threads (after all this field is accessed often), resulting in changes made by the animation thread being hidden to (not visible by) other threads.
If this is the problem, it is not that it takes so much to animate in this method, instead that the render thread becomes aware of changes of
currentFrame too late.
What you need in this case is
volatile. You can turn
volatile. You could also turn
volatile, or read from
In fact, you not only do not need
synchronized but it counterproductive, it is a waste of resources. The
synchronized keyword ensures is that at most one thread enters any of the
synchronized methods of the class at the same time. Well, multiple executions of
stop are idempotent so they don’t need this protection, and
getCurrentFrame reads a variable that is being set on a method that is not
run), thus using
getCurrentFrame is useless. Furthermore,
synchronized does not guarantee visibility… so yeah. Get rid of that.
Final note: You need a game loop that uses sleep to yield CPU time to the animation thread and any other thing you might have running in the background. If your game loop is eating all the CPU time and the animation threads are not getting enough then the animation are going to lag.
Yet, you probably should use thread for something more complex that a "flipbook animation", in particular something independent of the screen, so that there is some remarkable gain in using threads.*