Search code examples
javaswingjpanelpaintcomponentthread-sleep

Why isn't my JFrame object animating as I would expect?


I'm trying to animate a circle as per a Head First Java exercise. The original exercise calls for using an internal "MyDrawPanel" class within the "AnimatedBall" class, but I'm trying to do an external MyDrawPanel class. I have no errors, but there also is no animation. I'm assuming it has something to do with me passing values to the MyDrawPanel constructor that don't update when I run my for loop, but I'd like to know why this is.

AnimatedBall2.java:

import javax.swing.JFrame;

public class AnimatedBall2 {
    JFrame frame = new JFrame();
    int height = 300;
    int width = 300;
    int x = 70;
    int y = 70;


    public static void main(String[] args){
        AnimatedBall2 gui = new AnimatedBall2();
        gui.go();
    }

    public void go(){
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        MyDrawPanel panel = new MyDrawPanel(x, y, height, width);

        frame.getContentPane().add(panel);
        frame.setSize(height,width);
        frame.setVisible(true);

        for(int i = 0; i < 100; i++){
            x++;
            y++;
            panel.repaint();

            try{
                Thread.sleep(50);
            } catch(Exception ex){}
        }
    }   
}

MyDrawPanel.java:

import java.awt.Color;
import java.awt.Graphics;
import javax.swing.JPanel;


public class MyDrawPanel extends JPanel{
    int x;
    int y;
    int height;
    int width;

    public MyDrawPanel(int x1, int y1, int z1, int z2){
        x = x1;
        y = y1;
        height = z1;
        width = z2;

    }


    public void paintComponent(Graphics g){
        g.setColor(Color.white);
        g.fillRect(0, 0, height, width);
        g.setColor(Color.orange);
        g.fillOval(x, y, 100, 100); 

    }
}

Solution

  • Swing is a single threaded framework, that is, there is a single thread which is responsible for processing all events within the system and scheduling painting.

    Anything that blocks this thread, will prevent it from processing new events or paint requests.

    This is a very bad idea...

    public void go(){
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    
        MyDrawPanel panel = new MyDrawPanel(x, y, height, width);
    
        frame.getContentPane().add(panel);
        frame.setSize(height,width);
        frame.setVisible(true);
    
        for(int i = 0; i < 100; i++){
            x++;
            y++;
            panel.repaint();
    
            try{
                Thread.sleep(50);
            } catch(Exception ex){}
        }
    }   
    

    Add could potentially lock up your application...

    Take a look at Concurrency in Swing for more details. You should also have a look at Initial Threads

    In this case, I would recommended using a javax.swing.Timer, see How to use Swing Timers for more details.

    The main problem is, you've decarled x and y in your AnimatedBall2 class...

    public class AnimatedBall2 {
        //...
        int x = 70;
        int y = 70
    

    And in your MyDrawPanel class...

    public class MyDrawPanel extends JPanel{
        int x;
        int y;
    

    But your loop is updating the values in the AnimatedBall2 class, meaning that the values in MyDrawPanel never change

    You will need to add an "update" method of some kind which can tell the MyDrawPanel that it should update it's x/y values accordingly...

    Which brings us to...Swing is not thread safe, all modifications or interactions with the UI should be done within the context of the Event Dispatching Thread...This is why I'd recommend using a Swing Timer, as it triggers it's notifications within the context of the EDT.

    When performing custom painting, you should call super.paintComponent before performing any custom painting of your own. You should also not relay on "magic" numbers

    g.fillRect(0, 0, height, width);
    

    The size of the component can be changed by the layout manager depending on it's needs, instead, you should be using getWidth and getHeight which will tell you exactly what the size of the current component is.

    For example...

    import java.awt.Color;
    import java.awt.Dimension;
    import java.awt.EventQueue;
    import java.awt.Graphics;
    import java.awt.event.ActionEvent;
    import java.awt.event.ActionListener;
    import javax.swing.JFrame;
    import javax.swing.JPanel;
    import javax.swing.Timer;
    import javax.swing.UIManager;
    import javax.swing.UnsupportedLookAndFeelException;
    
    public class AnimatedBall2 {
    
        public static void main(String[] args) {
            new AnimatedBall2();
        }
    
        public AnimatedBall2() {
            EventQueue.invokeLater(new Runnable() {
                @Override
                public void run() {
                    try {
                        UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
                    } catch (ClassNotFoundException | InstantiationException | IllegalAccessException | UnsupportedLookAndFeelException ex) {
                        ex.printStackTrace();
                    }
    
                    final MyDrawPanel panel = new MyDrawPanel();
    
                    JFrame frame = new JFrame("I'm a banana");
                    frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
                    frame.getContentPane().add(panel);
                    frame.pack();
                    frame.setVisible(true);
    
                    Timer timer = new Timer(50, new ActionListener() {
                        @Override
                        public void actionPerformed(ActionEvent e) {
                            panel.update();
                        }
                    });
                    timer.start();
    
                }
            });
        }
    
        public class MyDrawPanel extends JPanel {
    
            int x;
            int y;
    
            int deltaX = 0;
            int deltaY = 1;
    
            public MyDrawPanel() {
                x = 150;
                y = 150;
                setBackground(Color.WHITE);
            }
    
            public void update() {
                x += deltaX;
                y += deltaY;
                if (x < 0) {
                    x = 0;
                    deltaX *= 1;
                } else if (x > getWidth()) {
                    x = getWidth();
                    deltaX *= 1;
                }
                if (y < 0) {
                    y = 0;
                    deltaY *= -1;
                } else if (y > getHeight()) {
                    y = getHeight();
                    deltaY *= -1;
                }
                repaint();
            }
    
            @Override
            public Dimension getPreferredSize() {
                return new Dimension(300, 300);
            }
    
            @Override
            protected void paintComponent(Graphics g) {
                super.paintComponent(g);
                g.setColor(Color.orange);
                g.fillOval(x - 50, y - 50, 100, 100);
    
            }
        }
    }