Absolute grid snapping

Authored by Campbell Barton (campbellbarton) on Nov 21 2014, 6:27 PM.



This patch adds another snap element type (GRID, which is actually already used in the node editor for snapping). Instead of snapping in increments relative to the current location, it will snap to increments based on the origin (0, 0, 0) (absolute).

A new blender_icons.svg with separate snap_grid and snap_increment icon:

Diff Detail

rB Blender
There are a very large number of changes, so older changes are hidden. Show Older Changes

@Paweł Łyczkowski (plyczkowski) I also like the clear difference between increment and grid in v2. Nice work! :)
Two things which bother me with the old grid icon are that the blue circle on the sphere has 2x pixels which is kind of visible on high dpi displays and that the center point is slightly off the grid. The latter might be so that the grid is more visible which is understandable.

@Sergey Sharybin (sergey) Regarding line 2476 in transform_snap.c: If I remember correctly the function only got called when the snap mode was SCE_SNAP_MODE_INCREMENT. I changed this so it also gets called when the mode is SCE_SNAP_MODE_GRID. That's why I added the print statement in the else part (it should never get executed). The only thing that's new is the behavior in case of SCE_SNAP_MODE_GRID. So old functionality shouldn't have changed.

I hope I can make another patch for testing once the icon is finalized. :)

@Fabio Arnold (donfabio)

blue circle on the sphere has 2x pixels which is kind of visible on high dpi displays

Please elaborate on that? What do you mean by 2x pixels?

once the icon is finalized

Keep in mind that I will require green light from someone higher up before doing the final vector icon.

Is this finished? If it is, and is going into master, then I can provide the final icon.

Based on @Sergey Sharybin (sergey)'s comments above I believe it needs another code review. I'd very much like to get this polished up for acceptance, though. It's really a crucial feature for architectural and game level development.

Just tested: the patch still works with current master. :)

As I explained 2.5 months ago, this doesn't break old snapping behavior, because the function which handles grid snapping only gets called when t->tsnap.mode is either SCE_SNAP_MODE_INCREMENT or SCE_SNAP_MODE_GRID. Before the patch the function was only called when t->tsnap.mode was set to SCE_SNAP_MODE_INCREMENT.

Although I didn't really change anything after the first review it would be great, if one of the devs could have a look at this again with regards to my explanation.

This comment was removed by Paweł Łyczkowski (plyczkowski).
Julian Eisel (Severin) commandeered this revision.EditedJun 24 2015, 3:29 PM
Julian Eisel (Severin) updated this revision to Diff 4478.

Made some minor edits to the patch to push things towards master

  • Cleanup: Codestyle, comments, naming, asserts instead of printfs
  • Removed unwanted(?) if-condition

(also made some minor edits to code not related to the patch, just to avoid cleanup commits if we work on this code anyway)

Now the only thing I see missing is an icon (@Paweł Łyczkowski (plyczkowski)'s seems fine, just need the .svg).

Feel free to commandeer back @Fabio Arnold (donfabio)

Patch wasn't "diffed" against master

Now the only thing I see missing is an icon

Coming soon.

@Fabio Arnold (donfabio), I think what Sergey meant was the if (!(t->con.mode & CON_APPLY) || t->con.mode & (CON_AXIS0 << i)) that seems like it was put there by accident (your last patch version, line 2476).

Edit: Noticed the failed patching mentioned by Jonathan was around that line, so guess it was a merge conflict.

It's quite some time ago but I think I put that line there intentionally. Probably because axis constraints weren't working otherwise.. I'm not sure.

Anyways, thanks for picking up this diff! :) I just tested it and everything works as expected.

Edit: Actually there is one issue: If you go to top view for example and move an object at some random location, then try to move it using absolute grid snapping, the object actually moves in increments...

Edit2: OK, now I know why I put that if-condition there: Absolute grid snapping should snap the object positions of even integers (or decimals when holding shift). If you're using an axis constraint it isn't desired that all the other axis get snapped to even integer, too. The check prevents that from happening. Note: this is not an issue when using incremental snapping, because it doesn't alter the origin of the translation.

Can't recreate the top-view issue here. Are you sure you're using the right mode (they are still using the same icon which is kinda confusing)?

See what you mean re. constraints, however, holding down shift I'd expect it to use the next smaller grid size just like it's already happening without the if (...). This is more a design question I think, but IMHO it's how it should work.

Fabio Arnold (donfabio) set the repository for this revision to rB Blender.
  • When constraining the transform by axis the transform won't snap to absolute grid positions of unconstrained axis
  • When holding shift the transform snaps to decimal steps (1.0, 0.1 up to 0.01) of the grid

