Modify

Opened 6 years ago

Closed 6 years ago

#9492 closed enhancement (fixed)

Tools>create Multipolygon should add ways to existing multipolygon

Reported by: CaptainCrunch Owned by: team
Priority: normal Milestone: 14.01
Component: Core multipoly Version: tested
Keywords: multipolygon Cc: ggeldenhuis, stoecker

Description

I can use create multipolygon to automatically create a multipolygon relation and assign the proper roles. However, if there already is a multipolygon relation, I have to manually add any new members, like forest clearings and assign their role. If I re-run the create multipolygon action, I end up with 2 relations.

Expected result: if one of the selected ways is already member of a multipolygon relation, try to add the other selected ways to that relation if possible, otherwise display an error message.
It should be really simple, because all inner-outer logic is identical, it just needs to use the existing relation and prevent duplicated members.

Attachments (0)

Change History (26)

comment:1 Changed 6 years ago by Don-vip

Milestone: 14.01
Type: defectenhancement

comment:2 Changed 6 years ago by simon04

Resolution: fixed
Status: newclosed

Fixed in r6564.

comment:3 Changed 6 years ago by CaptainCrunch

Thanks for the update, that was really quick! However, I have 2 issues with the changes:

  1. It does not work for me :-(

When selecting a newly created way and an existing multipolygon relation and then using "update multipolygon" I get the following exception an an info message on the lower left that says "Linie mit nur 0 Punkten kann nicht hinzugefügt werden."

Revision: 6565
java.lang.NullPointerException

at org.openstreetmap.josm.actions.CreateMultipolygonAction.actionPerformed(CreateMultipolygonAction.java:96)
at javax.swing.AbstractButton.fireActionPerformed(Unknown Source)
at javax.swing.AbstractButton$Handler.actionPerformed(Unknown Source)
at javax.swing.DefaultButtonModel.fireActionPerformed(Unknown Source)
at javax.swing.DefaultButtonModel.setPressed(Unknown Source)
at javax.swing.AbstractButton.doClick(Unknown Source)
at javax.swing.plaf.basic.BasicMenuItemUI.doClick(Unknown Source)
at javax.swing.plaf.basic.BasicMenuItemUI$Handler.mouseReleased(Unknown Source)
at java.awt.AWTEventMulticaster.mouseReleased(Unknown Source)
at java.awt.Component.processMouseEvent(Unknown Source)
at javax.swing.JComponent.processMouseEvent(Unknown Source)
at java.awt.Component.processEvent(Unknown Source)
at java.awt.Container.processEvent(Unknown Source)
at java.awt.Component.dispatchEventImpl(Unknown Source)
at java.awt.Container.dispatchEventImpl(Unknown Source)
at java.awt.Component.dispatchEvent(Unknown Source)
at java.awt.LightweightDispatcher.retargetMouseEvent(Unknown Source)
at java.awt.LightweightDispatcher.processMouseEvent(Unknown Source)
at java.awt.LightweightDispatcher.dispatchEvent(Unknown Source)
at java.awt.Container.dispatchEventImpl(Unknown Source)
at java.awt.Window.dispatchEventImpl(Unknown Source)
at java.awt.Component.dispatchEvent(Unknown Source)
at java.awt.EventQueue.dispatchEventImpl(Unknown Source)
at java.awt.EventQueue.access$200(Unknown Source)
at java.awt.EventQueue$3.run(Unknown Source)
at java.awt.EventQueue$3.run(Unknown Source)
at java.security.AccessController.doPrivileged(Native Method)
at java.security.ProtectionDomain$1.doIntersectionPrivilege(Unknown Source)
at java.security.ProtectionDomain$1.doIntersectionPrivilege(Unknown Source)
at java.awt.EventQueue$4.run(Unknown Source)
at java.awt.EventQueue$4.run(Unknown Source)
at java.security.AccessController.doPrivileged(Native Method)
at java.security.ProtectionDomain$1.doIntersectionPrivilege(Unknown Source)
at java.awt.EventQueue.dispatchEvent(Unknown Source)
at java.awt.EventDispatchThread.pumpOneEventForFilters(Unknown Source)
at java.awt.EventDispatchThread.pumpEventsForFilter(Unknown Source)
at java.awt.EventDispatchThread.pumpEventsForHierarchy(Unknown Source)
at java.awt.EventDispatchThread.pumpEvents(Unknown Source)
at java.awt.EventDispatchThread.pumpEvents(Unknown Source)
at java.awt.EventDispatchThread.run(Unknown Source)

Here's a screenshot: http://postimg.org/image/mcujzri1r/

  1. It works only when specifically selecting the Relation. When clicking a way that is outer-member of a multipolygon the first thing that gets selected is the way not the relation, so it would be easier if I could just select the way and JOSM would automatically use the multipolygon relation that this way is a member of. Note that it would then even work if I selected an existing inner way (like a nearby clearing in a large forest). Would this be possible?

comment:4 Changed 6 years ago by simon04

In 6569/josm:

see #9492 - Tools>"Update Multipolygon": check that relation is loaded completely

comment:5 Changed 6 years ago by simon04

Item 1 relates to the relation being incomplete; a check has been added in r6569.

Item 2: This would disallow creating a multipolygon relation if one way is already part of another relation. Also, if two of two selected ways are part of multipolygon relations, which is which?

comment:6 Changed 6 years ago by CaptainCrunch

A screenshot to illustrate my current workflow for adding inner members to existing multipolygons: http://postimg.org/image/agbsxydp1/

Note that I don't have to select the relation itself, any member (even inner) works. But it is a lot of clicking so I thought it must be possible to automate it. Note that it works on incomplete relations. But it obviously makes use of my knowledge of which relation is the correct one (if there is more than one) and which way should get which role.

I see, it's more complicated than I first thought. You are right, this would conflict with creating multipolygons, so perhaps it would be best to split this functionality off into a separate action. Then it could be implemented with the following special case in mind. (There is only this one, if I did not miss something.)

In the following always (re-)use the same logic as when creating a new multipolygon. (Which even supports closed rings of more than one way for both inner and outer.)

"Is the total number of unique relations of which the selected ways are members of greater than 1?"

  • If not: add members and assign roles as if a new multipolygon would be created, skipping ways that are already members.
  • If so: the right relation to add the new ways must be decided. Choose the one that will give an error-free multipolygon. If there is none, display an error message. If there is more than one: either abort with a message and let the user do it manually (easy way) or display a list and let the user select the correct relation. Third option would be to use the relation that is selected in the tags toolbox but that would not be obvious to the user and could lead to errors.

For all of this, whenever existing outer ways are needed for decision, they must be already or have to be downloaded, which could be done automatically.

Well, not that "really simple" at all, but in my opinion, if implemented it would save a lot of time and clicking when updating multipolygons.

comment:7 Changed 6 years ago by simon04

Resolution: fixed
Status: closedreopened

comment:8 Changed 6 years ago by simon04

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

comment:9 Changed 6 years ago by ggeldenhuis

Cc: ggeldenhuis added

comment:10 Changed 6 years ago by simon04

In 6597/josm:

see #9492 - split "Create Multipolyon" in % and "Update Multipolygon"; for the latter obtain the relation from the selected ways (if unambiguous)

comment:11 Changed 6 years ago by simon04

In 6600/josm:

see #9492 - Automatically download incomplete relations for "Update Multipolygon"

comment:12 Changed 6 years ago by simon04

I'm wondering when the first ticket "I didn't want that operation to download my huuuuuge multipolygon automatically" appears ;-)

comment:13 Changed 6 years ago by CaptainCrunch

I tested it with different cases and find it working very well.
Could you however please add a keyboard shortcut to that new action? Perhaps Ctrl+Alt+Y, or empty by default?

comment:14 Changed 6 years ago by simon04

It's hard to spot a free shortcut, see DevelopersGuide/ShortcutsList.

Btw: How does the "Create a multipolygon" command of the reltoolbox toolbox relate to the JOSM core command?

comment:15 Changed 6 years ago by CaptainCrunch

I did not have all the plugins in the scope when searching for a free shortcut in my JOSM preferences. I guess it's best to leave the shortcut empty by default then.

reltoolbox:
As far as I can tell, it does exactly the same thing. (Except that it changes the selection afterwards.)
(It seems to have a few bugs though, like not correctly rendering an outer way after creating a multipolygon and then directly undoing that with Ctrl+Z. The outer way is not filled then and the inner ways still have the style of the MP despite not being member anymore. I guess I should open a ticket for that.)

With reltoolbox it is possible to do what the new "update" action does, but almost as complicated as doing it manually, because you have to open the reltoolbox panel (which takes precious space on a small screen), search for the right relation (by selecting an existing member and clicking the relation or by searching for the name), then select the new ways, click +/- button. The role must be selected first or fixed with the "!" button after adding.

comment:16 Changed 6 years ago by simon04

Cc: stoecker added

Dirk, a shortcut re-mapping question … :-)

