Mystery (concurrency/component drawing?) bug in very simple Swing dice program

Question

Sorry for the poor question title, I'm stumped as to the cause of this bug and didn't know how to phrase the question.

I'm learning basic Swing and doing this exercise from the online book Introduction to Programming with Java.

I haven't followed the instructions to the letter and instead am trying to do this:

• have a window that shows a visual representation of two dice
• when you click on one of the dice, it 'rolls' and shows the new value

My implementation:

• a very basic JDie object that extends JPanel
• overridden paintComponent method to draw the die representation
• change die color every time value is changed just for a visual clue
• added a listener to 'roll' the dice when mouse is pressed and then
  repaint

The bug is quite specific:

1. Run the DieTest main method
2. Resize the window to fit the two die
3. Click on the second die to roll it
4. Now click on the first die to roll it
5. The second die's value changes back to its original value
6. If you resize the window the second die's value will also change back

If I click on the dice to roll them, before I resize the window, the bug won't appear...

I guess I'm making a basic mistake somewhere which has just disguised itself as this strange behaviour.

I've tried to whittle down the code as much as I could, it took ages just working out when the bug appeared and when it didn't:

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

class JDie extends JPanel {

    private Color color;
    private int value;

    JDie(){

        value = getValue();
        color = Color.BLACK;

        //add listener
        addMouseListener(new MouseAdapter(){
            @Override
            public void mousePressed(MouseEvent arg0) {
                value = getValue(); //'roll' the die
                repaint();
            }
        });
    }

    /*private helper methods */
    private int getValue(){
        int v =(int)(Math.random()*6) + 1;
        //change color just to show that the
        //value has changed
        color = getRandomColor();
        return v;
    }
    private Color getRandomColor(){
        float r = (float)Math.random();
        float g = (float)Math.random();
        float b = (float)Math.random();
        return new Color(r, g, b);
    }

    //draws the pips for the die
    @Override
    public void paintComponent(Graphics g){
        super.paintComponent(g);
        g.setColor(color);


        //draw pips
        //set pip size
        int pip_side = 10;
        switch(value){
        case 1:
            g.fillRect(3*pip_side, 3*pip_side, pip_side, pip_side);
            break;
        case 2:
            g.fillRect(5*pip_side, pip_side, pip_side, pip_side);
            g.fillRect(pip_side, 5*pip_side, pip_side, pip_side);
            break;
        case 3:
            g.fillRect(5*pip_side, pip_side, pip_side, pip_side);
            g.fillRect(pip_side, 5*pip_side, pip_side, pip_side);
            g.fillRect(3*pip_side, 3*pip_side, pip_side, pip_side);
            break;
        case 4:
            g.fillRect(pip_side, pip_side, pip_side, pip_side);
            g.fillRect(5*pip_side, 5*pip_side, pip_side, pip_side);
            g.fillRect(5*pip_side, pip_side, pip_side, pip_side);
            g.fillRect(pip_side, 5*pip_side, pip_side, pip_side);
            break;
        case 5:
            g.fillRect(pip_side, pip_side, pip_side, pip_side);
            g.fillRect(5*pip_side, 5*pip_side, pip_side, pip_side);
            g.fillRect(5*pip_side, pip_side, pip_side, pip_side);
            g.fillRect(pip_side, 5*pip_side, pip_side, pip_side);
            g.fillRect(3*pip_side, 3*pip_side, pip_side, pip_side);
            break;
        case 6:
            g.fillRect(pip_side, pip_side, pip_side, pip_side);
            g.fillRect(5*pip_side, 5*pip_side, pip_side, pip_side);
            g.fillRect(5*pip_side, pip_side, pip_side, pip_side);
            g.fillRect(pip_side, 5*pip_side, pip_side, pip_side);
            g.fillRect(pip_side, 3*pip_side, pip_side, pip_side);
            g.fillRect(5*pip_side, 3*pip_side, pip_side, pip_side);
            break;
        }
    }
}

public class DieTest extends JFrame{

