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

[cmuelle8]

solving this case might be easy, though - since the ws is long, the user can select is elsewhere.. so basically just filtering out long way segments when computing the nearest detection might work..

[bastiK]

Right, what if the imaginary segment snap area only applies for the start and end node of the segment and all other nodes override this? Something like:

  if (nearest_segment != null) {
    get nearest node excluding the 2 segment nodes
    if (node != null and within snap distance)
      return node
    else if (click is within imaginary snap area of nearest_segment)
      return segment
    else
      return one of the 2 segments nodes (if in snap distance)
  else
    return nearest node

(Btw. i suggested to use 0.2 as radius, so the imaginary snap area covers 40% of the line.)

[cmuelle8]

Replying to bastiK:

  if (nearest_segment != null) {
    get nearest node excluding the 2 segment nodes
    if (node != null and within snap distance)
      return node
    else if (click is within imaginary snap area of nearest_segment)
      return segment
    else
      return one of the 2 segments nodes (if in snap distance)
  else
    return nearest node

you would miss the way segment a lot of times, if a nearest Node is snapped to that is farther from p than a segment node, but within snapDistance (or it might even be a dup node to a segment node) .. your idea was to limit the snapDistance of nearestNode to give the way segment a chance. you also have to limit the snapDistance of nearestNodes that are not part of the nearestWaySeg if you want to make it "slick"..

but i agree that there must be some simple way :)

here is my current snippet:

    public final OsmPrimitive getNearestNodeOrWay(Point p, Predicate<OsmPrimitive> predicate) {
        OsmPrimitive osm = getNearestNode(p, predicate);

        if (osm == null) {
            osm = getNearestWay(p, predicate);
        } else if (p.distanceSq(getPoint((Node)osm)) > (3)*(3)) {
            WaySegment ws = getNearestWaySegment(p, predicate);
            if (ws != null) {
                Point wp1 = getPoint(ws.way.getNode(ws.lowerIndex));
                Point wp2 = getPoint(ws.way.getNode(ws.lowerIndex+1));

                double snapDistanceSq = Main.pref.getInteger("mappaint.segment.snap-distance", 10);
                snapDistanceSq *= snapDistanceSq;
                double segLengthSq = wp1.distanceSq(wp2);
                segLengthSq *= segLengthSq;
                if (!(segLengthSq>snapDistanceSq) &&
                        p.distanceSq(project(0.5, wp1, wp2)) < 0.2*segLengthSq) {
                    osm = ws.way;
                }
            }
        }
        return osm;
    }

(note that getNearestNodesOrWays also has to be changed - so this doesn't work out of the box for mousereleases if you copy+paste this into current code..)

[cmuelle8]

actually, isn't this just a matter of exactly the three points

• the point clicked (p)
• the point of the nearest node (pnn)
• the point of the middle of the nearest segment (pmiddle)

what about:

    if ( p.dist(pnn) <= 4) {
        take nearest node;
    } else if ( p.dist(pnn) > p.dist(pmiddle) ) {
        take nearest segment;
    } else {
        take nearest node;
    }

optionally test only for segments shorter than a certain length (which would then make the behavior for long way segments running through the node cloud consistent - else a long wayseg that has its pmiddle within the node cloud takes part in selection and a long wayseg that has its pmiddle outside of the node cloud but has otherwise the same position as the first, would not. also, if the middle of a long way segment is adjacent to a lonely node, that middle would take space away from the nearest node snap circle, even though the long way segment can probably be easily selected elsewhere. so restriction of seglength might be a good thing to still add to the pseudo code above, i suppose)

seb, do i completely miss something or would that be just the slickest way to do it?

[bastiK]

I didn't think it could be that simple! This should work indeed.

(a) only way segments within snap distance (length does not matter)

(b) only nodes within snap distance and segments with length >= L

(c) both nodes and small segments compete.

In case (c) your code would start. We should keep in mind that segments can be on top of each other and it should be possible to toggle them with ALT.

[bastiK]

Open attached file and zoom to ~ 230 m. Then click the lower 3 nodes in a random pattern. Sometimes the way gets selected on mouse down and after mouse up, the node is selected.

[anonymous]

Replying to bastiK:

Open attached file and zoom to ~ 230 m. Then click the lower 3 nodes in a random pattern. Sometimes the way gets selected on mouse down and after mouse up, the node is selected.

uh, yes. i also know why this happens. note, it exactly happens, if a node is selected and you try to select sth else. what happens is:

• nearest node found will be the one selected (this does not have to be the real) nearest node

• on mouse down, the nearest wayseg is closer than the selected, nearest node, so the wayseg is selected

• on mouse up, the wayseg is selected, so the nearest node is different (presumably it is the real nearest node now, that is returned) - if this nearest node is nearer to p than the wayseg, nearest node is taken

i'll come up with a solution, should not ben too hard..

[cmuelle8]

for the record, a major thing i found while working this out was:

• do not change the selection on mousePressed, IF there is something selected already

a lot of "randomness" for the user experience was taken out by fixing a bug in the p to way perpendicular distance calculation:

• multiple way segments with the _same_ end nodes sometimes had a slightly different distance to p because of rounding errors introduced by the double calculation od perDist and the fact that some ways run reversed compared to others, which switched the end points in the calculation around..

greetings,
christian

[bastiK]

select way then ALT-click one of its nodes

java.lang.NullPointerException
	at java.util.AbstractCollection.removeAll(AbstractCollection.java:336)
	at org.openstreetmap.josm.gui.NavigatableComponent.getNearestNodes(NavigatableComponent.java:494)
	at org.openstreetmap.josm.gui.NavigatableComponent.getNearestNodes(NavigatableComponent.java:526)
	at org.openstreetmap.josm.gui.NavigatableComponent.getNearestNodesOrWays(NavigatableComponent.java:815)
	at org.openstreetmap.josm.gui.NavigatableComponent.getNearestNodesOrWays(NavigatableComponent.java:839)
	at org.openstreetmap.josm.actions.mapmode.SelectAction.mouseReleased(SelectAction.java:431)
	at java.awt.AWTEventMulticaster.mouseReleased(AWTEventMulticaster.java:272)
	at java.awt.Component.processMouseEvent(Component.java:6263)
	at javax.swing.JComponent.processMouseEvent(JComponent.java:3267)
	at java.awt.Component.processEvent(Component.java:6028)
	at java.awt.Container.processEvent(Container.java:2041)
	at java.awt.Component.dispatchEventImpl(Component.java:4630)
	at java.awt.Container.dispatchEventImpl(Container.java:2099)
	at java.awt.Component.dispatchEvent(Component.java:4460)
	at java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:4574)
	at java.awt.LightweightDispatcher.processMouseEvent(Container.java:4238)
	at java.awt.LightweightDispatcher.dispatchEvent(Container.java:4168)
	at java.awt.Container.dispatchEventImpl(Container.java:2085)
	at java.awt.Window.dispatchEventImpl(Window.java:2478)
	at java.awt.Component.dispatchEvent(Component.java:4460)
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:599)
	at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:269)
	at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:184)
	at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:174)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:169)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:161)
	at java.awt.EventDispatchThread.run(EventDispatchThread.java:122)

