Changeset 2871 in josm

Timestamp:

2010-01-17T16:37:24+01:00 (14 years ago)

Author:

jttt

Message:

Partially fix situation after last layer removal - most objects still stay in memory but at least there are less references and forgotten listeners

Location:

trunk/src/org/openstreetmap/josm

Files:

• actions/JosmAction.java (modified) (1 diff)
• data/osm/DataSet.java (modified) (1 diff)
• data/osm/history/HistoryDataSet.java (modified) (1 diff)
• gui/IconToggleButton.java (modified) (3 diffs)
• gui/MapFrame.java (modified) (1 diff)
• gui/MapStatus.java (modified) (5 diffs)
• gui/dialogs/HistoryDialog.java (modified) (2 diffs)
• gui/dialogs/ToggleDialog.java (modified) (2 diffs)

trunk/src/org/openstreetmap/josm/actions/JosmAction.java

@@ -94 +94 @@
         }
         MapView.removeLayerChangeListener(layerChangeAdapter);
-        if (DataSet.selListeners != null) {
-            DataSet.selListeners.remove(selectionChangeAdapter);
-        }
+        DataSet.selListeners.remove(selectionChangeAdapter);
     }
 

trunk/src/org/openstreetmap/josm/data/osm/DataSet.java

@@ -271 +271 @@
      * dataset. (However, the selection does only change in the active layer)
      */
-    public static Collection<SelectionChangedListener> selListeners =
+    public static final Collection<SelectionChangedListener> selListeners =
         Collections.synchronizedList(new LinkedList<SelectionChangedListener>());
 

trunk/src/org/openstreetmap/josm/data/osm/history/HistoryDataSet.java

@@ -1 +1 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.data.osm.history;
-
-import static org.openstreetmap.josm.tools.I18n.tr;
 
 import java.text.MessageFormat;

trunk/src/org/openstreetmap/josm/gui/IconToggleButton.java

@@ -10 +10 @@
 import javax.swing.JToggleButton;
 
+import org.openstreetmap.josm.tools.Destroyable;
+
 /**
  * Just a toggle button, with smaller border and icon only to display in
@@ -16 +18 @@
  * @author imi
  */
-public class IconToggleButton extends JToggleButton implements PropertyChangeListener {
+public class IconToggleButton extends JToggleButton implements PropertyChangeListener, Destroyable {
 
     public boolean groupbutton;
@@ -49 +51 @@
         }
     }
+
+    public void destroy() {
+        Action action = getAction();
+        if (action instanceof Destroyable) {
+            ((Destroyable) action).destroy();
+        }
+    }
 }

trunk/src/org/openstreetmap/josm/gui/MapFrame.java

@@ -187 +187 @@
         for (int i = 0; i < toolBarActions.getComponentCount(); ++i)
             if (toolBarActions.getComponent(i) instanceof Destroyable) {
-                ((Destroyable)toolBarActions).destroy();
+                ((Destroyable)toolBarActions.getComponent(i)).destroy();
             }
         for (int i = 0; i < toolBarToggle.getComponentCount(); ++i)
             if (toolBarToggle.getComponent(i) instanceof Destroyable) {
-                ((Destroyable)toolBarToggle).destroy();
+                ((Destroyable)toolBarToggle.getComponent(i)).destroy();
             }
 

trunk/src/org/openstreetmap/josm/gui/MapStatus.java

@@ -69 +69 @@
      */
     final MapView mv;