Well, the issue wasn't related specifically to the top view. But when moving from top view in orthographic projection iter_fac will be 0.1 instead of 1.0. And absolute snapping wasn't working with iter_fac < 1.0.

I changed
val[i] = iter_fac * (floorf(t->center[i] + val[i] / (fac[action] * asp[i]) + 0.5f) - t->center[i]);
val[i] = iter_fac * floorf((t->center[i] + val[i]) / iter_fac + 0.5f) - t->center[i];
which does the trick.

The reason for the axis constraint thing is basically that if I want to move an object on the x-axis using grid snapping while constraining to the x-axis I don't want it to jump to 4.0 on the y-axis or whatever while it was on 4.4 or something. I think this makes sense.

Here is the Increment icon (for absolute snap use the existing Grid snap icon): https://drive.google.com/file/d/0B8V1o1Fl0ub1SWx6TVQ1TGZPZjQ/view?usp=sharing

I kind of got it working here.
But I think I messed up the blender_icons.svg, because the patch is now 4.7 MB... (I simply imported Pawel's icon).
Also blender_icons_update.py is generating some blankXXX icons. How do I clean up the SVG?

Edit: I just checked: The SVG in master also produces these blank icons. I will generate a patch now.

Referencing the SNAP_INCREMENT icon

patch is now 4.7 MB... (I simply imported Pawel's icon).

Don't know if that's relevant, but keep in mind that there are hidden layers in my svg - I guess to properly import into blender you have to copy just the icon from my file and paste it properly (do mind how it is placed on the pixels) in the blender's svg.

I noticed that the blank icons came from the macros in UI_icons.h...

patch is now 4.7 MB... (I simply imported Pawel's icon).

Don't know if that's relevant, but keep in mind that there are hidden layers in my svg - I guess to properly import into blender you have to copy just the icon from my file and paste it properly (do mind how it is placed on the pixels) in the blender's svg.

I saw that. I deleted all other layers and cleaned up the file using Inkscape's File > Clean up document. Otherwise the import would have taken forever. I also paid attention to line up the lines to the pixel grid. If you want to you can check the SVG. Could be that I missed something. I never did this before..

Campbell Barton (campbellbarton) requested changes to this revision.EditedJun 26 2015, 4:19 AM

I'm a bit disappointed in how this review goes, for such fundamental changes there is all sorts of icon discussion - but it seems nobody really tried testing much.

This isn't working in edit-mode with any transformation (very easy to test, default cube, grab/rotate/scale... then try snap to grid).

Update, multiplying t->center by the edit-mode objects matrix before using w/ absolute snapping resolves the issue.

Update #2, Committed rB09e89f01a6ee5314833cf8838ea124e0cf75c60e

So now you can just use t->center_global instead of t->center

This revision now requires changes to proceed.Jun 26 2015, 4:19 AM

Use t->center_global here fixes editmode behavior.

Use t->center_global as proposed by Campbell.

Fabio Arnold (donfabio) marked an inline comment as done.Jun 26 2015, 11:30 AM

Did some testing. There's a new problem: scale and rotate are affected by t->center_global and are having offsets based on the position. I think the desired behaviour would be that scale and rotate behave like snap_increment when snap_grid is active. What do you think?

Only snap to grid during translation.

Undid Severin's renames of snapGridIncrement -> snapGrid and applyGridIncrement -> applyGrid, because those functions are used by both snapping modes

I don't know what to do about other transform orientations than the global one when using the 3D manipulator or any axis constraints. Should the translation be snapped to the grid regardless of any axis constraints?

Re.: Function naming:
These functions execute for grid and grid-increment snapping. Our API-convention is to use the shared term, afaik (so would be snapGrid here). However, these are picky details we can check them later.

Re.: Other transformation orientations:
Yes, it should always snap to grid. If it didn't, it would conflict with what the user set. This might make things a bit more complicated, but IMHO that's a necessary change


Changing the order of icons shouldn't be done as the order here is dependent on the order in the blender_icons.svg (which would need to be updated to make this work). Use the empty slot below instead.


Not sure about disabling for rotate/scale. It's the same as with other transformation orientations: If snap mode is set to grid it should snap to grid. (At least for scale, rotation might be considered a bit special here)

@Jonathan Williamson (carter2422), @Paweł Łyczkowski (plyczkowski), @Sebastian Koenig (sebastian_k), or anyone else, what do you think, how would you expect it to behave with rotate/scale?


Also really minor but it keeps annoying me ;P: Codestyle convention would be

else {

See http://wiki.blender.org/index.php/Dev:Doc/Code_Style#Braces

Julian Eisel (Severin) requested changes to this revision.Jun 27 2015, 3:31 AM
This revision now requires changes to proceed.Jun 27 2015, 3:31 AM

And while you're already on it, just another super-minor thingy ;)


If you use:

else {

The next one who wants to add a snap mode into this function doesn't need to update the BLI_assert (which he even might miss/forget)

minor tweaks

  • keep increment first in the popup menu (since its default)
  • rename menu items to Grid (increment) , Grid (absolute)
  • use ELEM() where possible
  • use roundf instead of floorf
Campbell Barton (campbellbarton) added inline comments.

This only makes sense for absolute snapping, think its fine to do this.

remove notes, not really needed, can tell from reading code

correct icons (WIP, until we have real icon)

Campbell Barton (campbellbarton) marked an inline comment as done.

minor edits

Regarding the icon order: I had already uploaded an updated blender_icons.svg in the summary, where I grouped the snap icons (and moved older ones to fit) and added Pawel's final icon. Didn't want to add the SVG file to the patch because it was 4MB.

Severin: I agree snap_grid_increment and snap_grid_absolute would be a better naming. Then the function names would also make sense. Thanks for pointing out code style issues. I was a bit sloppy.

Campbell: Thanks for commandeering and commiting but is the behaviour for custom transform orientations resolved? For example: move an object to some uneven location. Switch transform orientation to view and drag an axis of the manipulator. The translation starts off at a weird offset because of t->center_global and ultimately the object doesn't snap to an even location. I think we shouldn't even snap to an even location then because that would void the axis constraint. So my suggestion would be to behave like snap_grid_increment in that case.

As for rotations I see two options:

  1. Start at 0° and snap to 5° increments (this is actually relative to the object's current rotation)
  2. Get the angle of the current rotation axis of the object and snap it 5° to increments

(1.) is how it currently behaves. (2.) would better fit to the notion of an absolute transform. At least when the rotation is around one of the global axis of the grid.

For scaling I see similiar options and another one would be snapping to the object's dimensions...

But actually I prefer the current behaviour of behaving just like snap_grid_increments for rotation and scaling. What do you guys think?

snap_grid_increment and snap_grid_absolute would be a better naming.

Why snap_grid_increment? The increment option adds increment snapping to many things, like rotation, and has not much to do with the grid. It's incidental when an object is placed at the grid, and moved in increments, that it looks as though it's snapping to the grid.

I would go with snap_increment, "Snap to increments during transform."/"Transform in increments."
and snap_grid "Snap to grid."

Eventually I'd like to see the snapping option as non-exclusive toggles, placed in a quick-settings popup, so you could have both snap to increments and snap to grid, or any other combination.

OK, based on Pawel's comment I have a new proposal which would be a bit more work: first get rid of the new snap_grid_absolute. Only keep snap_increment. Then add a toggle for absolute/relative snapping which is only shown when snap_increment is active. This toggle is only shown for transform orientation/spaces where it makes sense and an origin exists. So only show it in global/local orientation. Normal and view don't have an origin where absolute snapping would make sense AFAIK. When absolute is checked and incremental snapping is active make translation/rotation/scaling work as the second option I mentioned in my previous post. Also respect the orientation/space and don't override it as with snap_grid_absolute right now.

This would add a key thing to blender which it is currently lacking IMO: absolute transforms.

To go even further to be more consistent and complete when absolute transform is active the absolute location/rotation/scale should be displayed during translation/rotation/scaling operations. Right now only the delta of the operation is displayed and you have to look in the properties panel for transform info.

While I looked to get this feature finished up, it seems committing to master opened up new questions on how it might work.
Seems this feature is still too much in discussion, and an open-topic. See T45212.

@Paweł Łyczkowski (plyczkowski), if you think behavior should eventually be changed. It would be good to say so early on.

Disabled this patch in master until theres some more final design rB3d616ccba888ed678642356317745e8b638a908b

@Paweł Łyczkowski (plyczkowski), if you think behavior should eventually be changed. It would be good to say so early on.

Yup, sorry about that. I may have spoken too hastily, since it's not that obvious how all these modes could work as non-exclusive toggles. I'll add some thoughts in https://developer.blender.org/rB3d616ccba888ed678642356317745e8b638a908b

Note for future reference: This patch is now in master, but accessed as a toggle instead of a new snapping mode.

Added icon for incremental grid snapping rB14dd88e817432b