Skip to main content

layer added twice

8 replies [Last post]
dingfelder
Offline
Joined: 2010-09-12
Points: 0

I am seeing a wierd behavior where a dialog I am putting in a layer gets shown in 2 places on the screen.

I have made an example to demonstrate it.

Please have patience as it is not the smallest example, I have tried to make it as tidy as possible,

There are 4 classes:
LayerTest
ProgressPanel
ProgressMgr
TestMenu

The main class: LayerTest crerates the UI
The ProgressPanel is a JPanel that mimmics a progress bar
the ProgressMonitor class is a singleton that controls whether the progress bar is shown on top of the app or not
The menu is just used to turn on the progress bar

Here is what I am seeing:

If the LayerTest code calls the progress bar, and you manually close it, then the app behaves nicely from then on.

If you comment out that call however, and then show the progress bar via the menu, the progress bar is shown in the right place in the center of the app, and also a partial image of it is also shown at point 0,0 of the panel.

Once you close the menu and re-display it from the menu, the partial image doesnt get displayed from then on.

Can someone give me a clue what is going on here?

//////////////////////////////////////////////////////////////////////////
//
// UI class
//
//////////////////////////////////////////////////////////////////////////

package test;

import javax.swing.JComponent;
import javax.swing.JFrame;
import javax.swing.JPanel;

import org.jdesktop.jxlayer.JXLayer;

