Modify

Opened 14 years ago

Closed 14 years ago

#5457 closed enhancement (fixed)

[patch] make it easier to select ways and nodes, if there are multiple primitives under the cursor

Reported by: cmuelle8 Owned by: anonymous
Priority: major Milestone:
Component: Core Version:
Keywords: select selection multiple ways objects gui Cc:

Description

Hi,

my second patch for josm :) I modified SelectAction, so that on mouse release it toggles through the possible completions under the cursor. Note that it takes the semantics of Ctrl and Shift into account and does not modify the behaviour of area selects..

No modifier: select single object or rotate selection if there are more than one

Ctrl modifier: leave anything selected as is, but rotate through the possible completions /at the cursor/

Shift modifier: leave anything selected as is and gradually add all possible completions /at the cursor/ to the selection

Note that using Ctrl does not rotate through all objects without giving a deselect step

Attachments (6)

SelectAction.java.r3529.patch (5.4 KB ) - added by cmuelle8 14 years ago.
patches SelectionAction to enable the user to more easily select multiple primitives under the cursor
SelectAction.java.brains.patch (7.4 KB ) - added by cmuelle8 14 years ago.
brains patch
toggle-objects.xml (2.4 KB ) - added by bastiK 14 years ago.
SelectAction.java.patch (7.4 KB ) - added by cmuelle8 14 years ago.
fix GetNearestNodes() problem not returning all nearest nodes by using GetAllNearest() .. probably needs a new bug ticket for GetNearestNodes() problems !?
bulk2.osm (2.3 KB ) - added by bastiK 14 years ago.
2nd example
SelectAction.java.monster.patch (12.8 KB ) - added by cmuelle8 14 years ago.
fixes most issues, but please test.. if you want to apply it, please take out System.out.println debug lines

Download all attachments as: .zip

Change History (51)

by cmuelle8, 14 years ago

patches SelectionAction to enable the user to more easily select multiple primitives under the cursor

comment:1 by cmuelle8, 14 years ago

please comment or delete the System.out.println 's before commiting..
i just used them for testing..

comment:2 by bastiK, 14 years ago

Isn't that what Alt / Alt Gr is for? Apart from that, I was always wondering why this toggle mode is not default.

in reply to:  2 comment:3 by cmuelle8, 14 years ago

Replying to bastiK:

Isn't that what Alt / Alt Gr is for? Apart from that, I was always wondering why this toggle mode is not default.

for me using Alt is totally unreliable (except using it together with a mouse drawn selection area) -- it does toggle but the results are mediocre. it does not deliver all completions and together with ctrl it does not make a difference to the usual ctrl selection mode.

try the patch - you will notice the difference.

best regards,
christian

in reply to:  2 comment:4 by cmuelle8, 14 years ago

Replying to bastiK:

Isn't that what Alt / Alt Gr is for? Apart from that, I was always wondering why this toggle mode is not default.

when alt is pressed with current josm build, then

using shift does seem to work the way expected - it gradually selects anything

using no modifier it seems to just toggle between to alternatives

using ctrl it does not seem to make a difference..

comment:5 by stoecker, 14 years ago

Actually from description you do same as middle-click --> cycling through primitives.

As you describe it above I fear you will break a lot of stuff, as shift, alt, ... are overloaded in so many ways, that there simply is no free room to add anything without breaking other stuff. For example if you cycle through elements, then it is no longer possible that a click always selects already selected element. This is essential for a lot of operations.

in reply to:  5 ; comment:6 by cmuelle8, 14 years ago

Replying to stoecker:

Actually from description you do same as middle-click --> cycling through primitives.

As you describe it above I fear you will break a lot of stuff, as shift, alt, ... are overloaded in so many ways, that there simply is no free room to add anything without breaking other stuff. For example if you cycle through elements, then it is no longer possible that a click always selects already selected element. This is essential for a lot of operations.

