Modify

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#15625 closed defect (fixed)

georeferenced images can't be scaled with mouse wheel, NullPointerException

Reported by: anonymous Owned by: team
Priority: normal Milestone: 17.12
Component: Core image mapping Version: latest
Keywords: regression Cc: cmuelle8

Description

Only portrait images seem to be affected, landscape images are not.

URL:http://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2017-12-02 00:59:36 +0100 (Sat, 02 Dec 2017)
Build-Date:2017-12-02 02:33:20
Revision:13180
Relative:URL: ^/trunk

Identification: JOSM/1.5 (13180 de) Linux Debian GNU/Linux 9.1 (stretch)
Memory Usage: 1035 MB / 2000 MB (649 MB allocated, but free)
Java version: 1.8.0_151-8u151-b12-1~deb9u1-b12, Oracle Corporation, OpenJDK 64-Bit Server VM


=== REPORTED CRASH DATA ===
BugReportExceptionHandler#handleException:
No data collected.

Warning issued by: BugReportExceptionHandler#handleException

=== STACK TRACE ===
Thread: AWT-EventQueue-1 (17) of main
java.lang.NullPointerException
	at org.openstreetmap.josm.tools.ImageProvider.createScaledImage(ImageProvider.java:1411)
	at org.openstreetmap.josm.gui.layer.geoimage.ImageDisplay.paintComponent(ImageDisplay.java:659)
	at javax.swing.JComponent.paint(JComponent.java:1056)
	at javax.swing.JComponent.paintChildren(JComponent.java:889)
	at javax.swing.JComponent.paint(JComponent.java:1065)
	at javax.swing.JComponent.paintToOffscreen(JComponent.java:5210)
	at javax.swing.BufferStrategyPaintManager.paint(BufferStrategyPaintManager.java:290)
	at javax.swing.RepaintManager.paint(RepaintManager.java:1272)
	at javax.swing.JComponent._paintImmediately(JComponent.java:5158)
	at javax.swing.JComponent.paintImmediately(JComponent.java:4969)
	at javax.swing.RepaintManager$4.run(RepaintManager.java:831)
	at javax.swing.RepaintManager$4.run(RepaintManager.java:814)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:80)
	at javax.swing.RepaintManager.paintDirtyRegions(RepaintManager.java:814)
	at javax.swing.RepaintManager.paintDirtyRegions(RepaintManager.java:789)
	at javax.swing.RepaintManager.prePaintDirtyRegions(RepaintManager.java:738)
	at javax.swing.RepaintManager.access$1200(RepaintManager.java:64)
	at javax.swing.RepaintManager$ProcessingRunnable.run(RepaintManager.java:1732)
	at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:311)
	at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:756)
	at java.awt.EventQueue.access$500(EventQueue.java:97)
	at java.awt.EventQueue$3.run(EventQueue.java:709)
	at java.awt.EventQueue$3.run(EventQueue.java:703)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:80)
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:726)
	at org.GNOME.Accessibility.AtkWrapper$5.dispatchEvent(AtkWrapper.java:700)
	at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:201)
	at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116)
	at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93)
	at java.awt.EventDispatchThread.run(EventDispatchThread.java:82)

Attachments (1)

test.jpg (49.1 KB ) - added by anonymous 6 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 by Don-vip, 6 years ago

Cc: cmuelle8 added
Keywords: regression added
Milestone: 17.12
Owner: changed from team to anonymous
Status: newneedinfo

can you please attach your image?

comment:2 by Don-vip, 6 years ago

Component: CoreCore image mapping

comment:3 by anonymous, 6 years ago

You can create a image, which triggers this bug:

convert -size 4128x3096 xc:white jpeg:- | exiftool -Orientation#=6 -GPSLongitudeRef=E -GPSLongitude=139.0 -GPSLatitudeRef=N -GPSLatitude=35.0 - -o test.jpg

comment:4 by Don-vip, 6 years ago

The command gives me:

Error: Can't create JPEG files from scratch

Can you please attach your test.jpg?

by anonymous, 6 years ago

Attachment: test.jpg added

comment:5 by Don-vip, 6 years ago

Owner: changed from anonymous to team
Status: needinfonew