public class LayerTest extends JFrame {

private static final long serialVersionUID = 1L;
private JXLayer progressLayer = null;

public LayerTest() {
super("Test");

final JPanel content = new JPanel();// getContentPane();
setSize(800, 600);

try {
progressLayer = ProgressMgr.getInstance().getProgressLayer(content, "title",
"message", "section");
this.add(progressLayer);

// if this is uncommented, the problem goes away !
// ProgressMgr.getInstance().showProgressBar();

this.setJMenuBar(new TestMenu());
} catch (final Exception e) {
e.printStackTrace();
}

setVisible(true);

// what happens when user closes the JFrame.
this.setDefaultCloseOperation ( JFrame. DO_NOTHING_ON_CLOSE );

}

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

//////////////////////////////////////////////////////////////////////////
//
// ProgressMgr
//
//////////////////////////////////////////////////////////////////////////

package test;

import java.awt.Component;
import java.awt.Cursor;
import java.awt.Dimension;
import java.awt.GridBagConstraints;
import java.awt.GridBagLayout;
import java.awt.Insets;

import javax.swing.JComponent;
import javax.swing.JPanel;

import org.jdesktop.jxlayer.JXLayer;
import org.jdesktop.jxlayer.plaf.ext.LockableUI;

public final class ProgressMgr {

/** The _ instance. */
protected static ProgressMgr _INSTANCE = null;

/** The progress panel. */
final ProgressPanel progressPanel = new ProgressPanel("", "", "", true);

/** The is running. */
boolean isRunning = false;

/** The progress layer. */
private JXLayer progressLayer = null;
private LockableUI lockableUI = null;

/**
* The Constructor.
*/
private ProgressMgr() {

}

/**
* Gets the instance.
*
* @return the instance
*/

public static ProgressMgr getInstance() {
if (ProgressMgr._INSTANCE == null) {// don't want to block here
// two or more threads might be here!!!
synchronized (ProgressMgr.class) {
// must check again as one of the
// blocked threads can still enter
if (ProgressMgr._INSTANCE == null) {
ProgressMgr._INSTANCE = new ProgressMgr();
}
}

}
return ProgressMgr._INSTANCE;
}

/**
* Gets the progress layer.
*
* @return the progress layer
*
* @throws Exception
* the exception
*/
public JXLayer getProgressLayer() throws Exception {
getInstance();
if (getInstance().progressLayer == null) {
throw new Exception(
"Initialization Error. Progress Layer must be initialized first");
} else {
getInstance();
return getInstance().progressLayer;
}
}

/**
* Gets the progress layer.
*
* @param p
* the p
* @param title
* the title
* @param section
* the section
* @param msg
* the msg
*
* @return the progress layer
*
* @throws Exception
* the exception
*/
public JXLayer getProgressLayer(JPanel p, String title,
String section, String msg) throws Exception {
setProgressLayer(p);
// this.progressPanel.updateUI();
getInstance().progressPanel.setTitle(title);
getInstance().progressPanel.setMessage(msg);
getInstance().progressPanel.setSection(section);

return getInstance().progressLayer;
}

/**
* Sets the progress layer.
*
* @param p
* the new progress layer
*
* @throws Exception
* the exception
*/
private static void setProgressLayer(JPanel p) throws Exception {
if (p == null) {
throw new Exception("Initialization Error. the panel is null");
}

getInstance().initProgressLayer(p);
}

public static void setMinSize(Component c, int w, int h) {
c.setMinimumSize(new Dimension(w, h));
c.setPreferredSize(new Dimension(w, h));
c.setSize(new Dimension(w, h));
// dont set max
}

/**
* Inits the progress layer.
*
* @param p
* the p
*
* @throws Exception
* the exception
*/
private void initProgressLayer(JPanel p) throws Exception {
lockableUI = new LockableUI();
lockableUI.setLockedCursor(Cursor.getPredefinedCursor(Cursor.MOVE_CURSOR));
lockableUI.setLocked(true);

int width = p.getWidth();
int height = p.getHeight();

// System.out.println("setting progress layer width=" + width + ", height=" + height );

progressLayer = new JXLayer(p, lockableUI);
setMinSize(progressLayer, width, height);
progressLayer.getGlassPane().setLayout(new GridBagLayout());

lockableUI.setLockedCursor(Cursor.getPredefinedCursor(Cursor.MOVE_CURSOR));

hideProgressBar();
}

/**
* Cancel.
*/
public void cancel() {
hideProgressBar();
}

/**
* Hide progress bar.
*/
public static void hideProgressBar() {
System.out.println("hide requested");
getInstance().lockableUI.setLocked(false);
getInstance().isRunning = false;
getInstance().progressLayer.getGlassPane().removeAll();
}

/**
* Show progress bar.
*/
public static void showProgressBar() {
System.out.println("show requested");
// getInstance().progressLayer.getGlassPane().removeAll();
getInstance().lockableUI.setLocked(true);
getInstance().isRunning = true;
getInstance().progressLayer.getGlassPane().add(
getInstance().progressPanel,
new GridBagConstraints(0, 0, 1, 1, 1.0, 1.0,
GridBagConstraints.CENTER, GridBagConstraints.NONE,
new Insets(0, 0, 0, 0), 0, 0));
}

/**
* Gets the title.
*
* @return the title
*/
public String getTitle() {
return progressPanel.getTitle();
}

/**
* Sets the title.
*
* @param s
* the s
*/
public void setTitle(String s) {
progressPanel.setTitle(s);
}

/**
* Gets the section message.
*
* @return the message
*/
public String getSectionMessage() {
return progressPanel.getSection();
}

/**
* Sets the section message.
*
* @param s
* the s
*/
public void setSectionMessage(String s) {
progressPanel.setSection(s);
}

/**
* Gets the note message.
*
* @return the message
*/
public String getNoteMessage() {
return progressPanel.getMessage();
}

/**
* Sets the note message.
*
* @param s
* the s
*/
public void setNoteMessage(String s) {
progressPanel.setMessage(s);
}

/**
* Checks if is running.
*
* @return the isRunning
*/
public boolean isRunning() {
return isRunning;
}

/**
* Sets the is running.
*
* @param b
* the b
*/
private void setIsRunning(boolean b) {
isRunning = b;
}

/**
* Gets the progress bar title.
*
* @return the message
*/
public static String getProgressBarTitle() {
return getInstance().getTitle();
}

/**
* Sets the progress bar title.
*
* @param s
* the s
*/
public static void setProgressBarTitle(String s) {
getInstance().setTitle(s);
}

/**
* Gets the progress bar section message.
*
* @return the message
*/
public static String getProgressBarSectionMessage() {
return getInstance().getSectionMessage();
}

/**
* Sets the progress bar section message.
*
* @param s
* the s
*/
public static void setProgressBarSectionMessage(String s) {
getInstance().setSectionMessage(s);
}

/**
* Gets the progress bar note message.
*
* @return the message
*/
public static String getProgressBarNoteMessage() {
return getInstance().getNoteMessage();
}

/**
* Sets the progress bar note message.
*
* @param s
* the s
*/
public static void setProgressBarNoteMessage(String s) {
getInstance().setNoteMessage(s);
}

/**
* Checks if is progress bar running.
*
* @return the isRunning
*/
public static boolean isProgressBarRunning() {
return getInstance().isRunning();
}

/**
* Sets the progress bar is running.
*
* @param b
* the b
*/
public static void setProgressBarIsRunning(boolean b) {
getInstance().setIsRunning(b);
}

}

//////////////////////////////////////////////////////////////////////////
//
// ProgressPanel
//
//////////////////////////////////////////////////////////////////////////

package test;

import java.awt.Color;
import java.awt.Component;
import java.awt.Dimension;
import java.awt.Font;
import java.awt.GridBagConstraints;
import java.awt.GridBagLayout;
import java.awt.Insets;
import java.awt.event.ActionListener;

import javax.swing.JButton;
import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.SwingUtilities;
import javax.swing.border.LineBorder;

/**
* The Class ProgressPanel.
*
* @author DingfelderA
*/
public class ProgressPanel extends JPanel {

/** The Constant serialVersionUID. */
private static final long serialVersionUID = 1;

/** The j label title. */
private final JLabel jLabelTitle = new JLabel("title");

/** The j label section. */
private final JLabel jLabelSection = new JLabel("section");

/** The j label message. */
private final JLabel jLabelMessage = new JLabel("message");

/** The j button cancel. */
private JButton jButtonCancel = null;

/** The show cancel. */
private boolean showCancel = true;

/** The title panel. */
private final JPanel titlePanel = new JPanel();

/**
* Instantiates a new progress panel.
*
* @param t
* the t
* @param s
* the s
* @param m
* the m
* @param b
* the b
*/
public ProgressPanel(String t, String s, String m, boolean b) {
setTitleText(t);
setMessageText(m);
setSectionText(s);
showCancel = b;
init();

}

public static void setMinSize(Component c, int w, int h) {
c.setMinimumSize(new Dimension(w, h));
c.setPreferredSize(new Dimension(w, h));
c.setSize(new Dimension(w, h));
// dont set max
}

/**
* Inits the.
*/
public void init() {
setMinSize(this, 300, 150);
this.setBackground(Color.white);
setLayout(new GridBagLayout());
setBorder(new LineBorder(Color.black));

final Font titlefont = new Font(Font.SANS_SERIF, Font.BOLD, 13);

setMinSize(titlePanel, 300, 20);
titlePanel.setLayout(new GridBagLayout());
jLabelTitle.setFont(titlefont);
// jLabelTitle.setForeground(Constants.FONT_TITLE_COLOR);

titlePanel.add(jLabelTitle, new GridBagConstraints(0, 0, 1, 1, 1.0,
1.0, GridBagConstraints.CENTER, GridBagConstraints.HORIZONTAL,
new Insets(0, 10, 0, 0), 0, 0));

this.add(titlePanel, new GridBagConstraints(0, 0, 1, 1, 1.0, 1.0,
GridBagConstraints.NORTH, GridBagConstraints.NONE, new Insets(
0, 0, 0, 0), 0, 0));
this.add(jLabelSection, new GridBagConstraints(0, 1, 1, 1, 1.0, 1.0,
GridBagConstraints.CENTER, GridBagConstraints.BOTH, new Insets(
5, 10, 0, 5), 0, 0));
this.add(jLabelMessage, new GridBagConstraints(0, 2, 1, 1, 1.0, 1.0,
GridBagConstraints.CENTER, GridBagConstraints.BOTH, new Insets(
5, 10, 0, 5), 0, 0));

if (showCancel) {
jButtonCancel = new JButton("Cancel");
this.add(jButtonCancel, new GridBagConstraints(0, 3, 1, 1, 1.0,
1.0, GridBagConstraints.CENTER, GridBagConstraints.NONE,
new Insets(0, 0, 0, 0), 0, 0));
jButtonCancel
.addActionListener(new ProgressPanel_jButtonCancel_actionAdapter(
this));
}

}

/**
* Cancel.
*/
public void cancel() {

}

/**
* Gets the title.
*
* @return the Title
*/
public String getTitle() {
return jLabelTitle.getText();
}

/**
* Sets the title.
*
* @param s
* the s
*/
public void setTitle(final String s) {
SwingUtilities.invokeLater(new Runnable() {
public void run() {
setTitleText(s);
}
});
}

/**
* Sets the title text.
*
* @param s
* the new title text
*/
protected void setTitleText(String s) {
jLabelTitle.setText(s);
}

/**
* Gets the section.
*
* @return the Section
*/
public String getSection() {
return jLabelSection.getText();
}

/**
* Sets the section text.
*
* @param s
* the s
*/
protected void setSectionText(String s) {
jLabelSection.setText(s);
}

/**
* Sets the section.
*
* @param s
* the s
*/
public void setSection(final String s) {
SwingUtilities.invokeLater(new Runnable() {
public void run() {
setSectionText(s);
}
});
}

/**
* Gets the message.
*
* @return the Message
*/
public String getMessage() {
return jLabelMessage.getText();
}

/**
* Sets the message.
*
* @param s
* the s
*/
public void setMessage(final String s) {
SwingUtilities.invokeLater(new Runnable() {
public void run() {
setMessageText(s);
}
});
}

/**
* Sets the message text.
*
* @param s
* the new message text
*/
protected void setMessageText(String s) {
jLabelMessage.setText(s);
}

/**
* J button cancel_action performed.
*
* @param e
* the e
*/
public void jButtonCancel_actionPerformed(java.awt.event.ActionEvent e) {
try {
ProgressMgr.getInstance().cancel();
} catch (final Exception e1) {
e1.printStackTrace();
System.out.println("error cancelling\n" + e1.getMessage());
}
}

/**
* The Class ProgressPanel_jButtonCancel_actionAdapter.
*/
class ProgressPanel_jButtonCancel_actionAdapter implements ActionListener {

/** The adaptee. */
private final ProgressPanel adaptee;

/**
* The Constructor.
*
* @param adaptee
* the adaptee
*/
ProgressPanel_jButtonCancel_actionAdapter(ProgressPanel adaptee) {
this.adaptee = adaptee;
}

/*
* (non-Javadoc)
*
* @see
* java.awt.event.ActionListener#actionPerformed(java.awt.event.ActionEvent
* )
*/
public void actionPerformed(java.awt.event.ActionEvent e) {
adaptee.jButtonCancel_actionPerformed(e);
}
}

}

//////////////////////////////////////////////////////////////////////////
//
// menu
//
//////////////////////////////////////////////////////////////////////////

package test;

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

import javax.swing.JMenu;
import javax.swing.JMenuBar;
import javax.swing.JMenuItem;

/**
* The Class GenericMenu.
*/
public class TestMenu extends JMenuBar {

/** The Constant serialVersionUID. */
protected static final long serialVersionUID = 1;

/** The j menu file. */
JMenu jMenuFile = new JMenu("file");
JMenuItem jMenuTest = new JMenuItem("test progress bar", null);
public TestMenu() throws Exception {
super();

this.add(jMenuFile);
jMenuFile.add(jMenuTest);

jMenuTest.addActionListener(new MenuTest_ActionAdapter(this));

}

private void showProgressBar() {
ProgressMgr.showProgressBar();
}

/**
* The Class MenuFileBenchmark_ActionAdapter.
*/
class MenuTest_ActionAdapter implements ActionListener {

/** The adaptee. */
TestMenu adaptee;

MenuTest_ActionAdapter(TestMenu adaptee) {
this.adaptee = adaptee;
}
public void actionPerformed(ActionEvent actionEvent) {
showProgressBar();
}
}

} // end of menu code

Reply viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.
alexfromsun
Offline
Joined: 2005-09-05
Points: 0

Hello Dingfelder

Piet is absolutely right, here is the quote from the javadoc for Container.add() mehtod:

[code]
* Note: If a component has been added to a container that
* has been displayed, validate must be
* called on that container to display the new component.
[/code]
In Swing we usually call revalidate() rather than validate(),
so you should add

[code]
getInstance().progressLayer.getGlassPane().revalidate();
[/code]
at the end of the showProgressBar() method

and please create Swing GUI on the Event Dispatch Thread:

[code]
public static void main(String[] args) {
SwingUtilities.invokeLater(new Runnable() {
public void run() {
new LayerTest();
}
});
}
[/code]

Thanks
alexp

pietblok
Offline
Joined: 2003-07-17
Points: 0

Hi Dingfelder,

Something else struck me in your code, because until quite recently I did the same: the way you create a singleton.

field = new Object();

What I thought would happen is the following:

1) Obtain memory for the new object
2) Initialize the new object
3) Set a pointer to the new object in field