    DieTest(){
        setLayout(new GridLayout());
        add(new JDie());
        add(new JDie());

        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        //if I set the size smaller than the JDie is displaying
        //and resize the window before 'rolling' the dice
        //then the bug appears...?!
        setSize(80, 80);
        //setting the size larger than both JDie
        //and it works fine whether you resize or not
//      setSize(200, 200);

        setVisible(true);

    }

    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Runnable(){

            @Override
            public void run() {
                new DieTest();
            }

        });
    }

}

--------------EDIT-----------------

Running the code again, I've now noticed it doesn't happen 100% of the time, but the bug is still there. Here's a gif that I just took of it bugging that might illustrate the problem better:

You can clearly see the original value with the original color is being redrawn when I click the first die again. When I resize the window, the second die's value jumps back one, to the previous value it had... I really don't understand this...

---------------EDIT 2---------------------

• Tried the same code on another computer (mac) but couldn't replicate the problem.
• Compiled and ran the code on my computer outside of Eclipse, couldn't replicate the problem.
• Ran the code that Eclipse had compiled from command line, only got the problem once, couldn't replicate it today.
• Running the code from Eclipse, I still get the problem about 1 in 5-10 times? If it doesn't show up on the first 'pass' as it were, it doesn't show up at all. The gif illustrates it well.

So it seems my computer set up has some bearing on this as well, details are:

• Windows 7 64 bit
• Eclipse Kepler Service Release 1
• Java Version 7 Update 51
• Java SE Development Kit 7 Update 3 (64 bit)

This is a tricky one as I know longer know whether it's my code or some other program's that is causing the problem. As a newb how can I know whether any future problems are a result of my bad programming or something else... frustrating.

----------EDIT 3-----------

As a quick investigation into the concurrency side of things: I set all the instance fields to volatile I set all the methods including paintComponent to synchronized I removed the Math.random() call (although I read another thread saying that this was the thread safe implementation) and replaced it with instance Random objects instead

Unfortunately I still get the visual switchback.

Another thing I've noticed is that it seems to be happening a lot less often now about 1 in 10 times. I keep on getting my hopes up that it's fixed, and then the next attempt bugs again. In my original program I was working on, it seemed more like 1 in 3 (I've completely changed that program now so don't have it to hand).

--------EDIT 4--------- I have come up with a slightly more stripped down version which no longer uses any random values and still produces the visual switchback. It seems to happen more often with this code:

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

public class ColorPanelsWindow extends JFrame{

    class ColorPanel extends JPanel {

        //color starts off black
        //once it is changed should never be 
        //black again
        private Color color = Color.BLACK;

        ColorPanel(){
            //add listener
                    //click on panel to rotate color
            addMouseListener(new MouseAdapter(){
                @Override
                public void mousePressed(MouseEvent arg0) {
                    color = rotateColor();
                    repaint();
                }
            });
        }
        //rotates the color black/blue > red > green > blue
        private Color rotateColor(){
            if (color==Color.BLACK || color == Color.BLUE)
                return Color.RED;
            if (color==Color.RED)
                return Color.GREEN;
            else return Color.BLUE;
        }

        @Override
        public void paintComponent(Graphics g){
            g.setColor(color);
            g.fillRect(0, 0, 100, 100);
        }
    }
ColorPanelsWindow(){
    setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

    setLayout(new GridLayout(1,0));
    add(new ColorPanel());
    add(new ColorPanel());
    //the size must be set so that the window is too small
    // and the two ColorPanels are overlapping
    setSize(40, 40);
            //setSize(300, 200);

    setVisible(true);

}

public static void main(String[] args) {
    SwingUtilities.invokeLater(new Runnable(){

        @Override
        public void run() {
            new ColorPanelsWindow();
        }

    });
}

}

A couple of observations:

• afaict the panels must be overlapping at first
• if I use a flow layout, increase the size of the window, or any way that stops the panels from overlapping initially, then I don't seem to get the problem (or maybe it just happens much less often?)

Answer 1

Like Múna, I can't reproduce your findings, but I have a few observations:

• Override getPreferredSize() to define the initial geometry.

• Define constants that enforce the geometric relationships.

• Use a layout that respects the preferred size, e.g. FlowLayout to to see the effect.

• Use Color.getHSBColor() to get various saturated hues.

