Click confusion - setting to set pick radius, to make selecting stuff easier

Merged, abandoned or rejected pull requests are moved here to clear the main Pull Requests forum.
User avatar
DeepSOIC
Posts: 7810
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

Click confusion - setting to set pick radius, to make selecting stuff easier

Postby DeepSOIC » Tue Jun 21, 2016 11:11 pm

wmayer
Site Admin
Posts: 16461
Joined: Thu Feb 19, 2009 10:32 am

Re: Click confusion - setting to set pick radius, to make selecting stuff easier

Postby wmayer » Wed Jun 22, 2016 9:52 pm

Have to test it first...
wmayer
Site Admin
Posts: 16461
Joined: Thu Feb 19, 2009 10:32 am

Re: Click confusion - setting to set pick radius, to make selecting stuff easier

Postby wmayer » Thu Jul 07, 2016 10:55 am

There are a few things that should be fixed:
  • inconsistent default values for pick radius. In SoFCSelection and SoFCUnifiedSelection the default is 2 while in the preferences and View3DInventorViewer the default is 5. This leads to inconsistent behaviour. The default must be the same everywhere and IMO should be 2 to have the same standard behaviour as now
  • weird design: View3DInventorViewer got the new member "pickRadius" but the argument list of "seekToPoint" has been extended to get the pick radius.
    Nicer design is to add the member to SoQTQuarterAdaptor, restore the interface of seekToPoint and add a getter and a virtual setter method for the pick radius. View3DInventorViewer then can override the virtual setter method to pass the radius to the selection node
  • SoFCSelection:
    FIXME: use viewer's pick radius instead, somehow
    Remove this block and write a SoAction subclass which is applied to the scene graph in View3DInventorViewer::setPickRadius. This custom action class then passes the new pick radius to SoFCSelection. As an example for such a class look e.g. at SoHighlightElementAction
  • in the preferences there should be set a sensible upper and lower limit. Allowing a minimum of 0 and a maximum of 10000 definitely doesn't make any sense. Since the pick radius is in pixels a minimum value should be 1. A maximum could be e.g. 10 or maybe 20.
  • in several places it's checked whether the value is less than 0.001. As noted above it should be checked whether it's less than 1.0
User avatar
DeepSOIC
Posts: 7810
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

Re: Click confusion - setting to set pick radius, to make selecting stuff easier

Postby DeepSOIC » Fri Jul 08, 2016 6:41 pm

Thanks for the review!
I'm not currently ready to work on this now, updating will take quite some time.
wmayer wrote:inconsistent default values for pick radius. In SoFCSelection and SoFCUnifiedSelection the default is 2 while in the preferences and View3DInventorViewer the default is 5.
OK, that's easy.
wmayer wrote:The default must be the same everywhere and IMO should be 2 to have the same standard behaviour as now
I don't know what is now. I read in OpenInventor docs that if pick radius is not set, it defaults to 5. I only saw value of 2 in navigation styles.
wmayer wrote:Remove this block and write a SoAction subclass which is applied to the scene graph in View3DInventorViewer::setPickRadius. This custom action class then passes the new pick radius to SoFCSelection. As an example for such a class look e.g. at SoHighlightElementAction
That is completely out of my scope of competence. I'll have to learn all this "action" unintuitiveness.
wmayer wrote:in the preferences there should be set a sensible upper and lower limit. Allowing a minimum of 0 and a maximum of 10000 definitely doesn't make any sense. Since the pick radius is in pixels a minimum value should be 1. A maximum could be e.g. 10 or maybe 20.
My preferred value for pick radius seems to be 20. As for limits, I always prefer no hard limiting, so I would set the limit to at least an order of magnitude higher than my preferred one, that is, at least 200 (200 might make sense for a very insane remote possibility of running FreeCAD on smartphone). Lower limit: radius is not diameter, so 1 pixel has "radius" of 0.5 pixels. And again, I essentially wanted just limit "above zero", that's it.
User avatar
DeepSOIC
Posts: 7810
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

Re: Click confusion - setting to set pick radius, to make selecting stuff easier

Postby DeepSOIC » Fri Aug 05, 2016 11:04 pm

wmayer wrote:There are a few things that should be fixed:
  • SoFCSelection:
    FIXME: use viewer's pick radius instead, somehow
    Remove this block and write a SoAction subclass which is applied to the scene graph in View3DInventorViewer::setPickRadius. This custom action class then passes the new pick radius to SoFCSelection. As an example for such a class look e.g. at SoHighlightElementAction
I don't understand. Wht if SoFCSelection node is added to viewer scene graph as a part of viewprovider of some new object? how will it know the radius? Do I have to fire the setPickRadius action to act on all viewproviders being added?

Next. What if I have two viewers. Now, if I setPickRadius for one of viewers, the SoFCSelection nodes (which are shared between the viewers, right?) get the new pick radius, so in the other viewprovider, pick radii become inconsistent. In either solution.

Sorry, but I fail to understand, why the solution you are suggesting is better. It creates far more mess for achieving basically the same overall partially-broken result. Or I am missing something...