can you give examples for such operations? in the past i found it rather unintuitive that i "can't get to" the other elements under the cursor and instead get the same element over and over :)

i understand that this is a big change and it will need lot's of testing, but for the long run, i suppose it will be a real benefit to josm

if you don't want to take the risk of breaking things in trunk, we might want to consider replacing the previous behavior of ALT and subsitute that of the patch - this way we'd have at least some of the benefits..

having a list of operations depending on "clicking selects what's already selected" would be nice to have, so that eventually we could sort out those issues and make it the default selection behavior (instead of hiding it using ALT)

regards, christian

comment:7 by bastiK, 14 years ago

I think you like to do 2 things:

  • fix ALT
  • make ALT default

Maybe start with the first one, as you suggested?

in reply to:  5 comment:8 by cmuelle8, 14 years ago

just noticed a minor bug..

when ctrl is pressed and we hit a fresh spot, i should take the first object in selList to start with, not the last. else, if a node is under the cursor, the node is selected last in the rotation list, which is cumbersome..

updated patch follows..

comment:9 by stoecker, 14 years ago

You did not answer my main concern. Where is the difference between you proposal and middle click menu?

in reply to:  9 comment:10 by cmuelle8, 14 years ago

Replying to stoecker:

You did not answer my main concern. Where is the difference between you proposal and middle click menu?

  • middle click menu (mcm) only works if no modifier keys are used - this is what i found out by playing aroung with it
  • mcm is to easy to forget. i've used josm since a long time now and even though i remember having used the feature long ago it did, for whatever reason, not make it's way into my personal standard reportoire of ways to interact with the data. clearly, this is a highly subjective argument.
  • i would not want to miss mcm, it gives a detailed, graphical overview of what is under the mouse. in that it is user and newbie-friendly.

  • you cannot use mcm however, if there is no MMB (e.g. netbooks, touchpads, etc.)
  • even with a MMB: since it is often combined with the scrollwheel, scrolling gets in the way sometimes (this clearly depends on the user, the mouse model, and the reception of the user - some might like it that way, others may have problems with it)

  • from a workflow point of view I find popup-menus distracting, at least for the most /basic/ task of josm, which is selecting things
  • still arguing on the workflow: ideally the LMB and the two standard modifier keys CTRL and SHIFT ought to be enough to make every selection scenario that is /possible/ also /available/

->that said, i do not propose that LMB+SHIFT+CTRL should be /the one and only way/ and also, in some cases, it does not have to be /the shortest way/ to get a specific selection set

  • other methods are ) using search, ) using MMB for a single sel, ) using rectangular area selection mode [+ modifier keys] and ) using combinations of available methods

in summary, it is good to have that feature for an overview (see above), but it is impractical and slow for experienced users. moreover the menu also invalidates part of the mapview, so it repaints more often - i do not know, if this objectively slows down things, but just having to refocus with your eye on the popup is clearly a slowdown for human mappers :-) - so it'd be nice to have an alternative.

in reply to:  6 ; comment:11 by bastiK, 14 years ago

Replying to Christian Müller <cmue81@…>:

if you don't want to take the risk of breaking things in trunk, we might want to consider replacing the previous behavior of ALT and subsitute that of the patch - this way we'd have at least some of the benefits..

Please do that. A clear improvement is no problem to commit. The disputed part (i.e. making ALT default) can be sorted out later.

in reply to:  11 comment:12 by cmuelle8, 14 years ago

Replying to bastiK:

Replying to Christian Müller <cmue81@…>:

if you don't want to take the risk of breaking things in trunk, we might want to consider replacing the previous behavior of ALT and subsitute that of the patch - this way we'd have at least some of the benefits..

Please do that. A clear improvement is no problem to commit. The disputed part (i.e. making ALT default) can be sorted out later.