[cmuelle8]

yes, this was introduced with my fix to the node merge stuff.. but its still broken (the node merge stuff).. please have a coffee..

[bastiK]

Select way, then try to drag a node of the way. Result: way moves; Expected: node moves

[cmuelle8]

Replying to bastiK:

Select way, then try to drag a node of the way. Result: way moves; Expected: node moves

if the way is selected, the way should be moved..

• click and drag should either move the selection, if there is one, or it selects (only if there is nothing selected) and then moves..
• imagine that you have a more complex selection. e.g. consisting of some nodes and a way. what you would do with your expected behavior -if the user by chance drags this selection by clicking the node- is deselecting anything else of the complex selection and then move the node. this clearly cant be a desired behavior..

[cmuelle8]

Replying to bastiK:

Select way, then try to drag a node of the way. Result: way moves; Expected: node moves

there is a comment in SelectAction that predates all of my changes..

// Don't replace the selection now if the user clicked on a
// selected object (this would break moving of selected groups).
// We'll do that later in mouseReleased if the user didn't try to
// move.

one could argue that the node you want to click is not a selected object. but, indeed, this is really fragile. i fear that getting the node, when a way is selected will bring us back to behavior observed in #comment:12

[bastiK]

I'm not saying, you variant is clearly better or worse, but both makes sense to some degree. In this case we should stick with what is already established so that users don't have to relearn.

