Modify

Opened 9 months ago

Closed 8 months ago

Last modified 8 months ago

#15476 closed enhancement (fixed)

[patch] Antialiased text and better resize quality when viewing photos

Reported by: naoliv Owned by: team
Priority: normal Milestone: 17.10
Component: Core image mapping Version:
Keywords: picture photo resize quality geoimage Cc:

Description

If possible, could we have antialiased text and a better resize algorithm when viewing photos in JOSM?

https://i.imgur.com/ByJiIWW.png

If we compare to the same picture displayed when seeing a GPX file we can see the difference:
https://i.imgur.com/eK94s29.png

JOSM:

Build-Date:2017-10-24 09:00:44
Revision:13034
Is-Local-Build:true

Identification: JOSM/1.5 (13034 SVN pt_BR) Linux Debian GNU/Linux testing (buster)
Memory Usage: 3267 MB / 6372 MB (440 MB allocated, but free)
Java version: 1.8.0_144-8u144-b01-1-b01, Oracle Corporation, OpenJDK 64-Bit Server VM
Screen: :0.0 1600x900, :0.1 1280x1024
Maximum Screen Size: 1600x1024
Java package: openjdk-8-jre:amd64-8u144-b01-1
Java ATK Wrapper package: libatk-wrapper-java:all-0.33.3-13
VM arguments: [-Dawt.useSystemAAFontSettings=on]
Dataset consistency test: No problems found

Plugins:
+ Create_grid_of_ways (32699)
+ FastDraw (33583)
+ ImportImagePlugin (33563)
+ OpeningHoursEditor (33185)
+ PicLayer (33385)
+ RoadSigns (33579)
+ SimplifyArea (33004)
+ apache-commons (33668)
+ buildings_tools (33004)
+ conflation (0.5.5)
+ contourmerge (1032)
+ download_along (33710)
+ editgpx (33004)
+ ejml (32680)
+ geojson (66)
+ geotools (33380)
+ importvec (33564)
+ indoorhelper (33632)
+ jts (32699)
+ log4j (32699)
+ measurement (33088)
+ merge-overlap (33436)
+ opendata (33617)
+ pbf (33568)
+ pdfimport (33579)
+ poly (33570)
+ reverter (33572)
+ scripting (30775)
+ tageditor (33579)
+ todo (30303)
+ turnlanes (33294)
+ turnlanes-tagging (254)
+ turnrestrictions (33537)
+ undelete (33480)
+ utilsplugin2 (33704)

Attachments (2)

josm-r13123-geoimage-rendering-regression-from-13038.jpeg (37.6 KB) - added by cmuelle8 8 months ago.
josm-r13123-geoimage-rendering-regression-from-13038.jpeg
josm-13038-geoimage-zoom-regression-fix-and-minor-usability-enhancements.patch (34.7 KB) - added by cmuelle8 8 months ago.
josm-13038-geoimage-zoom-regression-fix-and-minor-usability-enhancements.patch

Download all attachments as: .zip

Change History (28)

comment:1 Changed 9 months ago by Don-vip

can you please attach the gpx file + the photo?

comment:2 Changed 9 months ago by Don-vip

Ah my bad, didn't see #15475

comment:3 Changed 9 months ago by naoliv

Yes, you can use the same example.
Sorry that I didn't refer to it.

comment:4 Changed 9 months ago by Don-vip

Milestone: 17.10

comment:5 Changed 9 months ago by Don-vip

Component: CoreCore image mapping

comment:6 Changed 9 months ago by Don-vip

Resolution: fixed
Status: newclosed

In 13038/josm:

fix #15476 - Antialiased text and better resize quality when viewing photos

comment:7 Changed 9 months ago by Don-vip

Should be ok now, please test :)

comment:8 Changed 9 months ago by naoliv

Much better now :-)
Thanks!

Changed 8 months ago by cmuelle8

josm-r13123-geoimage-rendering-regression-from-13038.jpeg

comment:9 Changed 8 months ago by cmuelle8