ok, here it goes - changes in latest patch:

  • ALT modifier activates new selection behavior
  • selectaction.rotates=true in preferences enables it as default mode
  • there is more consistency with the standard behavior while cycling
  • if there is at least one nearby node, _only_ cycle through the nodes and start with the one that would be selected by getNearestCollection
  • if there is no nearby node, cycle through all possible ways
  • separating the two cases "mouse nearby node" and "mouse not nearby node" is more intuitive than what i had used in the last patch (cycling through AllNearest regardless of wheter a node was directly under the cursor or not)

-> rationale1: if things are cluttered, it should be easy enough to just scroll in than to cycle through a large list containing both, nodes and ways
-> rationale2: practically most of the time, if the user hovers over a node, he wants to select that node (or a node very close by) and not a way

by cmuelle8, 14 years ago

brains patch

comment:13 by cmuelle8, 14 years ago

ideas to still take this further:

  • investigate getNearestCollectionVirtual (it builds virtualWays/virtualNode which are used by the drag action). if somebody could shed some light on this, i'd be glad. when/how/under what circumstances is the virtualNode used to create wnew? how do i trigger that code path using josm?
  • cleanup or change getNearestCollectionVirtual depending on wheter code is still needed (or depended upon by other operations)
  • (cache selectionList in mouseReleased if computation on each cycling step is too resource intensive) ??

