Made 2D zoom consistent with 3D zoom for touchpads #110375

Stefan Schumann · 2023-07-22T14:09:13+02:00

Stefan Schumann commented

2023-07-22 14:09:13 +02:00

First-time contributor

With "multi gesture touchpad" enabled in the preferences in editors using the 2DView interface one can zoom with a track pad pan gesture and pressing CTRL, e. g. in the Shader Editor or Graph Editor. In this case the resulting zoom behavior is reversed compared to the same operation in the 3DView or Image Editor. The same inconsistency shows in some other places of the UI.

This change makes the behavior consistent if "multi gesture touchpad" is enabled by using already existing code considering the "natural scrolling setting" of the OS. During the fixing some further enhancements have been made:

Properties handling has been fixed and harmonized for the Zoom/Dolly operators, it is now according to the specification
Dolly direction vector computation has been fixed, works now in all situations
Missing Dolly modal abort added
Zoom update calculation is now done from NDC coordinates with one matrix multiplication for all camera types
Input sensitivity for e. g. the Zoom/Dolly operators is now window resolution independent using UI_SCALE_FAC
Fly Navigation speed can be changed with a track pad swipe

With "multi gesture touchpad" enabled in the preferences in editors using the 2DView interface one can zoom with a track pad pan gesture and pressing CTRL, e. g. in the Shader Editor or Graph Editor. In this case the resulting zoom behavior is reversed compared to the same operation in the 3DView or Image Editor. The same inconsistency shows in some other places of the UI. This change makes the behavior consistent if "multi gesture touchpad" is enabled by using already existing code considering the "natural scrolling setting" of the OS. During the fixing some further enhancements have been made: - Properties handling has been fixed and harmonized for the Zoom/Dolly operators, it is now according to the specification - Dolly direction vector computation has been fixed, works now in all situations - Missing Dolly modal abort added - Zoom update calculation is now done from NDC coordinates with one matrix multiplication for all camera types - Input sensitivity for e. g. the Zoom/Dolly operators is now window resolution independent using `UI_SCALE_FAC` - Fly Navigation speed can be changed with a track pad swipe

Stefan Schumann added 1 commit 2023-07-22 14:09:20 +02:00

411227669a Made 2d zoom consistent with 3d zoom for touchpads.

Stefan Schumann changed title from ~~WIP: Made 2d zoom consistent with 3d zoom for touchpads.~~ to Made 2d zoom consistent with 3d zoom for touchpads.

2023-07-22 14:09:50 +02:00

Iliya Katushenock added the

Module

User Interface

label 2023-07-22 14:10:14 +02:00

Stefan Schumann referenced this pull request

2023-07-22 14:14:35 +02:00

Multi-touch gesture: Zooming is reverse in shader editor comparing to viewport #89906

Stefan Schumann changed title from ~~Made 2d zoom consistent with 3d zoom for touchpads.~~ to WIP: Made 2d zoom consistent with 3d zoom for touchpads.

2023-07-22 19:49:00 +02:00

Stefan Schumann added 1 commit 2023-07-22 20:02:20 +02:00

3e577c2994 Removed change in modal method used by mouse zoom only

Stefan Schumann changed title from ~~WIP: Made 2d zoom consistent with 3d zoom for touchpads.~~ to Made 2d zoom consistent with 3d zoom for touchpads.

2023-07-22 21:07:33 +02:00

Stefan Schumann added 1 commit 2023-07-23 20:08:47 +02:00

85f2899c48 Moved the change to the image and view3d editors

Yevgeny Makarov referenced this pull request

2023-07-24 11:54:41 +02:00

Multi-touch gesture: Zooming is reverse in shader editor comparing to viewport #89906

Yevgeny Makarov commented

2023-07-24 12:37:30 +02:00

delta = WM_event_absolute_delta_x(event) + WM_event_absolute_delta_y(event);

It's basically the right approach to fix the issue, but this code doesn't work correctly initially. Try moving your fingers diagonally. The correct code will be of the following form.

