Draw and move a circle in Java

Question

I'm using Swing to create a small GUI in Java. All I am trying to get it to do is take an ArrayListof Circles and draw them. I've run into two problems:

1) I have to call my draw method repeatedly before it draws the circle. If I just call my draw method once nothing happens, I get a blank drawing. If I call it in a loop that runs for less than 30 milliseconds it only draws the first of two circles that I want to draw. Finally, if I call it for more than 30 milliseconds it draws both circles I am trying to draw.

and

2) When I move one of the circles, I get a "flicker" on the drawing.

I'm not too familiar with Swing programming. I've looked at sample code and watched a few videos - and what I have looks right to me. But I figure I must have messed something up, because it doesn't look like this in the videos I've watched.

Here is my GUI class:

package gui;

import draw.*;
import java.util.List;
import javax.swing.*;

public class GUI extends JFrame {
    private CirclePainter drawingBoard = new CirclePainter();

    public GUI()
    {
        setSize(500, 500);
        this.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
        this.setVisible(true);
        this.add(drawingBoard);
        drawingBoard.setVisible(true);
    }

    public void draw(List<Circle> circles)
    {
        drawingBoard.paintComponent(drawingBoard.getGraphics(), circles);
    }
}

my CirclePainter class

package gui;

import draw.Circle;

import javax.swing.*;
import java.awt.*;
import java.util.List;

class CirclePainter extends JPanel
{
    public void paintComponent(Graphics graphics, List<Circle> circles)
    {
        super.paintComponent(graphics);
        for(Circle circle : circles)
            graphics.fillOval(circle.getX(), circle.getY(), circle.getRadius() * 2, circle.getRadius() * 2);
    }
}

EDIT: redacted some code since this is for a school project. The remaining code should be enough for someone visiting in the future to still understand the question.

Solution 1

Building on your code and the suggestions from Hovercraft Full Of Eels, a small step in the right direction could be taken with these modifications to the GUI and CirclePainter classes:

// GUI.draw
public void draw(List<Circle> circles)
{
//    drawingBoard.paintComponent(drawingBoard.getGraphics(), circles);
    drawingBoard.setCircles(circles);
    drawingBoard.repaint();
}


class CirclePainter extends JPanel
{
//    public void paintComponent(Graphics graphics, List<Circle> circles)
//    {
//        super.paintComponent(graphics);
//        for(Circle circle : circles)
//            graphics.fillOval(circle.getX(), circle.getY(), circle.getRadius() * 2, circle.getRadius() * 2);
//    }

    private List<Circle> circles;

    public void setCircles(final List<Circle> circles) {
        this.circles = circles;
    }

    @Override
    protected void paintComponent(final Graphics graphics) {
        super.paintComponent(graphics);
        for (Circle circle : circles)
            graphics.fillOval(circle.getX(), circle.getY(), circle.getRadius() * 2, circle.getRadius() * 2);
    }
}

This way, you might not have fixed all the fundamental issues, but you get your program to work with only minor changes. And Swing is a very nice library that can be much fun to learn more about.

Solution 2

1. Never call paintComponent(...) directly as you're doing.
2. Instead suggest a draw by calling repaint() on a component when necessary.
3. Don't draw with a Graphics object obtained via a getGraphics() call on a component. Instead, draw with the Graphics object provided in the paintComponent method.
4. Avoid using while (true) loops in a Swing GUI as you risk tying up the Swing event thread and freezing the GUI. Use a Swing Timer for simple animations.
5. You probably don't even need a Swing Timer since your animation can be driven by your MouseListener/MouseMotionListener.
6. Most important -- do read the Swing painting and other tutorials, as most of this information can be found there. It looks like you're guessing how to do some of your coding and that's a dangerous thing to do when it comes to drawing or animating a GUI. You can find most tutorials in the Swing info link.
7. Consider using a Shape object to represent your Circle, such as an ellipse2D. The reason that this will help is that it has some very useful methods, including a contains(Point p) method that will help you determine if a mouse click lands inside of your circle.
8. You will want to decide where _x and _y represent the center point of your circle or not. If so, then you'll need to adjust your drawing some, by shifting it left and up by _radius amount.
9. Consider casting your Graphics object into a Graphics2D object in order to use its extra methods and properties.
10. One such property are the RenderingHings. Set your Graphics2D RenderingHints to allow for anti-aliasing to get rid of your image "jaggies". This can be done with: g2.setRenderingHint(RenderingHints.KEY_ANTIALIASING, RenderingHints.VALUE_ANTIALIAS_ON); where g2 is your Graphics2D object.
11. Your paintComponent method is not a true paintComponent override and thus won't work correctly. It should be a protected method, not public, it should have one parameter, a Graphics object, and nto a second parameter, and you should place the @Override annotation above it.

For example, please have a look at this answer of mine to a similar problem.

An example of a paintComponent method that centers the circles on _x and _y and that uses rendering hints:

class CirclePainter extends JPanel implements Iterable<Circle> {
   private static final int PREF_W = 500;
   private static final int PREF_H = PREF_W;
   private CircleList circleList = new CircleList();

   @Override
   protected void paintComponent(Graphics graphics) {
      super.paintComponent(graphics);
      Graphics2D g2 = (Graphics2D) graphics;
      g2.setRenderingHint(RenderingHints.KEY_ANTIALIASING,
            RenderingHints.VALUE_ANTIALIAS_ON);
      for (Circle circle : circleList) {
         // if x and y are the center points, then you must subtract the radius.
         int x = circle.getX() - circle.getRadius();
         int y = circle.getY() - circle.getRadius();
         int width = circle.getRadius() * 2;
         int height = width;
         g2.fillOval(x, y, width, height);
      }
   }

Comments

[Freek de Bruijn]

These look like great suggestions. I think the best way to approach the problems mentioned in the question might depend on your preferred learning style. Some people prefer to learn by coding instead of reading the tutorials first.

[Hovercraft Full Of Eels]

@FreekdeBruijn: yes, experimentation is always a good thing, but in my experience something like Swing graphics is so counter-intuitive, most of us, myself included, guess wrong. Here you have to get some lead from the tutorials to move ahead. One plus up-vote for your answer by the way.

[Freek de Bruijn]

I agree, it's very easy to paint yourself into a corner by this type of experimentation with Swing graphics. Still, it's sometimes difficult to invest many hours of learning when you're trying to build something small.

[Hovercraft Full Of Eels]

@FreekdeBruijn: there are also many many examples of Swing graphics programs including animations to be found on this site, some written by me.

[ThomYorkkke]

I think Eels advice was better... but this one answered my question . So... I choose you. Thanks!

[ThomYorkkke]

Awesome. Someone gave an answer that actually answered my question, but your post helped a lot. Thanks.

[Hovercraft Full Of Eels]

@ThomYorkkke: no problem. please check additions to answer as well. And good luck on your project.

[Freek de Bruijn]

@ThomYorkkke: so my quick and dirty fix is chosen over the good advice of Hovercraft Full Of Eels... Thanks. Please do take a look at other examples (like Java Animate JLabel) to find more animation code and good luck with your circles!

[ThomYorkkke]

It's all about the easy way out... :-D

[ThomYorkkke]

@HovercraftFullOfEels Thanks for the addendum about making my paintComponent a true paintComponent. I just fixed that.