Can't draw more than one square in JFrame

Question

Can't get the program to print more than one square.

---

My code right now

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

public class MyApplication extends JFrame {

    private static final Dimension WindowSize = new Dimension(600, 600);
    private int xCord=9, yCord=32, width=80, height=80;

    public MyApplication() {
        //Create and set up the window
        this.setTitle("Squares");
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        //Display the window centered on the screen
        Dimension screensize = java.awt.Toolkit.getDefaultToolkit().getScreenSize();
        int x = screensize.width / 2 - WindowSize.width / 2;
        int y = screensize.height / 2 - WindowSize.height / 2;
        setBounds(x, y, WindowSize.width, WindowSize.height);
        setVisible(true);
    }

    public static void main(String args[]) {
        MyApplication window = new MyApplication();
    }

    public void paint(Graphics g) {
        int red = (int) (Math.random() * 255);
        int green = (int) (Math.random() * 255);
        int blue = (int) (Math.random() * 255);
        g.setColor(Color.getHSBColor(red, green, blue));
        g.fillRect(xCord, yCord, width, height);

        while((yCord+height)<600){
            if((xCord+width)>600){
                xCord=9;
                yCord+=80;
            }
            else xCord+=80;
            repaint();
        }
    }
}

I'm trying to fill a 600x600 window with squares of different colours that go to a new line once a row is full.

Answer 1

First of all, don't.

Don't override paint of top level containers, like JFrame.

JFrame is a composite component, meaning that they're a number of layers between its surface and the user and because of the way the paint system works, these can be painted independent of the frame, which can produce weird results.

Top level containers are not double buffered, meaning your updates will flash.

DO call a paint methods super method, unless you are absolutely sure you know what you're doing.

Start by taking a look at Performing Custom Painting and Painting in AWT and Swing for more details about how painting works in Swing and how you should work with it.

This...

Dimension screensize = java.awt.Toolkit.getDefaultToolkit().getScreenSize();
int x = screensize.width / 2 - WindowSize.width / 2;
int y = screensize.height / 2 - WindowSize.height / 2;
setBounds(x, y, WindowSize.width, WindowSize.height);

is a bad idea on a number of levels.

Toolkit#getScreenSize does not take into consideration the size of other UI elements which will reduce the available viewable area available on the screen, things like the taskbar/dock or menu bar on some OS

Using setBounds(x, y, WindowSize.width, WindowSize.height); on a window based class is also a bad idea, as the avaliable viewable area is the window size MINUS the window's decorations, meaning the actually viewable area is smaller then you have specified and because you're painting directly to the frame, you run the risk of painting under the frame decorations.

You can have a look at How can I set in the midst? for more details

One thing you should now about painting, painting is destructive, that is, each time a paint cycle occurs, you are expected to completely repaint the current state of the component.

Currently, this...

public void paint(Graphics g) {
    int red = (int) (Math.random() * 255);
    int green = (int) (Math.random() * 255);
    int blue = (int) (Math.random() * 255);
    g.setColor(Color.getHSBColor(red, green, blue));
    g.fillRect(xCord, yCord, width, height);

    while ((yCord + height) < 600) {
        if ((xCord + width) > 600) {
            xCord = 9;
            yCord += 80;
        } else {
            xCord += 80;
        }
        repaint();
    }
}

will only paint a single rectangle, base on the last value of xCord and yCord most likely AFTER the paint method has exited.

Swing uses a passive rendering engine, meaning that the system will make determinations about what to paint and when, you don't control it. You can make a "request" to the system through the use repaint, but it's up to the system to decide when and what will get painted, this means that multiple requests can be optimised down to a single paint pass.

Also, painting should do nothing more than paint the current state. It should avoid changing the state, directly or indirectly, especially if that change triggers a new paint pass, as this can suddenly reduce the performance of your program to 0, crippling it.

So, what's the answer?

Well, change everything...

import java.awt.Color;
import java.awt.Dimension;
import java.awt.EventQueue;
import java.awt.Graphics;
import java.awt.Graphics2D;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.UIManager;
import javax.swing.UnsupportedLookAndFeelException;

public class MyApplication {

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

    public MyApplication() {
        EventQueue.invokeLater(new Runnable() {
            @Override
            public void run() {
                try {
                    UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
                } catch (ClassNotFoundException | InstantiationException | IllegalAccessException | UnsupportedLookAndFeelException ex) {
                    ex.printStackTrace();
                }

                JFrame frame = new JFrame("Testing");
                frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
                frame.add(new TestPane());
                frame.pack();
                frame.setLocationRelativeTo(null);
                frame.setVisible(true);
            }
        });
    }

    public static class TestPane extends JPanel {

        private static final Dimension DESIRED_SIZE = new Dimension(600, 600);
        private int width = 80, height = 80;

        public TestPane() {
        }

        @Override
        public Dimension getPreferredSize() {
            return DESIRED_SIZE;
        }

        @Override
        protected void paintComponent(Graphics g) {
            super.paintComponent(g);
            Graphics2D g2d = (Graphics2D) g.create();

            int xCord = 0, yCord = 0;

            while ((yCord) < getHeight()) {
                int red = (int) (Math.random() * 255);
                int green = (int) (Math.random() * 255);
                int blue = (int) (Math.random() * 255);
                g2d.setColor(Color.getHSBColor(red, green, blue));
                g2d.fillRect(xCord, yCord, width, height);
                if ((xCord + width) > getWidth()) {
                    xCord = 0;
                    yCord += 80;
                } else {
                    xCord += 80;
                }
            }
            g2d.dispose();
        }

    }

}

Break down...

JFrame frame = new JFrame("Testing");
frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
frame.add(new TestPane());
frame.pack();
frame.setLocationRelativeTo(null);
frame.setVisible(true);

This creates an instance of Jframe, you don't really want extend from JFrame, you're not adding any new functionality to the class

frame.pack() is packing the window around the content, this ensures that the frame is always larger (by the amount of the frame decorations) then the desired content size

frame.setLocationRelativeTo(null); will centre the window in a system independent manner.

Next...

private static final Dimension DESIRED_SIZE = new Dimension(600, 600);
private int width = 80, height = 80;

public TestPane() {
}

@Override
public Dimension getPreferredSize() {
    return DESIRED_SIZE;
}

I've used DESIRED_SIZE to provide a sizing hint to the parent containers layout manager.

Finally...

@Override
protected void paintComponent(Graphics g) {
    super.paintComponent(g);
    Graphics2D g2d = (Graphics2D) g.create();
    
    int xCord = 0, yCord = 0;
    while ((yCord) < getHeight()) {
        int red = (int) (Math.random() * 255);
        int green = (int) (Math.random() * 255);
        int blue = (int) (Math.random() * 255);
        g2d.setColor(Color.getHSBColor(red, green, blue));
        g2d.fillRect(xCord, yCord, width, height);
        if ((xCord + width) > getWidth()) {
            xCord = 0;
            yCord += 80;
        } else {
            xCord += 80;
        }
    }
    g2d.dispose();
}

Note here, I've changed the xCord and yCord positions to zero, I no longer need to "guess" at the frame decorations. As well as making the local variables, so that when ever the method is called again, the values are reset to zero.

You don't "have" to cast the Graphics reference to Graphics2D, but Graphics2D is a more powerful API. I also like to copy it's state, but that's me, your code is simple enough so it's unlikely to have adverse effects on anything else that might be painted after your component.

Notice also, I've use getWidth and getHeight instead of "magic numbers", meaning you can resize the window and the painting will adapt.