Index: /trunk/src/org/openstreetmap/josm/data/osm/visitor/paint/StyledTiledMapRenderer.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/osm/visitor/paint/StyledTiledMapRenderer.java (revision 19219) +++ /trunk/src/org/openstreetmap/josm/data/osm/visitor/paint/StyledTiledMapRenderer.java (revision 19220) @@ -8,4 +8,5 @@ import java.awt.Font; import java.awt.Graphics2D; +import java.awt.GraphicsConfiguration; import java.awt.Image; import java.awt.Point; @@ -13,5 +14,4 @@ import java.awt.Transparency; import java.awt.event.MouseEvent; -import java.awt.geom.AffineTransform; import java.awt.image.BufferedImage; import java.util.ArrayList; @@ -22,7 +22,7 @@ import java.util.Map; import java.util.Objects; -import java.util.Optional; import java.util.Set; import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; import java.util.function.Consumer; import java.util.stream.Collectors; @@ -52,4 +52,5 @@ private int zoom; private Consumer notifier; + private final ExecutorService worker; /** @@ -65,4 +66,5 @@ public StyledTiledMapRenderer(Graphics2D g, NavigatableComponent nc, boolean isInactiveMode) { super(g, nc, isInactiveMode); + this.worker = MainApplication.worker; } @@ -74,9 +76,9 @@ return; } - final Executor worker = MainApplication.worker; + final Executor worker = this.worker; final BufferedImage tempImage; final Graphics2D tempG2d; // I'd like to avoid two image copies, but there are some issues using the original g2d object - tempImage = nc.getGraphicsConfiguration().createCompatibleImage(this.nc.getWidth(), this.nc.getHeight(), Transparency.TRANSLUCENT); + tempImage = createCompatibleImage(nc, this.nc.getWidth(), this.nc.getHeight()); tempG2d = tempImage.createGraphics(); tempG2d.setComposite(AlphaComposite.DstAtop); // Avoid tile lines in large areas @@ -94,5 +96,5 @@ final Point min = this.nc.getPoint(box2.getMin()); final Point max = this.nc.getPoint(box2.getMax()); - tileSize = max.x - min.x + BUFFER_PIXELS; + tileSize = max.x - min.x; } @@ -151,6 +153,7 @@ painted++; } - // There seems to be an off-by-one error somewhere. - tempG2d.drawImage(tileImage, point.x + 1, point.y + 1, null, null); + // There seems to be an off-by-one error somewhere. Seems to be tied to sign of lat/lon + final int offset = (tile.lat() > 0 ? 1 : 0) + (tile.lon() >= 0 ? 1 : 0); + tempG2d.drawImage(tileImage, point.x + 1, point.y + offset, null, null); } else { Logging.trace("StyledMapRenderer did not paint tile {1}", tile); @@ -250,11 +253,8 @@ temporaryView.zoomTo(bounds.getCenter().getEastNorth(ProjectionRegistry.getProjection()), mapState.getScale()); - BufferedImage bufferedImage = Optional.ofNullable(nc.getGraphicsConfiguration()) - .map(gc -> gc.createCompatibleImage(tileSize * xCount + xCount, tileSize * yCount + yCount, Transparency.TRANSLUCENT)) - .orElseGet(() -> new BufferedImage(tileSize * xCount + xCount, tileSize * yCount + yCount, BufferedImage.TYPE_INT_ARGB)); + BufferedImage bufferedImage = createCompatibleImage(nc, width, height); Graphics2D g2d = bufferedImage.createGraphics(); try { g2d.setRenderingHints(Map.of(RenderingHints.KEY_ANTIALIASING, RenderingHints.VALUE_ANTIALIAS_ON)); - g2d.setTransform(AffineTransform.getTranslateInstance(-BUFFER_TILES * (double) tileSize, -BUFFER_TILES * (double) tileSize)); final AbstractMapRenderer tilePainter = MapRendererFactory.getInstance().createActiveRenderer(g2d, temporaryView, false); tilePainter.render(data, true, bounds); @@ -262,5 +262,15 @@ g2d.dispose(); } - return bufferedImage; + final int bufferPixels = BUFFER_TILES * tileSize; + return bufferedImage.getSubimage(bufferPixels, bufferPixels, + width - 2 * bufferPixels + BUFFER_PIXELS, height - 2 * bufferPixels + BUFFER_PIXELS); + } + + private static BufferedImage createCompatibleImage(NavigatableComponent nc, int width, int height) { + final GraphicsConfiguration gc = nc.getGraphicsConfiguration(); + if (gc == null) { + return new BufferedImage(width, height, BufferedImage.TYPE_INT_ARGB); + } + return gc.createCompatibleImage(width, height, Transparency.TRANSLUCENT); } @@ -319,9 +329,9 @@ for (TileLoader loader : tileCollection) { final TileZXY txy = loader.tile; - final int x = (txy.x() - minX) * (tileSize - BUFFER_PIXELS) + BUFFER_PIXELS / 2; - final int y = (txy.y() - minY) * (tileSize - BUFFER_PIXELS) + BUFFER_PIXELS / 2; - final int wh = tileSize - BUFFER_PIXELS / 2; - - final BufferedImage tileImage = tImage.getSubimage(x, y, wh, wh); + final int x = (txy.x() - minX) * tileSize; + final int y = (txy.y() - minY) * tileSize; + final int wh = tileSize; + + final BufferedImage tileImage = tImage.getSubimage(x, y, wh + BUFFER_PIXELS, wh + BUFFER_PIXELS); loader.cacheTile(tileImage); } Index: /trunk/src/org/openstreetmap/josm/data/osm/visitor/paint/TileZXY.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/osm/visitor/paint/TileZXY.java (revision 19219) +++ /trunk/src/org/openstreetmap/josm/data/osm/visitor/paint/TileZXY.java (revision 19220) @@ -9,5 +9,6 @@ /** - * A record used for storing tile information for painting + * A record used for storing tile information for painting. + * The origin is upper-left, not lower-left (so more like Google tile coordinates than TMS tile coordinates). * @since 19176 */ @@ -144,7 +145,8 @@ */ public static TileZXY 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); + final double zoom2 = Math.pow(2, zoom); + final double latLog = Math.log(Math.tan(Math.toRadians(lat)) + 1 / Math.cos(Math.toRadians(lat))); + final int xCoord = (int) Math.floor(zoom2 * (180 + lon) / 360); + final int yCoord = (int) Math.floor(zoom2 * (1 - latLog / Math.PI) / 2); return new TileZXY(zoom, xCoord, yCoord); } Index: /trunk/test/functional/org/openstreetmap/josm/gui/mappaint/MapCSSRendererTest.java =================================================================== --- /trunk/test/functional/org/openstreetmap/josm/gui/mappaint/MapCSSRendererTest.java (revision 19219) +++ /trunk/test/functional/org/openstreetmap/josm/gui/mappaint/MapCSSRendererTest.java (revision 19220) @@ -222,4 +222,18 @@ } final BufferedImage reference = ImageIO.read(referenceImageFile); + assertImageEquals(testIdentifier, reference, image, thresholdPixels, thresholdTotalColorDiff, diffImageConsumer); + } + + /** + * Compares the reference image file with the actual images given as {@link BufferedImage}. + * @param testIdentifier a test identifier for error messages + * @param reference the reference image + * @param image the actual image + * @param thresholdPixels maximum number of differing pixels + * @param thresholdTotalColorDiff maximum sum of color value differences + * @param diffImageConsumer a consumer for a rendered image highlighting the differing pixels, may be null + */ + public static void assertImageEquals(String testIdentifier, BufferedImage reference, BufferedImage image, + int thresholdPixels, int thresholdTotalColorDiff, Consumer diffImageConsumer) { assertEquals(reference.getWidth(), image.getWidth()); assertEquals(reference.getHeight(), image.getHeight()); Index: /trunk/test/unit/org/openstreetmap/josm/data/osm/visitor/paint/StyledTiledMapRendererTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/data/osm/visitor/paint/StyledTiledMapRendererTest.java (revision 19220) +++ /trunk/test/unit/org/openstreetmap/josm/data/osm/visitor/paint/StyledTiledMapRendererTest.java (revision 19220) @@ -0,0 +1,148 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.data.osm.visitor.paint; + +import static org.openstreetmap.josm.gui.mappaint.MapCSSRendererTest.assertImageEquals; + +import java.awt.Graphics2D; +import java.awt.image.BufferedImage; +import java.io.File; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.Arrays; +import java.util.Map; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.function.Function; +import java.util.function.Supplier; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import java.util.stream.Stream; + +import javax.imageio.ImageIO; + +import org.apache.commons.jcs3.access.CacheAccess; +import org.awaitility.Awaitility; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.openstreetmap.josm.TestUtils; +import org.openstreetmap.josm.data.Bounds; +import org.openstreetmap.josm.data.cache.JCSCacheManager; +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.gui.MainApplication; +import org.openstreetmap.josm.gui.NavigatableComponent; +import org.openstreetmap.josm.testutils.annotations.Main; +import org.openstreetmap.josm.testutils.annotations.Projection; + +/** + * Test class for {@link StyledTiledMapRenderer} + */ +@Main +@Projection +class StyledTiledMapRendererTest { + static Stream testRender() { + final Function generateNodes = tile -> { + final Bounds bounds = TileZXY.tileToBounds(tile); + return new OsmPrimitive[] { + new Node(bounds.getCenter()), + new Node(bounds.getMin()), + new Node(bounds.getMax()), + new Node(new LatLon(bounds.getMinLat(), bounds.getMaxLon())), + new Node(new LatLon(bounds.getMaxLat(), bounds.getMinLon())), + }; + }; + // Everything but the 5 point nodes is just to make it easier to figure out why it is failing + final Function> generateTests = tile -> Stream.of( + Arguments.of("Center node", (Supplier) () -> new DataSet(generateNodes.apply(tile)[0]), tile) + ); + return Stream.concat( + // Tiles around 0, 0 + IntStream.rangeClosed(2097151, 2097152).mapToObj(x -> IntStream.rangeClosed(2097151, 2097152) + .mapToObj(y -> new TileZXY(22, x, y))).flatMap(Function.identity()), + // Tiles in the four quadrants far away from 0, 0 + Stream.of(new TileZXY(16, 13005, 25030), + new TileZXY(14, 5559, 10949), + new TileZXY(15, 31687, 21229), + new TileZXY(13, 8135, 2145))) + .flatMap(generateTests); + } + + @ParameterizedTest(name = "{0} - {2}") + @MethodSource + void testRender(String testIdentifier, final Supplier dataSetSupplier, final TileZXY tile) + throws InterruptedException, ExecutionException { + final int zoom = tile.zoom(); + final Bounds viewArea = TileZXY.tileToBounds(tile); + final CacheAccess cache = JCSCacheManager.getCache("StyledTiledMapRendererTest:testRender"); + cache.clear(); + final DataSet ds = dataSetSupplier.get(); + final NavigatableComponent nc = new NavigatableComponent() { + @Override + public int getWidth() { + return 800; + } + + @Override + public int getHeight() { + return 600; + } + }; + nc.zoomTo(viewArea); + final ExecutorService worker = MainApplication.worker; + final BufferedImage oldRenderStyle = render((g2d) -> new StyledMapRenderer(g2d, nc, false), ds, nc); + final Function newRenderer = (g2d) -> { + StyledTiledMapRenderer stmr = new StyledTiledMapRenderer(g2d, nc, false); + stmr.setCache(viewArea, cache, zoom, ignored -> { /* ignored */ }); + return stmr; + }; + // First "renders" schedules off-thread rendering. We need to loop here since a render call may only schedule a small amount of tiles. + int size = -1; + while (size != cache.getMatching(".*").size()) { + size = cache.getMatching(".*").size(); + render(newRenderer, ds, nc); + Awaitility.await().until(() -> cache.getMatching(".*").values().stream().allMatch(i -> i.imageFuture() == null)); + } + worker.submit(() -> { /* Sync */ }).get(); + // Second render actually does the painting + final BufferedImage newRenderStyle = render(newRenderer, ds, nc); + worker.submit(() -> { /* Sync */ }).get(); + var entries = cache.getMatching(".*").entrySet().stream().filter(e -> { + BufferedImage image = (BufferedImage) e.getValue().image(); + return Arrays.stream(image.getRGB(0, 0, image.getWidth(), image.getHeight(), + null, 0, image.getWidth())).anyMatch(i -> i != 0); + }).collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue().image())); + try { + assertImageEquals(testIdentifier, oldRenderStyle, newRenderStyle, 0, 0, diff -> { + try { + if (!Files.isDirectory(Paths.get(TestUtils.getTestDataRoot(), "output"))) { + Files.createDirectories(Paths.get(TestUtils.getTestDataRoot(), "output")); + } + final String basename = TestUtils.getTestDataRoot() + "output/" + + testIdentifier + ' ' + tile.zoom() + '-' + tile.x() + '-' + tile.y(); + ImageIO.write(diff, "png", new File(basename + "-diff.png")); + ImageIO.write(newRenderStyle, "png", new File(basename + "-new.png")); + ImageIO.write(oldRenderStyle, "png", new File(basename + "-old.png")); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }); + } finally { + cache.clear(); + } + } + + private static BufferedImage render(Function renderer, + final DataSet ds, final NavigatableComponent nc) { + final BufferedImage bufferedImage = new BufferedImage(nc.getWidth(), nc.getHeight(), BufferedImage.TYPE_INT_ARGB); + final Graphics2D g2d = bufferedImage.createGraphics(); + final StyledMapRenderer styledMapRenderer = renderer.apply(g2d); + styledMapRenderer.render(ds, true, nc.getRealBounds()); + g2d.dispose(); + return bufferedImage; + } +} Index: /trunk/test/unit/org/openstreetmap/josm/data/osm/visitor/paint/TileZXYTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/data/osm/visitor/paint/TileZXYTest.java (revision 19220) +++ /trunk/test/unit/org/openstreetmap/josm/data/osm/visitor/paint/TileZXYTest.java (revision 19220) @@ -0,0 +1,59 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.data.osm.visitor.paint; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.stream.Stream; + +import nl.jqno.equalsverifier.EqualsVerifier; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.openstreetmap.josm.data.Bounds; + +class TileZXYTest { + static Stream testBBoxCalculation() { + return Stream.of( + Arguments.of(new Bounds(-85.0511288, -180, 85.0511288, 180), new TileZXY(0, 0, 0)), + Arguments.of(new Bounds(0, -180, 85.0511288, 0), new TileZXY(1, 0, 0)), + Arguments.of(new Bounds(0, 0, 85.0511288, 180), new TileZXY(1, 1, 0)), + Arguments.of(new Bounds(-85.0511288, -180, 0, 0), new TileZXY(1, 0, 1)), + Arguments.of(new Bounds(-85.0511288, 0, 0, 180), new TileZXY(1, 1, 1)), + Arguments.of(new Bounds(0, 0, 0.0006866, 0.0006866), new TileZXY(19, 262144, 262143)), + Arguments.of(new Bounds(0, -0.0006866, 0.0006866, 0), new TileZXY(19, 262143, 262143)), + Arguments.of(new Bounds(-0.0006866, -0.0006866, 0, 0), new TileZXY(19, 262143, 262144)), + Arguments.of(new Bounds(-0.0006866, 0, 0, 0.0006866), new TileZXY(19, 262144, 262144)) + ); + } + + @ParameterizedTest + @MethodSource + void testBBoxCalculation(Bounds expected, TileZXY tile) { + assertEquals(expected, TileZXY.tileToBounds(tile)); + } + + static Stream testTileFromLatLon() { + final double delta = 0.00001; // Purely to get off of tile boundaries + return testBBoxCalculation().flatMap(arg -> { + final Bounds expected = (Bounds) arg.get()[0]; + final TileZXY tile = (TileZXY) arg.get()[1]; + return Stream.of(Arguments.of("UL", expected.getMaxLat() - delta, expected.getMinLon() + delta, tile), + Arguments.of("LL", expected.getMinLat() + delta, expected.getMinLon() + delta, tile), + Arguments.of("UR", expected.getMaxLat() - delta, expected.getMaxLon() - delta, tile), + Arguments.of("LR", expected.getMinLat() + delta, expected.getMaxLon() - delta, tile), + Arguments.of("Center", expected.getCenter().lat(), expected.getCenter().lon(), tile)); + }); + } + + @ParameterizedTest(name = "{3} - {0}") + @MethodSource + void testTileFromLatLon(String description, double lat, double lon, TileZXY tile) { + assertEquals(tile, TileZXY.latLonToTile(lat, lon, tile.zoom())); + } + + @Test + void testEqualsContract() { + EqualsVerifier.forClass(TileZXY.class).verify(); + } +}