I don't understand the code well enough to comment on the technical difficulties, but you have been warned. :)

[cmuelle8]

Replying to bastiK:

I'm not saying, you variant is clearly better or worse, but both makes sense to some degree. In this case we should stick with what is already established so that users don't have to relearn.

I don't understand the code well enough to comment on the technical difficulties, but you have been warned. :)

Since both "makes sense", both should be available, so I made this configurable with the old, usual behavior being the default - the setting in prefs is called "mappaint.select.waits-for-mouse-up"... (just a matter of adding the pref, actually, since the single parameter to toggle the behavior was already there)

Also I put node precedence code into a helper routine "isPrecedenceNode()" that is used by getNearestNodeOrWayImpl.. in it, node precedence rules are calculated. If the helper returns true, a nearest way is not looked for and the node is taken, else nearest wayseg gets a chance in the style of comment:9

The helper makes the code more readable and if you want to modify the behvaior of node/wayseg selection balance, this is most probably the spot to do it..

Furthermore, I tried to take care of icon nodes - we do not want to give small waysegs a chance, if they are covered by an icon node. So at the moment, what I do is use isTagged() property to determine if we should yield the node. Documentation says, that uninteresting tags (like source or created_by) are not used to calculate isTagged(), so we have a high chance, that the tagged node is also an icon node - of course, this is not a guarantee. It'd be better if class Node has a method isIconDrawn() or similar. You may modify the behavior, whether to yield tagged nodes or not, in "NavigatableComponent:isPrecedenceNode()"

The process I undertook of separating "nearest selected" and "true nearest" primtive stuff in getNearestNodeOrWay also solved the problem you had in click1.osm in combination with the usual-selection-behavior you expect.

Taking the true nearest node, regardless of current selection delivers the same result in mouseup and mousedown now and the toggling that appeared in click1.osm-testcase is gone.

updated patch in a minute..

[bastiK]

#5457/toggle-objects.xml seems to be broken again. (Toggles only 2 of 3 objects)

I try to test thoroughly, but it is so complex, that you can easily miss certain patterns and use cases. Therefore we can't apply this change before next release end of the week. Guess you can take your time. :)

[cmuelle8]

Replying to bastiK:

#5457/toggle-objects.xml seems to be broken again. (Toggles only 2 of 3 objects)

I try to test thoroughly, but it is so complex, that you can easily miss certain patterns and use cases. Therefore we can't apply this change before next release end of the week. Guess you can take your time. :)

hm. see - this is what i was talking about. i stated earlier that changing the selection on mouse down is not a good idea. once more i found out why :)

what happens technically is:

• change selection on mouse down, well, changes the selection.
• the cycling algorithm looks for the selected prim in the selectionList and then takes the next thereafter.
• that it toggles at all is related to selMorePrims detection.
• selMorePrims is set on mousePressed. it is true, if what is about to become selected is already selected in the mapview.
• say we have a node list (1,2,3).
  • on mousedown 1 is selected, selMorePrims will be false, so mouseup keeps 1.
  • second click. mousedown selects 1, selMorePrims will be true, so mouseup cycles to 2.
  • third click. mousedown selects 1, selMorePrims will be false (only 2 is selected atm), so mouseup keeps 1.
  • goto second click..

