Fix: Inconsistent input drag editing for factor properties #112004

Guillermo Venegas · 2023-09-06T06:38:09+02:00

Guillermo Venegas commented

2023-09-06 06:38:09 +02:00

Drag editing numeric inputs tagged as PROP_FACTOR is performed
as a slider, when the user start editing this numerical inputs
the mouse is hidden, the feedback that the user get is a slide
that fill the input.

However, when the mouse leave the input box, there is no more
feedback until the mouse enter again to the input box, this feels wrong
because the mouse is hidden, and there is no feedback that tells that
the mouse is outside the input box and need to be within.

This pr fixes the need of having the mouse withing the input box, instead
the slide is performed by mouse movement.

If the Continuous Grab preference is uncheck or if the user is using a tablet,
the slider will be exactly under the cursor whiting the input.

Drag editing numeric inputs tagged as `PROP_FACTOR` is performed as a slider, when the user start editing this numerical inputs the `mouse is hidden`, the feedback that the user get is a slide that fill the input. However, when the `mouse` leave the input box, there is no more feedback until the mouse enter again to the input box, this feels wrong because the mouse is hidden, and there is no feedback that tells that the mouse is outside the input box and need to be within. This pr fixes the need of having the mouse withing the input box, instead the slide is performed by mouse movement. If the Continuous Grab preference is uncheck or if the user is using a tablet, the slider will be exactly under the cursor whiting the input.

👍 1

Guillermo Venegas added 1 commit 2023-09-06 06:38:21 +02:00

0d71ea7bdd fix

Guillermo Venegas commented

2023-09-06 07:02:44 +02:00

This video shows what happens when drag editing a numeric input tagged as factor, i made changes to see the actual mouse position, the value only changes when the mouse is within the input box (well, not at all, it seems a bit translated), but this feels wrong because the mouse is normally hidden by a drag edit

Here the fix applied (the value changes by movement)

This video shows what happens when drag editing a numeric input tagged as factor, i made changes to see `the actual mouse position`, the value only changes when the mouse is within the input box (well, not at all, it seems a bit translated), but this feels wrong because the `mouse is normally hidden by a drag edit` <video src="/attachments/0b8eefdb-b8ed-4ea9-8270-9ba05af479e0" title="2023-09-05 22-22-36.mp4" controls></video> Here the fix applied (`the value changes by movement`) <video src="/attachments/8b7056d4-573c-4ac5-a46b-76fdd3ed8045" title="2023-09-05 22-24-44.mp4" controls></video>

2023-09-05 22-24-44.mp4

757 KiB

2023-09-05 22-22-36.mp4

912 KiB

Guillermo Venegas commented

2023-09-06 07:04:31 +02:00

In the second video a frame shows the input whit several decimals, wonder if its caused by the pr

In the second video a frame shows the input whit several decimals, wonder if its caused by the pr ![image](/attachments/8ae85923-9cb2-4335-9b1e-d06a1715a0ad)

image.png

864 KiB

Guillermo Venegas referenced this pull request

2023-09-06 07:07:23 +02:00

Inconsistent input drag editing #111195

Guillermo Venegas added 1 commit 2023-09-06 20:22:41 +02:00

8b62d8174f fix for use_continuous_grab uncheck or tablet

Guillermo Venegas added 1 commit 2023-09-06 20:22:54 +02:00

8d428fb120 Merge remote-tracking branch 'origin/main' into fix-mouse-drag-edit-factor

YimingWu commented

2023-09-07 05:45:57 +02:00

This looks pretty reasonable to me

YimingWu requested review from Pablo Vazquez 2023-09-07 05:46:09 +02:00

YimingWu added this to the User Interface project 2023-09-07 05:46:17 +02:00

Pablo Vazquez commented

2023-09-07 16:20:58 +02:00

Makes sense. Better to have @ideasman42 's input since this is related to Continuous Grab.

Harley Acheson commented

2023-09-11 23:30:40 +02:00

I do like this behavior. It took me a bit to understand the difference, but once I saw it, it feels much nicer.

Codewise, I'd personally minimize some changes and make the change more obvious by doing something like the following. So keeps the new use_continuous_grab with the new code.


  if (but->type == UI_BTYPE_NUM_SLIDER) {
    cursor_x_range = BLI_rctf_size_x(&but->rect);
    if (use_continuous_grab) {
      const float fac = ui_mouse_scale_warp_factor(shift);
      f = ((mx_fl - data->draglastx) / cursor_x_range) * fac + data->dragf;
    }
    else {
      mx_fl = clamp_f(mx_fl, but->rect.xmin, but->rect.xmax);
      f = (mx_fl - but->rect.xmin) / cursor_x_range;
    }
  }
  else ...