(just jotting this down in case anyone else wants to take a look at it. at this moment the current state of the code works quite well, i s'pose)

in reply to:  13 ; comment:14 by bastiK, 14 years ago

If i have the curser over 3 ways or 3 nodes, it just switches between 2 of them, but should toggle all 3.

Replying to Christian Müller <cmue81@…>:

ideas to still take this further:

  • investigate getNearestCollectionVirtual (it builds virtualWays/virtualNode which are used by the drag action). if somebody could shed some light on this, i'd be glad. when/how/under what circumstances is the virtualNode used to create wnew? how do i trigger that code path using josm?

I think it is for cross hair nodes in the middle of a way segment.

Generally, the part of the code you are touching is very fragile and it is quite easy to introduce regressions unless you are extremely careful. There is certainly some need for improvement and clean up here (e.g. ALT in add mode is broken), but don't run out on us, when bug reports are coming! ;-)

in reply to:  14 ; comment:15 by cmuelle8, 14 years ago

Replying to bastiK:

If i have the curser over 3 ways or 3 nodes, it just switches between 2 of them, but should toggle all 3.

can you give me a link with data to download and test? - on my side i can't reproduce this - I really get all of the ways/nodes when cycling..

I think it is for cross hair nodes in the middle of a way segment.

thx for the hint..

.. certainly some need for improvement and clean up here (e.g. ALT in add mode is broken)

do you mean my patch breaks ALT in add mode, or did you mean ALT in add mode was broken before with the standard code? if the latter - what was ALT supposed to do in add mode?

greetings..

by bastiK, 14 years ago

Attachment: toggle-objects.xml added

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

Replying to Christian Müller <cmue81@…>:

Replying to bastiK:

If i have the curser over 3 ways or 3 nodes, it just switches between 2 of them, but should toggle all 3.

can you give me a link with data to download and test? - on my side i can't reproduce this - I really get all of the ways/nodes when cycling..

See attachment: first time it toggles all 3 then only 2.

.. certainly some need for improvement and clean up here (e.g. ALT in add mode is broken)

do you mean my patch breaks ALT in add mode, or did you mean ALT in add mode was broken before with the standard code? if the latter - what was ALT supposed to do in add mode?

Was broken before. Shortcuts :

"Holding the "Alt" key creates a separated though connected way (same you could do afterwards by selecting a node and press P). This mode is marked by a small bar at the previous node."

in reply to:  15 ; comment:17 by cmuelle8, 14 years ago

Replying to bastiK:

If i have the curser over 3 ways or 3 nodes, it just switches between 2 of them, but should toggle all 3.

This seems to be an issue with GetNearestNodes().. It does not what you would expect from the name.. I use GetAllNearest() now to fix it, it has all three nodes, while the collection I retrieve with GetNearestNodes() does not. See patch..

greetings

by cmuelle8, 14 years ago

Attachment: SelectAction.java.patch added

fix GetNearestNodes() problem not returning all nearest nodes by using GetAllNearest() .. probably needs a new bug ticket for GetNearestNodes() problems !?

in reply to:  17 ; comment:18 by bastiK, 14 years ago

Replying to Christian Müller <cmue81@…>:

Replying to bastiK:

If i have the curser over 3 ways or 3 nodes, it just switches between 2 of them, but should toggle all 3.

This seems to be an issue with GetNearestNodes().. It does not what you would expect from the name.. I use GetAllNearest() now to fix it, it has all three nodes, while the collection I retrieve with GetNearestNodes() does not. See patch..

Cannot reproduce the problem: Main.map.mapView.getNearestNodes(e.getPoint(), OsmPrimitive.isSelectablePredicate); reports 3 nodes when I click the nodes pile.

in reply to:  18 comment:19 by cmuelle8, 14 years ago

Replying to bastiK:

Replying to Christian Müller <cmue81@…>:

Replying to bastiK:

If i have the curser over 3 ways or 3 nodes, it just switches between 2 of them, but should toggle all 3.

This seems to be an issue with GetNearestNodes().. It does not what you would expect from the name.. I use GetAllNearest() now to fix it, it has all three nodes, while the collection I retrieve with GetNearestNodes() does not. See patch..

Cannot reproduce the problem: Main.map.mapView.getNearestNodes(e.getPoint(), OsmPrimitive.isSelectablePredicate); reports 3 nodes when I click the nodes pile.

did you test this with your toggle-objects.xml? if so, then there must have been something wrong at if(!(c.contains(n)) c.add(n); statements I had before, since this is the only place where I changed things (and it works in latest patch..)

thx for the input,
christian

in reply to:  18 ; comment:20 by cmuelle8, 14 years ago

Replying to bastiK:

Cannot reproduce the problem: Main.map.mapView.getNearestNodes(e.getPoint(), OsmPrimitive.isSelectablePredicate); reports 3 nodes when I click the nodes pile.

Ok, the real scoop of this, is that

  • GetAllNearest always delivers primtives in the same order for a specific position, it does not matter what's selected in the MapView for this
  • GetNearestNodes places the prefers selected elements and puts them first in its returned collection

While cycling through the primtives my patch does not keep a history of the elements selects, so it depends on the way GetAllNearest behaves. Since GetAllNearest does not seem to take into account what is already selected in the MapView, I suggest that relying on its behavior is save.
If the order/sequence of primitives changes in between clicks on the _same_ spot, which is what GetNearestNodes does - it puts the primitive selected first -, I cannot rely on the order of the list returned and would have to keep track of the history of selected elements at a certain spot myself.

So my last patch, which does not use GetNearestNodes() and entirely depends on GetAllNearest() does indeed fix the bug, but GetNearestNodes() is not broken in the way I suspected in a prior comment - it does also deliver all nodes, but the ordering is unstable.

greetings,
cmuelle8

in reply to:  18 ; comment:21 by cmuelle8, 14 years ago

ok, one more thing:

  • I understood the virtualWays / virtualNode stuff (thanks for the hint!)
  • I suggest that, regardless of wheter alt is pressed or not (and therefor regardless if alt is the default selection behavior or not), that /always/ allWaySegements in getNearestCollectionVirtual() are considered:

/as long as/ no segment of the bulk was selected in mapview: ))

