Modify

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#14773 closed defect (fixed)

Exception when trying to set image offset via Plugin imagery_offset_db

Reported by: Andre68 Owned by: bastiK
Priority: normal Milestone:
Component: Plugin imagery_offset_db Version: latest
Keywords: template_report Cc: michael2402, Zverikk

Description

What steps will reproduce the problem?

  1. Activate Bing Areal Imager
  2. Load List of Image Offsets
  3. Set a Image Offset

What is the expected result?

Image Offset is set

What happens instead?

Nothing... Exception...

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

Build-Date:2017-05-11 08:20:10
Revision:12116
Is-Local-Build:true

Identification: JOSM/1.5 (12116 SVN de) Linux Mint 17.3 Rosa
Memory Usage: 1282 MB / 2731 MB (899 MB allocated, but free)
Java version: 1.8.0_131-b11, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Screen: :0.0 1920x1080
Maximum Screen Size: 1920x1080
Program arguments: [track-2017-04-18-02-03.gpx, track-2017-05-02-01-51.gpx, waypoints-2017-04-18-02-14.gpx, waypoints-2017-05-02-00-23.gpx, waypoints-2017-05-04-18-34.gpx, waypoints-2017-05-06-11-17.gpx]
Dataset consistency test: No problems found

Plugins:
+ FixAddresses (33182)
+ ImproveWay (12)
+ Mapillary (v1.5.3)
+ OpeningHoursEditor (33185)
+ RoadSigns (33204)
+ SimplifyArea (33004)
+ alignways (33182)
+ apache-commons (32994)
+ apache-http (32699)
+ download_along (32946)
+ editgpx (33004)
+ graphview (33004)
+ imagery_offset_db (33267)
+ lakewalker (33161)
+ log4j (32699)
+ measurement (33088)
+ merge-overlap (33154)
+ osmarender (33004)
+ pbf (33241)
+ photo_geotagging (33088)
+ public_transport (33166)
+ public_transport_layer (33088)
+ reverter (33088)
+ routing (33004)
+ tageditor (33021)
+ turnlanes (33294)
+ turnlanes-tagging (254)
+ turnrestrictions (33088)
+ undelete (33263)
+ utilsplugin2 (33297)
+ waydownloader (33167)

Map paint styles:
- https://josm.openstreetmap.de/josmfile?page=Styles/Enhanced_Lane_and_Road_Attributes&zip=1

Last errors/warnings:
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- W: java.io.IOException: Attribution is not loaded yet
- E: Handled by bug report queue: java.lang.NoSuchMethodError: org.openstreetmap.josm.gui.layer.ImageryLayer.setOffset(DD)V


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

Warning issued by: BugReportExceptionHandler#handleException

=== STACK TRACE ===
Thread: AWT-EventQueue-0 (19) of main
java.lang.NoSuchMethodError: org.openstreetmap.josm.gui.layer.ImageryLayer.setOffset(DD)V
	at iodb.ImageryOffsetTools.applyLayerOffset(ImageryOffsetTools.java:81)
	at iodb.OffsetDialog.applyOffset(OffsetDialog.java:273)
	at iodb.OffsetDialog.actionPerformed(OffsetDialog.java:258)
	at javax.swing.AbstractButton.fireActionPerformed(AbstractButton.java:2022)
	at javax.swing.AbstractButton$Handler.actionPerformed(AbstractButton.java:2348)
	at javax.swing.DefaultButtonModel.fireActionPerformed(DefaultButtonModel.java:402)
	at javax.swing.DefaultButtonModel.setPressed(DefaultButtonModel.java:259)
	at javax.swing.plaf.basic.BasicButtonListener.mouseReleased(BasicButtonListener.java:252)
	at java.awt.Component.processMouseEvent(Component.java:6533)
	at javax.swing.JComponent.processMouseEvent(JComponent.java:3324)
	at java.awt.Component.processEvent(Component.java:6298)
	at java.awt.Container.processEvent(Container.java:2236)
	at java.awt.Component.dispatchEventImpl(Component.java:4889)
	at java.awt.Container.dispatchEventImpl(Container.java:2294)
	at java.awt.Component.dispatchEvent(Component.java:4711)
	at java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:4888)
	at java.awt.LightweightDispatcher.processMouseEvent(Container.java:4525)
	at java.awt.LightweightDispatcher.dispatchEvent(Container.java:4466)
	at java.awt.Container.dispatchEventImpl(Container.java:2280)
	at java.awt.Window.dispatchEventImpl(Window.java:2746)
	at java.awt.Component.dispatchEvent(Component.java:4711)
	at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:758)
	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.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:90)
	at java.awt.EventQueue$4.run(EventQueue.java:731)
	at java.awt.EventQueue$4.run(EventQueue.java:729)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:80)
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:728)
	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 (0)