I'd also be a bit nervous about the removal of the "else" in ui_numedit_but_SLI. If there is something that is not UI_BTYPE_NUM_SLIDER or UI_BTYPE_SCROLL your "f" will be randomly 0 or 1.

I do like this behavior. It took me a bit to understand the difference, but once I saw it, it feels much nicer. Codewise, I'd personally minimize some changes and make the change more obvious by doing something like the following. So keeps the new use_continuous_grab with the new code. ``` if (but->type == UI_BTYPE_NUM_SLIDER) { cursor_x_range = BLI_rctf_size_x(&but->rect); if (use_continuous_grab) { const float fac = ui_mouse_scale_warp_factor(shift); f = ((mx_fl - data->draglastx) / cursor_x_range) * fac + data->dragf; } else { mx_fl = clamp_f(mx_fl, but->rect.xmin, but->rect.xmax); f = (mx_fl - but->rect.xmin) / cursor_x_range; } } else ... ``` I'd also be a bit nervous about the removal of the "else" in `ui_numedit_but_SLI`. If there is something that is not UI_BTYPE_NUM_SLIDER or UI_BTYPE_SCROLL your "f" will be randomly 0 or 1.

Guillermo Venegas added 1 commit 2023-09-12 06:17:35 +02:00

46e23a5c62 Merge remote-tracking branch 'origin/main' into fix-mouse-drag-edit-factor

Guillermo Venegas added 1 commit 2023-09-12 06:17:58 +02:00

e4e6a1d9ad requested changes

Guillermo Venegas commented

2023-09-12 06:30:24 +02:00

ui_numedit_but_SLI is only used in ui_do_but_SCROLL and ui_do_but_SLI, and they are used only once in ui_do_button in a switch like:

      break;
    case UI_BTYPE_SCROLL:
      retval = ui_do_but_SCROLL(C, block, but, data, event);
      break;
...
      break;
    case UI_BTYPE_NUM_SLIDER:
      retval = ui_do_but_SLI(C, block, but, data, event);
      break;

`ui_numedit_but_SLI` is only used in `ui_do_but_SCROLL` and `ui_do_but_SLI`, and they are used only once in `ui_do_button` in a switch like: ```cpp break; case UI_BTYPE_SCROLL: retval = ui_do_but_SCROLL(C, block, but, data, event); break; ... break; case UI_BTYPE_NUM_SLIDER: retval = ui_do_but_SLI(C, block, but, data, event); break; ```

Harley Acheson commented

2023-09-13 01:12:43 +02:00

ui_numedit_but_SLI is only used in ui_do_but_SCROLL and ui_do_but_SLI, and they are used only once in ui_do_button in a

No worries if you are sure. When there is any doubt a good trick is to leave that last else but with only a BLI_assert_unreachable(); inside.

> `ui_numedit_but_SLI` is only used in `ui_do_but_SCROLL` and `ui_do_but_SLI`, and they are used only once in `ui_do_button` in a No worries if you are sure. When there is any doubt a good trick is to leave that last else but with only a `BLI_assert_unreachable();` inside.

Campbell Barton requested changes 2024-01-17 05:32:38 +01:00

Campbell Barton left a comment

This PR seems to change behavior in ways that aren't ideal.

Without continuous grab (or when a tablet is used):

It's not longer possible to Shift-Drag for minor adjustments.
Clicking on the slider doesn't immediately set it's value to the cursors location (which feels like a glitch if the expectation is that dragging follows the cursor location instead of being an offset).
Changing the slider on the initial press wont work as a click is needed to edit the value.

I'd prefer if this PR only adjusts the behavior with continuous grab enabled, attached a diff that does this:

This PR seems to change behavior in ways that aren't ideal. Without continuous grab (or when a tablet is used): - It's not longer possible to Shift-Drag for minor adjustments. - Clicking on the slider doesn't immediately set it's value to the cursors location (which feels like a glitch if the expectation is that dragging follows the cursor location instead of being an offset). Changing the slider on the initial press wont work as a click is needed to edit the value. ---- I'd prefer if this PR only adjusts the behavior with continuous grab enabled, attached a diff that does this:

112004_update.diff

2.6 KiB

Guillermo Venegas commented

2024-01-18 23:22:08 +01:00

Seems fine, just checked my changes and they also introduces a regression to the UILits scroll sliders

This pull request has changes conflicting with the target branch.

source/blender/editors/interface/interface_handlers.cc

View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u fix-mouse-drag-edit-factor:guishe-fix-mouse-drag-edit-factor

git checkout guishe-fix-mouse-drag-edit-factor

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Fix: Inconsistent input drag editing for factor properties #112004

Checkout