Keywords: geoimage added
Resolution: fixed
Status: closedreopened

r13038 produced regression bugs with visible artefacts when image scaling some images.

openjdk version "1.8.0_151"
OpenJDK Runtime Environment (build 1.8.0_151-8u151-b12-0ubuntu0.17.04.2-b12)
OpenJDK 64-Bit Server VM (build 25.151-b12, mixed mode)

For a test image see e.g. https://commons.wikimedia.org/wiki/File:Vaals-Bokkebosje_(1).JPG

The thumbnail scaling (best fit) worked, but then, on each zoom level until the default scaling is employed, results are often just a white rectangle or a displaced fraction of the image. See https://josm.openstreetmap.de/attachment/ticket/15476/josm-r13123-geoimage-rendering-regression-from-13038.jpeg and #15511.

Attached patch fixes the regression and also makes geoimage plugin more configurable through prefs (adjustable max zoom, zoom-step, click zooming with mouse buttons (e.g. if a mouse wheel is not present). It also handles images with larger dimensions and/or larger maximum scaling.

Last edited 8 months ago by cmuelle8 (previous) (diff)

comment:10 Changed 8 months ago by cmuelle8

Summary: Antialiased text and better resize quality when viewing photos[patch] Antialiased text and better resize quality when viewing photos
Type: enhancementdefect

comment:11 Changed 8 months ago by cmuelle8

btw, what is the policy on malformed double prop in preferences? e.g. if user entered foo123.45bar instead of a valid double

prop.get() will throw an exception and log it as warning each time, before returning the default value of pref.
(the exception is caught and not promoted (thrown again) in AbstractProperty).

Should such malformed values be reset programmatically in a prefChangedListener?

    if (prop.get() == prop.getDefaultValue())
         prop.put(defaultValue);  // or prop.remove();

Or should that be left to the user to fix?
.. after all he will note that malformed value will not be in effect.
(I have not checked how its done over other parts of josm's code base, hence the question.)

Last edited 8 months ago by cmuelle8 (previous) (diff)

Changed 8 months ago by cmuelle8

josm-13038-geoimage-zoom-regression-fix-and-minor-usability-enhancements.patch

comment:12 Changed 8 months ago by cmuelle8

comment:13 Changed 8 months ago by cmuelle8

Related to this (and the progressive downscaling method imported by don-vip) there is
some more info here

eventually parts of twelvemonkeys lib might be worth looking at for import,
if there is a need to support more image formats or lanczos scaling.
its under BSD license and though source weighs a hefty 97 mb, zip compressed,
the compiled jars (common+jpeg) are around 0.5 mb to 1.0 mb.

comment:14 in reply to:  11 Changed 8 months ago by Don-vip

Replying to cmuelle8:

what is the policy on malformed double prop in preferences? e.g. if user entered foo123.45bar instead of a valid double
Should that be left to the user to fix?

Yes. Advanced preferences, as their name suggest, are for users who must know what they are doing.

comment:15 Changed 8 months ago by Don-vip

Thank you!
The patch looks nice, can you please just explain this property:

    private static final BooleanProperty AGPIFO_STYLE2 =
        new BooleanProperty("geoimage.agpifo-style-drag-and-zoom", false);

I need a small javadoc and understand why it is called "STYLE2".

comment:16 Changed 8 months ago by Don-vip

In 13127/josm:

see #15476, fix #15511 - fix image scaling regression and makes geoimage feature more configurable through prefs (adjustable max zoom, zoom-step, click zooming with mouse buttons (e.g. if a mouse wheel is not present). Patch by cmuelle8

comment:17 Changed 8 months ago by Don-vip

Type: defectenhancement

Switch back this ticket to enhancement. The defect part is traced in #15511.

comment:18 Changed 8 months ago by Don-vip

@naoliv: can you please check and confirm it works for you too? :)

comment:19 Changed 8 months ago by naoliv

Still working nicely for me!

comment:20 Changed 8 months ago by Don-vip

In 13129/josm:

see #15476 - fix checkstyle/pmd warnings, add some javadoc

comment:21 in reply to:  15 Changed 8 months ago by cmuelle8

Replying to Don-vip:

    private static final BooleanProperty AGPIFO_STYLE2 =
        new BooleanProperty("geoimage.agpifo-style-drag-and-zoom", false);

Ah, sorry. That just went in due to the way I refactored the props. I first went to

     private static final BooleanProperty AGPIFO_STYLE = Config.getPref().getBoolean("geoimage.agpifo-style-drag-and-zoom", false);

and still had that line in when moving to BooleanProperty. In the learning process about the differences between the two methods I then toggled a couple of times back and forth between AGPIFO_STYLE and AGPIFO_STYLE2 before deciding for the latter.

You can safely switch it back to AGPIFO_STYLE - there were no changes in semantic.

comment:22 Changed 8 months ago by Don-vip

OK. Can you please give me a javadoc for this property, and for public methods of VisRect missing after r13129? :)

