Transform: support navigation #105764
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Viewport & EEVEE
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Asset Browser Project
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Module
Viewport & EEVEE
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Severity
High
Severity
Low
Severity
Normal
Severity
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
5 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#105764
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "mano-wii/blender:transform_navigate"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Design Task: #106008 (Support navigation for transform operators)
Transform: upport navigation while transforming in 3D view
Implements a new option in keymap settings:
Note that with the option enabled, the keymap changes as follows:
As suggested in the design task, currently only 3 navigation operators are supported:
This patch only affects the following transform operators:
Navigation is not available when transforming with
Release Confirm
Builds
https://builder.blender.org/download/patch/PR105764
@blender-bot package
Package build started. Download here when ready.
05170c3810
tod2693be72d
d2693be72d
to01dbdf1df8
Germano Cavalcante referenced this pull request2023-03-20 20:04:11 +01:00
Is there a design for a non-experimental implementation? There are hacks here and it's not clear to me what the design would look like without them. The main questions I see looking at this code:
How to get non-conflicting keybindings for transform modal and navigation?
The proposed solution seems to be to configure the keymap in such a way that they are compatible, which is done by changing some transform modal bindings. For custom keymaps it is up to the user to make sure the few navigation operators are compatible with transform.
But I also see there is conflict handling in
modalkeymap_update_invoke
. It seems to automatically figure out modifier keys that can be added to navigation operators if they conflict?That seems rather unpredictable. What was the intent behind this? Are there conflicts that can not be resolved well in the default keymap, and some navigation keybindings have to be different when used during transform? Or is it to make things more automatic for custom keymaps?
How to get the WM event system to handle these additional key bindings?
The patch right now adds entries to the transform modal map with an operator, so that WM event handling as usual will recognize them as modal events. This solution is a hack that will need to be solved differently.
The most similar solution could be to add support for an operator making a dynamically allocated customized modal map at runtime when invoked. This is something that the WM could support.
Potentially this could be done generally in the WM, to add a given set of operators to a modal keymap to create a new temporary one. Some of the keymap handling in the 3D navigation code at least seems like it belongs more in the WM code but also has some hardcoded logic regarding the transform modal.
How to invoke the navigation operators from the transform operator?
This seems to be done by making some common functions called by both the operator callbacks and this new mechanism, bypassing the operator mechanism.
Maybe there is some way to use the actual operator callbacks with some changes elsewhere, but I haven't looked close enough to see if that's possible. Maybe it's ok roughly as is but needs some tweaks to make the implementation more clear.
Thanks for the comments @brecht!
I confess that there was no Design task. Many decisions were considered in chat.
But now I created a Design task. Any changes can be proposed there. #106008
After considering the downside of messing with users' muscle memory, and the fact that we can be less restrictive with experimental features, it seemed convenient to implement this feature as experimental.
In fact we could simply overwrite the user's keymap. But it takes an operator to do this, and since we have different default keymaps and the user can have custom keymaps, it also seemed convenient to resolve conflicts automatically.
I'm not sure what these additional key bindings refer to.
The
TRANSFORM_OT_modalkeymap_update
operator replaces each conflicting modal kmi with another non-conflicting kmi. The user can then save changes to the Preferences if they prefer.This
TRANSFORM_OT_modalkeymap_update
is something intended to be removed after this feature is no longer experimental.The patch also implements the
ed_view3d_kmi_navigation_fill_array
function inViewOpsData *ED_view3d_navigation_init(bContext *C)
. The data stored inViewOpsData *
is something that already exists for navigation operators. But now an array ofkmi
s has been added so we can detect which navigation operation was called.A user can modify the keymap while using Blender, so it's important to redetect what they are.
WM_event_match
was exposed.To perform the navigation operation the solution is:
WM_event_match
) to some kmi from the kmis array inViewOpsData*
_invoke
if it is not in modal mode or the corresponding_modal
. (Seeview3d_navigation_invoke
andview3d_navigation_modal
).Germano Cavalcante referenced this pull request2023-03-22 20:17:47 +01:00
01dbdf1df8
to299ae73e89
I carefully studied the solution (I noticed that merging the navigation operators into one wasn't going to make much difference).
I also split the branch into 3 commits:
And I simplified the code (removing unrelated cleanups) and added more documentation to it.
(Advantages and disadvantages are mentioned in the description of the first commit).
(I noticed some regressions due to old bad rebases - fixed).
My question was about how keymap configuration should work, ignoring compatibility for a moment. I can see a few ways:
It seems you are assuming it must be (1). If you have to have different keybinding for navigation with and without transform, then that or (2) may be necessary. If not, it's adding unnecessary complexity and work for users to configure bindings in two or more places.
So for me the first question is, do we need to have different navigation keybindings with and without transform, or can they be the same? If they can't be the same, what are the conflicts in the default keymap?
The answer to this depends on the right design for the previous question.
Right, I can see how the implementation was done, I was more question if it could be done in a way that doesn't involve leaking implementation internals of WM event into the 3D viewport navigation code.
Both to keep the code clean, and because we may want the same in 2D editors.
For the items mentioned in "how keymap configuration should work", the closest to the current solution is the
3
(Blender automatically uses items from the 3D viewport keymap ...)To better understand the conflicts with the transform's modal keymap, let's consider one of the cases: The
WHEELUPMOUSE
(mouse scrool).Conflict:
WHEELUPMOUSE
performs ZoomWHEELUPMOUSE
performs "PROPORTIONAL_SIZE_UP"Solution in patch:
Alt
+WHEELUPMOUSE
insteadWHEELUPMOUSE
With the current patch, what happens if we don't make this change in the modal keymap:
With a small change we could make both operations run. But this is not desirable.
Ok, then for the actual implementation that can land in main, the keymap changes should get made in
keymap_data/blender_default.py
andkeymap_data/industry_compatible_data.py
instead of throughTRANSFORM_OT_modalkeymap_update
, with review and approval by the UI team?If we don't want to enable this by default yet (because we may want to postpone breaking keymap changes to 4.0) or allow preserving compatibility, then still it seems better to implement this as a keymap option.
Yes, the
TRANSFORM_OT_modalkeymap_update
operator is just to enable this feature as Experimental.When we activate this operator in the
Experimental
tab, it displays a popup that shows all the conflicts that will be resolved automatically:I think it needs a more elaborate design. Would it be something like this?
Just a single checkbox should be enough I think. If someone wants to fine tune things they can edit the keymap.
299ae73e89
toce4277f062
Added a single checkbox to update the keymaps and avoid conflicts.
If the user does not enable this option they will not notice the new feature unless they try to navigate 3D View with the few cases where there is no conflict (such as Zoom without proportional editing).
ce4277f062
tob07cc42feb
b07cc42feb
tob226303639
I forced a branch update which promotes more localized changes.
The changes are:
transform.translate
,transform.rotate
,transform.resize
,transform.edge_slide
,transform.vert_slide
,transform.transform
When this property is not enabled, it prevents the user from navigating during transform operations even if there is no keymap conflict.
Another change is:
This configuration does not implement shortcuts for the main transform operators, leaving only those that come from tools and menus that are activated with "release_confirm", (thus not being able to have navigation).
With these changes, the option
Enable navigation and resolve keymap conflicts
, when disabled, does not bring any functional changes compared to current behavior without navigation.And since the only 5 chosen operators are affected, this change is unlikely to bring unpredictable results to the others (Skin Resize, To Sphere, Shear, Bend, Shrink/Fatten, Tilt, Trackball, Push/Pull, Crease, Bone Size, Bone Envelope, Bone Envelope Distance, Curve Shrink/Fatten, Grease Pencil Shrink/Fatten, Bone Roll, Bevel Weight, Align, Grease Pencil Opacity)
Not a detailed review, but mainly wondering if we can somehow call these operator through the regular operator interface and avoid some of the code complexity?
@ -253,0 +259,4 @@
"'Increase Max AutoIK Chain Length' will change from 'Wheel Down' to 'Alt Wheel Down' (as it conflicts with 'VIEW3D_OT_zoom').\n"
"'Decrease Max AutoIK Chain Length' will change from 'Wheel Up' to 'Alt Wheel Up' (as it conflicts with 'VIEW3D_OT_zoom').\n"
"'Automatic Constraint' will change from 'Middle Mouse' to 'Alt Middle Mouse' (as it conflicts with 'VIEW3D_OT_rotate').\n"
"'Automatic Constraint Plane' will change from 'Shift Middle Mouse' to 'Shift Alt Middle Mouse' (as it conflicts with 'VIEW3D_OT_move')"),
This doesn't need to be so detailed, I suggest:
@ -58,6 +59,24 @@ class Prefs(bpy.types.KeyConfigPreferences):
update=update_fn,
)
use_alt_in_transform_modals: BoolProperty(
I would not bother adding this to the 2.7 keymap at all.
@ -1659,0 +1753,4 @@
}
size_t kmi_array_size = sizeof(struct wmKeyMapItem *) * kmi_len;
ViewOpsData *vod = MEM_callocN(sizeof(ViewOpsData) + kmi_array_size, __func__);
I'd rather not do this kind of allocation trick, and zero sized arrays don't work in C++ which we are gradually porting files to.
I don't think we need to precompute this at all. Just iterating over the keymap on every event should not have a real performance impact, we do that continuously in the WM anyway.
@ -1659,0 +1866,4 @@
return viewmove_invoke_impl(vod, event);
case V3D_VIEW_PAN:
data = RNA_enum_get(ptr, "type");
return viewpan_invoke_impl(vod, data);
Can we avoid having these
_impl
functions by allocating a temporarywmOperator
? And then callingot->poll
,ot->invoke
,ot->modal
?The proposed changes to the utility make using
PASS_THROUGH
more feasible.Using
ot->poll
,ot->invoke
,ot->modal
makes it very difficult to use aViewOpsData
common to all operations and also makes it difficult to control the handles that are added to the window.@ -1659,0 +1866,4 @@
return viewmove_invoke_impl(vod, event);
case V3D_VIEW_PAN:
data = RNA_enum_get(ptr, "type");
return viewpan_invoke_impl(vod, data);
The idea of having a navigation utility had as one of the goals not to depend on the navigation operators.
The advantages would be:
However, the proposal seeks to go back to the
PASS_THROUGH
idea. So no changes in navigation operators are required.This was the initial idea and it was implemented at https://archive.blender.org/developer/D2624
It is not possible to use
ot->invoke
and capture handles created for modals without hacks and other complexities in the code.I would prefer use the utility, but I can let this idea for another occasion and create another PR proposing to use
PASS_THROUGH
(if preferred).3645623271
tod8073a6eff
@blender-bot build
d276604da3
toca70305618
Transform: support navigationto WIP: Transform: support navigationca70305618
to8ee1162d16
I removed the entire navigation utility in favor of a solution similar to Knife Tool (with
OPERATOR_PASS_THROUGH
).But in fact this solution is impractical, as it can miss custom navigation events and can also miss, during navigation, modal transform events. This can cause, for example, that the Snap remains enabled even with the
Ctrl
released.This indicates that a navigation utility is really needed. But related pull requests (!106279) are already out of date. (And creating a utility like that is not my expertise).
I feel like maybe someone more experienced is needed to work on this, otherwise this feature may be something that will never get implemented.
I thought the direction this was going is fine. Maybe try poking @ideasman42 in chat? And if he does not have time, maybe he can agree to have someone else review it?
I agree this is an important feature to solve.
I'm not so keen to have to support this functionality as it introduces key-conflicts and behavior which isn't well defined. Although if users really find this handy - each of these issues can be handled/resolved.
Conflicts:
Odd behavior:
Currently transform "jumps" on the first cursor motion after view navigation completes. While it's understandable some mouse pointer recalculation is needed, ideally this would happen immediately instead of being delayed until the user moves the cursor.
View locked cameras has some potential for strange behavior. You can for example, transform a camera that's locked to the view... from some quick tests it's not failing badly, so we could leave this as-is. Although having undo steps added by an action during transform seems a bit odd. It might be simplest to disable view locking while transforming if the view-locked object it's self is being transformed.
Arbitrary operator execution:
This change allows any operator to run, passing through view operators is only ensured by convention however there is nothing stopping someone from setting up a key-map that performs destructive edits. I'd assume we would not consider these cases bugs (if reported), as it seems like the user is going to some effort to shoot themselves in the foot.
Having said that - there may be cases add-ons use middle mouse input for other editors (sequencer for e.g.) that cause sequence strips to be added/removed.
Hard coded MMB:
Ideally this patch wouldn't hard code the key used for navigation.
Further work:
Would this be supported for editors besides the 3D viewport?
@ideasman42 my understanding is that this is essential for basepoint snapping to be usable, which in turn is an important feature that's been stuck for a while.
The design task #106279 has the complete list and proposed remapping.
The previous version of this patch did not allow arbitrary operator execution, and I think that was better. It was meant to work based on the refactoring in #106279.
@mano-wii, I'm not sure why you changed it since you comment it's impractical. Maybe in the hope of having a simpler change that is easier to get approved, but it's confusing this way.
This seems solvable, especially when not using operator pass through it should be straightforward to ensure refreshes happen.
In this case navigation can just be disabled.
8ee1162d16
to8b873dca71
WIP: Transform: support navigationto Transform: support navigationI reverted and updated the solution of the
ED_view3d_navigation_do
utility. Its implementation can be seen in this PR by commit a2971c8ca00e69318c7691e018c37ae3a2224735Compared to Knife Tool navigation (which uses PASS_THROUGH) the
ED_view3d_navigation_do
utility does not yet support NDOF events. I plan to work on that later.But unlike the Knife Tool this utility is adaptable to custom navigation keymaps.
Another thing that hasn't been implemented yet are the navigation modal keymaps. (For comparison, this doesn't work for the Knife either).
I also edited the descriptions of this feature in the UI.
8b873dca71
tof85a60c943
@ -377,0 +392,4 @@
# Also update the "allow navigation" property in the transform operators called through menus.
bpy.types.VIEW3D_MT_transform.allow_navigation = kc_prefs.use_transform_navigation
bpy.types.VIEW3D_MT_transform_object.allow_navigation = kc_prefs.use_transform_navigation
bpy.types.VIEW3D_MT_transform_armature.allow_navigation = kc_prefs.use_transform_navigation
This is a problem since this will not be properly reset when switching to another keymap. Instead the menu code could get the value like this:
@ -1939,0 +1995,4 @@
return NULL;
}
struct wmKeyMapItem *km_items[80];
Since this is C++ code, can just use a Vector and no use such hardcoded numbers.
@ -1939,0 +1997,4 @@
struct wmKeyMapItem *km_items[80];
int kmi_len;
ed_view3d_kmi_navigation_fill_array(C, ARRAY_SIZE(km_items), km_items, &kmi_len);
There is no need to cache this and doing these memory allocation tricks. Computing it every time in
ED_view3d_navigation_do
should not have a measurable performance impact.@ -1939,0 +2093,4 @@
}
if (op_return != OPERATOR_CANCELLED) {
/* XXX: Although this doubles the work, operators need to update some values using the updated
Not sure what the meaning of XXX is. Does it mean you expect this to be fixed still. If not, can XXX be left out?
For the UI, instead of a Transform section I suggest to add a
Navigate during Transform
boolean under 3D view, since it currently does not affect 2D views.f85a60c943
to483707ce3b
Updated as per suggestions.
Changes can be checked in 588394f506 (and 0ef1b22cee due a rebase error)
To know:
Navigate during Transform
boolean under3D view
in keymap settingsgetattr(context.window_manager.keyconfigs.active.preferences, "use_transform_navigation", False)
in Transform menusED_view3d_navigation_do
)XXX
comment (removing XXX)0ef1b22cee
toe937df1a7c
Wow, well done, it works well with snap base branch too.