so ATM it boils down to:

) do not change the selection on mouse down (which makes detection of the selected prim in the selectionList checked on mouseup reliable)

) rework the cycle detection in selectPrims somehow, or build something complex to track state changes.. (yuck)

) note, as using ctrl uses mouseReleases only to toggle the selection, your example should work with ctrl pressed (as mousedowns do not toggle selection state in between mouseups) - I guess you can confirm that !?

[bastiK]

Replying to cmuelle8:

) do not change the selection on mouse down (which makes detection of the selected prim in the selectionList checked on mouseup reliable)

Cannot imagine that.

) rework the cycle detection in selectPrims somehow, or build something complex to track state changes.. (yuck)

Why not? After all, from a user perspective it looks like a state change:

• certain clicks do not necessarily select the "best" object but the next in the cycle. ("rotation mode")
• clicking somewhere else seems to break the cycle, so it starts at the beginning. ("normal mode")

(Maybe suffices to remember selection before mousedown event?)

) note, as using ctrl uses mouseReleases only to toggle the selection, your example should work with ctrl pressed (as mousedowns do not toggle selection state in between mouseups) - I guess you can confirm that !?

Yes, can confirm.

[bastiK]

When I click in a pile of nodes with ALT, it selects one node. After mouse release, another node gets selected.

[cmuelle8]

Replying to bastiK:

When I click in a pile of nodes with ALT, it selects one node. After mouse release, another node gets selected.

this was a quick fix, i did not reset selMorePrims to false before looking if it should be true on mouse down..

i did reorganize SelectAction, put in more modularity, separated code into functions, so it is easier to see what the code parts do..

some stuff also makes more sense, now that things got a proper (function) name that lingered in huge function bodies before..

have fun..

[anonymous]

i still found some bugs i'm working on right now.. one is double precision related in NavigatableComponent..

• long ways with way segments lying way out of the MapView will make getPoint in NC overflow
• since getPoint depends on map scale, if you zoom in close it leads to wrong detection in (at least) all getNearest* methods for those way segments
• what's worse is, using Point you loose a lot of possible precision (since Point uses int for coordinate storage, but all coordination transforms are in double, which has some more room before overflowing)
• what's still worse is, there is an alternative - it is Point2D.Double
• luckily Point is derived from Point2D - so we can intermix without problems when doing calculations
• luckily Point.distanceSq() is Point2D.distanceSq() method for distance calculation (which in fact uses double..)
• ATM i use Point2D.Double in NavigatableComponent, but since there are lots of callers depending on getPoint delivering Point, i exchanged the public getPoint() methods with a wrapper around private getPoint2D() methods, that are used internally in NavigatableComponent to get more precision for the distance calculation..
• that way precision by casting from double to int is only lost for external callers of getPoint() methods and we don't have to change all the callers (which would be a mess, I suppose)

• of course, all of this only pushes the problem further away.. if you zoom in close enough, and have a long way with way segments far (only far enough) outside the mapview, the problem persists..

updated patch follows..

[cmuelle8]

fixes most issues, more documentation, more cleanup done, please test thoroughly

[bastiK]

I don't think there is much of a problem:

• If it were, we would see drawing artifacts all the time.
• There is no overflow when casting double to int. When double is too large it will return ±(2³¹-1)
• Way must be huge before it even happens. We usually don't have ways that large in osm.
• int is more efficient than double.

[anonymous]

Replying to bastiK:

I don't think there is much of a problem:

• If it were, we would see drawing artifacts all the time.
• There is no overflow when casting double to int. When double is too large it will return ±(2³¹-1)
• Way must be huge before it even happens. We usually don't have ways that large in osm.
• int is more efficient than double.

