JProgressBar not updating

Question

I have made a very simple code to show it here, i have a button that should show a JDialog to check the progress status, i am using the invoke late to go through EDT and my loop isn't in the run method, so why isn't my bar updating ? here is the code

import javax.swing.JButton;
import javax.swing.JDialog;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.JProgressBar;
import javax.swing.JTextField;
import javax.swing.SwingUtilities;

import java.awt.BorderLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

public class JBarEx extends JFrame {
private JTextField progStatus = new JTextField("Undefined");
private JButton dialogBtn = new JButton("Show Progression dialog");
final JDialog dlg = new JDialog((JFrame) null, "prog Title", false);
final JProgressBar dpb = new JProgressBar(0, 100);

public JBarEx() {
    JPanel pan = new JPanel();
    dialogBtn.addActionListener(new ActionListener() {

        @Override
        public void actionPerformed(ActionEvent e) {
            // TODO Auto-generated method stub
            showProgress();
        }
    });
    progStatus.setEditable(false);
    pan.add(progStatus);
    pan.add(dialogBtn);
    setContentPane(pan);
    this.setSize(200, 100);
    setVisible(true);
}

public void showProgress() {
    dlg.add(BorderLayout.CENTER, dpb);
    dlg.add(BorderLayout.NORTH, new JLabel("prog message"));
    dlg.setDefaultCloseOperation(JDialog.DISPOSE_ON_CLOSE);
    dlg.setSize(300, 75);
    dlg.setLocationRelativeTo(null);
    dlg.setVisible(true);

    for (int i = 0; i < 100; i++) {
        final int ii = i;
        try {
            Thread.sleep(25);
            SwingUtilities.invokeLater(new Runnable() {

                @Override
                public void run() {
                    updateBar(ii);

                }
            });
        }
        catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
    }

}

public void updateBar(int newValue) {
    dpb.setValue(newValue);
}

public static void main(String[] args) {
    JBarEx jbx = new JBarEx();
}

}

