2

A question similar to this has been asked several times. See e.g. here and here.

Yet I would really like to understand why it is that my code doesn't work. As has been answered in other versions of this question, a CardLayout would probably suffice, though in my case I'm not sure if it's ideal. In any case, what I'm interested in is understanding conceptually why this doesn't work.

I have a JFrame who's content pane listens to key events. When a key is pressed in the content pane, the content pane tells the JFrame to update itself with a new content pane. Here's a simple example of the problem:

This code is fully compilable. You can copy-paste it and run it as is.

Here's my JFrame:

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

public class SimpleSim extends JFrame{

    private static SimpleSim instance = null;

    public static SimpleSim getInstance(){
        if(instance == null){
            instance = new SimpleSim();
        }

        return instance;
    }

    private SimpleSim(){}

    public void initialize(){

        this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        this.setExtendedState(Frame.MAXIMIZED_BOTH);
        this.pack();
        this.setVisible(true);

        update();
    }

    public void update(){
        System.out.println("SIMPLE_SIM UPDATE THREAD: " + Thread.currentThread().getName());
        Random rand = new Random();

        float r = rand.nextFloat();
        float g = rand.nextFloat();
        float b = rand.nextFloat();

        SimplePanel simplePanel = new SimplePanel(new Color(r, g, b));
        JPanel contentPane = (JPanel) this.getContentPane();

        contentPane.removeAll();
        contentPane.add(simplePanel);
        contentPane.revalidate();
        contentPane.repaint();

        validate();
        repaint();

    }


}

And here's my JPanel that serves as my content pane:

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


public class SimplePanel extends JPanel implements KeyListener {

    public SimplePanel(Color c){

        setFocusable(true);
        setLayout(null);
        setBackground(c);
        setVisible(true);
        this.addKeyListener(this);
    }
    public void keyTyped(KeyEvent keyEvent) {
        if(keyEvent.getKeyChar() == 'a'){
            System.out.println("a");
            System.out.println("SIMPLE_PANEL KEY PRESS THREAD: "  + Thread.currentThread().getName());
            SimpleSim.getInstance().update();
        }
    }

    public void keyPressed(KeyEvent keyEvent) {
    }
    public void keyReleased(KeyEvent keyEvent) {
    }
}

Oddly, it works the first time you press a, but not after. My guess is that there is a threading issue going on here. I can see that when update is first called it's called on the main thread. The next time it's called on the EDT. I've tried calling update() using invokeLater() and that also didn't work. I've found a workaround using a different design pattern, but I'd really like to understand why this doesn't work.

Also, simple class to run:

public class Run {

    public static void main(String[] args){
        SimpleSim.getInstance().initialize();
    }
}

Note: The seemingly redundant call to validate and repaint the JFrame was done to try to appease the advice posted on the second link I provided, which stated that: Call validate() on the highest affected component. This is probably the muddiest bit of Java's rendering cycle. The call to invalidate marks the component and all of its ancestors as needing layout. The call to validate performs the layout of the component and all of its descendants. One works "up" and the other works "down". You need to call validate on the highest component in the tree that will be affected by your change. I thought this would cause it to work, but to no avail.

Community
  • 1
  • 1
  • While I have no evidence to the contrary (apart from the `main` method), but all interactions with the UI must be executed from within the context of the EDT. You may be doing this, but a couple of statements you made concerned me. If you are begin good and careful, then please feel free to ignore this statement... – MadProgrammer Nov 27 '12 at 04:13

1 Answers1

3

I made some modifications to your code, sorry, but it made testing SOooo much easier...

The import change that I can see is in the update method. Basically I simply called revalidate on the frame

Revalidate states

Revalidates the component hierarchy up to the nearest validate root.

This method first invalidates the component hierarchy starting from this component up to the nearest validate root. Afterwards, the component hierarchy is validated starting from the nearest validate root.

This is a convenience method supposed to help application developers avoid looking for validate roots manually. Basically, it's equivalent to first calling the invalidate() method on this component, and then calling the validate() method on the nearest validate root.

I think that last part is what you were missing in you own code.

public class SimpleSim extends JFrame {

    private static SimpleSim instance = null;

    public static SimpleSim getInstance() {
        if (instance == null) {
            instance = new SimpleSim();
        }

        return instance;
    }

    private SimpleSim() {
    }

    public void initialize() {

        this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        this.setSize(400, 400);
        this.setVisible(true);
        setLayout(new BorderLayout());

        update();

    }

    public void update() {
        System.out.println("NEXT: " + Thread.currentThread().getName());
        Random rand = new Random();

        float r = rand.nextFloat();
        float g = rand.nextFloat();
        float b = rand.nextFloat();

        SimplePanel simplePanel = new SimplePanel(new Color(r, g, b));
        JPanel contentPane = (JPanel) this.getContentPane();

        getContentPane().removeAll();
        add(simplePanel);

        revalidate();
    }

    public class SimplePanel extends JPanel {

