Details

Reviewers

William Reynish (billreynish)
Julian Eisel (Severin)
Brecht Van Lommel (brecht)
Pablo Vazquez (pablovazquez)

Commits

rB859505a3dae8: UI: Pan 2D Region When Dragging to Boundary

Summary

We can't really propose using dragging and dropping for moving modifiers without having something like this in my opinion.

This adds a modal operator called Edge Pan, which is meant to run invisibly while something inside a region is being dragged. This patch applies this to dragging panels, but it can be used elsewhere too.

Here I showed it with a small region just for a better aspect ratio. It looks like it skips a few times in the video, that's just a low framerate.

The speed and delay are easily adjustible, and the speed also increases subtly the start value to a max to make it feel more interactive and less robotic.

Diff Detail

Repository: rB Blender

Event Timeline

Hans Goudey (HooglyBoogly) requested review of this revision.Apr 18 2020, 12:07 AM

Hans Goudey (HooglyBoogly) created this revision.

Hans Goudey (HooglyBoogly) added a parent revision: D7462: UI: Fix dragging panels changing region size.

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)Apr 18 2020, 12:10 AM

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)

Duarte Farrajota Ramos (duarteframos) awarded a token.Apr 18 2020, 12:46 AM

Duarte Farrajota Ramos (duarteframos) added a subscriber: Duarte Farrajota Ramos (duarteframos).

This sounds great! Will this work for dragging any other type of data?
One recurring step in my workflow is copying a material diffuse color to the viewport display color.
This is usually done by drag&drop from the diffuse color box usually on top, to the viewport panel down below in the Properties Window.
With the Principled BSDF that list is quite long and the Viewport Display panel ends up out of screen in most 1080 monitors,.
This feature would be extremely welcome

It looks like the panel keeps moving when we're scrolled all the way to the top or the bottom and hold it there. I think I've found a fix for that, I just need to check if cur is already at its minimum or maximum). But I'm struggling to understand the purpose of these two lines:

dx *= (float)BLI_rctf_size_x(&region->v2d.cur) / (float)BLI_rcti_size_x(&region->winrct);
dy *= (float)BLI_rctf_size_y(&region->v2d.cur) / (float)BLI_rcti_size_y(&region->winrct);

(Commenting them out is helping solve the problem, which I feel like it shouldn't.)

Ryan Inch (Imaginer) awarded a token.Apr 18 2020, 2:28 AM

William Reynish (billreynish) awarded a token.Apr 18 2020, 5:55 AM

How general is this? Could we later use the same system for places like the Outliner?

stefano severi (Tetone) awarded a token.Apr 18 2020, 12:04 PM

Pieter Schiettecatte (Schiette) awarded a token.Apr 18 2020, 3:06 PM

Pieter Schiettecatte (Schiette) added a subscriber: Pieter Schiettecatte (Schiette).

In D7465#179191, @William Reynish (billreynish) wrote:

How general is this? Could we later use the same system for places like the Outliner?

In general the logic is not too bad so I'm guessing it would be pretty straightforward to add in other regions. But because this is wrapped up in the panel dragging it would have to be implemented separately. Could be nice!

Any chance you'd consider also enabling scrolling while dragging?

I've had some bad experiences with programs where it takes ages to move something from the bottom to the top of the list. Increasing the hardcoded speed is tricky because for some scenarios/users it might become too jumpy.

With mouse scrolling you can just aggressively scroll to speed it up a bit, and the behavior would be consistent with normal navigation.

In D7465#179191, @William Reynish (billreynish) wrote:

Outliner?

Yea, the outliner really needs it... 😉

In D7465#179171, @Hans Goudey (HooglyBoogly) wrote:
It looks like the panel keeps moving when we're scrolled all the way to the top or the bottom and hold it there. I think I've found a fix for that, I just need to check if cur is already at its minimum or maximum). But I'm struggling to understand the purpose of these two lines:
dx *= (float)BLI_rctf_size_x(&region->v2d.cur) / (float)BLI_rcti_size_x(&region->winrct);
dy *= (float)BLI_rctf_size_y(&region->v2d.cur) / (float)BLI_rcti_size_y(&region->winrct);

This is an adjustment for the amount of zoom in the properties editor. Removing it breaks dragging when you zoom in.

I think scroll also needs to be adjusted for this, see for example view_pan_apply_ex.
vpd->facx

source/blender/editors/interface/interface_panel.c
1349	sone -> zone
1363	I expect this needs some kind of clamping so `cur` remains within `tot`. This is usually done using `UI_view2d_curRect_validate`.