if (U.uiflag & USER_ZOOM_HORIZ) {
    delta = WM_event_absolute_delta_x(event);
}
else {
    delta = WM_event_absolute_delta_y(event);
}

See https://archive.blender.org/developer/D8521, just copy from this patch. It would be ideal if you update this entire patch to the current code. There are a lot of different bugs that overlap each other, I think it's easier to fix everything at once. In fact, this code is not complicated.

`delta = WM_event_absolute_delta_x(event) + WM_event_absolute_delta_y(event);` It's basically the right approach to fix the issue, but this code doesn't work correctly initially. Try moving your fingers diagonally. The correct code will be of the following form. ``` if (U.uiflag & USER_ZOOM_HORIZ) { delta = WM_event_absolute_delta_x(event); } else { delta = WM_event_absolute_delta_y(event); } ``` See https://archive.blender.org/developer/D8521, just copy from this patch. It would be ideal if you update this entire patch to the current code. There are a lot of different bugs that overlap each other, I think it's easier to fix everything at once. In fact, this code is not complicated.

Yevgeny Makarov changed title from ~~Made 2d zoom consistent with 3d zoom for touchpads.~~ to WIP: Made 2D zoom consistent with 3D zoom for touchpads

2023-07-24 12:38:49 +02:00

Yevgeny Makarov commented

2023-07-24 13:06:55 +02:00

You can create a new git brunch, rebase it to any commit around Oct 22 2021 (latest D8521 patch update). So you can apply my patch, build Blender, see the effect, learn how it works. And then make the same changes in the current code.

Stefan Schumann commented

2023-07-24 13:53:23 +02:00

First-time contributor

@jenkm, the image editor is scaling uniformly, that's probably why the whole movement vector was used. But using its x- or y-coordinate only is also possible, so I've applied your suggestion.

Yevgeny Makarov commented

2023-07-24 14:34:39 +02:00

If you move diagonally (top-left and bottom-right) the sum of the deltas would be zero, so it just can't work that way. But rather, it will oscillate between a slight zoom in and zoom out.

Stefan Schumann added 1 commit 2023-07-24 14:42:55 +02:00

1cb2b36938 Honour zoom direction in image editor and make clip editor consistent.

Stefan Schumann commented

2023-07-25 16:02:07 +02:00

First-time contributor

@jenkm, I took a look at your patch. It contains some rotation code which I would like to ignore here. This is only about zoom consistency. Also I checked the 3D dolly view. At first there was nothing in the preferences to enable it with a trackpad. After adding trackpad support in preferences for it and testing it, it seems broken for a trackpad. Do you think the same?

Yevgeny Makarov commented

2023-07-25 22:23:57 +02:00

Yeah, "Dolly View" is totally broken, but that's a small thing, you just have to copy the code from Zoom.

You can ignore the rotation code, but there's a different problem here. I completely forgot that there is no separate code for MOUSEZOOM (pinch gesture) it's mixed in with MOUSEPAN. So you'll need more code than I originally thought.

Yeah, "Dolly View" is totally broken, but that's a small thing, you just have to copy the code from Zoom. You can ignore the rotation code, but there's a different problem here. I completely forgot that there is no separate code for MOUSEZOOM (pinch gesture) it's mixed in with MOUSEPAN. So you'll need more code than I originally thought.

Stefan Schumann commented

2023-07-26 17:54:00 +02:00

First-time contributor

@jenkm, MOUSEPAN and MOUSEZOOM can be used in Dolly in the same way as in Zoom. But comparing the invoke methods of Dolly and Zoom I found that Dolly sets the mx/my properties while Zoom reads them. I think setting them is correct. What do you think?

Stefan Schumann added 1 commit 2023-07-27 21:26:39 +02:00

ac115d78e6 Dolly Zoom works with trackpad independent of window resolution

Stefan Schumann commented

2023-07-27 21:31:15 +02:00

First-time contributor

Property handling still needs fixing in Dolly and Zoom, including handling of "use cursor location" == false.