comment:23 Changed 8 months ago by cmuelle8

Thanks for applying the patch, btw.

The public methods of VisRect are unchanged from those that existed unbound in the outer class before. See

They did not have documentation before, and personally I think they did and do not need any at all. They are short enough (and easy enough) to be self explanatory. The chance of introducing a documentation to code mismatch is higher than the gain documentation could achieve at this point. The same applies for

If you really absolutely need javadoc because of code metrics, you're faster off writing a line per method yourself, than waiting for me. :) (But I do not request nor expect you to.)

I see the point in documenting the AGPIFO property however, as all other properties have doc. Problem is, I do not know what AGPIFO abbreviation stands for .. and have deducted its meaning myself from how it was/is used in the code. It essentially switches RMB and LMB (left/right mouse buttons). A proper documentation would explain the abbreviation aside from describing the behavior.

comment:24 Changed 8 months ago by cmuelle8

Currently I've made large jpeg loading work through the use of JNI bridge to turbojpeg system library. It is only employed if default toolkit fails to load jpeg. Are you interested in this patch as well?

Use case should be rather rare though, I guess. For one, such large images are rather uncommon, for second the pre-compiled libs on standard ubuntu are built without JNI bindings support (because of --with-java option missing in debian/rules *grmpf*).

I have tested with https://commons.wikimedia.org/wiki/File:Pieter_Bruegel_the_Elder_-_The_Tower_of_Babel_(Vienna)_-_Google_Art_Project_-_edited.jpg (a jpeg image with 30.000x21.952 pixels).

Trying to load this file with JOSM usually results in java.lang.OutOfMemoryError: Java heap space. I get even more horrible results when JOSM was started with -Xmx2G. Though the test machine has 8 gigs of ram, I won't get an exception in this case, but rather Xorg and the JVM terminate immediately and restart (bug in openjdk, I suppose). In this case the following won't work:

With the JNI patch, I catch java.lang.OutOfMemoryError, do System.gc() and then let turbojpeg load the image natively to a scaled BufferedImage, that is progressively constructed with smaller scales until the OOM condition is not met anymore. You only need to care about the memory footprint of the BufferedImage in this case (as it lands on the java heap), but not about the memory turbojpeg will use to decode, scale and write to this buffer (as this happens natively).

Using this method the test image loads nicely at about a third of its resolution (and JOSM scrolls and zooms on that copy as usual).

If you think this will be useful nonetheless, we should open another ticket for that though.

EDIT:
follow-up opened and filed under ticket:15574

Last edited 8 months ago by cmuelle8 (previous) (diff)

comment:25 Changed 8 months ago by Don-vip

Resolution: fixed
Status: reopenedclosed

OK thanks for your answer :) I thought the APIs were new, my mistake.
Please create a new ticket for the large jpeg loading, we'll discuss it there.

comment:26 Changed 8 months ago by Don-vip

In 13131/josm:

see #15476 - rename property constant

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.