Before you spend more time on this @Hans Goudey (HooglyBoogly), I'd like to propose doing this directly in View2D. There could simply be a modal operator there that scrolls based on mouse movements towards region borders, invoked through another operator/handler (e.g. the panel dragging). It would always return OPERATOR_PASS_THROUGH for the invoking, parallel running operator to catch the events.
I don't see this depending on anything but the region bounds and the mouse events.

If this can be made to work, we could *trivially* add the behavior to other editors that need this (e.g. Outliner).

Use a modal operator with pass-through return to achieve panning.
- Uses the same behavior for X and Y
- Generalized to View2D, so it can be easily used in other regions
Add the srcoll from the modal operator to the panel drag offset

Currently the panel jitters a bit while the region is scrolling.
I believe this is because the panel's offset is rounded to a multiple of some number. Not that it should do this... I remember coming across that a while ago, but I can't find it anymore!

Hans Goudey (HooglyBoogly) removed a parent revision: D7462: UI: Fix dragging panels changing region size.Apr 21 2020, 9:30 PM

Fix typo

Hans Goudey (HooglyBoogly) added inline comments.Apr 21 2020, 10:25 PM

source/blender/editors/interface/interface_panel.c
2538	Another complication: This discards `const` from context. I can just propagate that change through to all callers of `panel_activate_state`, but I wanted to mention it first.

Hans Goudey (HooglyBoogly) mentioned this in D7462: UI: Fix dragging panels changing region size.Apr 22 2020, 4:08 PM

Bataev Artem (shader) awarded a token.Apr 29 2020, 9:46 PM

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)May 15 2020, 7:03 PM

Brecht Van Lommel (brecht) added inline comments.May 15 2020, 7:36 PM

