Changeset 27326 in osm for applications/editors

Timestamp:

2011-12-28T13:17:21+01:00 (13 years ago)

Author:

oliverw

Message:

Bugfix #7181: FixAddresses plugin works very slow. Guessers work on smaller set now; result is changed after guess task has finished instead of updating the table continously.

Location:

applications/editors/josm/plugins/FixAddresses/src/org/openstreetmap/josm/plugins/fixAddresses

Files:

• GuessAddressRunnable.java (modified) (8 diffs)
• GuessedValueHandler.java (modified) (9 diffs)
• OSMAddress.java (modified) (3 diffs)
• gui/AddressEditTableModel.java (modified) (2 diffs)

applications/editors/josm/plugins/FixAddresses/src/org/openstreetmap/josm/plugins/fixAddresses/GuessAddressRunnable.java

@@ -22 +22 @@
 
 import org.openstreetmap.josm.Main;
-import org.openstreetmap.josm.data.coor.LatLon;
-import org.openstreetmap.josm.data.osm.Changeset;
 import org.openstreetmap.josm.data.osm.Node;
-import org.openstreetmap.josm.data.osm.OsmPrimitive;
-import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.Way;
-import org.openstreetmap.josm.data.osm.visitor.Visitor;
 import org.openstreetmap.josm.gui.PleaseWaitRunnable;
 import org.openstreetmap.josm.io.OsmTransferException;
@@ -43 +38 @@
         private List<OSMAddress> addressesToGuess;
         private List<IProgressMonitorFinishedListener> finishListeners = new ArrayList<IProgressMonitorFinishedListener>();
-        private double minDist;
-        private OSMAddress curAddressNode;
         private boolean isRunning = false;
         private boolean canceled;
 
