Modify

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#12964 closed defect (fixed)

NPE when creating imagery layer

Reported by: hjart Owned by: team
Priority: blocker Milestone: 16.06
Component: Core imagery Version: latest
Keywords: template_report regression gsoc-core Cc: wiktorn, michael2402, naoliv

Description

What steps will reproduce the problem?

What is the expected result?

What happens instead?

Please provide any additional information below. Attach a screenshot if possible.

URL:http://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2016-06-14 22:25:40 +0200 (Tue, 14 Jun 2016)
Build-Date:2016-06-15 01:34:12
Revision:10377
Relative:URL: ^/trunk

Identification: JOSM/1.5 (10377 da) Linux openSUSE Leap 42.1 (x86_64)
Memory Usage: 632 MB / 1324 MB (156 MB allocated, but free)
Java version: 1.8.0_91-b14, Oracle Corporation, OpenJDK 64-Bit Server VM
Dataset consistency test: No problems found

Plugins:
- DirectUpload (32158)
- FastDraw (32158)
- InfoMode (32158)
- Mapillary (32158)
- SimplifyArea (32158)
- apache-commons (32158)
- apache-http (32158)
- buildings_tools (32158)
- ejml (32158)
- geotools (31895)
- josm-geojson (31245)
- jts (31906)
- opendata (32158)
- reverter (32158)
- tagging-preset-tester (32158)
- todo (29154)
- utilsplugin2 (32158)
- wikipedia (32182)

Tagging presets:
- ${HOME}/Dokumenter/HjartsPresets.xml
- http://www.country-linedance.de/daten/Archaeologie-de.zip
- https://josm.openstreetmap.de/josmfile?page=Presets/OneClick&zip=1
- https://josm.openstreetmap.de/josmfile?page=Presets/Quick-highways&zip=1
- https://josm.openstreetmap.de/josmfile?page=Presets/Quick-stops&zip=1

Last errors/warnings:
- W: Old style SideButton usage for action org.wikipedia.WikipediaToggleDialog$PasteWikipediaArticlesAction@27174252
- W: Old style SideButton usage for action org.wikipedia.WikipediaToggleDialog$AddWikipediaTagAction@4f675dab
- E: org.openstreetmap.josm.tools.bugreport.ReportedException: java.lang.NullPointerException. Årsag: java.lang.NullPointerException
- E: org.openstreetmap.josm.tools.bugreport.ReportedException: java.lang.NullPointerException. Årsag: java.lang.NullPointerException
- E: org.openstreetmap.josm.tools.bugreport.ReportedException: java.lang.NullPointerException. Årsag: java.lang.NullPointerException

=== REPORTED CRASH DATA ===
MapView#paintLayer:
 - layer: org.openstreetmap.josm.gui.layer.TMSLayer@4f8b6382
 - bounds: Bounds[54.9469716,8.818596,55.0184521,8.9620823]

=== STACK TRACE ===
Thread: AWT-EventQueue-0 (18) of main
java.lang.NullPointerException
	at org.openstreetmap.josm.gui.layer.AbstractTileSourceLayer.getScaleFactor(AbstractTileSourceLayer.java:295)
	at org.openstreetmap.josm.gui.layer.AbstractTileSourceLayer.getBestZoom(AbstractTileSourceLayer.java:305)
	at org.openstreetmap.josm.gui.layer.AbstractTileSourceLayer.paint(AbstractTileSourceLayer.java:1449)
	at org.openstreetmap.josm.gui.MapView.paintLayer(MapView.java:754)
	at org.openstreetmap.josm.gui.MapView.paint(MapView.java:817)
	at javax.swing.JComponent.paintChildren(JComponent.java:880)
	at javax.swing.JComponent.paint(JComponent.java:1056)
	at javax.swing.JComponent.paintToOffscreen(JComponent.java:5201)
	at javax.swing.BufferStrategyPaintManager.paint(BufferStrategyPaintManager.java:290)
	at javax.swing.RepaintManager.paint(RepaintManager.java:1272)
	at javax.swing.JComponent._paintImmediately(JComponent.java:5149)
	at javax.swing.JComponent.paintImmediately(JComponent.java:4960)
	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:76)
	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:76)
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:726)
	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 (2)

patch-fix-12964.patch (5.5 KB) - added by michael2402 5 years ago.
patch-mapview-fix-zoom-to-bounds.patch (2.3 KB) - added by michael2402 5 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 5 years ago by Don-vip

Cc: wiktorn michael2402 added
Component: CoreCore imagery
Keywords: regression added
Milestone: 16.06

comment:2 Changed 5 years ago by Don-vip

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

comment:3 Changed 5 years ago by Don-vip

Cc: naoliv added

comment:4 Changed 5 years ago by malenki

To add a useful description:
Just adding any kind of imagery layer results in exception.

Tested with blank ~/.josm and r10377

comment:5 Changed 5 years ago by wiktorn

Resolution: fixed
Status: newclosed

In 10391/josm:

