Eames
Eames

Reputation: 319

Repaint taking up too much CPU

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();
}

}

Upvotes: 1

Views: 1522

Answers (4)

user1803551
user1803551

Reputation: 13427

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);
            }
        });
    }
}

Upvotes: 4

Ramesh-X
Ramesh-X

Reputation: 5055

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.

Upvotes: 0

Whateverest
Whateverest

Reputation: 86

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);
}

Upvotes: 1

Kayaman
Kayaman

Reputation: 73558

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

Upvotes: 2

Related Questions