Change History (26)

comment:1 by Don-vip, 7 years ago

Cc: michael2402 added

comment:2 by bastiK, 7 years ago

Owner: changed from team to bastiK

comment:3 by stoecker, 7 years ago

Integrate in core?

in reply to:  3 comment:4 by bastiK, 7 years ago

Replying to stoecker:

Integrate in core?

This should be coordinated with iD. Otherwise people will end up moving entire villages and city blocks back and forth a few meters. Even if JOSM users would then be "right" and iD users would be "wrong" (if they don't align manually), this kind of inconsistency leads to a lot of frustration.

comment:5 by stoecker, 7 years ago

Cc: Zverikk added

Agreed. Anyway I think the alignment database is a good idea. Probably first make it an expert option and somehow indicate if others have provided alignment and anyway are there conflicts - What happens when different users uploadoffsets for same area - is there a majority vote?

This plugin/idea is now 5 years old, but I don't see a maturing process of it. Maybe having it in core gives the whole concept a push forward?

comment:6 by stoecker, 7 years ago

Component: PluginPlugin imagery_offset_db

comment:7 by Zverikk, 7 years ago

Hi, thanks for adding me here. Regarding the exception, I noticed the deprecated calls only two days ago, and will update them this week. I was struggling with updating the svn repository.

The database and the plugin have been pretty stable for several years, so I felt no need to improve them. Many people in HOT and in the Russian community use the plugin. You can see their offsets and the data at http://offsets.textual.ru/

I am all for including the plugin in the core. Not sure if it should be restricted to experts: clicking the "download offsets" button should be the first thing everyone does after adding an imagery layer. Still, even the expert-only mode is better than having to install the plugin separately.

There is no voting in the database: people compare offsets and mark least suitable as deprecated. I have not seen any dispute over offsets yet.

The reason it is not in iD, is that american / european developers usually do not have to deal with offset imagery, and do not feel the importance of this feature. The ticket has been there from the start. By the way, Vespucci editor make use of the offset database.

Last edited 7 years ago by Zverikk (previous) (diff)

in reply to:  7 comment:8 by stoecker, 7 years ago

Replying to Zverikk:

The database and the plugin have been pretty stable for several years, so I felt no need to improve them. Many people in HOT and in the Russian community use the plugin. You can see their offsets and the data at http://offsets.textual.ru/

I meant maturing of the idea, not the software. There are several issues I can see, especially regarding permissions and malicious users which aren't addressed yet. These will come with a bigger userbase. Also there are mirrors and imagery updates and some other points which aren't taken care of. It works without, but a mature solution would address them - see how complicated our imagery database became over the time even if the minimum is still only name, type and url ;-)

I am all for including the plugin in the core. Not sure if it should be restricted to experts: clicking the "download offsets" button should be the first thing everyone does after adding an imagery layer. Still, even the expert-only mode is better than having to install the plugin separately.

Expert limits the scope to experienced users. Thats always good for new ideas which may be troublesome.

Last edited 7 years ago by stoecker (previous) (diff)

in reply to:  7 ; comment:9 by bastiK, 7 years ago

Replying to Zverikk:

Hi, thanks for adding me here. Regarding the exception, I noticed the deprecated calls only two days ago, and will update them this week. I was struggling with updating the svn repository.

The API was changed, so the offset can be converted when the projection changes. So far, it only remembered the displacement in east/north coordinates, but the conversion also requires a reference point (which is included in the OffsetBookmark class).

The database and the plugin have been pretty stable for several years, so I felt no need to improve them. Many people in HOT and in the Russian community use the plugin. You can see their offsets and the data at http://offsets.textual.ru/

I am all for including the plugin in the core. Not sure if it should be restricted to experts: clicking the "download offsets" button should be the first thing everyone does after adding an imagery layer. Still, even the expert-only mode is better than having to install the plugin separately.

I have tested the plugin a bit more and it works pretty nicely. One thing that can be improved, is to have a visual cue, that a certain offset is active and a way to look up the meta-data. It is so easy to set an offset, then forget about it and start mapping in another city (now with incorrect offset). Also, there is no obvious button to set it back to zero.

One way to deal with this, would be an auto-mode: It automatically applies the offset that is closest to the current location (up to a maximum distance).

The calibration geometry is a great concept, but I think not suitable for beginners and one could derive normal offsets for the most popular layers from it.

The reason it is not in iD, is that american / european developers usually do not have to deal with offset imagery, and do not feel the importance of this feature. The ticket has been there from the start. By the way, Vespucci editor make use of the offset database.

