commit 641cea8f4f7e5fc8a65a212a1d0ad30c8f8b38d7 Author: Simon Legner Date: 2020-11-23 23:26:51 +0100 see #20141 - ImageProvider: cache rendered SVG images using JCS diff --git a/src/org/openstreetmap/josm/data/cache/BufferedImageCacheEntry.java b/src/org/openstreetmap/josm/data/cache/BufferedImageCacheEntry.java index 3b908d5a9..2e637a12d 100644 --- a/src/org/openstreetmap/josm/data/cache/BufferedImageCacheEntry.java +++ b/src/org/openstreetmap/josm/data/cache/BufferedImageCacheEntry.java @@ -3,7 +3,9 @@ import java.awt.image.BufferedImage; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.UncheckedIOException; import javax.imageio.ImageIO; @@ -29,6 +31,21 @@ public BufferedImageCacheEntry(byte[] content) { super(content); } + /** + * Encodes the given image as PNG and returns a cache entry + * @param img the image + * @return a cache entry for the PNG encoded image + * @throws UncheckedIOException if an I/O error occurs + */ + public static BufferedImageCacheEntry pngEncoded(BufferedImage img) { + try (ByteArrayOutputStream output = new ByteArrayOutputStream()) { + ImageIO.write(img, "png", output); + return new BufferedImageCacheEntry(output.toByteArray()); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + /** * Returns BufferedImage from for the content. Subsequent calls will return the same instance, * to reduce overhead of ImageIO diff --git a/src/org/openstreetmap/josm/data/cache/JCSCacheManager.java b/src/org/openstreetmap/josm/data/cache/JCSCacheManager.java index a32238ad7..d3be3a6e3 100644 --- a/src/org/openstreetmap/josm/data/cache/JCSCacheManager.java +++ b/src/org/openstreetmap/josm/data/cache/JCSCacheManager.java @@ -177,6 +177,7 @@ private JCSCacheManager() { if (cachePath != null && cacheDirLock != null) { IDiskCacheAttributes diskAttributes = getDiskCacheAttributes(maxDiskObjects, cachePath, cacheName); + Logging.debug("Setting up cache: {0}", diskAttributes); try { if (cc.getAuxCaches().length == 0) { cc.setAuxCaches(new AuxiliaryCache[]{DISK_CACHE_FACTORY.createCache( diff --git a/src/org/openstreetmap/josm/gui/layer/geoimage/ThumbsLoader.java b/src/org/openstreetmap/josm/gui/layer/geoimage/ThumbsLoader.java index bd24e6bf8..9579ed839 100644 --- a/src/org/openstreetmap/josm/gui/layer/geoimage/ThumbsLoader.java +++ b/src/org/openstreetmap/josm/gui/layer/geoimage/ThumbsLoader.java @@ -8,14 +8,12 @@ import java.awt.Toolkit; import java.awt.geom.AffineTransform; import java.awt.image.BufferedImage; -import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; +import java.io.UncheckedIOException; import java.util.ArrayList; import java.util.Collection; -import javax.imageio.ImageIO; - import org.apache.commons.jcs3.access.behavior.ICacheAccess; import org.openstreetmap.josm.data.cache.BufferedImageCacheEntry; import org.openstreetmap.josm.data.cache.JCSCacheManager; @@ -164,10 +162,9 @@ private BufferedImage loadThumb(ImageEntry entry) { } if (!cacheOff && cache != null) { - try (ByteArrayOutputStream output = new ByteArrayOutputStream()) { - ImageIO.write(scaledBI, "png", output); - cache.put(cacheIdent, new BufferedImageCacheEntry(output.toByteArray())); - } catch (IOException e) { + try { + cache.put(cacheIdent, BufferedImageCacheEntry.pngEncoded(scaledBI)); + } catch (UncheckedIOException e) { Logging.warn("Failed to save geoimage thumb to cache"); Logging.warn(e); } diff --git a/src/org/openstreetmap/josm/tools/ImageProvider.java b/src/org/openstreetmap/josm/tools/ImageProvider.java index 67d424bcd..75a429f87 100644 --- a/src/org/openstreetmap/josm/tools/ImageProvider.java +++ b/src/org/openstreetmap/josm/tools/ImageProvider.java @@ -63,7 +63,9 @@ import javax.swing.ImageIcon; import javax.xml.parsers.ParserConfigurationException; +import org.apache.commons.jcs3.access.behavior.ICacheAccess; import org.openstreetmap.josm.data.Preferences; +import org.openstreetmap.josm.data.cache.JCSCacheManager; import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.OsmPrimitiveType; import org.openstreetmap.josm.io.CachedFile; @@ -983,7 +985,7 @@ private static ImageResource getIfAvailableHttp(String url, ImageType type) { URI uri = getSvgUniverse().loadSVG(is, Utils.fileToURL(cf.getFile()).toString()); svg = getSvgUniverse().getDiagram(uri); } - return svg == null ? null : new ImageResource(svg); + return svg == null ? null : new ImageResource(url, svg); case OTHER: BufferedImage img = null; try { @@ -991,7 +993,7 @@ private static ImageResource getIfAvailableHttp(String url, ImageType type) { } catch (IOException | UnsatisfiedLinkError e) { Logging.log(Logging.LEVEL_WARN, "Exception while reading HTTP image:", e); } - return img == null ? null : new ImageResource(img); + return img == null ? null : new ImageResource(url, img); default: throw new AssertionError("Unsupported type: " + type); } @@ -1040,7 +1042,7 @@ private static ImageResource getIfAvailableDataUrl(String url) { Logging.warn("Unable to process svg: "+s); return null; } - return new ImageResource(svg); + return new ImageResource(url, svg); } else { try { // See #10479: for PNG files, always enforce transparency to be sure tNRS chunk is used even not in paletted mode @@ -1049,7 +1051,7 @@ private static ImageResource getIfAvailableDataUrl(String url) { // hg.openjdk.java.net/jdk8u/jdk8u/jdk/file/dc4322602480/src/share/classes/com/sun/imageio/plugins/png/PNGImageReader.java#l656 // CHECKSTYLE.ON: LineLength Image img = read(new ByteArrayInputStream(bytes), false, true); - return img == null ? null : new ImageResource(img); + return img == null ? null : new ImageResource(url, img); } catch (IOException | UnsatisfiedLinkError e) { Logging.log(Logging.LEVEL_WARN, "Exception while reading image:", e); } @@ -1124,7 +1126,7 @@ private static ImageResource getIfAvailableZip(String fullName, File archive, St URI uri = getSvgUniverse().loadSVG(is, entryName); svg = getSvgUniverse().getDiagram(uri); } - return svg == null ? null : new ImageResource(svg); + return svg == null ? null : new ImageResource(fullName, svg); case OTHER: while (size > 0) { int l = is.read(buf, offs, size); @@ -1137,7 +1139,7 @@ private static ImageResource getIfAvailableZip(String fullName, File archive, St } catch (IOException | UnsatisfiedLinkError e) { Logging.warn(e); } - return img == null ? null : new ImageResource(img); + return img == null ? null : new ImageResource(fullName, img); default: throw new AssertionError("Unknown ImageType: "+type); } @@ -1159,7 +1161,7 @@ private static ImageResource getIfAvailableZip(String fullName, File archive, St private static ImageResource getIfAvailableLocalURL(URL path, ImageType type) { switch (type) { case SVG: - SVGDiagram svg = null; + return new ImageResource(path.toString(), () -> { synchronized (getSvgUniverse()) { try { URI uri = null; @@ -1175,12 +1177,13 @@ private static ImageResource getIfAvailableLocalURL(URL path, ImageType type) { uri = getSvgUniverse().loadSVG(betterPath); } } - svg = getSvgUniverse().getDiagram(uri); + return getSvgUniverse().getDiagram(uri); } catch (SecurityException | IOException e) { Logging.log(Logging.LEVEL_WARN, "Unable to read SVG", e); } } - return svg == null ? null : new ImageResource(svg); + return null; + }); case OTHER: BufferedImage img = null; try { @@ -1195,7 +1198,7 @@ private static ImageResource getIfAvailableLocalURL(URL path, ImageType type) { Logging.log(Logging.LEVEL_WARN, "Unable to read image", e); Logging.debug(e); } - return img == null ? null : new ImageResource(img); + return img == null ? null : new ImageResource(path.toString(), img); default: throw new AssertionError(); } @@ -1393,7 +1396,7 @@ static Image getCursorImage(String name, String overlay, UnaryOperator imgCache = new ConcurrentHashMap<>(4); + private static final ICacheAccess imgCache = JCSCacheManager.getCache( + "images", 10, 10000, + new File(Config.getDirs().getCacheDirectory(true), "images").getAbsolutePath()); + private final String cacheKey; /** * SVG diagram information in case of SVG vector image. */ private SVGDiagram svg; + private Supplier svgSupplier; /** * Use this dimension to request original file dimension. */ @@ -54,20 +66,27 @@ /** * Constructs a new {@code ImageResource} from an image. + * @param cacheKey the caching identifier of the image * @param img the image */ - public ImageResource(Image img) { - CheckParameterUtil.ensureParameterNotNull(img); - baseImage = img; + public ImageResource(String cacheKey, Image img) { + this.cacheKey = Objects.requireNonNull(cacheKey); + this.baseImage = Objects.requireNonNull(img); } /** * Constructs a new {@code ImageResource} from SVG data. + * @param cacheKey the caching identifier of the image * @param svg SVG data */ - public ImageResource(SVGDiagram svg) { - CheckParameterUtil.ensureParameterNotNull(svg); - this.svg = svg; + public ImageResource(String cacheKey, SVGDiagram svg) { + this.cacheKey = Objects.requireNonNull(cacheKey); + this.svg = Objects.requireNonNull(svg); + } + + public ImageResource(String cacheKey, Supplier svgSupplier) { + this.cacheKey = Objects.requireNonNull(cacheKey); + this.svgSupplier = Objects.requireNonNull(svgSupplier); } /** @@ -77,7 +96,9 @@ public ImageResource(SVGDiagram svg) { * @since 8095 */ public ImageResource(ImageResource res, List overlayInfo) { + this.cacheKey = res.cacheKey; this.svg = res.svg; + this.svgSupplier = res.svgSupplier; this.baseImage = res.baseImage; this.overlayInfo = overlayInfo; } @@ -158,6 +179,15 @@ ImageIcon getImageIcon(Dimension dim, boolean multiResolution, ImageResizeMode r return getImageIconAlreadyScaled(GuiSizesHelper.getDimensionDpiAdjusted(dim), multiResolution, false, resizeMode); } + private BufferedImage getImageFromCache(String cacheKey) { + try { + BufferedImageCacheEntry cacheEntry = imgCache.get(cacheKey); + return cacheEntry == null ? null : cacheEntry.getImage(); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + /** * Get an ImageIcon object for the image of this resource. A potential UI scaling is assumed * to be already taken care of, so dim is already scaled accordingly. @@ -180,9 +210,13 @@ ImageIcon getImageIconAlreadyScaled(Dimension dim, boolean multiResolution, bool } else if (resizeMode == null) { resizeMode = ImageResizeMode.BOUNDED; } - final int cacheKey = resizeMode.cacheKey(dim); - BufferedImage img = imgCache.get(cacheKey); + final String cacheKey = this.cacheKey + "--" + Integer.toHexString(resizeMode.cacheKey(dim)); + BufferedImage img = getImageFromCache(cacheKey); if (img == null) { + if (svgSupplier != null) { + svg = svgSupplier.get(); + svgSupplier = null; + } if (svg != null) { img = ImageProvider.createImageFromSvg(svg, dim, resizeMode); if (img == null) { @@ -210,7 +244,10 @@ ImageIcon getImageIconAlreadyScaled(Dimension dim, boolean multiResolution, bool img = new BufferedImage(img.getWidth(), img.getHeight(), BufferedImage.TYPE_4BYTE_ABGR); disabledIcon.paintIcon(new JPanel(), img.getGraphics(), 0, 0); } - imgCache.put(cacheKey, img); + if (img == null) { + return null; + } + imgCache.put(cacheKey, BufferedImageCacheEntry.pngEncoded(img)); } if (!multiResolution) diff --git a/test/unit/org/openstreetmap/josm/gui/tagging/presets/TaggingPresetReaderTest.java b/test/unit/org/openstreetmap/josm/gui/tagging/presets/TaggingPresetReaderTest.java index c4cf52276..bec7fc418 100644 --- a/test/unit/org/openstreetmap/josm/gui/tagging/presets/TaggingPresetReaderTest.java +++ b/test/unit/org/openstreetmap/josm/gui/tagging/presets/TaggingPresetReaderTest.java @@ -93,5 +93,6 @@ void testReadDefaulPresets() throws SAXException, IOException { String presetfile = "resource://data/defaultpresets.xml"; final Collection presets = TaggingPresetReader.readAll(presetfile, true); Assert.assertTrue("Default presets are empty", presets.size() > 0); + TaggingPresetsTest.waitForIconLoading(presets); } }