I'm thinking of setting pick radius for event action at the moment it is created... That will remove the need of FCSelection to know pickRadius altogether...
User avatar
DeepSOIC
Posts: 7810
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

Re: Click confusion - setting to set pick radius, to make selecting stuff easier

Postby DeepSOIC » Sat Aug 06, 2016 6:37 pm

DeepSOIC wrote:I'm thinking of setting pick radius for event action at the moment it is created...
I have first success regarding this!
in QuarterWidget, I added:

Code: Select all

  SoHandleEventAction* ea = this->pimpl->soeventmanager->getHandleEventAction();
  ea->setPickRadius(20.0);
And it works :D

so... rework in progress!
User avatar
DeepSOIC
Posts: 7810
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

Re: Click confusion - setting to set pick radius, to make selecting stuff easier

Postby DeepSOIC » Sat Aug 06, 2016 10:44 pm

Reworked.
wmayer wrote:inconsistent default values for pick radius
Addressed. Set to 5 everywhere (I actually measured that currently it is r=5px).

wmayer wrote:weird design: View3DInventorViewer got the new member "pickRadius" but the argument list of "seekToPoint" has been extended to get the pick radius.Nicer design is to add the member to SoQTQuarterAdaptor, restore the interface of seekToPoint and add a getter and a virtual setter method for the pick radius. View3DInventorViewer then can override the virtual setter method to pass the radius to the selection node
Addressed. Whole implementation is now in SoQtQuarterAdaptor; View3DInventrViewer not changed at all.

wmayer wrote:[*]SoFCSelection:
FIXME: use viewer's pick radius instead, somehow
Addressed. Neither SoFCSelection nor SoFCUnifiedSelection mess with pick radius anymore, at all.

wmayer wrote:in the preferences there should be set a sensible upper and lower limit. Allowing a minimum of 0 and a maximum of 10000 definitely doesn't make any sense. Since the pick radius is in pixels a minimum value should be 1. A maximum could be e.g. 10 or maybe 20.
New limits: 0.5 to 200

wmayer wrote:[*]in several places it's checked whether the value is less than 0.001. As noted above it should be checked whether it's less than 1.0
Only one place remains: in Py implementation of setPickRadius. I want to leave the ability to set value to subpixel, for the sake of experimentation at least. Maybe it's even worth removing this artificial constraint altogether - it is py interface, not UI.

EDIT: known problems
1. In sketcher, constraints do not respect pick radius.
2. In sketcher, when pick radius is high, it is hard to select two points one after another, when they are close to each other. I tried to fix that by disabling Z elevation of selection/highlighting, it helps, but not much. Further changes are required...
3. It is still hard to select vertices on concave corners. I'm thinking on how to fix it.
wmayer
Site Admin
Posts: 16461
Joined: Thu Feb 19, 2009 10:32 am

Re: Click confusion - setting to set pick radius, to make selecting stuff easier

Postby wmayer » Wed Aug 10, 2016 5:38 pm

Merged.
Addressed. Set to 5 everywhere (I actually measured that currently it is r=5px).
Since that's the default of OpenInventor it's fine, too.
Addressed. Whole implementation is now in SoQtQuarterAdaptor; View3DInventrViewer not changed at all.
Great.
1. In sketcher, constraints do not respect pick radius.
The responsible class is SoDatumLabel.
2. In sketcher, when pick radius is high, it is hard to select two points one after another, when they are close to each other. I tried to fix that by disabling Z elevation of selection/highlighting, it helps, but not much. Further changes are required...
That's exactly the problem you get with a too high pick radius. IMO the only good option is to choose a sensible value for the pick radius. Setting a ridiculously high value and then trying to fix its side-effects is insane.
3. It is still hard to select vertices on concave corners. I'm thinking on how to fix it.
Have a look at SoFCUnifiedSelection::getPickedPoint. It internally checks a list of all elements and if a point is picked it wins.
User avatar
DeepSOIC
Posts: 7810
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

Re: Click confusion - setting to set pick radius, to make selecting stuff easier

Postby DeepSOIC » Wed Aug 10, 2016 8:22 pm

wmayer wrote:Merged.
thanks!!
wmayer wrote:The responsible class is SoDatumLabel.
will check, but I don't feel lucky...
wmayer wrote:Setting a ridiculously high value and then trying to fix its side-effects is insane.
That's part of the goal to make FreeCAD touchscreen-friendly. At least I'm trying ;)
wmayer wrote:Have a look at SoFCUnifiedSelection::getPickedPoint. It internally checks a list of all elements and if a point is picked it wins.
Been there. I have some idea how to improve it, but haven't tried yet. The idea is to convert pick radius into depth region (kinda create confusion sphere), an apply priority selection rules there...
User avatar
yorik
Site Admin
Posts: 12034
Joined: Tue Feb 17, 2009 9:16 pm
Location: Brussels, Belgium
Contact:

Re: Click confusion - setting to set pick radius, to make selecting stuff easier

Postby yorik » Wed Aug 10, 2016 9:41 pm

DeepSOIC wrote:That's part of the goal to make FreeCAD touchscreen-friendly. At least I'm trying ;)
Now finally i see the point behind all this :D