• Use a single instance of Random as needed.

Addendum: The intermittent nature and platform variability of the problem strongly suggest incorrect synchronization. In the original program, both dice share a single, static instance of Random owned by Math; in the example below, each die has it's own instance. Note also that resizing the frame indirectly invokes paintComponent().

As tested:

import java.awt.*;
import java.awt.event.*;
import java.util.Random;
import javax.swing.*;

public class DieTest extends JFrame {

    DieTest() {
        setLayout(new FlowLayout());
        add(new JDie());
        add(new JDie());
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        pack();
        setVisible(true);
    }

    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Runnable() {

            @Override
            public void run() {
                new DieTest();
            }
        });
    }

    private static class JDie extends JPanel {

        private static final int SIDE = 32;
        private static final Random r = new Random();
        private Color color;
        private int value = getValue();
        private final Timer t = new Timer(500, null);

        JDie() {
            setBorder(BorderFactory.createEtchedBorder(color, color.darker()));
            value = getValue();
            addMouseListener(new MouseAdapter() {
                @Override
                public void mousePressed(MouseEvent arg0) {
                    value = getValue();
                    repaint();
                }
            });
            t.addActionListener(new ActionListener() {

                @Override
                public void actionPerformed(ActionEvent e) {
                    value = getValue();
                    repaint();
                }
            });
            t.start();
        }

        @Override
        public Dimension getPreferredSize() {
            return new Dimension(SIDE * 7, SIDE * 7);
        }

        private int getValue() {
            color = Color.getHSBColor(r.nextFloat(), 1, 1);
            return r.nextInt(6) + 1;
        }

        @Override
        public void paintComponent(Graphics g) {
            super.paintComponent(g);
            g.setColor(color);
            switch (value) {
                case 1:
                    g.fillRect(3 * SIDE, 3 * SIDE, SIDE, SIDE);
                    break;
                case 2:
                    g.fillRect(5 * SIDE, SIDE, SIDE, SIDE);
                    g.fillRect(SIDE, 5 * SIDE, SIDE, SIDE);
                    break;
                case 3:
                    g.fillRect(5 * SIDE, SIDE, SIDE, SIDE);
                    g.fillRect(SIDE, 5 * SIDE, SIDE, SIDE);
                    g.fillRect(3 * SIDE, 3 * SIDE, SIDE, SIDE);
                    break;
                case 4:
                    g.fillRect(SIDE, SIDE, SIDE, SIDE);
                    g.fillRect(5 * SIDE, 5 * SIDE, SIDE, SIDE);
                    g.fillRect(5 * SIDE, SIDE, SIDE, SIDE);
                    g.fillRect(SIDE, 5 * SIDE, SIDE, SIDE);
                    break;
                case 5:
                    g.fillRect(SIDE, SIDE, SIDE, SIDE);
                    g.fillRect(5 * SIDE, 5 * SIDE, SIDE, SIDE);
                    g.fillRect(5 * SIDE, SIDE, SIDE, SIDE);
                    g.fillRect(SIDE, 5 * SIDE, SIDE, SIDE);
                    g.fillRect(3 * SIDE, 3 * SIDE, SIDE, SIDE);
                    break;
                case 6:
                    g.fillRect(SIDE, SIDE, SIDE, SIDE);
                    g.fillRect(5 * SIDE, 5 * SIDE, SIDE, SIDE);
                    g.fillRect(5 * SIDE, SIDE, SIDE, SIDE);
                    g.fillRect(SIDE, 5 * SIDE, SIDE, SIDE);
                    g.fillRect(SIDE, 3 * SIDE, SIDE, SIDE);
                    g.fillRect(5 * SIDE, 3 * SIDE, SIDE, SIDE);
                    break;
            }
        }
    }
}

Answer 2

Yes, yes, I know exactly what you're talking about. I suffered from the same effect on my application.

Although I had a really hard time figuring out the reason, which I'm - even now - not sure if it's indeed what I think; that's happening, (too technical and not appropriate to post that all here) I did fix it, and I don't even know why it works.

So, instead of repainting the component itself, repaint its container (or its container's container - if still didn't fix).

getParent().repaint();

Hope that helps.