It would be interesting to learn more about the implementation in Vespucci.

comment:10 by Don-vip, 7 years ago

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

comment:11 by Zverikk, 7 years ago

Is there a reason TileSourceDisplaySettings#setDisplacement() was made private in r12093? I am fixing the deprecated methods, and this restriction looks very strange, given getDisplacement() is public.

in reply to:  9 comment:12 by michael2402, 7 years ago

Replying to bastiK:

One thing that can be improved, is to have a visual cue, that a certain offset is active and a way to look up the meta-data. It is so easy to set an offset, then forget about it and start mapping in another city (now with incorrect offset). Also, there is no obvious button to set it back to zero.

We currently have the pretty annoying dialog that notifies new users that they should use offsets (and a draw tool UI bug when it is active). We could add something like a "Auto-Apply nearest" button there. This should be pretty straight-forward to use.
As soon as the user moves a given distance from the reference point, the dialog pops open again and asks if the new offset should be applied.

in reply to:  11 comment:13 by bastiK, 7 years ago

Replying to Zverikk:

Is there a reason TileSourceDisplaySettings#setDisplacement() was made private in r12093? I am fixing the deprecated methods, and this restriction looks very strange, given getDisplacement() is public.

Yes, in addition to the east/north displacement, we also need to save the reference point. Otherwise it impossible to convert the displacement from one projection to another. For this purpose, I'm using the OffsetBookmark class that contains both a displacement and a reference/center.

comment:14 by Zverikk, 7 years ago

Yes, in addition to the east/north displacement, we also need to save the reference point.

But the setDisplacement() method does not use the reference point; its signature is the same. So why it was made private, again?

We currently have the pretty annoying dialog that notifies new users that they should use offsets

By the way, there is an annoying issue with that dialog: it is shown even for layers that render OpenStreetMap.

Last edited 7 years ago by Zverikk (previous) (diff)

in reply to:  14 comment:15 by stoecker, 7 years ago

We currently have the pretty annoying dialog that notifies new users that they should use offsets

By the way, there is an annoying issue with that dialog: it is shown even for layers that render OpenStreetMap.

There is a "valid-georeference" XML tag to suppress it for know correct imagery. Needs to be added when missing.

in reply to:  14 comment:16 by bastiK, 7 years ago

Replying to Zverikk:

Yes, in addition to the east/north displacement, we also need to save the reference point.

But the setDisplacement() method does not use the reference point; its signature is the same. So why it was made private, again?

The calling method setOffsetBookmark does. Maybe it is clearer to always query the OffsetBookmark and move the cache for the displacement in current projection to that class.

comment:17 by Zverikk, 7 years ago

If the setDisplacement() method can be called only from setOffsetBookmark(), maybe it makes sense to remove other direct calls to that from the core, since otherwise getOffsetBookmark() won't always return the same values as getDx() and getDy()?

comment:18 by bastiK, 7 years ago

Yes, that's right. The only other call is from loadFrom, which is used for session loading. Saving offset to session was always lacking for WMS, since east/north displacement is recorded, but not what projection the coordinates are in. I'm planning to fix that and get rid of the problem you pointed out at the same time.

comment:19 by bastiK, 7 years ago

In 12134/josm:

see #14773 - improve session export/import for imagery layer offset data

comment:20 by Zverikk, 7 years ago

Resolution: fixed
Status: newclosed

Fixed in ​[o33309:33310]. Thanks everyone for consultation.

Do note that the offset db uses the current projection, not a fixed one. I decided not to change that, since changing projections happens quite seldom, and we have a full database of offsets registered in that way.

comment:21 by bastiK, 7 years ago

Just to avoid misunderstanding: The imagery offset database stores the values as lat/lon (WGS 84), so there can be no confusion in terms of projection.

It is only when data is stored in the local preferences (key: iodb.stored.offsets) that east/north could be mistakenly loaded back in a different projection.

comment:22 by Don-vip, 7 years ago

In 12139/josm:

see #14773 - checkstyle

in reply to:  19 comment:23 by Don-vip, 7 years ago

Replying to bastiK:

In 12134/josm:

see #14773 - improve session export/import for imagery layer offset data

org.openstreetmap.josm.io.session.SessionReaderTest.testReadImage is failing.

comment:24 by bastiK, 7 years ago

In 12140/josm:

update unit test (see #14773)

comment:25 by bastiK, 7 years ago

Zverikk, it seems you forgot to update the main version in build.xml: #14800

comment:26 by Zverikk, 7 years ago

I did forget that. So sorry. Will be able to update this evening.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain bastiK.
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.