Thanks.
@cmuelle8: do you think this would be enough?

  • ImageDisplay.java

     
    652652
    653653            if (selectedRect == null && bilinLower < scale && scale < bilinUpper) {
    654654                BufferedImage bi = ImageProvider.toBufferedImage(image, r);
    655                 r.x = r.y = 0;
     655                if (bi != null) {
     656                    r.x = r.y = 0;
    656657
    657                 // See https://community.oracle.com/docs/DOC-983611 - The Perils of Image.getScaledInstance()
    658                 // Pre-scale image when downscaling by more than two times to avoid aliasing from default algorithm
    659                 image = ImageProvider.createScaledImage(bi, target.width, target.height,
    660                             RenderingHints.VALUE_INTERPOLATION_BILINEAR);
    661                 r.width = target.width;
    662                 r.height = target.height;
     658                    // See https://community.oracle.com/docs/DOC-983611 - The Perils of Image.getScaledInstance()
     659                    // Pre-scale image when downscaling by more than two times to avoid aliasing from default algorithm
     660                    image = ImageProvider.createScaledImage(bi, target.width, target.height,
     661                                RenderingHints.VALUE_INTERPOLATION_BILINEAR);
     662                    r.width = target.width;
     663                    r.height = target.height;
     664                }
    663665            } else {
    664666                // if target and r cause drawImage to scale image region to a tmp buffer exceeding
    665667                // its bounds, it will silently fail; crop with r first in such cases

comment:6 by cmuelle8, 6 years ago

@Don-vip: No, the problem is not the orientation of the plain image,
but what results from applying the exif orientation tag.

I.e. if you create images like this

PARAM="-GPSLongitudeRef=E -GPSLongitude=139.0 -GPSLatitudeRef=N -GPSLatitude=35.0"
convert -size 4128x3096 xc:white jpeg:- | exiftool $PARAM - -o test-landscape.jpg
convert -size 3096x4128 xc:white jpeg:- | exiftool $PARAM - -o test-portrait.jpg

they will work without problem.

If you looked at attachment:josm-image-loading_check-ram-constraints-to-prevent-oom-exceptions_do-not-use-expensive-scaling-while-dragging.patch:ticket:15574 you recognized that it already does contain the check if (bi != null) { .., but it will not solve the issue of this ticket.

This may not be a regression. Have you checked that the test image in comment:3 really works with older versions (e.g. r13037 or earlier)?

Since test-landscape.jpg and test-portrait.jpg work, I suspect the root cause to be within the range of line 251 and line 269 of trunk/src/org/openstreetmap/josm/gui/layer/geoimage/ImageDisplay.java?rev=13186.

It's probably better to create a new VisRect there (so that init is set correctly), instead of just swapping width and height of the one already created at this point. I'll update the attachments of ticket:15574 to contain this fix within some minutes, once I've tested the fix.

Last edited 6 years ago by cmuelle8 (previous) (diff)

comment:7 by cmuelle8, 6 years ago

@Don-vip:

  • src/org/openstreetmap/josm/gui/layer/geoimage/ImageDisplay.java

     
    242242                }
    243243
    244244                if (!error) {
    245                     ImageDisplay.this.image = img;
    246                     visibleRect = new VisRect(0, 0, img.getWidth(null), img.getHeight(null));
     245                    int width = img.getWidth(null);
     246                    int height = img.getHeight(null);
    247247
    248                     final int w = (int) visibleRect.getWidth();
    249                     final int h = (int) visibleRect.getHeight();
    250 
     248                    boolean switchedDim = false;
    251249                    if (ExifReader.orientationNeedsCorrection(orientation)) {
    252                         final int hh, ww;
    253250                        if (ExifReader.orientationSwitchesDimensions(orientation)) {
    254                             ww = h;
    255                             hh = w;
    256                         } else {
    257                             ww = w;
    258                             hh = h;
     251                            width = img.getHeight(null);
     252                            height = img.getWidth(null);
     253                            switchedDim = true;
    259254                        }
    260                         final BufferedImage rot = new BufferedImage(ww, hh, BufferedImage.TYPE_INT_RGB);
    261                         final AffineTransform xform = ExifReader.getRestoreOrientationTransform(orientation, w, h);
     255                        final BufferedImage rot = new BufferedImage(width, height, BufferedImage.TYPE_INT_RGB);
     256                        final AffineTransform xform = ExifReader.getRestoreOrientationTransform(orientation,
     257                                img.getWidth(null), img.getHeight(null));
    262258                        final Graphics2D g = rot.createGraphics();
    263                         g.drawImage(image, xform, null);
     259                        g.drawImage(img, xform, null);
    264260                        g.dispose();
    265 
    266                         visibleRect.setSize(ww, hh);
    267                         image.flush();
    268                         ImageDisplay.this.image = rot;
     261                        img.flush();
     262                        img = rot;
    269263                    }
     264
     265                    ImageDisplay.this.image = img;
     266                    visibleRect = new VisRect(0, 0, width, height);
     267
     268                    Logging.info("Loaded "+file.getPath()+
     269                            " with dimensions "+width+"x"+height+
     270                            " exifOrientationSwitchedDimension="+switchedDim);
    270271                }
    271272
    272273                selectedRect = null;

This patch will solve this ticket, but you know I'd rather like you to apply the smaller patch of ticket:15574. It will close this one and fix scaling while dragging, among some other minor stuff.

Greetings

in reply to:  6 comment:8 by anonymous, 6 years ago

Replying to cmuelle8:

This may not be a regression. Have you checked that the test image in comment:3 really works with older versions (e.g. r13037 or earlier)?

It works with r12976.

comment:9 by anonymous, 6 years ago

It also works with r13056!

comment:10 by anonymous, 6 years ago

r13119 is also working!

comment:11 by cmuelle8, 6 years ago

@Don-vip:
BTW: If exif orientation causes the AffineTransform to apply, rot is allocated. rot will need the same size as img, so we need memory available that can hold both rot and img until img = rot; is carried out and the garbage collector frees the old img.

This is another reason to rather directly apply the smaller patch of ticket:15574, because it will only load images, if there is enough mem left to carry out such an operation.

@anonym:
Thanks for checking. Issue should be fixed within the next couple of changesets. Please note that r13056 and r13119 had worse issues with image scaling, see ticket:15511.

comment:12 by anonymous, 6 years ago

r13128 is NOT working.
So the bug has been introduced between r13119 and r13128.

in reply to:  11 comment:13 by anonymous, 6 years ago

Replying to cmuelle8:

@anonym:
Thanks for checking. Issue should be fixed within the next couple of changesets. Please note that r13056 and r13119 had worse issues with image scaling, see ticket:15511.

Ok, thanks!

comment:14 by Don-vip, 6 years ago

Resolution: fixed
Status: newclosed

In 13191/josm:

fix #15625, see #15574 - geo image loading: do ram constraint checking; add some javadoc; do not use bilinear scaling when image is dragged (patch by cmuelle8, modified)

comment:15 by Don-vip, 6 years ago

Ticket #16196 has been marked as a duplicate of this ticket.

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. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.