Repaint taking up too much CPU

Question

I'm writing a simple drawing program that uses keyListeners. It works, but every time it needs to draw another circle, I have to use the repaint() method or it won't automatically repaint the screen after using one of the arrow keys. It would be fine except that it uses up way too much CPU (around 50%) for such a simple program. Any ideas on how to NOT use the repaint() method so that it can do whatever it needs without eating up all my CPU? Here is the source code:

import java.awt.BorderLayout;
import java.awt.Graphics;
import java.awt.event.KeyAdapter;
import java.awt.event.KeyEvent;

import javax.swing.JComboBox;
import javax.swing.JFrame;

public class Game extends JFrame {
int x, y;

public class AL extends KeyAdapter {

    @Override
    public void keyPressed(KeyEvent e) {
        int keyCode = e.getKeyCode();
        if (keyCode == e.VK_LEFT) {
            x--;
        }
        if (keyCode == e.VK_RIGHT) {
            x++;
        }
        if (keyCode == e.VK_UP) {
            y--;
        }
        if (keyCode == e.VK_DOWN) {
            y++;
        }

    }

    @Override
    public void keyReleased(KeyEvent e) {

    }
}

public static void main(String[] args) {
    Game game = new Game();
}

public Game() {
    addKeyListener(new AL());
    setTitle("Game");
    setSize(500, 500);
    setResizable(false);
    setVisible(true);
    setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

    x = 150;
    y = 150;

}

@Override
public void paint(Graphics g) {
    g.fillOval(x, y, 15, 15);
    repaint();
}

}

all answers missing two most important things 1. super.paint (clearing all pervious paintings) as 1st. code line, repaint inside paint creates enless loop, 2. use JPanel with paintComponent — mKorbel, Jan 02 '16 at 21:53

score 4 · Accepted Answer · edited May 23 '17 at 11:44

You are doing a few things wrong when it comes to painting:

Don't paint on a top level component like JFrame, instead add a JPanel to it and paint on it instead.
Don't override paint, override paintComponent instead.
Don't call repaint inside methods that paint (like paint and paintComponent), it will cause a recursion.

Also, use key bindings instead of key listeners. Here is an example of everything coming together:

class Example extends JPanel {

    int x = 0;
    int y = 0;

    Example() {

        getInputMap(JComponent.WHEN_IN_FOCUSED_WINDOW).put(KeyStroke.getKeyStroke("RIGHT"), "right");
        getActionMap().put("right", new AbstractAction() {

            @Override
            public void actionPerformed(ActionEvent e) {

                x++;
                repaint();
            }
        });
    }

    @Override
    public Dimension getPreferredSize() {

        return new Dimension(200, 200);
    }

    @Override
    public void paintComponent(Graphics g) {

        g.clearRect(0, 0, getWidth(), getHeight());
        g.drawRect(x, y, 30, 30);
    }

    public static void main(String args[]) {

        SwingUtilities.invokeLater(new Runnable() {

            @Override
            public void run() {

                JFrame frame = new JFrame();
                frame.add(new Example());
                frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
                frame.pack();
                frame.setVisible(true);
            }
        });
    }
}

score 2 · Answer 2 · answered Jan 02 '16 at 14:44

2

Don't call repaint(); inside paint(). Repaint schedules a paint(), so no wonder your CPU is having a hard time.

answered Jan 02 '16 at 14:44

Kayaman

72,141
5
83
121

score 1 · Answer 3 · answered Jan 02 '16 at 15:03

Like Kayaman said you should never call repaint() from within paint(). You can call the Frames repaint() Method in keyPressed() so the Frame will be repainted every time you press a key.

@Override
public void keyPressed(KeyEvent e) {
    int keyCode = e.getKeyCode();
    if (keyCode == e.VK_LEFT) {
        x--;
    }
    if (keyCode == e.VK_RIGHT) {
        x++;
    }
    if (keyCode == e.VK_UP) {
        y--;
    }
    if (keyCode == e.VK_DOWN) {
        y++;
    }
    Game.this.repaint();
}
/*...*/
@Override
public void paint(Graphics g) {
    g.fillOval(x, y, 15, 15);
}

You should also consider using the key bindings API instead of KeyListener and call super.paint — MadProgrammer, Jan 02 '16 at 21:13

score 0 · Answer 4 · answered Jan 02 '16 at 15:18

Call to repaint() will cause RepaintManager to call paint() method. So if you call repaint() inside paint() it will loop infinitely. Instead you can repaint your JFrame once the key-pressed action performed.

@Override
public void keyPressed(KeyEvent e) {
    int keyCode = e.getKeyCode();
    if (keyCode == KeyEvent.VK_LEFT) {
        x--;
    }
    if (keyCode == KeyEvent.VK_RIGHT) {
        x++;
    }
    if (keyCode == KeyEvent.VK_UP) {
        y--;
    }
    if (keyCode == KeyEvent.VK_DOWN) {
        y++;
    }
    repaint();
}

Remove repaint() call from paint() method and add it as above.

But if there are different places which you want the JFrame to be repainted and you choose the above method, it will be messy again. So, you can use a Timer to call repaint().

private final javax.swing.Timer timer;
private final int REFRESH_TIME = 100;
public Game() {
    addKeyListener(new AL());
    setTitle("Game");
    setSize(500, 500);
    setResizable(false);
    setVisible(true);
    setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

    x = 150;
    y = 150;

    timer = new javax.swing.Timer(REFRESH_TIME, new ActionListener() {

        @Override
        public void actionPerformed(ActionEvent e) {
            repaint();
        }
    });
    timer.start();
}

If you want, you can use another way to call repaint() once in a period. The calling Thread need not to be an EDT.

Repaint taking up too much CPU

4 Answers4