Add 'Transform Cursor' property to relevant Transform Operators
Needs ReviewPublic
Actions

Authored by Asher (ThatAsherGuy) on Aug 31 2019, 10:33 PM.

Details

Reviewers

Campbell Barton (campbellbarton)

Maniphest Tasks

T69550: 3D Cursor tool todo

Summary

This patch lets you move the 3D cursor with the rotate, trackball rotate, scale, shear, push/pull, mirror, and align operators. It tweaks how the transform operators calculate the Active Element pivot when you're moving the 3D cursor, but it doesn't change the other pivot types.

It doesn't change any keymaps, add any gizmos, or tweak any tools.

Diff Detail

Repository: rB Blender

Event Timeline

Asher (ThatAsherGuy) created this revision.Aug 31 2019, 10:33 PM

Asher (ThatAsherGuy) added reviewers: Campbell Barton (campbellbarton), Germano Cavalcante (mano-wii).Aug 31 2019, 10:42 PM

Asher (ThatAsherGuy) added a task: T69550: 3D Cursor tool todo.Sep 5 2019, 7:19 PM

Fixed issues with the Active Element pivot in edit mode. Added the property to more transform operators.

The real purpose of this patch is not well defined.
Adding this boolean is harmless, but activating it on theuser's keymap can completely change the result of the operation (this is not desirable).
Perhaps the only advantage I see is to define a new shortcut to perform these operations on the cursor.
If it is really useful, I recommend adding this parameter only to TRANSFORM_OT_transform, since it executes any of the other modes.

This revision now requires changes to proceed.Feb 23 2020, 2:43 AM

The property defaults to false, and TRANSFORM_OT_translate already has it. Limiting this to just TRANSFORM_OT_translate means we can't rotate, scale, or shear the cursor from a gizmo, which runs contrary to the notion of feature parity between the tool system and modal operators.

Correcting, I meant TRANSFORM_OT_transform! My bad.

(Edited)

How would limiting the feature to TRANSFORM_OT_transform improve it, or improve feature parity between tools and modal operators? It wouldn't make the code any easier to maintain, it wouldn't make the feature more discoverable, it wouldn't limit undesirable behavior, it wouldn't make it any easier to use it with a gizmo; what am I missing?

When the advantages of the patch are not very clear, I cannot make a decision.
I'll leave the review to Campbell.

This revision now requires review to proceed.Feb 24 2020, 2:03 AM

Germano Cavalcante (mano-wii) removed a reviewer: Germano Cavalcante (mano-wii).Feb 24 2020, 2:03 AM

You haven't described why the advantages aren't clear, or why you're looking for advantages (over what?) in the first place. I'd love to help you out, but it's hard to improve the patch description if I don't know what the source of your confusion is.

The only thing this patch does is lay the groundwork for tasks like T69550. It adds the transform_cursor property to the operators that can use it so the UI/UX folks can work on the user-facing side (gizmos, keymaps, etc) without having to worry about how the transform operators work. As I said on DevTalk, this more of a papercut fix than a new feature.

B (bnzs) awarded a token.Mar 4 2020, 1:31 AM

@Campbell Barton (campbellbarton) How busy is your calendar looking the next week or three? While I'm confident in the changes I made in transform_ops.c, I'm split on whether I should expand the pivot changes to accommodate other object types in this patch, or in a follow-up.

Revision Contents
Changeset List

Path

Size

source/

blender/

editors/

transform/

transform_ops.c

6 lines

Diff 17730

View Options

source/blender/editors/transform/transform_ops.c

Add 'Transform Cursor' property to relevant Transform OperatorsNeeds ReviewPublicActions