+        private GuessedValueHandler[] wayGuessers = new GuessedValueHandler[]{new GuessStreetValueHandler(TagUtils.ADDR_STREET_TAG)}; 
+        private GuessedValueHandler[] nodeGuessers = new GuessedValueHandler[]{
+                        new GuessedValueHandler(TagUtils.ADDR_POSTCODE_TAG, 500.0),
+                        new GuessedValueHandler(TagUtils.ADDR_CITY_TAG, 5000.0),
+                        new GuessedValueHandler(TagUtils.ADDR_STATE_TAG, 5000.0),
+                        new GuessedValueHandler(TagUtils.ADDR_COUNTRY_TAG, 5000.0),
+                        new GuessedValueHandler(TagUtils.ADDR_CITY_TAG, 2000.0)
+        };
 
         /**
@@ -110 +111 @@
         protected void fireFinished() {
                 for (IProgressMonitorFinishedListener l : finishListeners) {
-                        l.finished();
+                        l.finished();                   
                 }
                 // this event is fired only once, then we disconnect all listeners
                 finishListeners.clear();
         }
-        
+
         /* (non-Javadoc)
          * @see org.openstreetmap.josm.gui.PleaseWaitRunnable#cancel()
@@ -137 +138 @@
         @Override
         protected void realRun() throws SAXException, IOException,
-                        OsmTransferException {
-                
+        OsmTransferException {
+
                 if (Main.main.getCurrentDataSet() == null || addressesToGuess == null) return;
 
@@ -150 +151 @@
                         progressMonitor.setTicksCount(addressesToGuess.size());
 
-
-
                         List<OSMAddress> shadowCopy = new ArrayList<OSMAddress>(addressesToGuess);
                         for (OSMAddress aNode : shadowCopy) {
-                                minDist = Double.MAX_VALUE;
-                                curAddressNode = aNode;
-
-                                // setup guessing handlers for address tags
-                                GuessedValueHandler[] guessers = new GuessedValueHandler[]{
-                                                new GuessStreetValueHandler(TagUtils.ADDR_STREET_TAG, aNode),
-                                                new GuessedValueHandler(TagUtils.ADDR_POSTCODE_TAG, aNode, 500.0),
-                                                new GuessedValueHandler(TagUtils.ADDR_CITY_TAG, aNode, 5000.0),
-                                                new GuessedValueHandler(TagUtils.ADDR_STATE_TAG, aNode, 5000.0),
-                                                new GuessedValueHandler(TagUtils.ADDR_COUNTRY_TAG, aNode, 5000.0),
-                                                new GuessedValueHandler(TagUtils.ADDR_CITY_TAG, aNode, 2000.0)
-                                };
-
                                 if (!aNode.needsGuess()) { // nothing to do
                                         progressMonitor.worked(1);
@@ -176 +162 @@
                                         break;
                                 }
+
                                 // Update progress monitor
                                 progressMonitor.subTask(tr("Guess values for ") + aNode);
 
-                                // visit osm data
-                                for (OsmPrimitive osmPrimitive : Main.main.getCurrentDataSet().allPrimitives()) {
-                                        if (canceled) {
-                                                break;
-                                        }
-
-                                        // guess values
-                                        for (int i = 0; i < guessers.length; i++) {
-                                                osmPrimitive.visit(guessers[i]);
-
-                                                if (guessers[i].currentValue == null && i == 0) {
-                                                        //System.err.println("Guess #" + i + " failed for " + aNode);
+                                // Run way-related guessers
+                                for (int i = 0; i < wayGuessers.length; i++) {
+                                        GuessedValueHandler guesser = wayGuessers[i];
+                                        
+                                        guesser.setAddressNode(aNode);
+
+                                        // visit osm data
+                                        for (Way way : Main.main.getCurrentDataSet().getWays()) {
+                                                if (canceled) {
+                                                        break;
                                                 }
+                                                way.visit(guesser);                                             
+                                        }
+                                        
+                                        String guessedVal = guesser.getCurrentValue();
+                                        if (guessedVal != null) {
+                                                aNode.setGuessedValue(guesser.getTag(), guessedVal, guesser.getSourceNode());
+                                        }
+                                }
+                                
+                                // Run node-related guessers
+                                for (int i = 0; i < nodeGuessers.length; i++) {
+                                        GuessedValueHandler guesser = nodeGuessers[i];
+                                        
+                                        guesser.setAddressNode(aNode);
+
+                                        // visit osm data
+                                        for (Node node : Main.main.getCurrentDataSet().getNodes()) {
+                                                if (canceled) {
+                                                        break;
+                                                }
+                                                node.visit(guesser);                                            
+                                        }
+                                        
+                                        String guessedVal = guesser.getCurrentValue();
+                                        if (guessedVal != null) {
+                                                aNode.setGuessedValue(guesser.getTag(), guessedVal, guesser.getSourceNode());
                                         }
                                 }
@@ -204 +215 @@
         }
 
+        // TODO: Put in separate file
         private class GuessStreetValueHandler extends GuessedValueHandler {
+                public GuessStreetValueHandler(String tag) {
+                        this(tag, null);
+                }
 
                 public GuessStreetValueHandler(String tag, OSMAddress aNode) {
@@ -225 +240 @@
                         if (TagUtils.isStreetSupportingHousenumbers(w)) {
                                 OSMAddress aNode = getAddressNode();
-                                double dist = OsmUtils.getMinimumDistanceToWay(aNode.getCoor(), w);
-
-                                if (dist < minDist && dist < getMaxDistance()) {
-                                        System.out.println(String.format("New guess %s: %4.2f m", TagUtils.getNameValue(w), dist));
-                                        minDist = dist;
-                                        currentValue = TagUtils.getNameValue(w);
-                                        aNode.setGuessedValue(getTag(), currentValue, w);
-                                } else {
-                                        //System.out.println(String.format("Skipped %s: %4.2f m", TagUtils.getNameValue(w), dist));
+                                String newVal = TagUtils.getNameValue(w);
+                                
+                                if (newVal != null) {
+                                        double dist = OsmUtils.getMinimumDistanceToWay(aNode.getCoor(), w);
+                                        
+                                        if (dist < minDist && dist < getMaxDistance()) {
+                                                minDist = dist;
+                                                currentValue = newVal;
+                                                srcNode = w;
+                                                //aNode.setGuessedValue(getTag(), currentValue, w);
+                                        } else {
+                                                //System.out.println(String.format("Skipped %s: %4.2f m", TagUtils.getNameValue(w), dist));
+                                        }
                                 }
                         }

applications/editors/josm/plugins/FixAddresses/src/org/openstreetmap/josm/plugins/fixAddresses/GuessedValueHandler.java

@@ -16 +16 @@
 import org.openstreetmap.josm.data.osm.Changeset;
 import org.openstreetmap.josm.data.osm.Node;
+import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.Way;
@@ -37 +38 @@
         private OSMAddress aNode;
         private double maxDist = DEFAULT_MAX_DIST;
-
+        protected OsmPrimitive srcNode;
+
+        /**
+         * Instantiates a new guessed value handler without node and default maximum distance.
+         *
+         * @param tag the tag to find the guessed value for.
+         */
+        public GuessedValueHandler(String tag) {
+                this(tag, null, DEFAULT_MAX_DIST);
+        }
+        
+        /**
+         * Instantiates a new guessed value handler without node.
+         *
+         * @param tag the tag to find the guessed value for.
+         * @param maxDist the maximum distance for a node/way to be considered as guessed value.
+         */
+        public GuessedValueHandler(String tag, double maxDist) {
+                this(tag, null, maxDist);
+        }
+        
         /**
          * Instantiates a new guessed value handler.
@@ -63 +84 @@
                 }
 
-                if (aNode == null) {
-                        throw new RuntimeException("Address node must not be null!");
-                }
-
                 if (maxDist < 1.0) { // clip value
                         maxDist = 1.0;
                 }
+                
                 this.tag = tag;
-
-                minDist = Double.MAX_VALUE;
-                this.aNode = aNode;
                 this.maxDist = maxDist;
+                setAddressNode(aNode);          
         }
 
@@ -84 +100 @@
         protected OSMAddress getAddressNode() {
                 return aNode;
+        }
+        
+
+        /**
+         * Sets the address node to make the guess for.
+         * @param aNode
+         */
+        public void setAddressNode(OSMAddress aNode) {          
+                this.aNode = aNode;
+                // reset search results
+                minDist = Double.MAX_VALUE;
+                srcNode = null;
+                currentValue = null;            
         }
 