• think of boundaries or other long ways - like bigger wood areas or often power lines.. i only needed to multiply perDist with 100 to hit the problem (with a zoom at about 50 mtrs problem popped up, with, admittedly a very long way..)
• often, even if such long ways are rarely found in osm database, mappers convert long gpx tracks to ways, and then work with them (e.g. cut them into segments, etc.) - it's a use case you come across often, depending on the way people work with josm..
• if you have two layers, one with data from the db, the other with converted gpx2osm data, it is easy to imagine, that a user zooms in closely to know exactly where to split the segment
• as soon as people might start to map the inside of buildings (i've already read about this somewhere) you have high zoom levels very often and you will get the problem more often
• as to your argument about int efficiency: distanceSq uses double to do the calculation anyway, so the contrary might be the case - it should not be so inefficient to use Point2D.Double, because you avoid casting around to ints (or from ints) everytime, just for the sake of storage in between doing calculations..
• by casting double to int, you might get the nearest number possible with an int, but it is a clear loss of precision, if the double in question is exceeding INT_MAX or INT_MIN
• most cpus should not have a problem doing double calculation (additionally the question arises, why SUN's engineers didn't override distanceSq in Point to use int for the calculation..)
• ATM, NavigatableComponent hands out Point to external callers, so ATM there are few, overseeable, but vital places that benefit from the change

greetings

[anonymous]

Replying to bastiK:

I don't think there is much of a problem:

• If it were, we would see drawing artifacts all the time.

no, when drawing you do calculations the other way around. the user clicks points in a very defined rectangle (the viewport) and no distance calculation besides pixel distance of point clicked to point of center of the viewport is involved. the possibility of large numbers in this calculation is minimal..

the problem is that nodes of a way may lie far outside the viewport. what you calculate is pixel distance of that node to the center of the viewport. for any given way, if mapscale is only small enough, this pixel distance can grow insanely large.

actually, for getNearestWaySegmentsImpl at least, we could put in some simple detection to simply not do the calculation for Points that are far off the viewport. after all, it is unlikely that a single way segment will be interesting to select if it's points are close to DOUBLE_MAX/MIN values away from the viewport center.

do you know, if a double division by a really small number (scale), that busts DOUBLE_MAX or DOUBLE_MIN will reliably return positive or, respective, negative infinity?

greetings

[anonymous]

Replying to bastiK:

• There is no overflow when casting double to int. When double is too large it will return ±(2³¹-1)

what would be the correct term to describe this behavior? i see that overflow is used in a different context, but what terms would we use here?

• if double value is greater than the range of values an int can hold, casting "rounds to the nearest int possible".

correct?

[bastiK]

Ok, I guess it's better to be on the save site. Say you have a segment that is in screen coordinates (-5·10⁹, -6·10⁹) to (7·10⁹, 8·10⁹). After casting to int it would be (-a,-a) to (a,a) with a=2³¹-1. This means it would be painted right across the screen. Not sure if the line clipping can handle this, but theoretically it should.

do you know, if a double division by a really small number (scale), that busts DOUBLE_MAX or DOUBLE_MIN will reliably return positive or, respective, negative infinity?

No, but you can simply test. Should be consistent on all platforms.

There is no overflow when casting double to int. When double is too large it will return ±(2³¹-1)

what would be the correct term to describe this behavior?

Not sure, does "loss of magnitude" sound smart enought?

[cmuelle8]

Replying to bastiK:

Ok, I guess it's better to be on the save site. Say you have a segment that is in screen coordinates (-5·10⁹, -6·10⁹) to (7·10⁹, 8·10⁹). After casting to int it would be (-a,-a) to (a,a) with a=2³¹-1. This means it would be painted right across the screen. Not sure if the line clipping can handle this, but theoretically it should.

if it does, we could use the line clipping algo in getNearestWaySegementsImpl to sort out way segments before calculating perpendicular distances of p to the segment

still i think that it does not hurt to use Point2D.Double to avoid casts - since they are mostly used for viewport related calculation, it should not be expensive on memory either (maybe i'm wrong on this and there are some spots in the code where those Points pile up, but usually they are a job for the garbage collector after work has been done..)

best regards,
Christian

[bastiK]

The LineClip class supports only integer input at the moment. A simple envelope test might be enough, here: Dismiss the segment if it lies completely above / below the viewport or to the left / to the right of it. This should filter 90% of all off screen segments.

[bastiK]

In [3594/josm]:

applied #5500 (patch by cmuelle8) - improve SelectAction and NavigatableComponent

[bastiK]

see #5561