Index: trunk/src/org/openstreetmap/josm/actions/UpdateDataAction.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/UpdateDataAction.java (revision 11626) +++ trunk/src/org/openstreetmap/josm/actions/UpdateDataAction.java (revision 11627) @@ -51,5 +51,5 @@ List areas = new ArrayList<>(); - for (DataSource ds : editLayer.data.dataSources) { + for (DataSource ds : editLayer.data.getDataSources()) { areas.add(new Area(ds.bounds.asRect())); } Index: trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadOsmTask.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadOsmTask.java (revision 11626) +++ trunk/src/org/openstreetmap/josm/actions/downloadtasks/DownloadOsmTask.java (revision 11627) @@ -343,5 +343,5 @@ } // need to synthesize a download bounds lest the visual indication of downloaded area doesn't work - dataSet.dataSources.add(new DataSource(currentBounds != null ? currentBounds : + dataSet.addDataSource(new DataSource(currentBounds != null ? currentBounds : new Bounds(LatLon.ZERO), "OpenStreetMap server")); } Index: trunk/src/org/openstreetmap/josm/data/UndoRedoHandler.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/UndoRedoHandler.java (revision 11626) +++ trunk/src/org/openstreetmap/josm/data/UndoRedoHandler.java (revision 11627) @@ -52,5 +52,4 @@ CheckParameterUtil.ensureParameterNotNull(c, "c"); c.executeCommand(); - c.invalidateAffectedLayers(); commands.add(c); // Limit the number of commands in the undo list. @@ -81,4 +80,5 @@ } addNoRedraw(c); + c.invalidateAffectedLayers(); afterAdd(); Index: trunk/src/org/openstreetmap/josm/data/osm/DataSet.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/osm/DataSet.java (revision 11626) +++ trunk/src/org/openstreetmap/josm/data/osm/DataSet.java (revision 11627) @@ -4,4 +4,5 @@ import static org.openstreetmap.josm.tools.I18n.tr; +import java.awt.geom.Area; import java.util.ArrayList; import java.util.Arrays; @@ -25,4 +26,5 @@ import org.openstreetmap.josm.Main; +import org.openstreetmap.josm.data.Bounds; import org.openstreetmap.josm.data.Data; import org.openstreetmap.josm.data.DataSource; @@ -127,4 +129,7 @@ private final Object selectionLock = new Object(); + private Area cachedDataSourceArea; + private List cachedDataSourceBounds; + /** * Constructs a new {@code DataSet}. @@ -184,4 +189,29 @@ copyFrom.getReadLock().unlock(); } + } + + /** + * Adds a new data source. + * @param source data source to add + * @return {@code true} if the collection changed as a result of the call + * @since 11626 + */ + public synchronized boolean addDataSource(DataSource source) { + return addDataSources(Collections.singleton(source)); + } + + /** + * Adds new data sources. + * @param sources data sources to add + * @return {@code true} if the collection changed as a result of the call + * @since 11626 + */ + public synchronized boolean addDataSources(Collection sources) { + boolean changed = dataSources.addAll(sources); + if (changed) { + cachedDataSourceArea = null; + cachedDataSourceBounds = null; + } + return changed; } @@ -917,4 +947,20 @@ fireSelectionChanged(); } + } + + @Override + public synchronized Area getDataSourceArea() { + if (cachedDataSourceArea == null) { + cachedDataSourceArea = Data.super.getDataSourceArea(); + } + return cachedDataSourceArea; + } + + @Override + public synchronized List getDataSourceBounds() { + if (cachedDataSourceBounds == null) { + cachedDataSourceBounds = Data.super.getDataSourceBounds(); + } + return Collections.unmodifiableList(cachedDataSourceBounds); } @@ -1334,9 +1380,16 @@ * @param progressMonitor The progress monitor */ - public void mergeFrom(DataSet from, ProgressMonitor progressMonitor) { + public synchronized void mergeFrom(DataSet from, ProgressMonitor progressMonitor) { if (from != null) { new DataSetMerger(this, from).merge(progressMonitor); - dataSources.addAll(from.dataSources); - from.dataSources.clear(); + if (!from.dataSources.isEmpty()) { + if (dataSources.addAll(from.dataSources)) { + cachedDataSourceArea = null; + cachedDataSourceBounds = null; + } + from.dataSources.clear(); + from.cachedDataSourceArea = null; + from.cachedDataSourceBounds = null; + } } } Index: trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateNode.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateNode.java (revision 11626) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateNode.java (revision 11627) @@ -309,5 +309,4 @@ @Override public Command fixError(TestError testError) { - if (!isFixable(testError)) return null; Collection sel = new LinkedList<>(testError.getPrimitives()); Set nodes = new LinkedHashSet<>(OsmPrimitive.getFilteredList(sel, Node.class)); Index: trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateRelation.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateRelation.java (revision 11626) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateRelation.java (revision 11627) @@ -293,5 +293,5 @@ } - //Delete all relations in the list + // Delete all relations in the list relFix.remove(relationToKeep); commands.add(new DeleteCommand(relFix)); @@ -306,17 +306,17 @@ // We fix it only if there is no more than one relation that is relation member. Collection sel = testError.getPrimitives(); - Set relations = new HashSet<>(); + Set rels = new HashSet<>(); for (OsmPrimitive osm : sel) { if (osm instanceof Relation) { - relations.add((Relation) osm); - } - } - - if (relations.size() < 2) + rels.add((Relation) osm); + } + } + + if (rels.size() < 2) return false; int relationsWithRelations = 0; - for (Relation w : relations) { + for (Relation w : rels) { List rel = OsmPrimitive.getFilteredList(w.getReferrers(), Relation.class); if (!rel.isEmpty()) { Index: trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateWay.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateWay.java (revision 11626) +++ trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateWay.java (revision 11627) @@ -249,21 +249,21 @@ public Command fixError(TestError testError) { Collection sel = testError.getPrimitives(); - Set ways = new HashSet<>(); + Set wayz = new HashSet<>(); for (OsmPrimitive osm : sel) { if (osm instanceof Way && !osm.isDeleted()) { - ways.add((Way) osm); - } - } - - if (ways.size() < 2) + wayz.add((Way) osm); + } + } + + if (wayz.size() < 2) return null; long idToKeep = 0; - Way wayToKeep = ways.iterator().next(); + Way wayToKeep = wayz.iterator().next(); // Find the way that is member of one or more relations. (If any) Way wayWithRelations = null; List relations = null; - for (Way w : ways) { + for (Way w : wayz) { List rel = OsmPrimitive.getFilteredList(w.getReferrers(), Relation.class); if (!rel.isEmpty()) { @@ -284,5 +284,5 @@ // Fix relations. - if (wayWithRelations != null && wayToKeep != wayWithRelations) { + if (wayWithRelations != null && relations != null && wayToKeep != wayWithRelations) { for (Relation rel : relations) { Relation newRel = new Relation(rel); @@ -297,8 +297,8 @@ } - //Delete all ways in the list - //Note: nodes are not deleted, these can be detected and deleted at next pass - ways.remove(wayToKeep); - commands.add(new DeleteCommand(ways)); + // Delete all ways in the list + // Note: nodes are not deleted, these can be detected and deleted at next pass + wayz.remove(wayToKeep); + commands.add(new DeleteCommand(wayz)); return new SequenceCommand(tr("Delete duplicate ways"), commands); } @@ -309,22 +309,22 @@ return false; - //Do not automatically fix same ways with different tags + // Do not automatically fix same ways with different tags if (testError.getCode() != DUPLICATE_WAY) return false; // We fix it only if there is no more than one way that is relation member. Collection sel = testError.getPrimitives(); - Set ways = new HashSet<>(); + Set wayz = new HashSet<>(); for (OsmPrimitive osm : sel) { if (osm instanceof Way) { - ways.add((Way) osm); - } - } - - if (ways.size() < 2) + wayz.add((Way) osm); + } + } + + if (wayz.size() < 2) return false; int waysWithRelations = 0; - for (Way w : ways) { + for (Way w : wayz) { List rel = OsmPrimitive.getFilteredList(w.getReferrers(), Relation.class); if (!rel.isEmpty()) { Index: trunk/src/org/openstreetmap/josm/gui/conflict/tags/CombinePrimitiveResolverDialog.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/conflict/tags/CombinePrimitiveResolverDialog.java (revision 11626) +++ trunk/src/org/openstreetmap/josm/gui/conflict/tags/CombinePrimitiveResolverDialog.java (revision 11627) @@ -132,13 +132,19 @@ */ public void setTargetPrimitive(final OsmPrimitive primitive) { + setTargetPrimitive(primitive, true); + } + + /** + * Sets the primitive the collection of primitives is merged or combined to. + * + * @param primitive the target primitive + * @param updateTitle {@code true} to call {@link #updateTitle} in EDT (can be a slow operation) + * @since 11626 + */ + private void setTargetPrimitive(final OsmPrimitive primitive, boolean updateTitle) { this.targetPrimitive = primitive; - GuiHelper.runInEDTAndWait(() -> { - updateTitle(); - if (primitive instanceof Way) { - pnlRelationMemberConflictResolver.initForWayCombining(); - } else if (primitive instanceof Node) { - pnlRelationMemberConflictResolver.initForNodeMerging(); - } - }); + if (updateTitle) { + GuiHelper.runInEDTAndWait(this::updateTitle); + } } @@ -153,4 +159,5 @@ helpAction.setHelpTopic(ht("/Action/CombineWay#ResolvingConflicts")); getRootPane().putClientProperty("help", ht("/Action/CombineWay#ResolvingConflicts")); + pnlRelationMemberConflictResolver.initForWayCombining(); } else if (targetPrimitive instanceof Node) { setTitle(tr("Conflicts when merging nodes - target node is ''{0}''", targetPrimitive @@ -158,4 +165,5 @@ helpAction.setHelpTopic(ht("/Action/MergeNodes#ResolvingConflicts")); getRootPane().putClientProperty("help", ht("/Action/MergeNodes#ResolvingConflicts")); + pnlRelationMemberConflictResolver.initForNodeMerging(); } } @@ -295,6 +303,15 @@ */ public void prepareDefaultDecisions() { - getTagConflictResolverModel().prepareDefaultTagDecisions(); - getRelationMemberConflictResolverModel().prepareDefaultRelationDecisions(); + prepareDefaultDecisions(true); + } + + /** + * Prepares the default decisions for populated tag and relation membership conflicts. + * @param fireEvent {@code true} to call {@code fireTableDataChanged} (can be a slow operation) + * @since 11626 + */ + private void prepareDefaultDecisions(boolean fireEvent) { + getTagConflictResolverModel().prepareDefaultTagDecisions(fireEvent); + getRelationMemberConflictResolverModel().prepareDefaultRelationDecisions(fireEvent); } @@ -501,17 +518,22 @@ final CombinePrimitiveResolverDialog dialog = CombinePrimitiveResolverDialog.getInstance(); - dialog.getTagConflictResolverModel().populate(tagsToEdit, completeWayTags.getKeysWithMultipleValues()); - dialog.getRelationMemberConflictResolverModel().populate(parentRelations, primitives); - dialog.prepareDefaultDecisions(); + dialog.getTagConflictResolverModel().populate(tagsToEdit, completeWayTags.getKeysWithMultipleValues(), false); + dialog.getRelationMemberConflictResolverModel().populate(parentRelations, primitives, false); + dialog.prepareDefaultDecisions(false); // Ensure a proper title is displayed instead of a previous target (fix #7925) if (targetPrimitives.size() == 1) { - dialog.setTargetPrimitive(targetPrimitives.iterator().next()); + dialog.setTargetPrimitive(targetPrimitives.iterator().next(), false); } else { - dialog.setTargetPrimitive(null); + dialog.setTargetPrimitive(null, false); } // Resolve tag conflicts if necessary if (!dialog.isResolvedCompletely()) { + GuiHelper.runInEDTAndWait(() -> { + dialog.getTagConflictResolverModel().fireTableDataChanged(); + dialog.getRelationMemberConflictResolverModel().fireTableDataChanged(); + dialog.updateTitle(); + }); dialog.setVisible(true); if (!dialog.isApplied()) { @@ -520,5 +542,5 @@ } for (OsmPrimitive i : targetPrimitives) { - dialog.setTargetPrimitive(i); + dialog.setTargetPrimitive(i, false); cmds.addAll(dialog.buildResolutionCommands()); } Index: trunk/src/org/openstreetmap/josm/gui/conflict/tags/RelationMemberConflictResolverModel.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/conflict/tags/RelationMemberConflictResolverModel.java (revision 11626) +++ trunk/src/org/openstreetmap/josm/gui/conflict/tags/RelationMemberConflictResolverModel.java (revision 11627) @@ -129,5 +129,5 @@ case 4: /* decision */ d.decide((RelationMemberConflictDecisionType) value); - refresh(); + refresh(false); break; default: // Do nothing @@ -159,4 +159,17 @@ */ public void populate(Collection relations, Collection memberPrimitives) { + populate(relations, memberPrimitives, true); + } + + /** + * Populates the model with the relation members belonging to one of the relations in relations + * and referring to one of the primitives in memberPrimitives. + * + * @param relations the parent relations. Empty list assumed if null. + * @param memberPrimitives the child primitives. Empty list assumed if null. + * @param fireEvent {@code true} to call {@code fireTableDataChanged} (can be a slow operation) + * @since 11626 + */ + void populate(Collection relations, Collection memberPrimitives, boolean fireEvent) { decisions.clear(); relations = relations == null ? Collections.emptyList() : relations; @@ -169,5 +182,5 @@ this.relations = relations; this.primitives = memberPrimitives; - refresh(); + refresh(fireEvent); } @@ -199,5 +212,17 @@ */ public void prepareDefaultRelationDecisions() { - + prepareDefaultRelationDecisions(true); + } + + /** + * Prepare the default decisions for the current model. + * + * Keep/delete decisions are made if every member has the same role and the members are in consecutive order within the relation. + * For multiple occurrences those conditions are tested stepwise for each occurrence. + * + * @param fireEvent {@code true} to call {@code fireTableDataChanged} (can be a slow operation) + * @since 11626 + */ + void prepareDefaultRelationDecisions(boolean fireEvent) { if (primitives.stream().allMatch(Node.class::isInstance)) { final Collection primitivesInDecisions = new HashSet<>(); @@ -256,5 +281,5 @@ } - refresh(); + refresh(fireEvent); } @@ -300,6 +325,18 @@ */ public void refresh() { + refresh(true); + } + + /** + * Refreshes the model state. Invoke this method to trigger necessary change + * events after an update of the model data. + * @param fireEvent {@code true} to call {@code fireTableDataChanged} (can be a slow operation) + * @since 11626 + */ + void refresh(boolean fireEvent) { updateNumConflicts(); - GuiHelper.runInEDTAndWait(this::fireTableDataChanged); + if (fireEvent) { + GuiHelper.runInEDTAndWait(this::fireTableDataChanged); + } } Index: trunk/src/org/openstreetmap/josm/gui/conflict/tags/TagConflictResolverModel.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/conflict/tags/TagConflictResolverModel.java (revision 11626) +++ trunk/src/org/openstreetmap/josm/gui/conflict/tags/TagConflictResolverModel.java (revision 11627) @@ -73,4 +73,13 @@ */ public void rebuild() { + rebuild(true); + } + + /** + * initializes the model from the current tags + * @param fireEvent {@code true} to call {@code fireTableDataChanged} (can be a slow operation) + * @since 11626 + */ + void rebuild(boolean fireEvent) { if (tags == null) return; for (String key: tags.getKeys()) { @@ -102,5 +111,7 @@ refreshNumConflicts(); sort(); - GuiHelper.runInEDTAndWait(this::fireTableDataChanged); + if (fireEvent) { + GuiHelper.runInEDTAndWait(this::fireTableDataChanged); + } } @@ -113,4 +124,17 @@ */ public void populate(TagCollection tags, Set keysWithConflicts) { + populate(tags, keysWithConflicts, true); + } + + /** + * Populates the model with the tags for which conflicts are to be resolved. + * + * @param tags the tag collection with the tags. Must not be null. + * @param keysWithConflicts the set of tag keys with conflicts + * @param fireEvent {@code true} to call {@code fireTableDataChanged} (can be a slow operation) + * @throws IllegalArgumentException if tags is null + * @since 11626 + */ + void populate(TagCollection tags, Set keysWithConflicts, boolean fireEvent) { CheckParameterUtil.ensureParameterNotNull(tags, "tags"); this.tags = tags; @@ -120,5 +144,5 @@ } decisions = new HashMap<>(); - rebuild(); + rebuild(fireEvent); } @@ -250,4 +274,13 @@ */ public void prepareDefaultTagDecisions() { + prepareDefaultTagDecisions(true); + } + + /** + * Prepare the default decisions for the current model + * @param fireEvent {@code true} to call {@code fireTableDataChanged} (can be a slow operation) + * @since 11626 + */ + void prepareDefaultTagDecisions(boolean fireEvent) { for (MultiValueResolutionDecision decision: decisions.values()) { List values = decision.getValues(); @@ -260,5 +293,5 @@ // else: Do not suggest to keep all values in order to reduce the wrong usage of semicolon values, see #9104! } - rebuild(); + rebuild(fireEvent); } Index: trunk/src/org/openstreetmap/josm/gui/dialogs/ValidatorDialog.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/dialogs/ValidatorDialog.java (revision 11626) +++ trunk/src/org/openstreetmap/josm/gui/dialogs/ValidatorDialog.java (revision 11627) @@ -57,4 +57,5 @@ import org.openstreetmap.josm.tools.JosmRuntimeException; import org.openstreetmap.josm.tools.Shortcut; +import org.openstreetmap.josm.tools.Utils; import org.xml.sax.SAXException; @@ -595,7 +596,16 @@ protected void fixError(TestError error) throws InterruptedException, InvocationTargetException { if (error.isFixable()) { + long start = System.currentTimeMillis(); final Command fixCommand = error.getFix(); + long phase1 = System.currentTimeMillis() - start; + start = System.currentTimeMillis(); if (fixCommand != null) { SwingUtilities.invokeAndWait(() -> Main.main.undoRedo.addNoRedraw(fixCommand)); + if (Main.isDebugEnabled()) { + long phase2 = System.currentTimeMillis() - start; + Main.debug(String.format("%s fix: %s + %s", + error.getTester().getClass().getSimpleName(), Utils.getDurationString(phase1), + Utils.getDurationString(phase2))); + } } // It is wanted to ignore an error if it said fixable, even if fixCommand was null Index: trunk/src/org/openstreetmap/josm/gui/dialogs/relation/ParentRelationLoadingTask.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/dialogs/relation/ParentRelationLoadingTask.java (revision 11626) +++ trunk/src/org/openstreetmap/josm/gui/dialogs/relation/ParentRelationLoadingTask.java (revision 11627) @@ -161,5 +161,5 @@ // copy the merged layer's data source info - getLayer().data.dataSources.addAll(referrers.dataSources); + getLayer().data.addDataSources(referrers.getDataSources()); // FIXME: this is necessary because there are dialogs listening // for DataChangeEvents which manipulate Swing components on this thread. Index: trunk/src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java (revision 11626) +++ trunk/src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java (revision 11627) @@ -406,5 +406,5 @@ // draw the hatched area for non-downloaded region. only draw if we're the active // and bounds are defined; don't draw for inactive layers or loaded GPX files etc - if (active && Main.pref.getBoolean("draw.data.downloaded_area", true) && !data.dataSources.isEmpty()) { + if (active && Main.pref.getBoolean("draw.data.downloaded_area", true) && !data.getDataSources().isEmpty()) { // initialize area with current viewport Rectangle b = mv.getBounds(); @@ -496,7 +496,7 @@ // copy the merged layer's data source info. // only add source rectangles if they are not contained in the layer already. - for (DataSource src : from.dataSources) { + for (DataSource src : from.getDataSources()) { if (a == null || !a.contains(src.bounds.asRect())) { - data.dataSources.add(src); + data.addDataSource(src); } } @@ -846,9 +846,9 @@ // we'll assume that if this has no data sources // that it also has no borders - if (this.data.dataSources.isEmpty()) + if (this.data.getDataSources().isEmpty()) return true; boolean layerBoundsPoint = false; - for (DataSource src : this.data.dataSources) { + for (DataSource src : this.data.getDataSources()) { if (src.bounds.contains(coor)) { layerBoundsPoint = true; Index: trunk/src/org/openstreetmap/josm/io/OsmReader.java =================================================================== --- trunk/src/org/openstreetmap/josm/io/OsmReader.java (revision 11626) +++ trunk/src/org/openstreetmap/josm/io/OsmReader.java (revision 11627) @@ -198,5 +198,5 @@ } DataSource src = new DataSource(bounds, origin); - ds.dataSources.add(src); + ds.addDataSource(src); } else { throwException(tr("Missing mandatory attributes on element ''bounds''. " + Index: trunk/src/org/openstreetmap/josm/io/OsmWriter.java =================================================================== --- trunk/src/org/openstreetmap/josm/io/OsmWriter.java (revision 11626) +++ trunk/src/org/openstreetmap/josm/io/OsmWriter.java (revision 11627) @@ -174,5 +174,5 @@ public void writeDataSources(DataSet ds) { - for (DataSource s : ds.dataSources) { + for (DataSource s : ds.getDataSources()) { out.println("