Instantiated components from within a thread aren't repainting into a JFrame in Java

Question

Hi, I have a single class like this one

public class BlockSpawner implements Runnable{

public static long timeToSpawn;
private GtrisJFrame frame;

public BlockSpawner(GtrisJFrame frame)
{

    this.frame = frame;
    timeToSpawn = 2000;
}

public void run()
{
    while(true)
    {
        try
        {
            Thread.sleep(timeToSpawn);
        }
        catch(InterruptedException e)
        {
            //Unhandled exception
        }

        //After awake, instanciate 2 blocks
        //get the position of the first one
        int index = Block.getRandomStartPosition();
        new Block(frame, index);
        new Block(frame, index+1);
    }
}

}

I instance this class in the JFrame main class, and start it's thread like this:

private void initBlockSpawner()
{
    spawner = new BlockSpawner(this);
    new Thread(spawner).start();
}

I call this initBlockSpawner() function from within the JFrame Constructor. Block Class is really a little big, but in a nutshell, it implements runnable, and calls its run() method at the end of its constructor. The run() method only makes a block to fall at certain speed. I've been tried to manually instantiate new blocks in the JFrame constructor and they work, they repaint and fall. But whenever I want to instantiate Blocks from other threads, they appear to fall (i.e. its properties update each loop), but they don't paint in the JFrame.

As additional information, I am using NetBeans, and since the application entry point is on the JFrame class, the main method looks like this:

public static void main(String args[])
{
    java.awt.EventQueue.invokeLater(new Runnable() {
        public void run() {
            new GtrisJFrame().setVisible(true);
        }
    });

}

I don't have that much experience with Java Threads, awt events and swing components. But something I read here makes me think that my problem is that only one thread has the control over the swing components, or something... Is there any way to solve my problem?

Thanks in advance.

EDIT: Additional info, whenever I check the method toString on the Instantiated cubes from threads, they give me this [,0,0,0x0], but when I instantiate them within the same JFrame class they give me this result [,0,0,328x552] and they appear on the frame. this 328x552 value it's the same as the component's Dimension returned by getPreferredSize()... I tried to force them to that dimension by instantiate them like this:

new Block(this, index).setPreferredSize(new Dimension(328, 552));

But it didn't work, anyone knows what this [,0,0,328x552] value could mean?

Thanks everyone, I think we're almost there!

EDIT 2: I realized that the Size of the component is x:0 y:0, why is this? I change the BlockSpawner's run() method to something like this:

public void run()
{
    while(true)
    {
        System.out.println("SPAWN");
        int index = Block.getRandomStartPosition();
        new Thread(new Block(frame, index)).start();
        new Thread(new Block(frame, index+1)).start();

        try
        {
            Thread.sleep(timeToSpawn);
        }
        catch(InterruptedException e)
        {
            //Unhandled exception
        }
    }
}

On the first run, everything goes ok! even the pair of Blocks paint on the JFrame and fall down correctly, but after the Thread.sleep(), the rest of them just get instantiated, but their getSize() method gives me x:0 y:0; Is this still somehow related to the One Dispatcher Thread issue? or it's something different now?

Answer 1

