[patch] improve SelectAction and NavigatableComponent

[cmuelle8]

the following patch improves upon the patches to SelectAction checked in previously (see #5457 and #5480). it also does some HouseKeeping in NavigatableComponent, delivers better documentation and has some improvements in clarity of the code (I hope). other minor things is to make the keyboard preferences sortable by clicking on the column header and using ProxySelector in PropertiesDialog for fetching of help pages (see #5464 and #5465).

i marked some methods in NavigatableComponent deprecated. josm core does not need them anymore, after applying the patch - for plugin developers I left the old methods intact, but deprecated them - I hope this is the right way to do it..

have a good day,
christian

[bastiK]

Hm, seems to be quite a radical change: It is now relatively hard to select node ways, at all. And even for "icon-nodes" it does not select if you click the outer region of the icon.

I did a little mockup for possible snap regions:

The snap regions for the way are not drawn. The idea is, that you can select a way by clicking the middle of a segment. If the segment is long enough to show a cross hair node, selecting the way is no problem anyway. What do you think?

[cmuelle8]

there is two ways to interpret your graphic.

first, making room to select the way, if the snapDistances of two nodes overlap. this will not work:

• the closer the nodes become, the higher the possibility you hit the way (this is counterintuitive, we really want to heighten the probability to get one of the nodes in that case)

• the more the nodes move away from each other, the less probable a way hit becomes, until, if the dist between n1 and n2 is exactly 2*snapDistance you do not have a chance to select the way at all

a second interpretation might be what you want.

• an imaginary node with its snapCircle in the middle of the way segment takes space away from the snapCircles of n1 and n2 as they approach.. (the snapCircle of the imaginary node shrinks in its diameter to be always smaller than the distance between n1 and n2 - so that it will never contain n1 and n2)

the computation cost of this is higher than the noticable benefit, i suppose - but the idea is appealing. having shrunk the bounding box for getNearestNodes() to node-size works fairly well for non-icon nodes (since node snap distance was configurable before, i kept it configurable via prefs, btw).

using the old node-snap-distance value (10px), icon nodes were relatively easy to select, but it made selecting node ways (= ways with many close nodes at a certain zoom level) impossible before - this is possible at the moment, at the single cost of having to hit the 4px-bbox in the middle of the icon.

doing the computation with an imaginary node will have a higher cost, regardless of the implementation, here is why:

• it does not suffice to get only nodes within bbox anymore, you additionally have to lookup waysegs as well - always (since you must not do the imaginary node thingy if the nodes are not connected by a way segment)

however, since some math is already done, implementing simulated behavior to the above might be possible with less computation cost:

• getNodesImpl and getWaySegementsImpl deliver the distances to p of the primitives they have found, in a sorted map

• when calling getNearestNodeOrWay, and resp. getNearestNodesOrWays what we could do is:

• call getNearestNodeImpl() (with a 10px bound..)

• find nn = (selected node or a node within 4px reach)

• if nn was found, return nn, else nn = nearest

• call getNearestWaysImpl() (with a 10px bound..)

• take the nearest way segment (nn is not necessarily part of it, nor have to be any of the other nodes returned by getNearestNodesImpl())

• for all nodes nd that satisfy dist(point(nn), point(nd)) > 2*4px do :
  • if (dist(closest_point_of_wayseg, point_middle_of_nn_nd) < dist(point_nn, point_middle_of_nn_nd))
    • return nearest wayseg;

• return nn;

greetings

[bastiK]

using the old node-snap-distance value (10px), icon nodes were relatively easy to select, but it made selecting node ways (= ways with many close nodes at a certain zoom level) impossible before - this is possible at the moment, at the single cost of having to hit the 4px-bbox in the middle of the icon.

Imho, decreasing the node snap distance from 10px to 4px is quite a change because every user will notice the difference. And they won't think "hey nice, now i can select ways more easily" but "what the heck, I cannot select nodes anymore". Indeed for fast and efficient work, a larger snap distance is useful.

We should try to get better "selectiblity of node ways" without these drawbacks.

there is two ways to interpret your graphic.

The 2nd interpretation is what i meant.

No need to worry about computational costs too much. We have a spacial index to get only the relevant ways and calculating the perpendicular distance is really cheap. This should be orders of magnitude away from being noticeable to the user.

Regarding your pseudo code: It should work, but why not simply:

   if (dist(p, middle_of_nearest_way_segment) < 0.2 * length_of_nearest_way_segment)
      return nearest_way_segment

--

Sebastian

[stoecker]

To inject some knowledge from long-time-josm-admin: Don't reduce the node selection quality we now have. It has been a long process to get it. The idea of Sebastian sounds good, but general reductions of node-snap-distance or the way it currently works aren't acceptable.

So: Improvements to better select short ways are welcome as long as they don't make selecting nodes harder. We always have the option to zoom in to get more free way area, but we can't change the mouse handling of our users.

[cmuelle8]

Replying to stoecker:

nobody wants to reduce node selection quality. we are in the process of experimenting with it, since seb suggested that there is still room for improvement on selecting visually short way segments (this has some history in #5457 already). reducing the node snap distance was a quick try to find out and then document that it cannot be done by a simple change. the tickets document why :)

@Seb: impl of my pseudo code does work, but your suggestion seems all the better:

• it shrinks the snapCircle for the wayseg to a fifth of its length with the center of the snapCircle lying at the middle_of_nearest_way_segment

• it honors the case that the nodes might not be part of the way segment (they might be entirely unrelated), by simply not using any near nodes for computation

• it is almost as precise as my pseudo code. there are some corner cases, where it does not deliver the right answer:
  • imagine a pretty long way segment, so its center lies in a node cloud
  • the snapCircle for the wayseg might then be as big (or bigger) then the circumcircle of the node cloud
  • effectively you then always snap to the way, if the nearest node was not within 4px to p
  • note: the center of the way segment might also lie outside of the node cloud's circumcircle - as long as the snapCircle of the way segment will contain the circumcircle of the nodes, one will wrongly snap to the way segement..
  • see graphic (red node symbolizes clicked point p)