Subject: [PATCH] #11487: Try to improve render performance --- Index: src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java b/src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java --- a/src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java (revision 18721) +++ b/src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java (date 1683049713596) @@ -11,11 +11,13 @@ import java.awt.Composite; import java.awt.Graphics2D; import java.awt.GridBagLayout; +import java.awt.Point; import java.awt.Rectangle; import java.awt.TexturePaint; import java.awt.datatransfer.Transferable; import java.awt.datatransfer.UnsupportedFlavorException; import java.awt.event.ActionEvent; +import java.awt.geom.AffineTransform; import java.awt.geom.Area; import java.awt.geom.Path2D; import java.awt.geom.Rectangle2D; @@ -37,6 +39,7 @@ import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Supplier; import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -49,6 +52,9 @@ import javax.swing.JPanel; import javax.swing.JScrollPane; +import org.apache.commons.jcs3.access.CacheAccess; +import org.openstreetmap.gui.jmapviewer.OsmMercator; +import org.openstreetmap.gui.jmapviewer.TileXY; import org.openstreetmap.josm.actions.AutoScaleAction; import org.openstreetmap.josm.actions.ExpertToggleAction; import org.openstreetmap.josm.actions.RenameLayerAction; @@ -58,6 +64,7 @@ import org.openstreetmap.josm.data.Data; import org.openstreetmap.josm.data.ProjectionBounds; import org.openstreetmap.josm.data.UndoRedoHandler; +import org.openstreetmap.josm.data.cache.JCSCacheManager; import org.openstreetmap.josm.data.conflict.Conflict; import org.openstreetmap.josm.data.conflict.ConflictCollection; import org.openstreetmap.josm.data.coor.EastNorth; @@ -70,6 +77,7 @@ import org.openstreetmap.josm.data.gpx.GpxTrackSegment; import org.openstreetmap.josm.data.gpx.IGpxTrackSegment; import org.openstreetmap.josm.data.gpx.WayPoint; +import org.openstreetmap.josm.data.osm.BBox; import org.openstreetmap.josm.data.osm.DataIntegrityProblemException; import org.openstreetmap.josm.data.osm.DataSelectionListener; import org.openstreetmap.josm.data.osm.DataSet; @@ -77,7 +85,9 @@ import org.openstreetmap.josm.data.osm.DatasetConsistencyTest; import org.openstreetmap.josm.data.osm.DownloadPolicy; import org.openstreetmap.josm.data.osm.HighlightUpdateListener; +import org.openstreetmap.josm.data.osm.INode; import org.openstreetmap.josm.data.osm.IPrimitive; +import org.openstreetmap.josm.data.osm.IWay; import org.openstreetmap.josm.data.osm.Node; import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.OsmPrimitiveComparator; @@ -104,6 +114,7 @@ import org.openstreetmap.josm.gui.MapFrame; import org.openstreetmap.josm.gui.MapView; import org.openstreetmap.josm.gui.MapViewState.MapViewPoint; +import org.openstreetmap.josm.gui.PrimitiveHoverListener; import org.openstreetmap.josm.gui.datatransfer.ClipboardUtils; import org.openstreetmap.josm.gui.datatransfer.data.OsmLayerTransferData; import org.openstreetmap.josm.gui.dialogs.LayerListDialog; @@ -144,7 +155,8 @@ * @author imi * @since 17 */ -public class OsmDataLayer extends AbstractOsmDataLayer implements Listener, DataSelectionListener, HighlightUpdateListener { +public class OsmDataLayer extends AbstractOsmDataLayer + implements Listener, DataSelectionListener, HighlightUpdateListener, PrimitiveHoverListener { private static final int HATCHED_SIZE = 15; // U+2205 EMPTY SET private static final String IS_EMPTY_SYMBOL = "\u2205"; @@ -155,6 +167,16 @@ private boolean requiresUploadToServer; /** Flag used to know if the layer is being uploaded */ private final AtomicBoolean isUploadInProgress = new AtomicBoolean(false); + /** + * A cache used for painting + * TODO: add dirty bit (to avoid "blank" spaces) + */ + private final CacheAccess cache = JCSCacheManager.getCache("osmDataLayer:" + this); + /** The last zoom that was painted (used to invalidate {@link #cache}, TODO investigate if it is worth it to cache multiple zoom levels) */ + private int lastZoom; + /** The map paint index that was painted (used to invalidate {@link #cache}) */ + private int lastDataIdx; + private boolean hoverListenerAdded; /** * List of validation errors in this layer. @@ -497,6 +519,10 @@ * Draw nodes last to overlap the ways they belong to. */ @Override public void paint(final Graphics2D g, final MapView mv, Bounds box) { + if (!hoverListenerAdded) { + MainApplication.getMap().mapView.addPrimitiveHoverListener(this); + hoverListenerAdded = true; + } boolean active = mv.getLayerManager().getActiveLayer() == this; boolean inactive = !active && Config.getPref().getBoolean("draw.data.inactive_color", true); boolean virtual = !inactive && mv.isVirtualNodesEnabled(); @@ -537,13 +563,144 @@ } } - AbstractMapRenderer painter = MapRendererFactory.getInstance().createActiveRenderer(g, mv, inactive); - painter.enableSlowOperations(mv.getMapMover() == null || !mv.getMapMover().movementInProgress() - || !PROPERTY_HIDE_LABELS_WHILE_DRAGGING.get()); - painter.render(data, virtual, box); + final double scale = mv.getScale(); + // We might have to fall back to the old method if user is reprojecting + final double topResolution = 2 * Math.PI * OsmMercator.EARTH_RADIUS / 256; // 256 is the "target" size (TODO check HiDPI!) + // Used to invalidate cache + int zoom; + for (zoom = 0; zoom < 30; zoom++) { // Use something like imagery.{generic|tms}.max_zoom_lvl (20 is a bit too low for our needs) + if (scale > topResolution / Math.pow(2, zoom)) { + zoom = zoom > 0 ? zoom - 1 : zoom; + break; + } + } + // We paint at a few levels higher, note that the tiles are appropriately sized (if 256 is the "target" size, the tiles should be + // 64px square) + zoom += 2; + if (!Config.getPref().getBoolean("mappaint.fast_render", false) || zoom > Config.getPref().getInt("mappaint.fast_render.zlevel", 16)) { + AbstractMapRenderer painter = MapRendererFactory.getInstance().createActiveRenderer(g, mv, inactive); + painter.enableSlowOperations(mv.getMapMover() == null || !mv.getMapMover().movementInProgress() + || !PROPERTY_HIDE_LABELS_WHILE_DRAGGING.get()); + painter.render(this.data, virtual, box); + } else { + if (zoom != this.lastZoom || this.data.getMappaintCacheIndex() != this.lastDataIdx) { + this.cache.clear(); + this.lastZoom = zoom; + this.lastDataIdx = this.data.getMappaintCacheIndex(); + Logging.trace("OsmDataLayer {0} paint cache cleared", this.getName()); + } + final List toRender = boundsToTiles(box.getMinLat(), box.getMinLon(), box.getMaxLat(), box.getMaxLon(), zoom); + toRender.stream().map(tile -> tileToBounds(tile, lastZoom)).forEach(box::extend); // TODO is the box sent in reused? + final int tileSize; + if (toRender.isEmpty()) { + tileSize = 256; // Mostly to keep the compiler happy + } else { + final TileXY tile = toRender.get(0); + final Bounds bounds = tileToBounds(tile, zoom); + final Point min = mv.getPoint(bounds.getMin()); + final Point max = mv.getPoint(bounds.getMax()); + tileSize = max.x - min.x; + } + for (TileXY tile : toRender) { + final int actualZoom = zoom; + final double lat = yToLat(tile.getYIndex(), zoom); + final double lon = xToLon(tile.getXIndex(), zoom); + final Point point = mv.getPoint(new LatLon(lat, lon)); + final BufferedImage tileImage; + // Needed to avoid having tiles that aren't rendered properly + final String tileString = tile.toString(); + final Supplier tileSupplier = + () -> generateTile(this.data, mv, point, inactive, virtual, tile, tileSize, actualZoom); + final BufferedImage tImg = this.cache.get(tileString); + // 16.66 ms gives us 60 fps, but we want to ''always'' render at ''least'' one tile + if (tImg == null) { + // We could do this render in a separate thread. Worker thread maybe? Caveat: mv changes will kind of mess it up. + // TODO do we want to use the worker thread, or do we need/want a dedicated thread(s)? + // Note that the paint code is *not* thread safe, so all tiles must be painted on the same thread. + // FIXME figure out how to make this thread safe? Probably not necessary, since UI isn't blocked. + MainApplication.worker.execute(() -> { + // These checkpoints ensure that the mapview is the same as when we scheduled the tile + // This prevents tearing. This check occurs first to avoid a pointless render. + final Point checkPoint = mv.getPoint(new LatLon(lat, lon)); + if (checkPoint.equals(point)) { + this.cache.put(tileString, tileSupplier.get()); + // A second check just in case the map moved during render. + final Point checkPoint2 = mv.getPoint(new LatLon(lat, lon)); + if (checkPoint2.equals(checkPoint)) { + GuiHelper.runInEDT(this::invalidate); + } else { + this.cache.remove(tileString); + } + } + }); + tileImage = null; + } else { + tileImage = tImg; + } + if (tileImage != null) { + // Get the lowerright point of the tile to avoid render gaps + // This does very slightly stretch a tile though. There is something like a 1px border if we use the tile size. + final double lat2 = yToLat(tile.getYIndex() + 1, zoom); + final double lon2 = xToLon(tile.getXIndex() + 1, zoom); + final Point point2 = mv.getPoint(new LatLon(lat2, lon2)); + g.drawImage(tileImage, point.x, point.y, point2.x - point.x, point2.y - point.y, null, null); + } + } + } MainApplication.getMap().conflictDialog.paintConflicts(g, mv); } + private static List boundsToTiles(double minLat, double minLon, double maxLat, double maxLon, int zoom) { + final List tiles = new ArrayList<>(); + final TileXY upperRight = latLonToTile(maxLat, maxLon, zoom); + final TileXY lowerLeft = latLonToTile(minLat, minLon, zoom); + for (int x = lowerLeft.getXIndex(); x <= upperRight.getXIndex(); x++) { + for (int y = upperRight.getYIndex(); y <= lowerLeft.getYIndex(); y++) { + tiles.add(new TileXY(x, y)); + } + } + return tiles; + } + + private static BufferedImage generateTile(DataSet data, MapView mv, Point point, boolean inactive, boolean virtual, TileXY tile, + int tileSize, int zoom) { + BufferedImage bufferedImage = new BufferedImage(tileSize, tileSize, BufferedImage.TYPE_4BYTE_ABGR); + Graphics2D g2d = bufferedImage.createGraphics(); + g2d.setTransform(AffineTransform.getTranslateInstance(-point.x, -point.y)); + try { + AbstractMapRenderer tilePainter = MapRendererFactory.getInstance().createActiveRenderer(g2d, mv, inactive); + // Render to the surrounding tiles for continuity -- this probably needs to be tweaked + int buffer = 2; + Bounds bounds = tileToBounds(new TileXY(tile.getXIndex() - buffer, tile.getYIndex() - buffer), zoom); + bounds.extend(tileToBounds(new TileXY(tile.getXIndex() + buffer, tile.getYIndex() + buffer), zoom)); + tilePainter.render(data, virtual, bounds); + } finally { + g2d.dispose(); + } + return bufferedImage; + } + + private static Bounds tileToBounds(TileXY tile, int zoom) { + return new Bounds(yToLat(tile.getYIndex() + 1, zoom), xToLon(tile.getXIndex(), zoom), + yToLat(tile.getYIndex(), zoom), xToLon(tile.getXIndex() + 1, zoom)); + } + + private static double xToLon(int x, int zoom) { + return (x / Math.pow(2, zoom)) * 360 - 180; + } + + private static double yToLat(int y, int zoom) { + double t = Math.PI - (2 * Math.PI * y) / Math.pow(2, zoom); + return 180 / Math.PI * Math.atan((Math.exp(t) - Math.exp(-t)) / 2); + } + + private static TileXY latLonToTile(double lat, double lon, int zoom) { + int xCoord = (int) Math.floor(Math.pow(2, zoom) * (180 + lon) / 360); + int yCoord = (int) Math.floor(Math.pow(2, zoom) * + (1 - Math.log(Math.tan(Math.toRadians(lat)) + 1 / Math.cos(Math.toRadians(lat))) / Math.PI) / 2); + return new TileXY(xCoord, yCoord); + } + @Override public String getToolTipText() { DataCountVisitor counter = new DataCountVisitor(); for (final OsmPrimitive osm : data.allPrimitives()) { @@ -1147,6 +1304,10 @@ validationErrors.clear(); removeClipboardDataFor(this); recentRelations.clear(); + if (hoverListenerAdded) { + hoverListenerAdded = false; + MainApplication.getMap().mapView.removePrimitiveHoverListener(this); + } } protected static void removeClipboardDataFor(OsmDataLayer osm) { @@ -1165,6 +1326,7 @@ @Override public void processDatasetEvent(AbstractDatasetChangedEvent event) { + resetTiles(event.getPrimitives()); invalidate(); setRequiresSaveToFile(true); setRequiresUploadToServer(event.getDataset().requiresUploadToServer()); @@ -1172,9 +1334,21 @@ @Override public void selectionChanged(SelectionChangeEvent event) { + Set primitives = new HashSet<>(event.getAdded()); + primitives.addAll(event.getRemoved()); + resetTiles(primitives); invalidate(); } + private void resetTiles(Iterable primitives) { + for (IPrimitive primitive : primitives) { + final BBox bounds = primitive.getBBox(); + for (TileXY tile : boundsToTiles(bounds.getMinLat(), bounds.getMinLon(), bounds.getMaxLat(), bounds.getMaxLon(), lastZoom)) { + this.cache.remove(tile.toString()); + } + } + } + @Override public void projectionChanged(Projection oldValue, Projection newValue) { // No reprojection required. The dataset itself is registered as projection @@ -1307,6 +1481,25 @@ invalidate(); } + @Override + public void primitiveHovered(PrimitiveHoverEvent e) { + for (IPrimitive primitive : Arrays.asList(e.getHoveredPrimitive(), e.getPreviousPrimitive())) { + if (primitive == null || primitive.getDataSet() != this.getDataSet()) continue; + if (primitive instanceof IWay) { + for (INode n : ((IWay) primitive).getNodes()) { + final TileXY tile = latLonToTile(n.lat(), n.lon(), lastZoom); + this.cache.remove(tile.toString()); + } + } else { + final BBox box = primitive.getBBox(); + for (TileXY tile : boundsToTiles(box.getMinLat(), box.getMinLon(), box.getMaxLat(), box.getMaxLon(), lastZoom)) { + this.cache.remove(tile.toString()); + } + } + } + this.invalidate(); + } + @Override public void setName(String name) { if (data != null) {