But this is not necessarily the case. It might well be:

1) Obtain memory for the new object
2) Set a pointer to the new object in field
3) Initialize the new object.

The (unwanted) result may be that a second thread thinks that an object has been created while it has not yet been initialized.

No, I did not discover this myself. The FindBugs plugin for Eclipse warned me for this construct. (I explain it here in my own words, as I understood the problem).

From then, I coded the construct differently:

[code]
public class Demo {

private static Demo singleton;

/**
* Not like this
*/
public static Demo getInstanceWrong() {
if (singleton == null) {
synchronized (ProgressMgr.class) {
if (singleton == null) {
singleton = new Demo();
}
}
}
return singleton;
}

/**
* But like this
*/
public static Demo getInstanceOK() {
if (singleton == null) {
synchronized (ProgressMgr.class) {
if (singleton == null) {
Demo demo = new Demo();
// Postpone setting singleton until singleton is really
// initialized, but not sure if it is necessary to
// have it deferred by a dummy if condition.
if (!demo.equals(null)) {
singleton = demo;
}
}
}
}
return singleton;
}
}
[/code]

The documentation in FindBugs warned that there is no way to get a construct like this right. Always synchronize without checking for null. I have confidence in my solution, but maybe one should adhere to what the gurus say.

Secondly: why do you create a singleton of your LockableUI? A LockableUI can never be shared by multiple JXLayers, because it inherits from
org.jdesktop.jxlayer.plaf.AbstractBufferedLayerUI
(see the documentation) so you are probably preparing a mess if you want to use your ProgressMgr in a more general way than just create a demo.

