Bounding box snapping mode should use only the bounding box of the grabbed item

[Yann Papouin]

Actually the bounding box for snapping is computed from all the current selection but in my case it will be smarter if it only use the bounding box of the main item of a selection when multiple items are selected. And to return to the way it works actually, I will just have to group the current selection to have a single object.

See attachment for an example:
I just want to snap to the middle bounding box of the "middle man" not the bounding box of all the selection. In the case I want to snap to all the selection, I will group it.

[Yann Papouin (yann-papouin)]

This is a patch to fix this.

[Diederik van Lierop (mail-diedenrezi)]

If we make it behave like you suggest, then this will only work for translations and not for scaling/stretching. That's because for these operations we use the handles of the selection and we cannot use only a single boundingbox (from one of the items in that selection). Although I like your proposal, I don't like the inconsistency we will get in return, i.e. snapping for translations is different from the snapping for scaling/skewing. Do you happen to see a way to work around this?

[Yann Papouin (yann-papouin)]

As a graphic designer, I'm not able to see the case I need to snap a specific point of my shape when scaling it as it's an operation that abstract the shape itself.

And as scaling uses handles, I think there is nothing to do at the moment.

The only would be to implement a functionnality named "Select and link" that woud allow multiple scaling with individual snapping.

There is some examples:
http://yurl.fr/adegc
http://yurl.fr/adegd

[Diederik van Lierop (mail-diedenrezi)]

I haven't applied this patch yet, because I'm not fully sure that it won't affect other parts in the code. The member variables that are being affected here are also used for example for updating the handles, although I haven't seen any evidence yet of things going wrong. I'm just being careful not to screw up this close before the new release. One way to handle this would be to add another member variable specifically for translations, but this would require some changes downstream in the code because we now have two member variables that need to be handled. For example, what should we then do with the setting of the snapindicator, some lines further down in the grab() method? We cannot tell here yet whether we're translating or not, so we cannot decide where to put the snap indicator. Anyway, this is just to let you know that I haven't forgotten your patch.

Something else I just realized is that when snapping the bbox with your patch applied, only the grabbed item is being considered,
whereas when snapping the nodes still all selected items are being considered. Shouldn't we just use all four bbox corners of each selected item, instead of only the grabbed item? This way you can still snap the bbox corners of the grabbed item instead of the four corners of the bbox of the selection, just as you intended to with your patch. This makes bbox snapping consistent to node snapping (although only for translations).

[Vladimir Savic (vladimir-firefly-savic)]

Maybe I'm missing something obvious here, but isn't the point of "Snapping node to..." supposed to do what you ask? Bounding box relates to farther points in four directions of selected object(s). You are free to turn on node snapping and then the relevant snapping points becomes the one closest to mouse pointer at the moment of click event.

[Yann Papouin (yann-papouin)]

I like this idea Diederik, using all objects corners is more consistent. And in the case the corner's count this will be > 100 just apply the "closest mode" if checked otherwise use an equivalent of the convex hull mode (why not comming back to the actual mode active in current svn, just using the global bounding box).

Maybe in that case the following option should be renamed to be more generic :
Only snap the node closest to the pointer.

@Vladimir

Sometimes, snapping node is not enought as some shape do not have a node at the place to be snapped and I do not chose to add a node to it only for snapping, and sometimes using the bounding box is just enought.

[Diederik van Lierop (mail-diedenrezi)]

It was a bit more effort than expected (but hey, what's new ;-) ?), but I think I got it right now. Please test revision #22028 or newer.

For details see

http://inkscape.svn.sourceforge.net/viewvc/inkscape?view=rev&revision=22028

[Yann Papouin (yann-papouin)]

That's so better now, it's really a good job what you did.

There is just one thing, "keepClosestPointOnly" seems to be applied to the whole selection.

At line 336, the IF condition should be true if "item.size" is in the allowed range but also if "/options/snapclosestonly/value" is true:

if (((_items.size() > 0) && (_items.size() < 50)) || (prefs->getBool("/options/snapclosestonly/value", false)))

[Diederik van Lierop (mail-diedenrezi)]

Good catch and thanks for testing Yann, I've just fixed this too. You're
contribution has been very valuable! Would you like to get direct commit
access to our SVNrepository? You can get that after supplying two
patches, and you passed that milestone.

[Yann Papouin (yann-papouin)]

Yes it could be interesting :)
Maybe can you review the following : https://bugs.launchpad.net/inkscape/+bug/289774