Modernize org.openstreetmap.josm.gui.layer.geoimage.ImageDisplay

[simon04]

1. ImageDisplay is using java.awt.MediaTracker.

According to ​https://stackoverflow.com/a/7369580/205629 (written in 2010):

MediaTracker was useful in 1995. Back then the primary GUI use of java was Applets, and Applets would usually load images slowly over the network.
...
These days you can typically load images synchronously, and it is best to use ImageIO.

2. ImageDisplay creates a new thread for each image to be loaded. We can use MainApplication.worker

attachment:20813-alpha.patch​​ contains 1. + 2.

3. Omit rescaling via ImageProvider.toBufferedImage and ImageProvider.createScaledImage? When zapping through images, approx. 50% of heap allocations are due to ImageIO.read() and approx. 50% are due to ImageProvider.toBufferedImage/ImageProvider.createScaledImage.

[taylor.smock]

My major concern is that this doesn't seem to handle large image files (e.g. 56 MP) or low memory conditions gracefully.

I'd have to double check with a high-res image and a low max memory, but I suspect it will throw an OOM.

mayFitMemory(((long) width)*height*4*2)

If I didn't screw up the math:
6 MP -> 48 MB
12 MP -> 96 MB
56 MP -> 448 MB (I think I've seen a 56 MP camera on a phone somewhere).

[Don-vip]

Ticket retargeted after milestone closed

[simon04]

Replying to taylor.smock:

My major concern is that this doesn't seem to handle large image files (e.g. 56 MP) or low memory conditions gracefully.

True. We could make use of ​IIOParam.setSourceSubsampling. Example code: ​https://stackoverflow.com/a/3296139/205629

Replying to Don-vip:

Ticket retargeted after milestone closed

Ups, thanks. I've meant to look into this in milestone:21.05.

[taylor.smock]

Replying to simon04:

[...]
True. We could make use of ​IIOParam.setSourceSubsampling. Example code: ​https://stackoverflow.com/a/3296139/205629

I think this would be significantly better than our current potential OOM solution (not showing the image).

From the linked discussion:

For extra optimization, I render the buffered image in tiles also...

I'll have to look into this -- it will probably make 360 imagery a bit more performant (I've been intending to copy/move the Mapillary Image viewer code into JOSM, but priorities...)

[simon04]

In 17834/josm:

see #20813 - Add ImageDisplayTest.testLoadImageRunnablePerformance

[simon04]

attachment:20813-beta.patch​​​ contains improvements 1. + 2. + 3. TODO: load higher resolution or correct source region when zooming in.

The patch reduces heap allocations for reading and rendering 85 images with size 6000x4000 from 2 GiB to 1 MiB.

 Method                                           Objects          Size (Allocations)
-ImageDisplayTest.testLoadImageRunnablePerformance() 2374 2_103_274_448
+ImageDisplayTest.testLoadImageRunnablePerformance()  569       983_304

[GerdP]

Sounds great!
Sorry for my poor understanding: In what situation do I want to load images into JOSM instead of using a dedicated program like IrfanView? I've tried that once but found that most of my images "were not found to be GPS tagged". This maybe caused by a bad configuration of my rather old smartfone (Sony Xperia Z3) or quite normal. I've not invested much time in this so far.
What can I do with my images in JOSM that I can't do when I open them in IrfanView? Is it about mapping or about modifying the EXIF data in the imgage?

[StephaneP]

Replying to GerdP:

Sounds great!
Sorry for my poor understanding: In what situation do I want to load images into JOSM instead of using a dedicated program like IrfanView?

Yes, Josm is very useful to:

• Geolocalize images with a gnss trace and an aerial imagery
• Edit images locations

And mappping with geolocalized images is very very useful. Here is an example:
​https://pbs.twimg.com/media/EhpPyrfWsAAlMy0.jpg

[GerdP]

My current workflow with mapping is this:
I cycle somewhere and stop from time to time to take some fotos of things that I want to map later PLUS one foto that shows my Garmin Oregon navi so that I know where I took this pictures. At home I

