Modify

Opened 6 years ago

Closed 6 years ago

Last modified 5 years ago

#10037 closed enhancement (fixed)

[Patch] Double click selection mode

Reported by: simon04 Owned by: team
Priority: normal Milestone: 14.05
Component: Core Version:
Keywords: Cc: akks, Don-vip, Balaitous

Description (last modified by simon04)

Based on the very nice new feature of utilsplugin2 ([o30419]), I would like to propose to add this feature to the core. One could easily support adding/removing objects from the selection based on whether [alt]/[ctrl] is pressed.

What do you think (of the attached patch which is based on [o30419])?

Attachments (4)

dblclkselect.patch (5.1 KB) - added by simon04 6 years ago.
insideMultipolygon.osm (1.5 KB) - added by akks 6 years ago.
bankInsideBank.osm (2.0 KB) - added by akks 6 years ago.
validator not detecting bank inside bank (move one of the nodes upper and the warning arise)
2014-05-19 18_09_59-_ Редактор OpenStreetMap на Java.png (19.2 KB) - added by akks 6 years ago.
illustration

Download all attachments as: .zip

Change History (23)

Changed 6 years ago by simon04

Attachment: dblclkselect.patch added

comment:1 Changed 6 years ago by bastiK

Could you explain what this is about?

comment:2 Changed 6 years ago by simon04

Description: modified (diff)

Sorry, I forgot that. A double-click selects inside selects a surrounding (multi)polygon.

comment:3 Changed 6 years ago by bastiK

Okay, why not ...

comment:4 Changed 6 years ago by Don-vip

Milestone: 14.05

Don't forget the Javadoc :)

comment:5 Changed 6 years ago by akks

It would be nice to see it in core :) It is nice that ctrl-and shift click works too.

I had some integration problems when adding this to plugin, here are the ideas:
1) Some actions in SelectionMode are triggered with single mouse press. Will it work correctly in SelectionManager?
2) There was incorrect behavior of Geometry.isNodeInsideMultiPolygon for some cases. I'll check it once more and post an example if it still exists.
3) This action can be really CPU and time-consuming for big dataset. Maybe some parameters should be added (maxWays and maxRelations, for example) to avoid possible several seconds delay in dbl-click?

comment:6 Changed 6 years ago by akks

After testing:
1) All actions seems to work perfectly, there are no event problems.
3) Action is rather fast, maybe it can work without tuning parameters.

2) There are still some cases when Geometry.isNodeInsideMultiPolygon works incorrectly (not related to the patch, however), so I had to keep separate version in Utilsplugin2.
Example: download relation r398514 via ctrl-shift-o and try to select the boundary relation with double click.
Utilsplugin2 version does, Geometry.isNodeInsideMultiPolygon does not :)

My opinion - the patch itself is clean and ready for inclusion (with slightly expanded JavaDoc).

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

comment:7 Changed 6 years ago by akks

Here is the simplified example for bad Geometry.isNodeInsideMultiPolygon behavior (try selecting it from left and right internal point).

Changed 6 years ago by akks

Attachment: insideMultipolygon.osm added

comment:8 Changed 6 years ago by akks

After checking: the algorithm of isPolygonInsideMultiPolygon is not completely correct - multipolygon with multiple outer parts is not a sum of its outers (some validator tests should also work incorrectly). This is better to be fixed...

I can not fix it myself without replacing or duplicating a lot of existing code (utilsplugin2 may have correct but completely different checking).

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

Changed 6 years ago by akks

Attachment: bankInsideBank.osm added

validator not detecting bank inside bank (move one of the nodes upper and the warning arise)

comment:9 Changed 6 years ago by akks

Cc: Don-vip added

comment:10 Changed 6 years ago by Don-vip

Cc: Balaitous added

Yes, I'm not absolutely confident in the correctness of this method.
@Balaitous: interested in checking this point? ;)

If anyone thinks it's too complicated for this release, feel free to postpone to next milestone, we should release the next stable release this week-end.

comment:11 Changed 6 years ago by akks

This patch is easy and harmless enough and can be included now (there will be enough time to detect possible event bugs) :)

However, then some users can notice nodeInsidePolygon problem (currently it is used mostly in mapcss).
We can temporarily use the code of isPointInsideMultipolygon from Utilsplugin2 for isNodeInsideMultiPolygon only (it needs isPointInsidePolygon, buildPointList, getWayPoints, getRayIntersectionsCount as the private functions). Then the existing behavior will not change but some hidden duplicate functionality will exist in Geometry.java.

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

comment:12 Changed 6 years ago by simon04

(How) does isPointInsideMultipolygon from Utilsplugin2 handle inner roles of multipolygon relations?

    static boolean isPointInsideMultipolygon(EastNorth p, Relation rel) {
        Set<Way> usedWays = OsmPrimitive.getFilteredSet(rel.getMemberPrimitives(), Way.class);
        return isPointInsidePolygon(p, buildPointList(usedWays));
    }

https://trac.openstreetmap.org/browser/subversion/applications/editors/josm/plugins/utilsplugin2/src/org/openstreetmap/josm/plugins/utilsplugin2/selection/NodeWayUtils.java?rev=30421

comment:13 Changed 6 years ago by simon04

In 7144/josm:

see #10037 - Add double-click selection mode - based on akks's implementation in utilsplugin2

This allows to select polygons/multipolygons by a double-click on an internal point.

comment:14 Changed 6 years ago by simon04

In 7145/josm:

see #10037 - Improve node-inside-multipolygon matching by handling rings consisting of several ways correctly

comment:15 Changed 6 years ago by simon04

In 7146/josm:

see #10037 - Improve class name

comment:16 Changed 6 years ago by simon04

I've removed the implementation from utilsplugin2 in [o30459], [o30461].

comment:17 Changed 6 years ago by simon04

Resolution: fixed
Status: newclosed

The feature is implemented, the bug demonstrated in attachment:insideMultipolygon.osm has been fixed. A big thanks to all people involved!

comment:18 Changed 6 years ago by akks

Wow! That was really impressing.

Have tested the code on ~80 administrative boundaries collection - it is working and fast enough, suitable for tested version.

Reconstructing the rings for all the dataset multipolygons for each double click is a rather big task. If later we encounter the performance issues, we can do something about caching the MultipolygonCreate.joinWays results.

The algorithm from Utilsplugin2 does not handle the roles or try to orient the ways. It just counts the intersections of OX-parallel ray and all multipolygon parts (inner and outer fragments). If the number is 2k, the ray starting point is outside the multipolygon. This can potentially work faster, detects points on edges or vertices but does not support polygonIntersection (no Java2d area can be used).

comment:19 Changed 5 years ago by Don-vip

Ticket #4945 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.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.