I made a class capable of moving Polygons around on a Polygon in a mostly smooth manner. Disclaimer: this was part of a school project (thus the use of Polygon rather than Image), but this code is entirely outside of the scope of the assignment, and the assignment has already been graded.The project was a Bejeweled clone, so the Animator is capable of the two animations needed for that clone: it can take two ColoredPolygons (a simple wrapper around a Color and a Polygon) and swap their position, or a List of ColoredPolygons and translate them all some dx/dy. A built in assumption in this animation is that only one animation will be active at a time, and that the area covered by the animation only contains the Polygons that were passed in.package cad97.bejeweled.animation;import javax.swing.*;import java.awt.*;import java.awt.geom.Area;import java.util.List;/** * Animate Polygons around on a Graphics object. * * @author Christopher Durham * @date 11/25/2016 */public final class Animator {    public static final Animator INSTANCE = new Animator();    private final static int ANIMATION_FRAMES = 30;    private final static int ANIMATION_FPS = 60;    private final static int ANIMATION_INTERVAL = 1000 / ANIMATION_FPS;    /**     * Convenience class bundling a {@link Color} and a {@link Polygon}     */    public static class ColoredPolygon {        final Color color;        final Polygon polygon;        public ColoredPolygon(Color color, Polygon polygon) {            this.color = color;            this.polygon = polygon;        }    }    /**     * Animate the swap of two Polygons on a Graphics object.     * <p>     * The parameter Polygons are mutated during the execution of this method.     * <p>     * A redraw of the animated area in the {@code after} callback is suggested,     * as the final frame of this animation is not guaranteed to be fully finished.     * (E.g. the frame before a fully finished position.)     *     * @param bgColor the background Color used to erase frames     * @param first   the first ColoredPolygon being switched     * @param second  the second ColoredPolygon being switched     * @param g       the Graphics object to draw on     * @param after   callback run on UI thread after animation is finished     */    public void translateSwap(final Color bgColor, final ColoredPolygon first, final ColoredPolygon second,                              final Graphics g, final Runnable after) {        new SwingWorker<Void, Void>() {            @Override            protected Void doInBackground() throws Exception {                final Rectangle b1 = first.polygon.getBounds();                final Rectangle b2 = second.polygon.getBounds();                final int deltaX = (b1.x - b2.x) / ANIMATION_FRAMES;                final int deltaY = (b1.y - b2.y) / ANIMATION_FRAMES;                final Rectangle animationBounds = b1.union(b2);                for (int i = 0; i < ANIMATION_FRAMES; i++) {                    first.polygon.translate(-deltaX, -deltaY);                    second.polygon.translate(deltaX, deltaY);                    SwingUtilities.invokeAndWait(() -> {                        g.setColor(bgColor);                        g.fillRect(animationBounds.x, animationBounds.y, animationBounds.width, animationBounds.height);                        g.setColor(first.color);                        g.fillPolygon(first.polygon);                        g.setColor(second.color);                        g.fillPolygon(second.polygon);                    });                    Thread.sleep(ANIMATION_INTERVAL);                }                SwingUtilities.invokeLater(after);                return null;            }        }.execute();    }    /**     * Translate a group of Polygons a direction on a Graphics object.     * <p>     * The passed Polygons are mutated during the execution of this method.     * <p>     * A redraw of the animated area in the {@code after} callback is suggested,     * as the final frame of this animation is not guaranteed to be fully finished.     * (E.g. the frame before a fully finished position.)     *     * @param bgColor  the background Color used to erase frames     * @param polygons a list of ColoredPolygons to translate     * @param dx       the delta x to translate the polygons     * @param dy       the delta y to translate the polygons     * @param g        the Graphics object to draw on     * @param after    callback run on UI thread after animation is finished     */    public void batchTranslate(final Color bgColor, final List<ColoredPolygon> polygons,                               final int dx, final int dy, final Graphics2D g, final Runnable after) {        new SwingWorker<Void, Void>() {            @Override            protected Void doInBackground() throws Exception {                final Area animationBounds = polygons.stream().sequential()                        .map(it -> it.polygon).map(Polygon::getBounds).peek(it -> {                            it.grow(dx / 2, dy / 2);                            it.translate(dx / 2, dy / 2);                        }).map(Area::new).reduce((lhs, rhs) -> {                            if (lhs == null) return rhs;                            rhs.add(lhs);                            return rhs;                        })                        .orElseThrow(AssertionError::new);                final int deltaX = dx / ANIMATION_FRAMES;                final int deltaY = dy / ANIMATION_FRAMES;                for (int i = 0; i < ANIMATION_FRAMES; i++) {                    polygons.forEach(it -> it.polygon.translate(deltaX, deltaY));                    SwingUtilities.invokeAndWait(() -> {                        g.setColor(bgColor);                        g.fill(animationBounds);                        polygons.forEach(it -> {                            g.setColor(it.color);                            g.fill(it.polygon);                        });                    });                    Thread.sleep(ANIMATION_INTERVAL);                }                SwingUtilities.invokeLater(after);                return null;            }        }.execute();    }}I've never done actual animation before, so this is likely a hacky solution that goes against best practices. This is within a Swing context, so any suggestions using the Swing library and not just AWT are welcome.Here is a minimal harness for this code on Gist. This is not part of the review, merely a way to enable ease of testing. In the creation of this harness, I noticed a bug on line 102 of this class, the fix to which is commented in the Gist.
Animation of java.awt.Polygons
java;swing;animation;awt
Conceptual ProblemsYou have hard coded the animation FPS as ANIMATION_FPS = 60. This is a big no-no! You are assuming that your code will be called exactly with exactly 1000/ANIMATION_FPS = 16.7 ms intervals. But you are using Thread.sleep(long) which is:subject to the precision and accuracy of system timers and schedulers.Remember that on Windows the scheduling time slice is on the order of 10 ms (see here where a Vista system has 15ms). So this means that your calling accuracy on the animation is about the same order of magnitude as your time step, this is bad news! Your animation will either be jittery or time slide. This is not Windows specific, all OSes have some manner of time slice, some may have dynamic ones. They all just differ in how long these slices are but in general they are on the milli second scale. Even if the OS sleep was accurate, your way of doing it with a fixed sleep would break down if the CPU was busy and couldn't keep up with your paint in a fast enough manner causing a time dilation in your animation.What you really should do is to for each call to your painting algorithm figure out the time since the last call and multiply this by the speed you want your figure to move/rotate whatever.Pseudocode:void paint(float deltaTimeSeconds){    int xPos = oldXPos + velocityX*deltaTimeSeconds;    drawAt(XPos);}If you don't do it in the way described above you may encounter problems such as your animation feeling jittery or that the animation slides and doesn't maintain the correct speed on the screen. Useless threadingYou are doing the paint computations in a background thread and then signalling the result to the main thread at an interval. To me this looks like unnecessary work. I believe that you will have better performance and less trouble by simply doing the drawing in the application thread.Yes I realise that you use the background thread to trigger the draw because you can't trigger it from the UI thread. But there is a better way of doing this, simply use javax.swing.Timer.From the documentation:An example use is an animation object that uses a Timer as the trigger for drawing its frames.Also interesting:Although all Timers perform their waiting using a single, shared thread (created by the first Timer object that executes), the action event handlers for Timers execute on another thread -- the event-dispatching thread. This means that the action handlers for Timers can safely perform operations on Swing components. However, it also means that the handlers must execute quickly to keep the GUI responsive.Example implementation in AnimationPanel (some assembly required):long lastFrameNs;boolean animationCompleted = false;Timer animationTimer = new Timer(ANIMATION_INTERVAL, event -> {    long currentFrameNs = System.currentNano();    long deltaNs = currentFrameNs - lastFrameNs;    lastFrameNs = currentFrameNs;    float deltaS = deltaNs /1E9f;    // Update polygon positions here using `deltaS`     // Detect when the animation finished....    if(done){        animationCompleted = true;    }    // Tell SWING that the component is dirty and to repaint it    // Bonus: Figure out the affected `Rectangle` and pass that into     // repaint to avoid repainting the whole shebang.     repaint();}); // Repeats by default// Add method to trigger the animation and parametersvoid animateSwap(Jewel A, Jewel B, ...){    // Setup necessary data here...    animationCompleted = false;    lastFrameNs = System.currentNano();    animationTimer.start();}@Overridevoid paint(Graphics g){    // Draw all the jewels at their positions here...    if(animationCompleted){        animationTimer.stop();        animationCompleted = false;    }}At this point I feel that further review of the code is pointless from my part as the above already highlights problems that would mandate a rewrite that would make further review obsolete.