if we have multiple segments on top of each other and the user wants to add a virtualNode in their middle, it does not make sense to only insert the virtualNode into one of the ways, . reasons for this:

  • visually the bulk appears to the user as a single way
  • say we have two or more nearby areas sharing the segment. in almost any case, if the user wants to correct the border of an area by adding a node, he wants to correct all other areas that shared the segment as well.
  • speaking for myself, i previously found myself often adding a node to the segment, thereafter consecutively adding nodes to all areas left with the old segment as well and joining them with the first created virtualNode. this was cumbersome and effectively i could have avoided this if i knew that the josm code provided for this case with the ALT modifier (but actually it has nothing to do with what ALT does for selection, since adding a virtualNode does not change the selection)
  • again, since the user has no feedback to which way the virtualNode will be added (if nothing is selected at that spot), adding the virtualNode to allWaySegments under the cursor make sense, is clean and doesn't get into the mappers way when using josm..

if there are waySegments selected in the mapView /at the spot the virtualNode is about to be added/: ))

  • just add the virtualNode to all the waySegments selected (this is the case the user has a visual feedback, through the selection and it will intuitively make sense to him that the virtualNode is only added to selected waySegments)

updated patch within the next hours..

in reply to:  21 comment:22 by cmuelle8, 14 years ago

Replying to Christian Müller <cmue81@…>:

(but actually it has nothing to do with what ALT does for selection, since adding a virtualNode does not change the selection)

this is not true, it changes the selection to the node added. what i meant is, that ALT at the moment a (admittedly slightly) different meaning in different contexts:

-use all waySegments and add virtualNode
-toggle possible waySegments
-select anything cut by the selection area

while you have a visual feedback for the latter two cases, you do not for the addition of the virtualNode. this is why I think the behavior proposed in the last post is better.. in a way it reverses things:

  • at the moment: adds Node to single way, but you don't know which one, if nothing is selected
  • at the moment: with ALT pressed, adds Node to all segments
  • at the moment: what's selected in the mapview does not matter!
  • proposed: if nothing is selected, add Node to all segments
  • proposed: if segments selected, add Node to those segments
  • proposed: no modifier for adding virtualNode used, selection used to modify its behavior..

in reply to:  21 comment:23 by bastiK, 14 years ago

Replying to Christian Müller <cmue81@…>:

updated patch within the next hours..

Please create new ticket or wait until the first part is done.

in reply to:  20 ; comment:24 by bastiK, 14 years ago

Replying to Christian Müller <cmue81@…>:

Replying to bastiK:

Cannot reproduce the problem: Main.map.mapView.getNearestNodes(e.getPoint(), OsmPrimitive.isSelectablePredicate); reports 3 nodes when I click the nodes pile.

Ok, the real scoop of this, is that

  • GetNearestNodes places the prefers selected elements and puts them first in its returned collection

No, you do! ;)
Line 432 changed from c = new ArrayList<OsmPrimitive>(c); to c = new ArrayList<OsmPrimitive>();. The other changes don't really matter.

Have a look at the code for getNearestNodes and getAllNearest! It is almost identical.
(No need to update the patch.)

by bastiK, 14 years ago

Attachment: bulk2.osm added

2nd example

comment:25 by bastiK, 14 years ago

In the 2nd example, if you zoom out, the nodes appear to be on one spot. But it toggles at most 2 of them. Is this intended?

in reply to:  21 ; comment:26 by bastiK, 14 years ago

Replying to Christian Müller <cmue81@…>:

ok, one more thing:

  • I understood the virtualWays / virtualNode stuff (thanks for the hint!)
  • I suggest that, regardless of wheter alt is pressed or not (and therefor regardless if alt is the default selection behavior or not), that /always/ allWaySegements in getNearestCollectionVirtual() are considered:

I don't feel competent to appraise these ideas, but it sounds good.

(One thing that annoys me about virtual / cross hair nodes: It always selects the way for a split second, so it slows you down when painting the whole area takes long.)

in reply to:  26 comment:27 by bastiK, 14 years ago

Replying to bastiK:

Replying to Christian Müller <cmue81@…>:

ok, one more thing:

  • I understood the virtualWays / virtualNode stuff (thanks for the hint!)
  • I suggest that, regardless of wheter alt is pressed or not (and therefor regardless if alt is the default selection behavior or not), that /always/ allWaySegements in getNearestCollectionVirtual() are considered:

I don't feel competent to appraise these ideas, but it sounds good.

@Dirk: Can you have a look?

in reply to:  24 comment:28 by cmuelle8, 14 years ago

Replying to bastiK:

No, you do! ;)
Line 432 changed from c = new ArrayList<OsmPrimitive>(c); to c = new ArrayList<OsmPrimitive>();. The other changes don't really matter.

true, sometimes two eyes and one brain are not enough.. ;-)

by cmuelle8, 14 years ago

fixes most issues, but please test.. if you want to apply it, please take out System.out.println debug lines

in reply to:  25 comment:29 by cmuelle8, 14 years ago

Replying to bastiK:

In the 2nd example, if you zoom out, the nodes appear to be on one spot. But it toggles at most 2 of them. Is this intended?

latest patch solves most issues for me - i left my debug statements in, so you can follow along if you still find some mysteries in the data..

it basically takes on getNearestCollectionVirtual() to better fit what I do in mouseReleased() ..

please test it thoroughly, I might have overlooked some cases, when testing..

greetings

comment:30 by stoecker, 14 years ago

Actually I'm not 100% sure what I should comment about, but I try nevertheless. Virtual nodes only affect one way ATM. It has been requested already (there is a ticket for this probably) that they affect all ways. I did not like this in the beginning, but I think I changed my opinion and probably now agree that all ways should be affect when they overlap with IDENTICAL segments (i.e. when also the virtual nodes overlap). But I think there should be a visual indication if one or more ways are affected.

Did I answer correct question?

When we change JOSM behavior as proposed I hope Christian is available at least the next two weeks to fix related issues and bug reports, which surely will come in. Please create a login, so we can assign bugs to you.

comment:31 by bastiK, 14 years ago

(In [3550]) see #5457 (patch by Christian Müller) - make it easier to select ways and nodes, if there are multiple primitives under the cursor

in reply to:  30 comment:32 by bastiK, 14 years ago

Ok, applied it. If there are regressions, we will find out the hard way.

Replying to stoecker:

Did I answer correct question?

Yes, as Christian noted, this feature (virtual node pulls all segments under the curser) was already implemented as ALT behaviour, which is now set to default.

comment:33 by bastiK, 14 years ago

I think the following is not optimal:

Set selectaction.cycles.multiple.matches=true or hold ALT. Then move to an area where multiple nodes are within snap distance. Now select one of the nodes then move the mouse a little and try to select another node in addition nearby (holding CTRL). This deselects the current and selects a "random" node in snap distance.

Expected: In addition to the first node, select the nearest to cursor.

in reply to:  33 comment:34 by anonymous, 14 years ago

Owner: changed from team to anonymous
Status: newassigned

Replying to bastiK:

I think the following is not optimal:

Set selectaction.cycles.multiple.matches=true or hold ALT. Then move to an area where multiple nodes are within snap distance. Now select one of the nodes then move the mouse a little and try to select another node in addition nearby (holding CTRL). This deselects the current and selects a "random" node in snap distance.

Expected: In addition to the first node, select the nearest to cursor.

  • what you would actually use in this situation is SHIFT (add node to selection - and it should deliver the nearest node to add to your selection (or else its a bug, which is likely to be the case, since getNearestNodes() in NavigatableComponent is not sorting the node list by distance yet, patch on this later.. getNearestWaySegements already sorts by distance though))
  • you may also resolve this by zooming in (so that nodes are not in one cycle group anymore)
  • a third way would be to change snap distance (but i think its hardcoded to 10px in NavigatableComponent !? and not user configurable using prefs !?)

if you still prefer to change the behavior, the following question arises. if a node group (nodes within snapDist) be neglected all together, so that just the nearest is taken in any case, how would you cycle through dup nodes (identical position) then?

