This patch enables the option for Snap Cursor to Active to align the 3D cursor to the normal orientation of the active item.
This patch helps reduce the number of steps to accurately place and orient the 3D cursor.
During a session, the operation saves the state that it was last set to, so the default value for the option is not too critical. With that in mind, I chose to enable alignment by default based on my perception of its utility. I think that most of the time aligning when snapping would be a benefit or at least neutral.
Details
Details
- Reviewers
Germano Cavalcante (mano-wii)
Diff Detail
Diff Detail
- Repository
- rB Blender
- Branch
- Snap_Improvements (branched from master)
- Build Status
Buildable 20543 Build 20543: arc lint + arc unit
Event Timeline
Comment Actions
The idea is good but the code doesn't convince me much.
An option should just target a few extra lines of code, not redo the whole thing.
This gives me the impression that the code is duplicated.
There are also style issues that need to be addressed (https://wiki.blender.org/wiki/Style_Guide/C_Cpp#Clang_Format)
Comment Actions
The code looks good, but I think the design still needs to be discussed a little more.
Users can expect the orientation to be the one set in the scene.
While NORMAL (with a fallback to LOCAL) is preferable in most cases, this might break expectations a bit in my opinion.
Is there a strong reason for the orientation to be V3D_ORIENT_NORMAL?
Comment Actions
Is there a strong reason for the orientation to be V3D_ORIENT_NORMAL?
The idea is to align the cursor to the active item.
I think V3D_ORIENT_NORMAL is the best option for this goal since it enables aligning to the active element when in edit/pose mode and to the active object when in object mode.
I cannot think of an alternative aside from including a dropdown list of orientation options.
The only time I have seen the fallback defy my expectations is an edge case where I tried snapping to a wire edge with one vertex at (0,0,0) and another at (1,1,1) in local coordinates. I expected the alignment to ignore the vertex normals and still be able to align with Z perpendicular to the edge. However, it falls back to local in that scenario. I would only expect a fallback to local for an edge when the edge has zero length, but I figured that cleaning up edge cases like this would be the domain of a separate commit.
There is one thing that I am not happy about design wise. When unchecking the box, the 3D cursor does not return to its former orientation. Perhaps you have insight on how to deal with that.
Comment Actions
Makes sense.
Cursor position is not saved in undo steps, so I don't see a simple solution for this.
Another thing that makes me unsure about this change is that only the cursor-to-active operator will be affected.
Users may not understand why this option does not appear in other snap operators (selection to active, cursor to selected...).
Also, since this operator looks more and more like a transform operator, I wonder if instead of adding new features, wouldn't it be better to make a new transform operator? (See D7780: Transform: Replace View3d Snap operators with a transform ops version).
But these are shallow concerns, maybe they don't really justify blocking this feature.
In conclusion, the code looks ok, the feature is simple, but I'm not sure about making the final decision here.
I feel like it needs more discussion with the Modeling team.