Assistance with java repaint() method?

Question

I'm trying to make a rectangle to move across the screen, but instead it's just repainting and not getting rid of the preceding rectangle making it look like a giant rectangle across the screen. Any help would be appreciated, here's my code:

import javax.swing.JComponent;
import javax.swing.JFrame;
import javax.swing.Timer;

import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.geom.*;

@SuppressWarnings("serial")
public class Test extends JFrame implements ActionListener{
int x=50;
Timer tm = new Timer(30,this);

public static void main(String[] args){
    new Test();
}

public Test(){
    this.setSize(700, 500);
    this.setTitle("Drawing Shapes");
    this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    this.setLocationRelativeTo(null);
    this.setVisible(true);
}

public void paint(Graphics g){
     Graphics2D graph2 = (Graphics2D)g;

     Shape Rect = new Rectangle2D.Float(x, 50, 50, 30);
     graph2.setColor(Color.RED);
     graph2.fill(Rect);
     tm.start();
    }

public void actionPerformed(ActionEvent e){
    x=10+x;
    repaint();
}
}

Answer 1

• Draw in a JPanel that is held by and displayed within the JFrame. Don't draw directly in the JFrame as this risks drawing over things that shouldn't be messed with such as root panes, borders, child components,... Also you lose the benefits of automatic double buffering by drawing directly in the JFrame's paint method leading to choppy animation.
• You should override the JPanel's paintComponent method not its paint method.
• Always call the super's painting method in your own painting method (here it again should be paintComponent). This is your problem. So again, your paintComponent(Graphics g) override painting method should call super.paintComponent(g); on its first line. This will erase the old images.

Answer 2

You're breaking the paint chain...

public void paint(Graphics g){
    // Broken here...
    Graphics2D graph2 = (Graphics2D)g;

    Shape Rect = new Rectangle2D.Float(x, 50, 50, 30);
    graph2.setColor(Color.RED);
    graph2.fill(Rect);
    tm.start();

}

You MUST call super.paint. See Painting in AWT and Swing and Performing Custom Painting for more details about painting in Swing...

Top level containers are not double buffered and it is not recommended to paint directly to them, instead, create a custom component which extends from something like JPanel and override it's paintComponent method (calling super.paintComponent before performing any custom painting)

As a general rule of thumb, you should avoid extending from top level containers like JFrame, as you are not adding any new functionality to the class and they lock you into a single use-case, reducing the re-usability of your classes

DON'T call tm.start inside the paint method, you should do nothing in the paint methods except paint, never try and modify the state or otherwise perform an action which might indirectly modify the state of a component, this is a good way to have you program consume your CPU

Answer 3

To add to what other have already stated,

I've noticed that you use this.setLocationRelativeTo(null) for a simple application. Not saying it is bad, but you might want to check this thread to make sure it is what you want.

How to best position Swing GUIs?