commit 59feb376093a782eb0fae018e7cfe28b04ea3222 Author: Simon Legner Date: 2020-03-10 12:27:12 +0100 fix #18907 - Initialize Territories+RightAndLefthandTraffic together, remove "Edit boundaries", save memory diff --git a/scripts/TagInfoExtract.java b/scripts/TagInfoExtract.java index 901072ecb..eab619448 100644 --- a/scripts/TagInfoExtract.java +++ b/scripts/TagInfoExtract.java @@ -582,7 +582,6 @@ private void init() throws IOException { System.setProperty("josm.home", tmpdir.toString()); DeleteCommand.setDeletionCallback(DeleteAction.defaultDeletionCallback); Territories.initialize(); - RightAndLefthandTraffic.initialize(); Files.createDirectories(options.imageDir); } } diff --git a/src/org/openstreetmap/josm/gui/MainInitialization.java b/src/org/openstreetmap/josm/gui/MainInitialization.java index a145698f7..a1fd49357 100644 --- a/src/org/openstreetmap/josm/gui/MainInitialization.java +++ b/src/org/openstreetmap/josm/gui/MainInitialization.java @@ -39,7 +39,6 @@ import org.openstreetmap.josm.tools.OpenBrowser; import org.openstreetmap.josm.tools.OverpassTurboQueryWizard; import org.openstreetmap.josm.tools.PlatformManager; -import org.openstreetmap.josm.tools.RightAndLefthandTraffic; import org.openstreetmap.josm.tools.Shortcut; import org.openstreetmap.josm.tools.Tag2Link; import org.openstreetmap.josm.tools.Territories; @@ -82,9 +81,7 @@ public MainInitialization(MainApplication application) { // help shortcut MainApplication.registerActionShortcut(MainApplication.menu.help, Shortcut.registerShortcut("system:help", tr("Help"), KeyEvent.VK_F1, Shortcut.DIRECT)); - }), - // This needs to be done before RightAndLefthandTraffic::initialize is called - new InitializationTask(tr("Initializing internal boundaries data"), Territories::initialize) + }) ); } @@ -114,7 +111,7 @@ public MainInitialization(MainApplication application) { Logging.warn(Logging.getErrorMessage(Utils.getRootCause(e))); } }), - new InitializationTask(tr("Initializing internal traffic data"), RightAndLefthandTraffic::initialize), + new InitializationTask(tr("Initializing internal boundaries data"), Territories::initialize), new InitializationTask(tr("Initializing validator"), OsmValidator::initialize), new InitializationTask(tr("Initializing presets"), TaggingPresets::initialize), new InitializationTask(tr("Initializing map styles"), MapPaintPreference::initialize), diff --git a/src/org/openstreetmap/josm/gui/mappaint/RenderingCLI.java b/src/org/openstreetmap/josm/gui/mappaint/RenderingCLI.java index ac565d763..7973ee370 100644 --- a/src/org/openstreetmap/josm/gui/mappaint/RenderingCLI.java +++ b/src/org/openstreetmap/josm/gui/mappaint/RenderingCLI.java @@ -43,7 +43,7 @@ import org.openstreetmap.josm.tools.OptionParser; import org.openstreetmap.josm.tools.OptionParser.OptionCount; import org.openstreetmap.josm.tools.OptionParser.OptionParseException; -import org.openstreetmap.josm.tools.RightAndLefthandTraffic; +import org.openstreetmap.josm.tools.Territories; /** * Command line interface for rendering osm data to an image file. @@ -438,7 +438,7 @@ void initialize() { String projCode = Optional.ofNullable(argProjection).orElse("epsg:3857"); ProjectionRegistry.setProjection(Projections.getProjectionByCode(projCode.toUpperCase(Locale.US))); - RightAndLefthandTraffic.initialize(); + Territories.initialize(); } private Level getLogLevel() { diff --git a/src/org/openstreetmap/josm/gui/preferences/advanced/AdvancedPreference.java b/src/org/openstreetmap/josm/gui/preferences/advanced/AdvancedPreference.java index 5339e127d..1ad501304 100644 --- a/src/org/openstreetmap/josm/gui/preferences/advanced/AdvancedPreference.java +++ b/src/org/openstreetmap/josm/gui/preferences/advanced/AdvancedPreference.java @@ -46,7 +46,6 @@ import org.openstreetmap.josm.gui.dialogs.LogShowDialog; import org.openstreetmap.josm.gui.help.HelpUtil; import org.openstreetmap.josm.gui.io.CustomConfigurator; -import org.openstreetmap.josm.gui.layer.MainLayerManager; import org.openstreetmap.josm.gui.layer.OsmDataLayer; import org.openstreetmap.josm.gui.preferences.DefaultTabPreferenceSetting; import org.openstreetmap.josm.gui.preferences.PreferenceSetting; @@ -60,7 +59,6 @@ import org.openstreetmap.josm.spi.preferences.StringSetting; import org.openstreetmap.josm.tools.GBC; import org.openstreetmap.josm.tools.Logging; -import org.openstreetmap.josm.tools.Territories; import org.openstreetmap.josm.tools.Utils; /** @@ -89,21 +87,6 @@ public void clear() { } } - private static final class EditBoundariesAction extends AbstractAction { - EditBoundariesAction() { - super(tr("Edit boundaries")); - } - - @Override - public void actionPerformed(ActionEvent ae) { - DataSet dataSet = Territories.getOriginalDataSet(); - MainLayerManager layerManager = MainApplication.getLayerManager(); - if (layerManager.getLayersOfType(OsmDataLayer.class).stream().noneMatch(l -> dataSet.equals(l.getDataSet()))) { - layerManager.addLayer(new UnclearableOsmDataLayer(dataSet, tr("Internal JOSM boundaries"))); - } - } - } - private final class ResetPreferencesAction extends AbstractAction { ResetPreferencesAction() { super(tr("Reset preferences")); @@ -391,8 +374,6 @@ private JPopupMenu buildPopupMenu() { menu.addSeparator(); menu.add(getProfileMenu()); menu.addSeparator(); - menu.add(new EditBoundariesAction()); - menu.addSeparator(); menu.add(new ResetPreferencesAction()); return menu; } diff --git a/src/org/openstreetmap/josm/tools/RightAndLefthandTraffic.java b/src/org/openstreetmap/josm/tools/RightAndLefthandTraffic.java index a4372725a..4ed51e38e 100644 --- a/src/org/openstreetmap/josm/tools/RightAndLefthandTraffic.java +++ b/src/org/openstreetmap/josm/tools/RightAndLefthandTraffic.java @@ -1,16 +1,13 @@ // License: GPL. For details, see LICENSE file. package org.openstreetmap.josm.tools; -import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Set; import org.openstreetmap.josm.data.coor.LatLon; -import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.Relation; -import org.openstreetmap.josm.data.osm.RelationMember; import org.openstreetmap.josm.data.osm.Way; /** @@ -19,9 +16,9 @@ */ public final class RightAndLefthandTraffic { - private static final String DRIVING_SIDE = "driving_side"; - private static final String LEFT = "left"; - private static final String RIGHT = "right"; + static final String DRIVING_SIDE = "driving_side"; + static final String LEFT = "left"; + static final String RIGHT = "right"; private static volatile GeoPropertyIndex rlCache; @@ -42,32 +39,11 @@ public static synchronized boolean isRightHandTraffic(LatLon ll) { /** * Initializes Right and lefthand traffic data. + * @param geoProperty the property containing the traffic data * TODO: Synchronization can be refined inside the {@link GeoPropertyIndex} as most look-ups are read-only. */ - public static synchronized void initialize() { - rlCache = new GeoPropertyIndex<>(computeLeftDrivingBoundaries(), 24); - } - - private static DefaultGeoProperty computeLeftDrivingBoundaries() { - Collection ways = new ArrayList<>(); - // Find all outer ways of left-driving countries. Many of them are adjacent (African and Asian states) - DataSet data = Territories.getDataSet(); - for (Way w : data.getWays()) { - if (LEFT.equals(w.get(DRIVING_SIDE))) { - addWayIfNotInner(ways, w); - } - } - for (Relation r : data.getRelations()) { - if (r.isMultipolygon() && LEFT.equals(r.get(DRIVING_SIDE))) { - for (RelationMember rm : r.getMembers()) { - if (rm.isWay() && "outer".equals(rm.getRole()) && !RIGHT.equals(rm.getMember().get(DRIVING_SIDE))) { - addWayIfNotInner(ways, (Way) rm.getMember()); - } - } - } - } - // Combine adjacent countries into a single polygon - return new DefaultGeoProperty(ways); + static synchronized void initialize(DefaultGeoProperty geoProperty) { + rlCache = new GeoPropertyIndex<>(geoProperty, 24); } /** @@ -76,7 +52,7 @@ private static DefaultGeoProperty computeLeftDrivingBoundaries() { * @param ways ways * @param w way */ - private static void addWayIfNotInner(Collection ways, Way w) { + static void addWayIfNotInner(Collection ways, Way w) { Set s = Collections.singleton(w); for (Relation r : OsmPrimitive.getParentRelations(s)) { if (r.isMultipolygon() && LEFT.equals(r.get(DRIVING_SIDE)) && diff --git a/src/org/openstreetmap/josm/tools/Territories.java b/src/org/openstreetmap/josm/tools/Territories.java index a315e9f96..54db37894 100644 --- a/src/org/openstreetmap/josm/tools/Territories.java +++ b/src/org/openstreetmap/josm/tools/Territories.java @@ -30,9 +30,12 @@ import org.openstreetmap.josm.data.coor.LatLon; import org.openstreetmap.josm.data.osm.DataSet; +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.RelationMember; import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.data.osm.visitor.paint.relations.MultipolygonCache; import org.openstreetmap.josm.io.CachedFile; import org.openstreetmap.josm.io.IllegalDataException; import org.openstreetmap.josm.io.OsmReader; @@ -52,8 +55,6 @@ private static final String ISO3166_2_LC = ISO3166_2.toLowerCase(Locale.ENGLISH); private static final String TAGINFO = "taginfo"; - private static DataSet dataSet; - private static volatile Map> iso3166Cache; private static volatile Map taginfoCache; private static volatile Map taginfoGeofabrikCache; @@ -98,23 +99,6 @@ public static synchronized boolean isIso3166Code(String code, LatLon ll) { return Boolean.TRUE.equals(gpi.get(ll)); // avoid NPE, see #16491 } - /** - * Returns the original territories dataset. Be extra cautious when manipulating it! - * @return the original territories dataset - * @since 15565 - */ - public static synchronized DataSet getOriginalDataSet() { - return dataSet; - } - - /** - * Returns a copy of the territories dataset. - * @return a copy of the territories dataset - */ - public static synchronized DataSet getDataSet() { - return new DataSet(dataSet); - } - /** * Initializes territories. * TODO: Synchronization can be refined inside the {@link GeoPropertyIndex} as most look-ups are read-only. @@ -127,12 +111,14 @@ public static synchronized void initialize() { private static void initializeInternalData() { iso3166Cache = new HashMap<>(); taginfoCache = new TreeMap<>(); + Collection traffic = new ArrayList<>(); try (CachedFile cf = new CachedFile("resource://data/" + FILENAME); InputStream is = cf.getInputStream()) { - dataSet = OsmReader.parseDataSet(is, null); - Collection candidates = new ArrayList<>(dataSet.getWays()); - candidates.addAll(dataSet.getRelations()); - for (OsmPrimitive osm : candidates) { + DataSet dataSet = OsmReader.parseDataSet(is, null); + for (OsmPrimitive osm : dataSet.allPrimitives()) { + if (osm instanceof Node) { + continue; + } String iso1 = osm.get(ISO3166_1); String iso2 = osm.get(ISO3166_2); if (iso1 != null || iso2 != null) { @@ -154,7 +140,22 @@ private static void initializeInternalData() { iso3166Cache.put(iso2, gpi); } } + // Find all outer ways of left-driving countries. Many of them are adjacent (African and Asian states) + if (RightAndLefthandTraffic.LEFT.equals(osm.get(RightAndLefthandTraffic.DRIVING_SIDE))) { + if (osm instanceof Way) { + RightAndLefthandTraffic.addWayIfNotInner(traffic, ((Way) osm)); + } else if (osm instanceof Relation && osm.isMultipolygon()) { + for (RelationMember rm : ((Relation) osm).getMembers()) { + if (rm.isWay() && "outer".equals(rm.getRole()) + && !RightAndLefthandTraffic.RIGHT.equals(rm.getMember().get(RightAndLefthandTraffic.DRIVING_SIDE))) { + RightAndLefthandTraffic.addWayIfNotInner(traffic, (Way) rm.getMember()); + } + } + } + } } + RightAndLefthandTraffic.initialize(new DefaultGeoProperty(traffic)); + MultipolygonCache.getInstance().clear(dataSet); } catch (IOException | IllegalDataException ex) { throw new JosmRuntimeException(ex); } diff --git a/test/unit/org/openstreetmap/josm/testutils/JOSMTestRules.java b/test/unit/org/openstreetmap/josm/testutils/JOSMTestRules.java index 9161e3fa9..f41f80283 100644 --- a/test/unit/org/openstreetmap/josm/testutils/JOSMTestRules.java +++ b/test/unit/org/openstreetmap/josm/testutils/JOSMTestRules.java @@ -64,7 +64,6 @@ import org.openstreetmap.josm.tools.JosmRuntimeException; import org.openstreetmap.josm.tools.Logging; import org.openstreetmap.josm.tools.MemoryManagerTest; -import org.openstreetmap.josm.tools.RightAndLefthandTraffic; import org.openstreetmap.josm.tools.Territories; import org.openstreetmap.josm.tools.bugreport.ReportedException; import org.openstreetmap.josm.tools.date.DateUtils; @@ -99,7 +98,6 @@ private boolean usePresets; private boolean useHttps; private boolean territories; - private boolean rlTraffic; private boolean metric; private boolean main; @@ -269,10 +267,11 @@ public JOSMTestRules territories() { * Use right and lefthand traffic dataset in this test. * @return this instance, for easy chaining * @since 12556 + * @deprecated Use {@link #territories} */ + @Deprecated public JOSMTestRules rlTraffic() { territories(); - rlTraffic = true; return this; } @@ -552,10 +551,6 @@ protected void before() throws InitializationError, ReflectiveOperationException Territories.initialize(); } - if (rlTraffic) { - RightAndLefthandTraffic.initialize(); - } - if (this.edtAssertionMockingRunnable != null) { this.edtAssertionMockingRunnable.run(); }