NPE when creating imagery layer

[hjart]

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)

[Don-vip]

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

[malenki]

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

Tested with blank ~/.josm and r10377

[wiktorn]

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

[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.

[Don-vip]

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

[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.

[wiktorn]

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

[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.

[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]

[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 :-)

[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.

[wiktorn]

In 10394/josm:

Fix zoom to bounds.

Patch by: michael2402

See: #12964

[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.

[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.