Yevgeny Makarov commented

2023-07-28 18:45:34 +02:00

I've thought about it some more, there's no easy solution here. You can easily fix MOUSEPAN, but you will need to do something about MOUSEZOOM.

I don't think there's any point in you taking the time to sort all this out. I've already been down that road. The best you can do is just very carefully update my patch to the current code.

And we'll also need a developer to add GHOST code for Windows. It sucks that patches aren't reviewed in a timely manner.

I've thought about it some more, there's no easy solution here. You can easily fix MOUSEPAN, but you will need to do something about MOUSEZOOM. I don't think there's any point in you taking the time to sort all this out. I've already been down that road. The best you can do is just very carefully update my patch to the current code. And we'll also need a developer to add GHOST code for Windows. It sucks that patches aren't reviewed in a timely manner.

Stefan Schumann commented

2023-07-28 19:44:57 +02:00

First-time contributor

Okay, but looking at the Dolly code which is easier to understand than the Zoom code I saw some strange assumptions about preference settings. I found that the preference settings are in the keymap preferences for Dolly input events. And these set the properties when the operator is called. This also made me think about the properties handling. I came up with the following rules:

If a property is set, we use it. If a property is not set, we set it according to an input device state/event or some internal assumption.

I found also that setting the delta property for Dolly does not make sense. It is an int and the deltas based on trackpad panning are close to zero. So rounding from float to int would always result in 0. Delta should be a float, too bad.

Okay, but looking at the Dolly code which is easier to understand than the Zoom code I saw some strange assumptions about preference settings. I found that the preference settings are in the keymap preferences for Dolly input events. And these set the properties when the operator is called. This also made me think about the properties handling. I came up with the following rules: If a property is set, we use it. If a property is not set, we set it according to an input device state/event or some internal assumption. I found also that setting the delta property for Dolly does not make sense. It is an int and the deltas based on trackpad panning are close to zero. So rounding from float to int would always result in 0. Delta should be a float, too bad.

Stefan Schumann added 1 commit 2023-07-28 20:07:04 +02:00

86b1ea7f1d Add initial property handling to Dolly taking also care of "use cursor init"

Yevgeny Makarov commented

2023-07-29 16:43:13 +02:00

I actually have a patch for View3D Zoom and Dolly: https://archive.blender.org/developer/D8527

And for Clip/Image Zoom get the code from D8521,
but for MOUSEZOOM use the following instead of event->factor:

else { /* MOUSEZOOM */
    delta = WM_event_absolute_delta_x(event);
    factor = 1.0f + delta / 300.0f;
}

Need to test it, but it seems like a good and simple fix for the zoom issues.

I actually have a patch for View3D Zoom and Dolly: https://archive.blender.org/developer/D8527 And for Clip/Image Zoom get the code from [D8521](https://archive.blender.org/developer/D8521), but for `MOUSEZOOM` use the following instead of `event->factor`: ``` else { /* MOUSEZOOM */ delta = WM_event_absolute_delta_x(event); factor = 1.0f + delta / 300.0f; } ``` Need to test it, but it seems like a good and simple fix for the zoom issues.

Stefan Schumann commented

2023-07-29 17:09:44 +02:00

First-time contributor

I've looked at your patches but at least for Dolly I think I found a different solution. It also handles the properties correctly I think.
I've tested it with trackpad and mouse. I also tested it with Python using this little script:

import bpy

area = [area for area in bpy.context.screen.areas if area.type == "VIEW_3D"][0]
region = [region for region in area.regions if region.type == 'WINDOW'][0]

with bpy.context.temp_override(area=area, region=region):
    bpy.ops.view3d.dolly(mx=0, my=0, delta=2, use_cursor_init=False)

I will post an update soon.