still another solution, which is more a compromise to the above:

  • make snapDist for node groups smaller, but keep the old value for multiple way bulks, e.g. 10px for ways, 2px for nodes

comment:35 by bastiK, 14 years ago

Yes, SHIFT works as expected. But I still think that CTRL should be similar. Personally I don't use SHIFT very often. (With CTRL, when you click an object accidentally, you can deselect it without switching modifiers.)

The criterion could be: If the mouse cursor has moved, such that the nearest node is different, then break the cycle and toggle the selection for the new nearest node.

Another (maybe unrelated) feature request: Select way, if the mouse cursor is closer to the way segment than to any node, unless it is very close to the node, say 2-4 px.

in reply to:  35 ; comment:36 by bastiK, 14 years ago

Replying to bastiK:

Another (maybe unrelated) feature request: Select way, if the mouse cursor is closer to the way segment than to any node, unless it is very close to the node, say 2-4 px.

Basically make it possible to select a way even if it is filled with nodes at the current zoom level. One could also do like this: If the "nearest node" distance d1 and the "nearest node but one" distance d2 is very close (e.g. are only 30% different from each other) and the way segment is much closer than d1 and d2, then the user probably tries to click in the middle of a segment in order to select the way.

in reply to:  36 ; comment:37 by cmuelle8, 14 years ago

Replying to bastiK:

Replying to bastiK:

Resetting the node cycle and just starting a new one when the mouse has moved slightly is not an option, the two reasons being:

1) technical: if the old node is still within reach i'd then have two selected nodes in the cycle, which breaks the way i coded the toggling entirely, since i look for the selected node in the list, then take the next one (two selected nodes add ambiguousity). please note, that you can already break the cycle by using shift to add a second primitive within reach - in this case, if you switch back to CTRL, it already is somewhat random what primitive will be selected next _for a particular node/way group_

2) user-perspective: since most mouses have jitter of 1 or 2 px, esp. optical ones, again, it depends on the user, if he can hold still in between clicks, so basically I do not want to take slight, unnoticable moves to make state changes.



Ok, so I think your feature request and the node group selection problem are related and can be solved by coding the compromise I stated earlier:

  • make the snapDistance for getNearestNodes() smaller (but: it should not be smaller than the rectangle size used to draw a node in the mapview, imho)



There might be people that do not edit cluttered areas, generally zoom in more into an area to edit the geometry _and_ then expect the usual snapDistance behavior, so that the node in question is replied on a generous fuzzy click around its center). The problem is, that while you would like to have more precision within that 10px bound, others might not, because they like the fact that they do not have to precisely click the nodes spot to get it selected. So the question that arises is:

Should snapDistance just be configurable, with sensible defaults?

-- mappaint.node.snap-distance with a default of 10px (precision lovers would decrease this to the radius of the circumcircle of the node rectangle)

-- mappaint.way.snap-distance with a default of 10px



The lower the snap distance for nodes, the better you will be able to select the way segment, _but_ the more precision you need in order to select that node. The way you compute this in the background (changing snapDistance) or (comparing three calculated distance of the point clicked to the primitives nearby) does not really matter, imho.



Yet another solution could be to not consider node groups at all, and always take the nearest node within reach of the point clicked..

comment:38 by cmuelle8, 14 years ago

ps: for the last "yet another solution" the benefit of cycling between dup nodes would be gone, of course (this was mentioned somewhere earlier in the thread)

greetins

in reply to:  38 ; comment:39 by cmuelle8, 14 years ago

btw, the problem (if we categorize it as such)

"nearest way under cursor unselectable when node is nearby"

persisted before. the behavior stems from getNearest() in NavigatableComponent - it first looks for a NearestNode(), if one is found, waySegments are not searched for anymore (regardless of whether or not one might be closer to cursor position).

i'm not saying that we can't change this, but after all, i'd like to state that it's indeed a "feature request"..

what is your opinion? i'd say aiming for making it configurable with sensible defaults might be the best to aim for..