• load images with IrfanView
• load recorded gpx track into JOSM
• look at first image and find out where I took it
• load data around that place and use the images to improve the OSM data

If I got that right I can do this:

• load gpx track into JOSM
• load all images into JOSM
• let JOSM find out where the images where taken using the timestamps
• load data around the located places and use the images to improve the OSM data

I tried this but the results of the automatic correlation was very poor, so I gave up. Maybe I too stupid, maybe there is an error in JOSM or in my images. If this is supposed to work easily should I open a new ticket and upload my data?

[simon04]

Please don't discuss the relevance of ImageDisplay here, but focus on its performance/improvements. Thanks!

[Bjoeni]

Replying to GerdP:

I tried this but the results of the automatic correlation was very poor, so I gave up. Maybe I too stupid, maybe there is an error in JOSM or in my images. If this is supposed to work easily should I open a new ticket and upload my data?

Actually the autoguess function never worked for me either. That only works when the first picture is taken a the same time the track recording starts, so I guess it's only useful for very few cases. Usually you just have to set timezone and offset manually. Maybe the "Auto-Guess" function should be renamed or behave differently, it confused me as well. New ticket I guess?

---

Replying to simon04:

attachment:20813-beta.patch​​​ contains improvements 1. + 2. + 3. TODO: load higher resolution or correct source region when zooming in.

The patch reduces heap allocations for reading and rendering 85 images with size 6000x4000 from 2 GiB to 1 MiB.

 Method                                           Objects          Size (Allocations)
-ImageDisplayTest.testLoadImageRunnablePerformance() 2374 2_103_274_448
+ImageDisplayTest.testLoadImageRunnablePerformance()  569       983_304

Looks good! I agree that it's long overdue. I thought about doing it in #20341, but didn't have the time.

Some remarks about the getSubsampling() method:

• Sometimes the subsampling is a bit too agressive. It looks much better when using Math.floor() instead of Math.round(), since only pixels that are definitely not going to be shown should be discarded.
• I think the method doesn't work the way you intended it to: When both dimensions of the source image d1 are larger than those of the viewer d2 (which is the case most of the time), the subsampling is always based on the width, even though the height might be the limiting factor.

I think it should look something like this:

        private int getSubsampling(Dimension d1, Dimension d2) {
            if (d1.getWidth() > d2.getWidth() || d1.getHeight() > d2.getHeight()) {
                return (int) Math.floor(Math.max(
                        d1.getWidth() / d2.getWidth(),
                        d1.getHeight() / d2.getHeight()));
            }
            
            return 1;
        }

[simon04]

In 17856/josm:

see #20813 - Add ThumbsLoaderTest.testPerformance

[Adrian]

Replying to taylor.smock:

I'll have to look into this -- it will probably make 360 imagery a bit more performant (I've been intending to copy/move the Mapillary Image viewer code into JOSM, but priorities...)

(OT) +1 for making the image viewer handle 360° panoramas. As a workaround, I coded a patch #13815 to open an image in an external viewer, but the code was not sufficiently polished to be incorporated into JOSM. I am still using the patch, updated for changes in JOSM.

[simon04]

In 17870/josm:

see #20813 - ThumbsLoaderTest: add assertions

[simon04]

In 17871/josm:

see #20813 - Modernize ImageDisplay using ImageIO and subsampling

[simon04]

In 17872/josm:

see #20813 - Apply EXIF rotation in ImageEntry.load

[simon04]

In 17873/josm:

see #20813 - Modernize ThumbsLoader using ImageIO and subsampling

[simon04]

Replying to simon04:

attachment:20813-beta.patch​​​ contains improvements 1. + 2. + 3. TODO: load higher resolution or correct source region when zooming in.

The patch reduces heap allocations for reading and rendering 85 images with size 6000x4000 from 2 GiB to 1 MiB.

I haven't implemented this aggressive subsampling to keep the high resolution for zooming in. If anybody is interested to implement reloading in a higher resolution only when necessary, please go ahead. I will not be working on that in the forseable future.

The maximum width/height (given in pixels) to load can be changed using the advanced preference geoimage.maximum-width (defaulting to 6000px).