I've looked at your patches but at least for Dolly I think I found a different solution. It also handles the properties correctly I think. I've tested it with trackpad and mouse. I also tested it with Python using this little script: ``` import bpy area = [area for area in bpy.context.screen.areas if area.type == "VIEW_3D"][0] region = [region for region in area.regions if region.type == 'WINDOW'][0] with bpy.context.temp_override(area=area, region=region): bpy.ops.view3d.dolly(mx=0, my=0, delta=2, use_cursor_init=False) ``` I will post an update soon.

Stefan Schumann added 1 commit 2023-07-29 17:44:06 +02:00

86fd5e5ae3 Properties work for Dolly. Added missing RMB cancel support.

Stefan Schumann added 1 commit 2023-08-05 19:21:59 +02:00

18e4853cac Zoom: analog properties handling to Dolly, easier update calculation.

Stefan Schumann commented

2023-08-05 20:06:48 +02:00

First-time contributor

Now Zoom works with Python as well, can be tested with this script:

import bpy

area = [area for area in bpy.context.screen.areas if area.type == "VIEW_3D"][0]
region = [region for region in area.regions if region.type == 'WINDOW'][0]

with bpy.context.temp_override(area=area, region=region):
    bpy.ops.view3d.zoom(mx=0, my=0, delta=-2, use_cursor_init=True)

Now Zoom works with Python as well, can be tested with this script: ``` import bpy area = [area for area in bpy.context.screen.areas if area.type == "VIEW_3D"][0] region = [region for region in area.regions if region.type == 'WINDOW'][0] with bpy.context.temp_override(area=area, region=region): bpy.ops.view3d.zoom(mx=0, my=0, delta=-2, use_cursor_init=True) ```

Stefan Schumann commented

2023-08-05 21:37:21 +02:00

First-time contributor

Okay, I tested it again and Zoom/Dolly work now consistently across editors. A few things were done along the way so far:

Properties handling fixed for Zoom/Dolly, is now according to specification
Dolly direction vector computation fixed, works now in all situations
Missing Dolly modal abort fixed
Zoom update calculation now done from NDC coordinates with one matrix multiplication for all camera types
Zooming input sensitivity is now window resolution independent using UI_SCALE_FAC
Fly Navigation speed can be changed with a track pad swipe

Okay, I tested it again and Zoom/Dolly work now consistently across editors. A few things were done along the way so far: - Properties handling fixed for Zoom/Dolly, is now according to specification - Dolly direction vector computation fixed, works now in all situations - Missing Dolly modal abort fixed - Zoom update calculation now done from NDC coordinates with one matrix multiplication for all camera types - Zooming input sensitivity is now window resolution independent using `UI_SCALE_FAC` - Fly Navigation speed can be changed with a track pad swipe

Stefan Schumann added 2 commits 2023-08-06 16:46:38 +02:00

112883b813 Merge branch 'main' into 2dzoomfix

0b2dd13695 Merged previous changes back. Dolly code has been tightened.

Stefan Schumann added 1 commit 2023-08-06 20:39:40 +02:00

c92e1a7a97 Added event delta computation via WM_event_absolute_delta_ to Dolly and Zoom.

Yevgeny Makarov commented

2023-08-07 09:54:10 +02:00

Regarding MOUSEZOOM, MOUSEROTATE: the flag indicating the "natural scrolling on/off" is only changing for MOUSEPAN. I've tested it. So:
if (U.uiflag & USER_ZOOM_HORIZ) {
      delta = WM_event_absolute_delta_x(event);
}
else {
      delta = WM_event_absolute_delta_y(event);
}
if (U.uiflag & USER_ZOOM_INVERT) {
      delta *= -1;
}
is sufficient for all three events. Ie. the 2 patches at the top of your list are already included.

This cannot work properly for MOUSEZOOM (pinch zoom).

The direction of MOUSEZOOM must not depend on USER_ZOOM_HORIZ, on USER_ZOOM_INVERT, and on the
"natural scrolling" (but WM_event_absolute_delta_x does).

