Why is this JLabel's ImageIcon not updating?

Question

SSCCE, as small as I could get it with keeping all the logic in the same order:

import java.awt.Color;
import java.awt.Graphics2D;
import java.awt.event.MouseEvent;
import java.awt.event.MouseListener;
import java.awt.image.BufferedImage;
import java.io.File;
import javax.imageio.ImageIO;
import javax.swing.ImageIcon;
import javax.swing.JFileChooser;
import javax.swing.JFrame;
import javax.swing.JLabel;

public class Test {

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

    BufferedImage img = null; // <-- needs this scope
    JFrame mainWindow = new JFrame();
    JLabel mainImage = new JLabel();

    public Test() {
        mainWindow.add(mainImage);
        mainWindow.setVisible(true);
        mainWindow.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
// step 5
        mainImage.addMouseListener(new MouseListener() {
            @Override public void mouseClicked(MouseEvent e) {
                dostuff();
            }
            @Override public void mouseEntered(MouseEvent e) {}
            @Override public void mouseExited(MouseEvent e) {}
            @Override public void mousePressed(MouseEvent e) {}
            @Override public void mouseReleased(MouseEvent e) {}
        });
        dostuff();
    }

    private void dostuff() {
// step 1
        try {
            JFileChooser fc = new JFileChooser();
            fc.showOpenDialog(null);
            File file = fc.getSelectedFile();
            img = ImageIO.read(file);
        } catch (Exception e) {
            e.printStackTrace(); 
            System.exit(1);
        }
//step 2
        mainImage.setIcon(new ImageIcon(img));
        mainWindow.pack();
        mainWindow.setLocationRelativeTo(null);
        Graphics2D g = img.createGraphics();
        g.setColor(new Color(0xFFFF0000));
        g.drawOval(10, 10, 10, 10);
        try{Thread.sleep(2000);}catch(Exception e){}
// step 3
        BufferedImage img2 = new BufferedImage(400,300,BufferedImage.TYPE_INT_RGB);
        for (int i = 10 ; i < 20 ; i++) {
            for (int j = 10 ; j < 20 ; j++) {
                img2.setRGB(i,j,0xFF0000FF);
            }
        }
// step 4
        mainImage.setIcon(new ImageIcon(img2));
        mainWindow.pack();
        mainWindow.setLocationRelativeTo(null);
    }

}

It should be obvious what I'm trying to do, and compiling will show that it's not doing that. But I want this post to have a question mark so here is the problem description and question:

What I want to happen:

1. The program loads, and the user is prompted to select a file (an image).

2. Upon a selecting an image, that image is displayed in a JFrame, and some Graphics2D drawings happen on it. (I included the sleep() because these drawings take a while in the actual program)

3. When the drawings are finished, a new image is created and it is also drawn on.

4. The new image replaces the old image in the JFrame

5. When the user clicks on the image, they are prompted to select a new image, and we repeat from step 1.

What is happening:

Steps one through four work fine. On step five, after selecting an image, the JFrame is populated by a black rectangle the size of the user-selected image, with the image from step three overlayed in the top left corner, step two does not happen, and steps three through five appear to work just fine.

What's more, in my actual program, I can tell that the new user-selected-image is doing its job just fine by the output that step three produces.

So the question is, how can I fix later repetitions of step two such that the appropriate image is shown in the JFrame?

EDIT: This imgur album shows step by step the results I am getting. http://imgur.com/a/xW051

EDIT2: updated without Thread.sleep()

import java.awt.Color;
import java.awt.Graphics2D;
import java.awt.event.MouseEvent;
import java.awt.event.MouseListener;
import java.awt.image.BufferedImage;
import java.io.File;
import javax.imageio.ImageIO;
import javax.swing.ImageIcon;
import javax.swing.JFileChooser;
import javax.swing.JFrame;
import javax.swing.JLabel;

public class Test {

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

    BufferedImage img = null; // <-- needs this scope
    JFrame mainWindow = new JFrame();
    JLabel mainImage = new JLabel();

    public Test() {
        mainWindow.add(mainImage);
        mainWindow.setVisible(true);
        mainWindow.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
// step 5
        mainImage.addMouseListener(new MouseListener() {
            @Override public void mouseClicked(MouseEvent e) {
                dostuff();
            }
            @Override public void mouseEntered(MouseEvent e) {}
            @Override public void mouseExited(MouseEvent e) {}
            @Override public void mousePressed(MouseEvent e) {}
            @Override public void mouseReleased(MouseEvent e) {}
        });
        dostuff();
    }

    private void dostuff() {
// step 1
        try {
            JFileChooser fc = new JFileChooser();
            fc.showOpenDialog(null);
            File file = fc.getSelectedFile();
            img = ImageIO.read(file);
        } catch (Exception e) {
            e.printStackTrace(); 
            System.exit(1);
        }
//step 2
        mainImage.setIcon(new ImageIcon(img));
        mainWindow.pack();
        mainWindow.setLocationRelativeTo(null);
        Graphics2D g = img.createGraphics();
        g.setColor(new Color(0xFFFF0000));
        for (int h = 0 ; h < 0xFF ; h++) {
            for (int i = 0 ; i < img.getWidth() ; i++) {
                for (int j = 0 ; j < img.getHeight()/2 ; j++) {
                    img.setRGB(i,j,0x88FF0000 + h);
                }
            }
            mainImage.repaint();
        }
// step 3
        BufferedImage img2 = new BufferedImage(400,300,BufferedImage.TYPE_INT_RGB);
        for (int i = 10 ; i < 20 ; i++) {
            for (int j = 10 ; j < 20 ; j++) {
                img2.setRGB(i,j,0xFF0000FF);
            }
        }
// step 4
        mainImage.setIcon(new ImageIcon(img2));
        mainWindow.pack();
        mainWindow.setLocationRelativeTo(null);
    }

}

Answer 1

Edit:

If I then load a 1000x1000 image, I see a 1000x1000 black square...

I would guess the black images are because you are causing the Event Dispatch Thread (EDT) to sleep. so although the frame is resizing with the pack() statement the newly loaded image is not being painted.

Don't use Thread.sleep() in code that is executing on the EDT. All code executed from a listener executes on the EDT. Instead you should be using a Swing Timer to schedule the event to update the image of the label.

Edit 2:

Read the section from the Swing tutorial on Concurrency. In general all event code executes on the EDT. When the EDT is blocked, Swing components can't be repainted.

  mainImage.setIcon(new ImageIcon(img));

The above statement will cause repaint() to be invoked on the "mainImage" label. The repaint() request is passed to the RepaintManager and a painting request is added to the end of the EDT.

So the label will be painted AFTER all the code in the "doStuff()" method has finished executing.

However you also invoke

 mainImage.setIcon(new ImageIcon(img2));

at the end of the method. So by the time the image actually gets painted its Icon has been changed a second time so only the second Icon is painted.

In between those two statements, after reading the image the frame is resized when the pack() method is invoked. I believe this is because the packing of the frame results in OS level painting since a frame is an OS widget not a Swing component. In other words the frame can be resized even if the components on the frame are not repainted.

If you want a responsive GUI then you can't execute long running code on the EDT. In the case of the second SSCCE the "for loop" you added takes a long time to run which is effectively blocking the EDT and preventing the newly read Icon from being painted.

Therefore long running tasks should be executed on a separate Thread. The concurrency tutorial will explain how you can use a SwingWorker for long running tasks.