Hope this helps

Piet

dingfelder
Offline
Joined: 2010-09-12
Points: 0

thanks for the tips. I am always trying to improve my code to those comments are great to see

RE the singleton:

I can re-evaluate the process depending on the outcome of this discussion but my thought up to now is:
1. there are about 10 GUI screens that each speak to different classes that have different long running tasks
2. the theory was that this singleton would just keep track of the layer and display the progress bar (and control it's updating as needed) as different threads communicate what they are doing.
3. Rather than have each screen or thread send messages to the layer themselves, just the singleton progress mgr would have access to it.

If you think I will have problems with this, I am open to suggestions.

Cheers,

Andy

pietblok
Offline
Joined: 2003-07-17
Points: 0

Hi Andy,

When looking more carefully at your code, I see that a new LockableUI is instantiated before setting it. I think that I got confused by all the getInstance() invocations for the monitor and assuming it also had a singleton LockableUI.

So: no objections.

Piet

dingfelder
Offline
Joined: 2010-09-12
Points: 0

Ok, good to hear :)

I have a different question now. This code seems to work file as the progress screen on my application but someone asked me if I can modify it slight so the panel in the center of the screen is dragable instead of locked in position.

Is this possible? and if so, are there any example of how to do this?

Cheers,

Andy

pietblok
Offline
Joined: 2003-07-17
Points: 0