Fixes for LayerManager.

  • LayerManager: invoke hookupMapView at the end of addLayer so layer's initialization code depending on presence of MapView will be executed
  • Main: moved creation of mapFrame from addLayer(...) to anonymous LayerManager listener, so hookupMapView can depend on the fact, that mapFrame exists
  • AbstractTileSourceLayer: forced generation of the event for this layer, so MouseAdapter listeners will be properly registered

Closes: #12964, #12965

comment:6 Changed 5 years ago by wiktorn

@michael2402:
Source of the problem was, that Main.addLayer(final Layer layer, ViewportData viewport) was doing a bit more, than LayerManager.addLayer(Layer layer). It still does, so it's wise to either move it to LayerManager or deprecate Main.addLayer methods.

For the change in AbstractTileSourceLayer - I'm not sure about this solution. Maybe I shall move layerAdded code outside listener, and just listen for layer removals.

comment:7 Changed 5 years ago by Don-vip

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

comment:8 Changed 5 years ago by michael2402

Sorry for the inconvenience. The main reason for the change was that I have a patch waiting to be finished that removes the requirement of the addLayer() code in Main.

That way we can make JOSM more modular by removing the dependency on main and support multiple mapviews/layer managers in the future.

Changed 5 years ago by michael2402

Attachment: patch-fix-12964.patch added

comment:9 Changed 5 years ago by wiktorn

@michael2402:
patch-fix-12964.patch is a replacement for changes in [10391]? (apart from changes to properly register MouseAdapter listeners)

comment:10 Changed 5 years ago by michael2402

It was what I had written. Yo do not need to apply it, I'll send you a small patch for your patch in a few minutes.

comment:11 Changed 5 years ago by michael2402

First of all, thanks for investigating - this was about the same way I wanted to do it. I'm sorry to not have seen that dependency for the tests.

hookupMapView was called before the layer was added and the layer change listeners were fired (before my change with the LayerManager as well). Is there a special reason you moved it? It currently does not have any semantic if multiple layer managers or multiple map views would exists. We might add a new hookUpMapView(view) method.

if (map != null) { This should always return true. I have a fix attached that fixes the zoom problem when creating a second data layer.

patch-mapview-fix-zoom-to-bounds.patch​ is the patch that can be applied to [10391]

Last edited 5 years ago by michael2402 (previous) (diff)

Changed 5 years ago by michael2402

comment:12 in reply to:  11 ; Changed 5 years ago by wiktorn

Replying to michael2402:

hookupMapView was called before the layer was added and the layer change listeners were fired (before my change with the LayerManager as well). Is there a special reason you moved it? It currently does not have any semantic if multiple layer managers or multiple map views would exists. We might add a new hookUpMapView(view) method.

Actually it wasn't. For calls Main.getLayerManager().addLayer(), like in AddImageryAction, hookupMapView was never called.

Only if DownloadOsmTask.addNewLayerIfRequired() was called, which called Main.main.addLayer() which in turn, called hookupMapView.

So we had two code paths, one including hookUpMapView and one not. As the invocation is moved inside LayerManager now - all code paths will call this method.

if (map != null) { This should always return true. I have a fix attached that fixes the zoom problem when creating a second data layer.

Good point indeed.

patch-mapview-fix-zoom-to-bounds.patch​ is the patch that can be applied to [10391]

I'll commit it right away

EDIT: with some tweaks :-)

Last edited 5 years ago by wiktorn (previous) (diff)

comment:13 in reply to:  12 Changed 5 years ago by michael2402

Replying to wiktorn:

Replying to michael2402:

hookupMapView was called before the layer was added and the layer change listeners were fired (before my change with the LayerManager as well). Is there a special reason you moved it? It currently does not have any semantic if multiple layer managers or multiple map views would exists. We might add a new hookUpMapView(view) method.

Actually it wasn't. For calls Main.getLayerManager().addLayer(), like in AddImageryAction, hookupMapView was never called.

Yes, this was my mistake because I replaced that call. Before that, Main.addLayer() was used. And there hookupMapView was called before fireing the listeners - before I started changing anything.

Only if DownloadOsmTask.addNewLayerIfRequired() was called, which called Main.main.addLayer() which in turn, called hookupMapView.

So we had two code paths, one including hookUpMapView and one not. As the invocation is moved inside LayerManager now - all code paths will call this method.

Yes, this was the path I intended.

comment:14 Changed 5 years ago by wiktorn

In 10394/josm:

Fix zoom to bounds.

Patch by: michael2402

See: #12964

comment:15 Changed 5 years ago by wiktorn

I finally applied the patch as it is, I was mislead by the fact, that earlier version contained changes also in MapView constructor which you've stripped down.

comment:16 Changed 5 years ago by michael2402

Thanks. I'm currently working on the zoom state code, so the whole ViewportData might become obsolete. But I don't make it a priority for now.

comment:17 Changed 5 years ago by stoecker

Keywords: gsoc-core added

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.