> Regarding `MOUSEZOOM`, `MOUSEROTATE`: the flag indicating the "natural scrolling on/off" is only changing for `MOUSEPAN`. I've tested it. So: > ``` > if (U.uiflag & USER_ZOOM_HORIZ) { > delta = WM_event_absolute_delta_x(event); > } > else { > delta = WM_event_absolute_delta_y(event); > } > if (U.uiflag & USER_ZOOM_INVERT) { > delta *= -1; > } > ``` > is sufficient for all three events. Ie. the 2 patches at the top of your list are already included. This cannot work properly for `MOUSEZOOM` (pinch zoom). The direction of `MOUSEZOOM` must not depend on `USER_ZOOM_HORIZ`, on `USER_ZOOM_INVERT`, and on the "natural scrolling" (but `WM_event_absolute_delta_x` does).

Stefan Schumann commented

2023-08-07 13:02:17 +02:00

First-time contributor

Agreed. The y value of MOUSEZOOM is still a problem. So I changed the logic to this, so far in the clip and image editors:

if (event->type == MOUSEPAN) {
      if (U.uiflag & USER_ZOOM_HORIZ) {
        delta = WM_event_absolute_delta_x(event);
      }
      else {
        delta = WM_event_absolute_delta_y(event);
      }
        
      if (U.uiflag & USER_ZOOM_INVERT) {
        delta *= -1;
      }
}
else { /* MOUSEZOOM */
      delta = WM_event_absolute_delta_x(event);
}

Agreed. The y value of `MOUSEZOOM` is still a problem. So I changed the logic to this, so far in the clip and image editors: ``` if (event->type == MOUSEPAN) { if (U.uiflag & USER_ZOOM_HORIZ) { delta = WM_event_absolute_delta_x(event); } else { delta = WM_event_absolute_delta_y(event); } if (U.uiflag & USER_ZOOM_INVERT) { delta *= -1; } } else { /* MOUSEZOOM */ delta = WM_event_absolute_delta_x(event); } ```

Stefan Schumann added 2 commits 2023-08-07 13:06:34 +02:00

e0292c90bb Added proper MOUSEZOOM support to Clip and Image editors.

e130076535 Added proper MOUSEZOOM support to Dolly and Zoom.

Stefan Schumann added 3 commits 2023-08-07 18:59:00 +02:00

c6811ee4a5 Adapted View3D rotation and Proportional Editing tool.

741697f206 Adapted Loop Cut and Bevel tools.

267547b314 Adapted color button.

Stefan Schumann commented

2023-08-07 19:10:58 +02:00

First-time contributor

Okay, so far I've added the patches directly except for Dolly and Zoom where some minor adjustments were necessary.

Also U.dpi_fac seems to have been removed from the source code. This again raises the question if the input sensitivity should depend only via a factor on dx, dy. This makes it window resolution dependent. I think it should depend on dx/width, dy/height. What do you think?

So far I was not able to test the fly navigation with the track pad. Seems to have problems with it.

Okay, so far I've added the patches directly except for Dolly and Zoom where some minor adjustments were necessary. Also `U.dpi_fac` seems to have been removed from the source code. This again raises the question if the input sensitivity should depend only via a factor on dx, dy. This makes it window resolution dependent. I think it should depend on dx/width, dy/height. What do you think? So far I was not able to test the fly navigation with the track pad. Seems to have problems with it.

Yevgeny Makarov commented

2023-08-07 20:15:14 +02:00

Yeah, U.dpi_fac has been renamed to U.scale_factor / UI_SCALE_FAC.

The delta that trackpad sends depends on the screen resolution, on Retina it's 2x than on a standard screen. So on Retina the value changes will be twice as fast, there will be too much sensitivity. There's no access to the screen resolution multiplier here, but scale_factor matches this at the default "UI Scale" in Blender. Is to equalize the Retina screen (2x) with the regular one (1x).

There is a bug with fly navigation, see "Fly Navigation using the trackpad is broken 110848".