Hi Andy,

Just for the fun of it I tried. And yes, it's quite easy. I tested it on my version of your demo (where I removed all singletons and getInstance() methods, hope you don't mind) so maybe you have to alter something.

I have no experience with GridBagLayout, so I tried not to touch that too much.

Firstly, I changed the layout manager for the glass pane to a BorderLayout.

Then I changed the method showProgressBar as follows:

[code]
/**
* Show progress bar.
*/
public void showProgressBar() {
System.out.println("show requested");
lockableUI.setLocked(true);
JComponent glassPane = progressLayer.getGlassPane();
final JViewport viewport = new JViewport();
glassPane.add(viewport, BorderLayout.CENTER);
JPanel view = new JPanel(new GridBagLayout());
view.setPreferredSize(new Dimension(glassPane.getWidth() * 2, glassPane
.getHeight() * 2));
view.add(progressPanel, new GridBagConstraints(0, 0, 1, 1, 1.0, 1.0,
GridBagConstraints.CENTER, GridBagConstraints.NONE, new Insets(
0, 0, 0, 0), 0, 0));
viewport.setView(view);
viewport.setViewPosition(new Point(glassPane.getWidth() / 2, glassPane
.getHeight() / 2));

MouseAdapter mouseAdapter = new MouseAdapter() {

Point oldPoint = null;

@Override
public void mousePressed(MouseEvent event) {
oldPoint = event.getPoint();
}

@Override
public void mouseReleased(MouseEvent event) {
scroll(event);
oldPoint = null;
}

@Override
public void mouseDragged(MouseEvent event) {
scroll(event);
}

private void scroll(MouseEvent event) {
if (oldPoint != null) {
Point newPoint = event.getPoint();
Point currentViewPosition = viewport.getViewPosition();
Point newViewPosition = new Point(currentViewPosition.x
+ oldPoint.x - newPoint.x, currentViewPosition.y
+ oldPoint.y - newPoint.y);
viewport.setViewPosition(newViewPosition);
oldPoint = newPoint;
}
}
};
viewport.addMouseMotionListener(mouseAdapter);
viewport.addMouseListener(mouseAdapter);
glassPane.revalidate();
}
[/code]

You may notice that I don't invoke removeAll() on the glass pane, because, when showing, the glass pane is already empty.

You may also note that mouse dragging works on the total surface, not on the bar only. That was easier to implement and also it has the advantage that when the bar has completely scrolled out, it can be scrolled in again.

I guess there are more ways to accomplish it, but I thought this to be fun to implement.

Piet

pietblok
Offline
Joined: 2003-07-17
Points: 0

Hi Andy,

One more thing. What I wrote in an earlier reply about double checking (to obtain a singleton) is still true, But my solution is unsafe as well.

Just a few days after I posted my answer, I read a thread on one of the sun forums, discussing a similar problem. I could not resist to ask advice for my circumvention of the problem. The answer: that will also fail.

http://forums.sun.com/thread.jspa?messageID=10453813

The following link explains why double checking may fail.

http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html

Sorry for misleading you with my unsafe homegrown solution, but you should really change your construct.

Piet

pietblok
Offline
Joined: 2003-07-17
Points: 0

Hi,

After you add your progress component on the glass pane, you should invoke revalidate() on that same glass pane.

Hope this helps,

Piet