You are blocking the Aedt, preventing it form processing any further paint request. Until after the loop has competed. A bett solution would be to use a SwingWorker to run your task/loop in he background and fire progress updates which can be processed in the EDT. See [*this*](http://stackoverflow.com/questions/15199091/progress-bar-java/15199220#15199220) for an example — MadProgrammer, Sep 16 '13 at 20:09

score 10 · Accepted Answer · answered Sep 16 '13 at 21:52

Your showProgress method is being executed within the context of the Event Dispatching Thread. The EDT is responsible for, amongst other things, processing paint requests. This means that so long as your for-loop is executing, the EDT can not process any new paint requests (or handle the invokeLater events either) as it is blocking the EDT.

While there are any number of possible ways to solve the problem, based on your code example, the simplest would be to use a SwingWorker.

It has the capacity to allow your to execute the long running task the a background thread (freeing up the EDT), but also allows you means for publishing updates (if required) so that they can be processed in the EDT and also provides handy progress notification.

For example...

enter image description here

import java.awt.BorderLayout;
import java.awt.EventQueue;
import java.awt.GridBagConstraints;
import java.awt.GridBagLayout;
import java.awt.Insets;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.beans.PropertyChangeEvent;
import java.beans.PropertyChangeListener;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JProgressBar;
import javax.swing.SwingWorker;
import javax.swing.UIManager;
import javax.swing.UnsupportedLookAndFeelException;
import javax.swing.border.EmptyBorder;

public class SwingWorkerProgress {

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

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

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

    public class TestPane extends JPanel {

        private JProgressBar pbProgress;
        private JButton start;

        public TestPane() {

            setBorder(new EmptyBorder(10, 10, 10, 10));
            pbProgress = new JProgressBar();
            setLayout(new GridBagLayout());
            GridBagConstraints gbc = new GridBagConstraints();
            gbc.insets = new Insets(4, 4, 4, 4);
            gbc.gridx = 0;
            gbc.gridy = 0;
            add(pbProgress, gbc);

            start = new JButton("Start");
            gbc.gridy++;
            add(start, gbc);

            start.addActionListener(new ActionListener() {
                @Override
                public void actionPerformed(ActionEvent e) {
                    start.setEnabled(false);
                    ProgressWorker pw = new ProgressWorker();
                    pw.addPropertyChangeListener(new PropertyChangeListener() {

                        @Override
                        public void propertyChange(PropertyChangeEvent evt) {
                            String name = evt.getPropertyName();
                            if (name.equals("progress")) {
                                int progress = (int) evt.getNewValue();
                                pbProgress.setValue(progress);
                                repaint();
                            } else if (name.equals("state")) {
                                SwingWorker.StateValue state = (SwingWorker.StateValue) evt.getNewValue();
                                switch (state) {
                                    case DONE:
                                        start.setEnabled(true);
                                        break;
                                }
                            }
                        }

                    });
                    pw.execute();
                }
            });

        }
    }

    public class ProgressWorker extends SwingWorker<Object, Object> {

        @Override
        protected Object doInBackground() throws Exception {

            for (int i = 0; i < 100; i++) {        
                setProgress(i);
                try {
                    Thread.sleep(25);
                } catch (Exception e) {
                    e.printStackTrace();
                }
            }

            return null;
        }
    }
}

Check out Concurrency in Swing for more details

kiheru · Answer 2 · 2013-09-16T19:51:49.550

1

Even if you fix the loop as others have pointed out, you'd still block the event dispatch thread. The for loop is run in showProgress() which is called from an event listener. The updates are pushed to the event queue, but that does not get processed until the loop has completed.

Use a Swing Timer instead. Something like this:

Timer timer = new Timer(25, new ActionListener() {
    private int position;

    @Override
    public void actionPerformed(ActionEvent e) {
         position++;
         if (position < lastPosition) {
             updateBar(position);
         } else {
             ((Timer) e.getSource).stop();
         }
    }
});
timer.start();

where lastPosition would be the state where you want the progress bar to stop.

Unrelated to that bug, but a bug still, you should not create swing components outside the event dispatch thread. It's best to do it right from the start:

public static void main(String[] args) {
    SwingUtilities.invokeLater(new Runnable() {
        @Override
        public void run() {
            JBarEx jbx = new JBarEx();
        }
    });
}

edited Sep 16 '13 at 19:51

answered Sep 16 '13 at 19:43

kiheru

6,588
25
31

1

+1 for blocking the EDT, -1 for javax.swing.Timer, it would not solve the problem, as it will only trigger a callback at regular intervals. There are better solutions... – MadProgrammer Sep 16 '13 at 20:05
@MadProgrammer running at regular intervals is what the question code does. Depending on what the *actual* application does, a `SwingWorker` may of course be a better solution. – kiheru Sep 16 '13 at 20:15
SwingWorker is designed exactly for the purpose the OP has stated. It can run the task in the background and provides two means to report back to the EDT, `publish` and `setProgress`. No need to pool for updates, they are sent back automatically.. – MadProgrammer Sep 16 '13 at 21:43
I know that, and they're very useful indeed. But still they are not always the complete solution. Like when 1. the needed time/steps are not determinate, 2. the background task has no good intermediate steps or calling `publish()` would make mess of responsibilities, 3. the background task can't be relied to update the status for other reasons, like that it can block waiting a network connection. – kiheru Sep 16 '13 at 21:50
Pooling the state of some background task has all the same problems you just listed. There's no guarantee that the background task will have updated anything useful, so you're just wasting CPU cycles trying. Because `SwingWorker` is `abstract`, you are forced/encouraged to ensure proper responsibility for processing the output, if that's required, because you should be writing them to accomplish a particular task. In case "1", a progress bar isn't the best choice (unless you place in indeterminate state), then you wouldn't need the `Timer` anyway... – MadProgrammer Sep 16 '13 at 21:56

score 0 · Answer 3 · answered Sep 16 '13 at 19:35

for (int i = 0; i < 0; i++) {

You will never enter this code so will never call the updateBar(..) method

i needs to be greater than 0 in this case. If it is 1 then updateBar will be called once, if 2 then updateBar will be called twice etc

Also rather than doing

Thread.sleep(25);

take a look at java executors as these will help with your scheduling and remove the need for the sleep

JProgressBar not updating

3 Answers3

Linked

Related