Avoid re-reading the image file on every scaled drawImage - #3506
Open
vogella wants to merge 1 commit into
Open
Conversation
CachedImageAtSize.loadImageDataAtExactSize asked ImageDataLoader.isDynamicallySizable(fileName) whether the backing file can be loaded at an arbitrary size, then discarded the answer. For a format that cannot (PNG, GIF, JPEG, so nearly every icon) the cached image stayed null, so the open plus format sniff repeated on every draw. For SVG each cache miss opened the file twice, to sniff and to load. Sizability depends on the file alone and never on the requested size, so remember the file already found not to be sizable. The memo is keyed on the resolved file name, because an Image may be backed by different files. The new ImageLoader.loadBySizeIfDynamicallySizable does the check and the load from a single open. Opens per draw over 100 draws of one Image, strace on Linux/GTK: PNG, stable draw size 1.00 -> 0.01 PNG, alternating draw size 1.00 -> 0.01 SVG, stable draw size 0.02 -> 0.01 SVG, alternating draw size 2.00 -> 1.00 That is 1.5 to 2.5 us per draw on a 16x16 icon, and rendering is bit-identical. One behavior change: a file replaced in place at the same path is no longer re-sniffed for the lifetime of the Image. Fixes eclipse-platform#3505
Contributor
This was referenced Aug 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The scaled
GC.drawImageoverload askedisDynamicallySizable(fileName)on every draw and then discarded the answer, so for any PNG, GIF or JPEG the backing file was opened and sniffed again for each repaint, forever.Since sizability is a property of the file and never depends on the requested size, this remembers the file already found not to be sizable, keyed on the resolved file name so that an
Imagebacked by different files over its lifetime still behaves correctly.A new
ImageLoader.loadBySizeIfDynamicallySizabledoes the format check and the load from a single open, which also closes the window where the file could change between the two reads.Measured with
strace -f -e trace=openatover 100 draws of oneImageon Linux/GTK at device zoom 100, opens per draw drop from 1.00 to 0.01 for PNG (stable and alternating draw sizes) and from 2.00 to 1.00 for SVG at alternating sizes.That is roughly 1.5 to 2.5 us per draw on a 16x16 icon, and the rendered output is bit-identical before and after.
Andrey's concern on the issue about NFS-mounted config areas applies directly here, since this was one
openatper scaled draw on that path.Two limits of that table are worth stating so nobody reads it as covering HiDPI.
At device zoom 200 the same PNG case measures 2.00 opens per draw before and 1.00 after, rather than 0.01.
The open that remains is a different defect, #3507: the internal
drawImagereads the source dimensions fromgetImageData(), which re-decodes the file at any zoom other than 100.That one is fixed separately in #3510, and with both applied the case reaches 0.00.
This PR also does not fix
CachedImageAtSize.isReusable, which compares a pixel value against a point value:refreshconverts the requested size withDPIUtil.pointToPixel(destWidth, getDeviceZoom()), while the cached image'swidthis divided back down to points inImage.init(ImageData, zoom).At zoom 200 that comparison can never be true, so the single-entry size cache never hits there and every draw is a miss.
The change here makes each of those misses cheap rather than making them stop happening; the unit mismatch deserves its own fix.
One behavior change worth noting: a file replaced in place at the same path is no longer re-sniffed for the lifetime of the
Image.Fixes #3505