Index: trunk/src/org/openstreetmap/josm/actions/CombineWayAction.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/CombineWayAction.java (revision 3229) +++ trunk/src/org/openstreetmap/josm/actions/CombineWayAction.java (revision 3230) @@ -103,10 +103,10 @@ } - public void combineWays(Collection ways) { + public Way combineWays(Collection ways) { // prepare and clean the list of ways to combine // if (ways == null || ways.isEmpty()) - return; + return null; ways.remove(null); // just in case - remove all null ways from the collection ways = new HashSet(ways); // remove duplicates @@ -119,5 +119,5 @@ if (path == null) { warnCombiningImpossible(); - return; + return null; } // check whether any ways have been reversed in the process @@ -142,5 +142,5 @@ } if ((reversedWays != null) && !reversedWays.isEmpty()) { - if (!confirmChangeDirectionOfWays()) return; + if (!confirmChangeDirectionOfWays()) return null; // filter out ways that have no direction-dependent tags unreversedWays = ReverseWayTagCorrector.irreversibleWays(unreversedWays); @@ -167,5 +167,5 @@ } catch(UserCancelException ex) { - return; + return null; } } @@ -207,5 +207,5 @@ dialog.setVisible(true); if (dialog.isCancelled()) - return; + return null; } @@ -218,18 +218,7 @@ cmds.add(new DeleteCommand(deletedWays)); final SequenceCommand sequenceCommand = new SequenceCommand(tr("Combine {0} ways", ways.size()), cmds); - - // update gui - final Way selectedWay = targetWay; - Runnable guiTask = new Runnable() { - public void run() { - Main.main.undoRedo.add(sequenceCommand); - getCurrentDataSet().setSelected(selectedWay); - } - }; - if (SwingUtilities.isEventDispatchThread()) { - guiTask.run(); - } else { - SwingUtilities.invokeLater(guiTask); - } + Main.main.undoRedo.add(sequenceCommand); + + return targetWay; } @@ -248,5 +237,19 @@ return; } - combineWays(selectedWays); + // combine and update gui + final Way selectedWay = combineWays(selectedWays); + if(selectedWay != null) + { + Runnable guiTask = new Runnable() { + public void run() { + getCurrentDataSet().setSelected(selectedWay); + } + }; + if (SwingUtilities.isEventDispatchThread()) { + guiTask.run(); + } else { + SwingUtilities.invokeLater(guiTask); + } + } } Index: trunk/src/org/openstreetmap/josm/actions/JoinAreasAction.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/JoinAreasAction.java (revision 3229) +++ trunk/src/org/openstreetmap/josm/actions/JoinAreasAction.java (revision 3230) @@ -184,7 +184,12 @@ boolean hadChanges = false; if(!same) { + int i = 0; if(checkForTagConflicts(a, b)) return true; // User aborted, so don't warn again - hadChanges = joinAreas(a, a); - hadChanges = joinAreas(b, b) || hadChanges; + if(joinAreas(a, a)) + ++i; + if(joinAreas(b, b)) + ++i; + hadChanges = i > 0; + cmdsCount = i; } @@ -212,5 +217,5 @@ // Fix Multipolygons if there are any - Collection newInnerWays = fixMultigons(innerWays, outerWay); + Collection newInnerWays = fixMultipolygons(innerWays, outerWay, same); // Delete the remaining inner ways @@ -251,4 +256,5 @@ // FIXME: This is mostly copied and pasted from CombineWayAction.java and one day should be moved into tools + // We have TagCollection handling for that now - use it here as well Map> props = new TreeMap>(); for (Way w : ways) { @@ -274,6 +280,6 @@ if("created_by".equals(e.getKey())) { - ax.put("created_by", "JOSM"); - bx.put("created_by", "JOSM"); + ax.remove("created_by"); + bx.remove("created_by"); } else { JComboBox c = new JComboBox(e.getValue().toArray()); @@ -409,4 +415,6 @@ */ private void addNodesToWay(Way a, ArrayList nodes) { + if(nodes.size() == 0) + return; Way ax=new Way(a); Collections.sort(nodes); @@ -640,11 +648,7 @@ a = b; } - Main.main.getCurrentDataSet().setSelected(ways); - // TODO: It might be possible that a confirmation dialog is presented even after reversing (for - // "strange" ways). If the user cancels this, makeCommitsOneAction will wrongly consume a previous - // action. Make CombineWayAction either silent or expose its combining capabilities. - new CombineWayAction().actionPerformed(null); - cmdsCount++; - return (Way)(Main.main.getCurrentDataSet().getSelectedWays().toArray())[0]; + if((a = new CombineWayAction().combineWays(ways)) != null) + cmdsCount++; + return a; } @@ -656,5 +660,5 @@ * @return ArrayList The List of newly created inner ways */ - private ArrayList fixMultigons(Collection uninterestingWays, Way outerWay) { + private ArrayList fixMultipolygons(Collection uninterestingWays, Way outerWay, boolean selfintersect) { Collection innerNodes = getNodesFromWays(uninterestingWays); Collection outerNodes = outerWay.getNodes(); @@ -672,7 +676,8 @@ for(Node n : w.getNodes()) { if(outerNodes.contains(n)) { - continue wayIterator; - } - if(!hasInnerNodes && innerNodes.contains(n)) { + if(!selfintersect) // allow outer point for self intersection + continue wayIterator; + } + else if(!hasInnerNodes && innerNodes.contains(n)) { hasInnerNodes = true; } @@ -686,40 +691,70 @@ // This removes unnecessary ways that might have been added. removeAlmostAlikeWays(possibleWays); - removePartlyUnconnectedWays(possibleWays); - - // Join all ways that have one start/ending node in common - Way joined = null; - outerIterator: do { - joined = null; - for(Way w1 : possibleWays) { - if(w1.isClosed()) { - if(!wayIsCollapsed(w1)) { - uninterestingWays.remove(w1); - newInnerWays.add(w1); + + // loop twice + // in k == 0 prefer ways which allow no Y-joining (i.e. which have only 1 solution) + for(int k = 0; k < 2; ++k) + { + // Join all ways that have one start/ending node in common + Way joined = null; + outerIterator: do { + removePartlyUnconnectedWays(possibleWays); + joined = null; + for(Way w1 : possibleWays) { + if(w1.isClosed()) { + if(!wayIsCollapsed(w1)) { + uninterestingWays.remove(w1); + newInnerWays.add(w1); + } + joined = w1; + possibleWays.remove(w1); + continue outerIterator; } - joined = w1; - possibleWays.remove(w1); - continue outerIterator; - } - for(Way w2 : possibleWays) { - // w2 cannot be closed, otherwise it would have been removed above - if(!waysCanBeCombined(w1, w2)) { - continue; + ArrayList secondary = new ArrayList(); + for(Way w2 : possibleWays) { + int i = 0; + // w2 cannot be closed, otherwise it would have been removed above + if(w1.equals(w2)) + continue; + if(w2.isFirstLastNode(w1.firstNode())) + ++i; + if(w2.isFirstLastNode(w1.lastNode())) + ++i; + if(i == 2) // this way closes w1 - take it! + { + if(secondary.size() > 0) + secondary.clear(); + secondary.add(w2); + break; + } + else if(i > 0) + secondary.add(w2); } - - ArrayList joinThem = new ArrayList(); - joinThem.add(w1); - joinThem.add(w2); - uninterestingWays.removeAll(joinThem); - possibleWays.removeAll(joinThem); - - // Although we joined the ways, we cannot simply assume that they are closed - joined = joinWays(joinThem); - uninterestingWays.add(joined); - possibleWays.add(joined); - continue outerIterator; - } - } - } while(joined != null); + if(k == 0 ? secondary.size() == 1 : secondary.size() > 0) + { + ArrayList joinThem = new ArrayList(); + joinThem.add(w1); + joinThem.add(secondary.get(0)); + // Although we joined the ways, we cannot simply assume that they are closed + if((joined = joinWays(joinThem)) != null) + { + uninterestingWays.removeAll(joinThem); + possibleWays.removeAll(joinThem); + + List nodes = joined.getNodes(); + // check if we added too much + /*for(int i = 1; i < nodes.size()-2; ++i) + { + if(nodes.get(i) == nodes.get(nodes.size()-1)) + System.out.println("Joining of ways produced unexpecteded result\n"); + }*/ + uninterestingWays.add(joined); + possibleWays.add(joined); + continue outerIterator; + } + } + } + } while(joined != null); + } return newInnerWays; } @@ -795,22 +830,4 @@ if(count == 2) return true; } - return false; - } - - /** - * Checks if two ways share one starting/ending node - * @param Way first way - * @param Way second way - * @return boolean Wheter the ways share a starting/ending node or not - */ - private boolean waysCanBeCombined(Way w1, Way w2) { - if(w1.equals(w2)) return false; - - if(w1.getNode(0).equals(w2.getNode(0))) return true; - if(w1.getNode(0).equals(w2.getNode(w2.getNodesCount()-1))) return true; - - if(w1.getNode(w1.getNodesCount()-1).equals(w2.getNode(0))) return true; - if(w1.getNode(w1.getNodesCount()-1).equals(w2.getNode(w2.getNodesCount()-1))) return true; - return false; }