It sounds to me (although I can't tell from your code above) that you are trying to add components to a live JFrame (i.e. one that has been shown on screen, or 'realized') from a thread other than the event dispatch thread. That's a violation of the Swing threading model, and will cause you no end of problems.

If you want to make changes to a Swing object from a different thread, you package the change up in a Runnable and submit it to the dispatch thread using EventQueue.invokeLater() or invokeAndWait().

EDIT: more info

Some additional comments (not directly related to your problem, but important nonetheless): Performing activity in a constructor is probably not a good idea. Subclassing JFrame to add components to it is also probably not a good idea. For that matter, doing these operations in a JFrame instead of a JPanel is probably not the best approach either.

Taking these in turn:

1. Constructors should be used to perform initial configuration on objects - not invoke behavior. This separation helps to keep your design clean and maintainable. Even though it might seem to be easier to do it this way, I recommend against. At some point in your design, you may decide that it's more efficient to create these objects in advance, and only use them later.

2. Subclassing a JFrame to add components is usually not a hot idea. Why? What if you decide that you want to use a specialized JFrame that has some other behavior that you want? What if you decide to use an application framework that provides you with the JFrame (this is typical in cases where the framework might want to track window closing events so it can save window size and location). Anyway - tons of reasons. Package your behavior into a non-GUI related class, and use that to inject behavior into the JFrame (or JPanel).

3. Consider using a JPanel instead of a JFrame. You can always add the JPanel to a JFrame if you want. IF you word directly with JFrame, what happens when you decide that you want to have two of these panels side by side in a single container?

So, I would suggest that you do something more along the lines of:

BlockAnimator animator = new BlockAnimator();
DispatchThread.invokeLater( 
  new Runnable(){
    public void run(){
      JPanel blockAnimationPanel = new JPanel();
      Block block = new Block(...);
      blockAnimationPanel.add(block);
      JFrame mainFrame = new JFrame();
      mainFrame.add(blockAnimationPanel);
      animator.start(); // note that we probably should start the thread *after* the panel is realized - but we don't really have to.
    }
  }

public class BlockAnimator extends Thread{
  private final List<Block> blocks = new CopyOnWriteArray<Block>(); // either this, or synchronize adds to the list
  public void addBlock(Block block){
    blocks.add(block);
  }
  public void run(){
    while(true){ // either put in a cancel check boolean, or mark the thread as daemon!
      DispatchThread.invokeAndWait(
        new Runnable(){
          public void run(){
            for(Block block: blocks){
              block.moveTo(....); // do whatever you have to do to move the block
            }
          }
        }
      ); // I may have missed the brace/paren count on this, but you get the idea
      spawnNewBlockObjects();
      Thread.sleep(50);
    }
  }
}

Code above hasn't been checked for accuracy, etc...

You could, theoretically, have a separate thread for spawning the new blocks, but the above is pretty straightforward. If you do decide to implement with a single background thread like I've shown a above, you can use a simple ArrayList for the blocks list because there will be no race condition on that object.

A couple of other thoughts on this:

1. In the above, the block animator can be managed independently from the blocks themselves. You could, for example, add a pause() method that would pause all blocks.

2. I have the animation update for all blocks occuring in the same dispatch thread call. Depending on the cost of the animation, it may be better to compute the new coordinates on the background thread, and only post the actual position update on the EDT. And you may choose to issue a separate invokeAndWait (or possibly use invokeLater) for each block update. It really depends on the character of what you are doing.

If the computation for where to move the block is gnarly, consider separating the computation from the actual move. So you'd have a call that would get the new Point for the object, then another call that would actually do the move.

Answer 2

Swing does not support multithreading so whenever you need to interact with it you need to do in from the AWT event thread.

This is what is happening in the main() method added by netbeans. java.awt.EventQueue.invokeLater schedules a runnable to be executed on the AWT event queue.

Normally you could do the same with your BlockSpawner Runnable but because you need to have a delay the sleep() would block the event queue and cause issues/delays with the user input.

To work around this I suggest you use a SwingWorker which allows for tasks to be executed in the background and then re-synchronised with the event queue when they are finished.

In your case you should perform the sleep() in the doInBackground() method and then create your new components in the done() method.

Answer 3

An alternative to my other answer is to use a javax.swing.Timer

This provides the ability to schedule an action to occur on the event dispatch thread at a specified rate and does not require Java 6

You could schedule your BlockSpawner with the following code:

  int timeToSpawn = 2000;

  ActionListener blockSpawner = new ActionListener() {
      public void actionPerformed(ActionEvent evt) {
          int index = Block.getRandomStartPosition();
          new Block(frame, index);
          new Block(frame, index+1);
      }
  };
  new Timer(timeToSpawn, blockSpawner).start();

This is probably the simplest solution as it does not require additional threads. Just make sure you use the Timer class in javax.swing and not java.util otherwise you may not be executing on the event dispatch thread.