        public SimplePanel(Color c) {

            setFocusable(true);
            setLayout(null);
            setBackground(c);
            setVisible(true);
            requestFocusInWindow();
            InputMap im = getInputMap(WHEN_IN_FOCUSED_WINDOW);
            ActionMap am = getActionMap();

            im.put(KeyStroke.getKeyStroke(KeyEvent.VK_A, 0), "A");
            am.put("A", new AbstractAction() {
                @Override
                public void actionPerformed(ActionEvent e) {
                    System.out.println("a");
                    System.out.println("KEY: " + Thread.currentThread().getName());
                    SimpleSim.getInstance().update();
                }
            });

        }
    }

    public static void main(String[] args) {
        EventQueue.invokeLater(new Runnable() {
            @Override
            public void run() {
                try {
                    UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
                } catch (ClassNotFoundException | InstantiationException | IllegalAccessException | UnsupportedLookAndFeelException ex) {
                }

                SimpleSim.getInstance().initialize();
            }
        });
    }
}

Also, I'd suggest you take advantage of the key bindings API instead of using KeyListener. It will solve some of the focus issues ;)

UPDATE

After some time testing various permutations, we've come to the conclusion that the major issue is related to a focus problem.

While the SimplePanel was focusable, nothing was giving it focus, meaning that the key listener fails to be triggered.

Added simplePanel.requestFocusInWindow AFTER it was added to the frame seems to have allowed the key listener to remain active.

From my own testing, with out a call to revalidate the panels did not update.

MadProgrammer
  • 343,457
  • 22
  • 230
  • 366
  • +1. However, I don't think the issue is with revalidate(). You can add that too my code (in fact, I had tried that too) and it doesn't fix it. I put the excessive calls of (re)validate, repaint, pack, etc. to illustrate that I don't think the issue was with getting swing to validate the component heirarchy. The issue here is a threading one, and the reason why your code fixes it is because you initially run the program using EventQueue's invokeLater call. This is what I wanted to clarify with my question. Why does this fix it? What's wrong with the threading in my original code? –  Nov 27 '12 at 05:03
  • Also, the key bindings API was mention on the other questions, and it is very sound advice. I'll be using that in the future –  Nov 27 '12 at 05:04
  • Swing is not thread safe. It's possible you've been visited by a race condition or other threading anomaly. When testing you're code, `revalidate` was the only thing that actually worked – MadProgrammer Nov 27 '12 at 05:07
  • 1
    I just took out the `EventQueue.invokeLater` call from the `main` method and called your initalizer directly and it worked fine. The first call came from `main` the remainder came from `AWT-EventQueue-0`. This could be a combination of issues (focus, key listener, event queue) accumulating against you – MadProgrammer Nov 27 '12 at 05:11
  • I agree, it's one of (or a combination of) several issues: threading/event queue, key bindings, focus. I've basically tried permutations of all suggestions you've given with my code and yours, and I think you hit the nail on the head by pointing out that it's a focus, key listener, or event queue issue. I've been able to get it to work without any calls to revalidate, so I'm quite sure that's not it. All good stuff though, I think your code gives me a better snapshot of how it should be written to avoid these issues in the first place. –  Nov 27 '12 at 05:31
  • Also, calling revalidate on just my code in itself doesn't fix it. The issue is clearly with how KeyListener and the EDT are working in my code, and I guess I'm still wondering what it might be. As you say though, it could be a lot of things, and it may not really be possible to tell what specifically is going on. –  Nov 27 '12 at 05:55
  • I've tried a few permutations and can't seem to replicate the original issue. As soon as I add `revalidate` it seems to work as expected – MadProgrammer Nov 27 '12 at 05:59
  • Thanks for sticking with this, here's the permutations I've tried: If you run my code *exactly as is*, it fails. If you run it and *only* add a revalidate, it fails (in fact, revalidate had no effect on any combinations I tried). If I use your initializer on my code, still doesn't work. Finally, though, if I use keybindings like you suggested, it works! Ah, I think that's it! Looking at all other trials, this seems the key ingredient that fixes it. I'll look into how KeyBindings works differently and why it fixes it, perhaps suggest an edit to your answer, and then select your answer as final. –  Nov 27 '12 at 06:09
  • 1
    I think it's a focus issue. If I call `simplePanel.requestFocusInWindow` AFTER I add it, I can get the key listener to fire, but unless I add `revalidate`, it will never update – MadProgrammer Nov 27 '12 at 06:13
  • Edit looks good! You're correct in that you do have to make some validation, but my point is that it doesn't have to be the revalidate call, it could just be the validate call I use in my original code. If you add revalidate to my code exactly as it is on the page without doing anything else, it doesn't work, because you immediately lose focus after it first switches. But you do have to do some kind of validation, be it pack, validate, or revalidate, but I was doing that in my original code already. The issue was definitely a focus one. I'll stop commenting now :) –  Nov 27 '12 at 06:45