source/blender/blenkernel/BKE_lib_id.h
77 ↗	(On Diff #23949)	Seems there are some unrelated changes in this patch.

Update patch with Arcanist

Harbormaster completed remote builds in B8068: Diff 24813.May 15 2020, 7:57 PM

Brecht Van Lommel (brecht) accepted this revision.May 18 2020, 11:50 PM

Brecht Van Lommel (brecht) added inline comments.

source/blender/editors/interface/view2d_ops.c
385–393 ↗	(On Diff #24813)	The `exec` function can be removed, you wouldn't redo this operation or call it from the console.

This revision is now accepted and ready to land.May 18 2020, 11:50 PM

Remove exec function, add define
Merge master

Harbormaster completed remote builds in B8137: Diff 24966.May 20 2020, 5:34 PM

A couple of issues:

Usually with such edge-panning features, the speed is controlled by the mouse distance from the edge. So if I drag the mouse further away from the edge, it scrolls faster.
The panel indeed jitters a bit while the view is being scrolled. Do you plan to address that still?
If the lower-most panel is closed (or empty), you can't drag a panel lower than it:
There's some noticeable, seemingly random jumping going on. I think it happens if you hover the scrollbar while dragging over the edge:

source/blender/editors/interface/interface_panel.c
2551	/Users/julianeisel/blender/software/dev/default/src/source/blender/editors/interface/interface_panel.c:2975:29: warning: passing 'const bContext ' (aka 'const struct bContext ') to parameter of type 'struct bContext ' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers] WM_operator_name_call(C, "VIEW2D_OT_edge_pan", WM_OP_INVOKE_DEFAULT, NULL); ^ /Users/julianeisel/blender/software/dev/default/src/source/blender/windowmanager/WM_api.h:422:44: note: passing argument to parameter 'C' here int WM_operator_name_call(struct bContext C, ^ I think it's reasonable to let `panel_activate_state()` have a non-const context, most callers pass it like this anyway. I think you'd have to cast away const in `ED_region_panels_layout_ex()` though when calling `UI_panels_end()`. I prefer doing these casts higher up the call-stack, rather than having them buried down at a hard to find place. However, a cleaner solution would be using the UI after-funcs for this, so the general UI handling code does the operator call. Turns out there's a simple helper for this already: `ui_handle_afterfunc_add_operator()`.

This revision now requires changes to proceed.Jun 2 2020, 2:20 PM

Julian Eisel (Severin) awarded a token.Jun 2 2020, 2:21 PM

Cleanup: Fix unecessary casts
Use handler after func to call the edge pan operator
Fix dragging panel below the last panel if it is collapsed

Usually with such edge-panning features, the speed is controlled by the mouse distance from the edge. So if I drag the mouse further away from the edge, it scrolls faster.

The issue with that is that it's much easier to move the panel up then down because the properties area usually goes
to the bottom of the screen. We could do it that way but I think that asymmetry makes it a frustrating solution.

The panel indeed jitters a bit while the view is being scrolled. Do you plan to address that still?

Yes, I'm working on that and I would consider fixing it a prerequisite for committing this. I haven't found the cause yet
though. Printing out the offset at the beginning of every redraw doesn't show it jittering at all.

If the lower-most panel is closed (or empty), you can't drag a panel lower than it:

Fixed in this update.

There's some noticeable, seemingly random jumping going on. I think it happens if you hover the scrollbar while dragging over the edge:

I also need to look into this. Hopefully it's related to the jittering.

Harbormaster completed remote builds in B8367: Diff 25474.Jun 4 2020, 1:43 AM

Hans Goudey (HooglyBoogly) added inline comments.Jun 4 2020, 1:45 AM

source/blender/editors/interface/interface_panel.c
2551	This worked out very well! Thanks for the tip

Fix the remaining issues:

Jittering while "edge panning"
Jumping when "edge panning" and dragging over the scroll bar.

So it turns out issue 1 (jittering) happens because there are a few things happening at different rates.

VIEW EDGE PAN MODAL
VIEW EDGE PAN MODAL
UI DO DRAG
UI PANELS DRAW
VIEW EDGE PAN MODAL
UI PANELS DRAW
VIEW EDGE PAN MODAL
VIEW EDGE PAN MODAL
UI DO DRAG
UI PANELS DRAW
VIEW EDGE PAN MODAL
UI PANELS DRAW
VIEW EDGE PAN MODAL
VIEW EDGE PAN MODAL
UI DO DRAG
UI PANELS DRAW
VIEW EDGE PAN MODAL
UI PANELS DRAW
VIEW EDGE PAN MODAL
VIEW EDGE PAN MODAL
UI DO DRAG
UI PANELS DRAW

The key is that ui_do_drag happens slower than both view_edge_pan_modal and ui_panels_draw, meaning the panel's location isn't always updated to reflect the change in region scroll before a redraw.

I see two possible solutions to this, but some feedback on which to pursue would be useful.

Figure out a way to get the panel's UI handler to run before every redraw.
Pass a pointer to the panel's offset to the operator and change it there directly.

I'm not sure which I prefer, I feel the changes for 1 might be deeper than this patch would call for, but on the other hand option 2 doesn't seem as clean.

ui_do_drag() isn't actually executed on mouse-move events in the current version, only on timer events (which always pass through all handlers). Returning OPERATOR_RUNNING_MODAL from the modal callback means that the event is swallowed, even if combined with OPERATOR_PASS_THROUGH -- for whatever reason.

Fix panel jittering while dragging
Ignore non-mousemove events in the edge pan operator (also "in-between" mousemoves)

Harbormaster completed remote builds in B8371: Diff 25482.Jun 4 2020, 1:36 PM

Julian Eisel (Severin) added inline comments.Jun 4 2020, 1:37 PM

source/blender/editors/interface/view2d_ops.c
381–383 ↗	(On Diff #25482)	Unused.

Remove unused variables, struct
Separate logic for each axis with a timer for both X and Y

The issue I thought had to do with the scroll bar was the time accumulating for the X axis panning
also applying to the Y axis. It makes more sense to track each axis separately anyway.

Harbormaster completed remote builds in B8376: Diff 25491.Jun 4 2020, 6:07 PM

Works pretty nice now, feels much more finished!

I still think this should work based on mouse distance though, that's how I know this from any other app. Right now there is no way to make it scroll faster, which I find *very* annoying. If I want to move a panel from the beginning to the end, it takes ages if there are a bunch of open panels (doesn't have to be many).

The asymmetry is a trade0off, but I think a much more acceptable one than being bound to a predefined, rather slow scrolling speed. In other apps I notice this trade-off too.

Fix dragging below the last panel if it's collapsed

Harbormaster completed remote builds in B8378: Diff 25493.Jun 4 2020, 7:18 PM

Actually fix dragging past a bottom collapsed panel, for real this time
Use only deltas, calculate speed based on distance and fade it in over one second

Thanks @Julian Eisel (Severin) for pushing me to make the panning feel better, I'm very happy with the way it works now.

I rewrote the speed logic combining a 1 second smootherstep fade-in with a speed gradient with the distance from the edge.

The logic also gets simpler because the starttime is only used for the fade-in. Instead of managing the total scroll the operator now only applies deltas.

Harbormaster completed remote builds in B8384: Diff 25501.Jun 4 2020, 10:08 PM