+    final Collector collector;
 
     /**
@@ -143 +144 @@
          */
         public void run() {
-            for (;;) {
-
-                MouseState ms = new MouseState();
-                synchronized (this) {
-                    try {wait();} catch (InterruptedException e) {}
-                    ms.modifiers = mouseState.modifiers;
-                    ms.mousePos = mouseState.mousePos;
-                }
-                if (parent != Main.map)
-                    return; // exit, if new parent.
-
-                // Do nothing, if required data is missing
-                if(ms.mousePos == null || mv.center == null) {
-                    continue;
-                }
-
-                // Freeze display when holding down CTRL
-                if ((ms.modifiers & MouseEvent.CTRL_DOWN_MASK) != 0) {
-                    // update the information popup's labels though, because
-                    // the selection might have changed from the outside
-                    popupUpdateLabels();
-                    continue;
-                }
-
-                // This try/catch is a hack to stop the flooding bug reports about this.
-                // The exception needed to handle with in the first place, means that this
-                // access to the data need to be restarted, if the main thread modifies
-                // the data.
-                try {
-                    // Set the text label in the bottom status bar
-                    statusBarElementUpdate(ms);
-
-                    // The popup != null check is required because a left-click
-                    // produces several events as well, which would make this
-                    // variable true. Of course we only want the popup to show
-                    // if the middle mouse button has been pressed in the first
-                    // place
-                    boolean isAtOldPosition = (oldMousePos != null
-                            && oldMousePos.equals(ms.mousePos)
-                            && popup != null);
-                    boolean middleMouseDown = (ms.modifiers & MouseEvent.BUTTON2_DOWN_MASK) != 0;
-
-                    // Popup Information
-                    // display them if the middle mouse button is pressed and
-                    // keep them until the mouse is moved
-                    if (middleMouseDown || isAtOldPosition)
-                    {
-                        Collection<OsmPrimitive> osms = mv.getAllNearest(ms.mousePos);
-
-                        if (osms == null) {
-                            continue;
+            registerListeners();
+            try {
+                for (;;) {
+
+                    MouseState ms = new MouseState();
+                    synchronized (this) {
+                        // TODO Would be better if the timeout wasn't necessary
+                        try {wait(1000);} catch (InterruptedException e) {}
+                        ms.modifiers = mouseState.modifiers;
+                        ms.mousePos = mouseState.mousePos;
+                    }
+                    if (parent != Main.map)
+                        return; // exit, if new parent.
+
+                    // Do nothing, if required data is missing
+                    if(ms.mousePos == null || mv.center == null) {
+                        continue;
+                    }
+
+                    // Freeze display when holding down CTRL
+                    if ((ms.modifiers & MouseEvent.CTRL_DOWN_MASK) != 0) {
+                        // update the information popup's labels though, because
+                        // the selection might have changed from the outside
+                        popupUpdateLabels();
+                        continue;
+                    }
+
+                    // This try/catch is a hack to stop the flooding bug reports about this.
+                    // The exception needed to handle with in the first place, means that this
+                    // access to the data need to be restarted, if the main thread modifies
+                    // the data.
+                    try {
+                        // Set the text label in the bottom status bar
+                        statusBarElementUpdate(ms);
+
+                        // The popup != null check is required because a left-click
+                        // produces several events as well, which would make this
+                        // variable true. Of course we only want the popup to show
+                        // if the middle mouse button has been pressed in the first
+                        // place
+                        boolean isAtOldPosition = (oldMousePos != null
+                                && oldMousePos.equals(ms.mousePos)
+                                && popup != null);
+                        boolean middleMouseDown = (ms.modifiers & MouseEvent.BUTTON2_DOWN_MASK) != 0;
+
+                        // Popup Information
+                        // display them if the middle mouse button is pressed and
+                        // keep them until the mouse is moved
+                        if (middleMouseDown || isAtOldPosition)
+                        {
+                            Collection<OsmPrimitive> osms = mv.getAllNearest(ms.mousePos);
+
+                            if (osms == null) {
+                                continue;
+                            }
+
+                            final JPanel c = new JPanel(new GridBagLayout());
+                            final JLabel lbl = new JLabel(
+                                    "<html>"+tr("Middle click again to cycle through.<br>"+
+                                    "Hold CTRL to select directly from this list with the mouse.<hr>")+"</html>",
+                                    null,
+                                    JLabel.HORIZONTAL
+                            );
+                            lbl.setHorizontalAlignment(JLabel.LEFT);
+                            c.add(lbl, GBC.eol().insets(2, 0, 2, 0));
+
+                            // Only cycle if the mouse has not been moved and the
+                            // middle mouse button has been pressed at least twice
+                            // (the reason for this is the popup != null check for
+                            // isAtOldPosition, see above. This is a nice side
+                            // effect though, because it does not change selection
+                            // of the first middle click)
+                            if(isAtOldPosition && middleMouseDown) {
+                                // Hand down mouse modifiers so the SHIFT mod can be
+                                // handled correctly (see funcion)
+                                popupCycleSelection(osms, ms.modifiers);
+                            }
+
+                            // These labels may need to be updated from the outside
+                            // so collect them
+                            List<JLabel> lbls = new ArrayList<JLabel>();
+                            for (final OsmPrimitive osm : osms) {
+                                JLabel l = popupBuildPrimitiveLabels(osm);
+                                lbls.add(l);
+                                c.add(l, GBC.eol().fill(GBC.HORIZONTAL).insets(2, 0, 2, 2));
+                            }
+
+                            popupShowPopup(popupCreatePopup(c, ms), lbls);
+                        } else {
+                            popupHidePopup();
                         }
 
-                        final JPanel c = new JPanel(new GridBagLayout());
-                        final JLabel lbl = new JLabel(
-                                "<html>"+tr("Middle click again to cycle through.<br>"+
-                                "Hold CTRL to select directly from this list with the mouse.<hr>")+"</html>",
-                                null,
-                                JLabel.HORIZONTAL
-                        );
-                        lbl.setHorizontalAlignment(JLabel.LEFT);
-                        c.add(lbl, GBC.eol().insets(2, 0, 2, 0));
-
-                        // Only cycle if the mouse has not been moved and the
-                        // middle mouse button has been pressed at least twice
-                        // (the reason for this is the popup != null check for
-                        // isAtOldPosition, see above. This is a nice side
-                        // effect though, because it does not change selection
-                        // of the first middle click)
-                        if(isAtOldPosition && middleMouseDown) {
-                            // Hand down mouse modifiers so the SHIFT mod can be
-                            // handled correctly (see funcion)
-                            popupCycleSelection(osms, ms.modifiers);
-                        }
-
-                        // These labels may need to be updated from the outside
-                        // so collect them
-                        List<JLabel> lbls = new ArrayList<JLabel>();
-                        for (final OsmPrimitive osm : osms) {
-                            JLabel l = popupBuildPrimitiveLabels(osm);
-                            lbls.add(l);
-                            c.add(l, GBC.eol().fill(GBC.HORIZONTAL).insets(2, 0, 2, 2));
-                        }
-
-                        popupShowPopup(popupCreatePopup(c, ms), lbls);
-                    } else {
-                        popupHidePopup();
+                        oldMousePos = ms.mousePos;
+                    } catch (ConcurrentModificationException x) {
+                        //x.printStackTrace();
+                    } catch (NullPointerException x) {
+                        //x.printStackTrace();
                     }
-
-                    oldMousePos = ms.mousePos;
-                } catch (ConcurrentModificationException x) {
-                    //x.printStackTrace();
-                } catch (NullPointerException x) {
-                    //x.printStackTrace();
-                }
+                }
+            } finally {
+                unregisterListeners();
             }
         }
@@ -496 +503 @@
     MouseState mouseState = new MouseState();
 
+    private AWTEventListener awtListener = new AWTEventListener() {
+        public void eventDispatched(AWTEvent event) {
+            if (event instanceof ComponentEvent &&
+                    ((ComponentEvent)event).getComponent() == mv) {
+                synchronized (collector) {
+                    mouseState.modifiers = ((InputEvent)event).getModifiersEx();
+                    if (event instanceof MouseEvent) {
+                        mouseState.mousePos = ((MouseEvent)event).getPoint();
+                    }
+                    collector.notify();
+                }
+            }
+        }
+    };
+
+    private MouseMotionListener mouseMotionListener = new MouseMotionListener() {
+        public void mouseMoved(MouseEvent e) {
+            synchronized (collector) {
+                mouseState.modifiers = e.getModifiersEx();
+                mouseState.mousePos = e.getPoint();
+                collector.notify();
+            }
+        }
+
+        public void mouseDragged(MouseEvent e) {
+            mouseMoved(e);
+        }
+    };
+
+    private KeyAdapter keyAdapter = new KeyAdapter() {
+        @Override public void keyPressed(KeyEvent e) {
+            synchronized (collector) {
+                mouseState.modifiers = e.getModifiersEx();
+                collector.notify();
+            }
+        }
+
+        @Override public void keyReleased(KeyEvent e) {
+            keyPressed(e);
+        }
+    };
+
+    private void registerListeners() {
+        // Listen to keyboard/mouse events for pressing/releasing alt key and
+        // inform the collector.
+        try {
+            Toolkit.getDefaultToolkit().addAWTEventListener(awtListener,
+                    AWTEvent.KEY_EVENT_MASK | AWTEvent.MOUSE_EVENT_MASK | AWTEvent.MOUSE_MOTION_EVENT_MASK);
+        } catch (SecurityException ex) {
+            mv.addMouseMotionListener(mouseMotionListener);
+            mv.addKeyListener(keyAdapter);
+        }
+    }
+
+    private void unregisterListeners() {
+        try {
+            Toolkit.getDefaultToolkit().removeAWTEventListener(awtListener);
+        } catch (SecurityException e) {
+            // Don't care, awtListener probably wasn't registered anyway
+        }
+        mv.removeMouseMotionListener(mouseMotionListener);
+        mv.removeKeyListener(keyAdapter);
+    }
+
+
     /**
      * Construct a new MapStatus and attach it to the map view.
@@ -502 +574 @@
     public MapStatus(final MapFrame mapFrame) {
         this.mv = mapFrame.mapView;
+        this.collector = new Collector(mapFrame);
 
         JumpToAction JumpToAct = new JumpToAction();
@@ -539 +612 @@
 
         // The background thread
-        final Collector collector = new Collector(mapFrame);
         thread = new Thread(collector, "Map Status Collector");
         thread.setDaemon(true);
         thread.start();
-
-        // Listen to keyboard/mouse events for pressing/releasing alt key and
-        // inform the collector.
-        try {
-            Toolkit.getDefaultToolkit().addAWTEventListener(new AWTEventListener(){
-                public void eventDispatched(AWTEvent event) {
-                    if (event instanceof ComponentEvent &&
-                            ((ComponentEvent)event).getComponent() == mapFrame.mapView) {
-                        synchronized (collector) {
-                            mouseState.modifiers = ((InputEvent)event).getModifiersEx();
-                            if (event instanceof MouseEvent) {
-                                mouseState.mousePos = ((MouseEvent)event).getPoint();
-                            }
-                            collector.notify();
-                        }
-                    }
-                }
-            }, AWTEvent.KEY_EVENT_MASK | AWTEvent.MOUSE_EVENT_MASK | AWTEvent.MOUSE_MOTION_EVENT_MASK);
-        } catch (SecurityException ex) {
-            mapFrame.mapView.addMouseMotionListener(new MouseMotionListener() {
-                public void mouseMoved(MouseEvent e) {
-                    synchronized (collector) {
-                        mouseState.modifiers = e.getModifiersEx();
-                        mouseState.mousePos = e.getPoint();
-                        collector.notify();
-                    }
-                }
-
-                public void mouseDragged(MouseEvent e) {
-                    mouseMoved(e);
-                }
-            });
-
-            mapFrame.mapView.addKeyListener(new KeyAdapter() {
-                @Override public void keyPressed(KeyEvent e) {
-                    synchronized (collector) {
-                        mouseState.modifiers = e.getModifiersEx();
-                        collector.notify();
-                    }
-                }
-
-                @Override public void keyReleased(KeyEvent e) {
-                    keyPressed(e);
-                }
-            });
-        }
     }
 

trunk/src/org/openstreetmap/josm/gui/dialogs/HistoryDialog.java

@@ -64 +64 @@
     protected ShowHistoryAction showHistoryAction;
     protected ReloadAction reloadAction;
+
+    public HistoryDialog() {
+        super(tr("History"), "history", tr("Display the history of all selected items."),
+                Shortcut.registerShortcut("subwindow:history", tr("Toggle: {0}", tr("History")), KeyEvent.VK_H,
+                        Shortcut.GROUP_LAYER, Shortcut.SHIFT_DEFAULT), 150);
+        build();
+        DataSet.selListeners.add(model);
+    }
 
     /**
@@ -128 +136 @@
     }
 
-    public HistoryDialog() {
-        super(tr("History"), "history", tr("Display the history of all selected items."),
-                Shortcut.registerShortcut("subwindow:history", tr("Toggle: {0}", tr("History")), KeyEvent.VK_H,
-                        Shortcut.GROUP_LAYER, Shortcut.SHIFT_DEFAULT), 150);
-        build();
-        DataSet.selListeners.add(model);
+    @Override
+    public void showNotify() {
         HistoryDataSet.getInstance().addHistoryDataSetListener(this);
+    }
+
+    @Override
+    public void hideNotify() {
+        HistoryDataSet.getInstance().removeHistoryDataSetListener(this);
     }
 

trunk/src/org/openstreetmap/josm/gui/dialogs/ToggleDialog.java

@@ -106 +106 @@
         setMinimumSize(new Dimension(0,0));
         this.preferredHeight = preferredHeight;
-        toggleAction = new ToggleDialogAction(name, "dialogs/"+iconName, tooltip, shortcut, iconName);
+        toggleAction = new ToggleDialogAction(this, name, "dialogs/"+iconName, tooltip, shortcut, iconName);
         String helpId = "Dialog/"+getClass().getName().substring(getClass().getName().lastIndexOf('.')+1);
         toggleAction.putValue("help", helpId.substring(0, helpId.length()-6));
@@ -134 +134 @@
      *
      */
-    public final class ToggleDialogAction extends JosmAction {
-        private ToggleDialogAction(String name, String iconName, String tooltip, Shortcut shortcut, String prefname) {
+    public final static class ToggleDialogAction extends JosmAction {
+
+        private ToggleDialog dialog;
+
+        private ToggleDialogAction(ToggleDialog dialog, String name, String iconName, String tooltip, Shortcut shortcut, String prefname) {
             super(name, iconName, tooltip, shortcut, false);
+            this.dialog = dialog;
         }
 
         public void actionPerformed(ActionEvent e) {
-            toggleButtonHook();
-            if (isShowing) {
-                hideDialog();
-                dialogsPanel.reconstruct(Action.ELEMENT_SHRINKS, null);
+            dialog.toggleButtonHook();
+            if (dialog.isShowing) {
+                dialog.hideDialog();
+                dialog.dialogsPanel.reconstruct(Action.ELEMENT_SHRINKS, null);
             } else {
-                showDialog();
-                if (isDocked && isCollapsed) {
-                    expand();
+                dialog.showDialog();
+                if (dialog.isDocked && dialog.isCollapsed) {
+                    dialog.expand();
                 }
-                if (isDocked) {
-                    dialogsPanel.reconstruct(Action.INVISIBLE_TO_DEFAULT, ToggleDialog.this);
+                if (dialog.isDocked) {
+                    dialog.dialogsPanel.reconstruct(Action.INVISIBLE_TO_DEFAULT, dialog);
                 }
             }
+        }
+
+        @Override
+        public void destroy() {
+            super.destroy();
+            dialog = null;
         }
     }