in reply to:  37 ; comment:40 by bastiK, 14 years ago

Replying to cmuelle8:

Replying to bastiK:

Replying to bastiK:

Resetting the node cycle and just starting a new one when the mouse has moved slightly is not an option, the two reasons being:

1) technical: if the old node is still within reach i'd then have two selected nodes in the cycle, which breaks the way i coded the toggling entirely, since i look for the selected node in the list, then take the next one (two selected nodes add ambiguousity). please note, that you can already break the cycle by using shift to add a second primitive within reach - in this case, if you switch back to CTRL, it already is somewhat random what primitive will be selected next _for a particular node/way group_

Well, this can be changed, if really necessary...

2) user-perspective: since most mouses have jitter of 1 or 2 px, esp. optical ones, again, it depends on the user, if he can hold still in between clicks, so basically I do not want to take slight, unnoticable moves to make state changes.

Would it help to sort the nearestNodes by distance?

Try to imagine typical use:

  • User has selected some stuff and wants to add a certain node from a pile of nodes. He/she could use SHIFT but decides for CTRL which should work as well.
  • click the node -> (a) the wrong node got selected -> click it again to deselect; try again

-> (b) done; now select a 2nd node in the same pile -> the original node should stay selected.

Actually the cycling is only needed when segments are on top of each other or nodes are very close. So one could have both a "toggle distance" and a "snap distance".



Ok, so I think your feature request and the node group selection problem are related and can be solved by coding the compromise I stated earlier:

  • make the snapDistance for getNearestNodes() smaller (but: it should not be smaller than the rectangle size used to draw a node in the mapview, imho)

yes



There might be people that do not edit cluttered areas, generally zoom in more into an area to edit the geometry _and_ then expect the usual snapDistance behavior, so that the node in question is replied on a generous fuzzy click around its center). The problem is, that while you would like to have more precision within that 10px bound, others might not, because they like the fact that they do not have to precisely click the nodes spot to get it selected. So the question that arises is:

Should snapDistance just be configurable, with sensible defaults?

-- mappaint.node.snap-distance with a default of 10px (precision lovers would decrease this to the radius of the circumcircle of the node rectangle)

-- mappaint.way.snap-distance with a default of 10px


I don't think, that configuration options will improve the situation, here. Better make it "just work".

in reply to:  39 comment:41 by bastiK, 14 years ago

Replying to cmuelle8:

btw, the problem (if we categorize it as such)

"nearest way under cursor unselectable when node is nearby"

persisted before. the behavior stems from getNearest() in NavigatableComponent - it first looks for a NearestNode(), if one is found, waySegments are not searched for anymore (regardless of whether or not one might be closer to cursor position).

i'm not saying that we can't change this, but after all, i'd like to state that it's indeed a "feature request"..

Right, just a random idea for further improvement.

in reply to:  40 comment:42 by cmuelle8, 14 years ago

ok, I'll go for implementing the following (to make it "just" work):

1) _keep_ cycling through nodes (since we have the dup case, or using your language, the very-close case)

2) use a really small bounding box to build the nearest nodes list used, in plain english: depart from using the generous snapDistance (that's currently used for ways) for nodes

ps: I've already modified NavigatableComponent (and streamlined lots of stuff, added missing methods, renamed some methods for clarity (yes, I've read the coding guidelines..), it's a big change, but a sorted by distance list as output of getNearestNodes() is also included in the changes ;-) ). checking this change in, however, might eventually break some stuff that depends on a certain behavior of NavigatableComponent..

comment:43 by bastiK, 14 years ago

Can we close this ticket? Sorry, I lost track a little ...

comment:44 by cmuelle8, 14 years ago

yes, we can :) i'll take care of #5561 soon (it's on my todo list since a couple of days, thx for the analysis you already did on this..

this ticket can be closed

greetings

comment:45 by bastiK, 14 years ago

Resolution: fixed
Status: assignedclosed

Modify Ticket

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