Yeah, `U.dpi_fac` has been renamed to `U.scale_factor` / `UI_SCALE_FAC`. The delta that trackpad sends depends on the screen resolution, on Retina it's 2x than on a standard screen. So on Retina the value changes will be twice as fast, there will be too much sensitivity. There's no access to the screen resolution multiplier here, but `scale_factor` matches this at the default "UI Scale" in Blender. Is to equalize the Retina screen (2x) with the regular one (1x). There is a bug with fly navigation, see "Fly Navigation using the trackpad is broken 110848".

Stefan Schumann added 1 commit 2023-08-08 17:54:56 +02:00

31ca6787e8 Added UI scaling to Dolly and Zoom.

Stefan Schumann commented

2023-08-08 18:03:18 +02:00

First-time contributor

So far I've added UI_SCALE_FAC to Dolly and Zoom. As I have modified these quite a bit I knew where to do it. Also now a scaling factor of 1/300 is present which I saw was used already somewhere in the source code for a similar purpose.

For the remaining cases it must be checked if the result of the division of dx, dy by UI_SCALE_FAC is passed on as a float or an int. If it's int the best way would be to defer the division to a place where a conversion to float takes place.

So far I've added `UI_SCALE_FAC` to Dolly and Zoom. As I have modified these quite a bit I knew where to do it. Also now a scaling factor of 1/300 is present which I saw was used already somewhere in the source code for a similar purpose. For the remaining cases it must be checked if the result of the division of dx, dy by `UI_SCALE_FAC` is passed on as a `float` or an `int`. If it's `int` the best way would be to defer the division to a place where a conversion to `float` takes place.

Stefan Schumann added 2 commits 2023-08-08 19:04:48 +02:00

0a2126eb77 Added UI_SCALE_FAC to Clip and Image Editors

7401f29821 Added UI_SCALE_FAC to Proportional Editing and Loop Cut tool.

Stefan Schumann added 1 commit 2023-08-08 20:08:29 +02:00

235a2ba808 Added UI_SCALE_FAC to Bevel Tool and color button

Stefan Schumann commented

2023-08-09 15:35:20 +02:00

First-time contributor

Looking at the Fly Navigation the event FLY_MODAL_SPEED cannot be configured in the preferences. An alternative would be to detect the track pad in an acceleration event similar to the Zoom operator. Do you think this is possible?

Looking at the Fly Navigation the event `FLY_MODAL_SPEED` cannot be configured in the preferences. An alternative would be to detect the track pad in an acceleration event similar to the Zoom operator. Do you think this is possible?

Stefan Schumann added 1 commit 2023-08-09 20:25:30 +02:00

375cefbd7c Track pad for Fly Navigation works now as pointer handling. Sensitivity adjustments to Zoom and Dolly.

Stefan Schumann commented

2023-08-09 20:29:09 +02:00

First-time contributor

I have found that the modal "key map" always replaces the original event. But handling MOUSEPAN directly to increase and decrease the speed works now.

I have found that the modal "key map" always replaces the original event. But handling `MOUSEPAN` directly to increase and decrease the speed works now.

Stefan Schumann added 1 commit 2023-08-10 19:20:16 +02:00

7d80f016e3 Some refinements.

Stefan Schumann commented

2023-08-12 10:51:50 +02:00

First-time contributor

Okay, at the moment I do not have anything to add. Who can now review this?

Stefan Schumann added 1 commit 2023-08-12 18:30:07 +02:00

d9107d2009 Make Zoom/Dolly delta computation distance dependent for key press

Stefan Schumann changed title from ~~WIP: Made 2D zoom consistent with 3D zoom for touchpads~~ to Made 2D zoom consistent with 3D zoom for touchpads

2023-08-13 10:11:31 +02:00

Pratik Borhade referenced this pull request

2023-08-16 05:07:13 +02:00

Multi-touch gesture: Zooming is reverse in shader editor comparing to viewport #89906

YimingWu referenced this pull request

2023-08-16 05:33:18 +02:00

Multi-touch gesture: Zooming is reverse in shader editor comparing to viewport #89906

This pull request has changes conflicting with the target branch.