Atm we have a "Create multipolygon" action in core, and there's a similar action in the reltoolbox plugin. Since r6597, the core contains an "Update multipolygon" action.

I suggest the following re-mapping.

command current shortcut suggested re-mapping
core "Create multipolygon" ALT+CTRL+A CTRL+B
core "Update multipolygon" (none) CTRL+SHIFT+B
reltoolbox "Update multipolygon" CTRL+B ALT+CTRL+A

Is it okay to do so?

comment:17 Changed 6 years ago by Don-vip

Do you think we could also drop the reltoolbox action ?

comment:18 in reply to:  17 Changed 6 years ago by simon04

Replying to Don-vip:

Do you think we could also drop the reltoolbox action ?

According to the source code CreateMultipolygonAction.java of the plugin, there's some special handling for boundary relations which assigns roles like admin_centre and label. The JOSM core has this functionality in the standard relation editor.

comment:19 Changed 6 years ago by stoecker

If it does the same, I'd drop that action. If some additional functionality is there, then port it into core. There is no sense in multiple equal functions.

comment:20 Changed 6 years ago by simon04

Then there's also #7012 to be implemented eventually.

I'm unsure whether the reltoolbox's handling of boundary relations fits into JOSM core. For instance, there's a dialog asking for "Enter admin level and name for the border relation". A different approach would be to alter reltoolbox's implementation to "Create boundary multipolygon" and keep all the specific handling there.

comment:21 in reply to:  20 Changed 6 years ago by skyper

Replying to simon04:

Then there's also #7012 to be implemented eventually.

Yes, I still miss this function at least the old behaviour of the plugin

comment:22 Changed 6 years ago by simon04

Since the next tested version is to appear shortly, I would suggest to:

  • remap the shortcuts as in comment:16 now
  • and keep the functionality integration/differentiation from #7012 and comment:15 for later

comment:23 Changed 6 years ago by Don-vip

Agreed.

comment:24 Changed 6 years ago by simon04

In 6721/josm:

see #9492 - Remap shortcuts for "create/update multipolygon"

comment:25 Changed 6 years ago by simon04

Adapted reltoolbox shortcut in [o30209] and [o30210].

comment:26 Changed 6 years ago by simon04

Resolution: fixed
Status: reopenedclosed

Created #9585 for functionality integration/differentiation of reltoolbox action.

#7012 is a separate ticket anyway.

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.