@@ -120 +149 @@
          * @return the currentValue
          */
-        protected String getCurrentValue() {
+        public String getCurrentValue() {
                 return currentValue;
+        }
+        
+        
+        /**
+         * Gets the node/way which has been selected for the guess.
+         * @return The source node or null; if no appropriate source primitive has been found
+         */
+        public OsmPrimitive getSourceNode() {
+                return srcNode;
         }
 
@@ -138 +176 @@
         @Override
         public void visit(Node n) {
+                assert aNode != null;
+                
                 if (n.hasKey(tag)) {
                         double dist = n.getCoor().greatCircleDistance(aNode.getCoor());
@@ -143 +183 @@
                                 minDist = dist;
                                 currentValue = n.get(tag);
-                                aNode.setGuessedValue(tag, currentValue, n);
+                                srcNode = n;
                         }
                 }
@@ -153 +193 @@
         @Override
         public void visit(Way w) {
+                assert aNode != null;
+                
                 if (w.hasKey(tag)) {
                         double dist = OsmUtils.getMinimumDistanceToWay(aNode.getCoor(), w);
@@ -158 +200 @@
                                 minDist = dist;
                                 currentValue = w.get(tag);
-                                aNode.setGuessedValue(tag, currentValue, w);
+                                srcNode = w;
                         }
                 }

applications/editors/josm/plugins/FixAddresses/src/org/openstreetmap/josm/plugins/fixAddresses/OSMAddress.java

@@ -21 +21 @@
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.plugins.fixAddresses.gui.actions.AddressActions;
-import org.openstreetmap.josm.plugins.fixAddresses.gui.actions.ApplyAllGuessesAction;
 import org.openstreetmap.josm.tools.CheckParameterUtil;
 
@@ -548 +547 @@
          * @param tag the tag to set the guess for
          * @param value the value of the guessed tag.
+         * @param osm the (optional) object which was used for the guess 
          */
         public void setGuessedValue(String tag, String value, OsmPrimitive osm) {
@@ -713 +713 @@
                 AddressSolution s = new AddressSolution(
                                 String.format("%s '%s'", tr("Assign to"), getGuessedValue(tag)),
-                                new ApplyAllGuessesAction(tag),
+                                AddressActions.getApplyGuessesAction(),
                                 SolutionType.Change);
 

applications/editors/josm/plugins/FixAddresses/src/org/openstreetmap/josm/plugins/fixAddresses/gui/AddressEditTableModel.java

@@ -19 +19 @@
 
 import javax.swing.JTable;
+import javax.swing.SwingUtilities;
 import javax.swing.event.TableModelEvent;
 import javax.swing.table.DefaultTableModel;
@@ -44 +45 @@
         @Override
         public void containerChanged(AddressEditContainer container) {
-                fireTableDataChanged(); // update model
+                if (SwingUtilities.isEventDispatchThread()) {
+                        fireTableDataChanged(); // update model
+                } else {
+                        SwingUtilities.invokeLater(new Runnable() {
+                                
+                                @Override
+                                public void run() {
+                                        fireTableDataChanged(); // update model                                 
+                                }
+                        });
+                }
         }