UI: Basic tree-view drag & drop reordering and inserting support #109825

Merged
Julian Eisel merged 11 commits from JulianEisel/blender:temp-tree-view-drag-drop into main 2023-07-11 14:30:34 +02:00
Member

No user visible changes expected, these are just the internal API preparations.

Modifies the Drop API for views so that tree-views can choose to insert items before, after and into other items.

From #109826:

Note: While there is support for drag-tooltips that can explain how an item will be inserted, there is no drawing yet like in the Outliner, that indicates if an item is inserted before, after or into. There is some work on that but that can be done separately.

Changes:

  • Removes AbstractViewDropTarget that was shared between tree- and grid-views, and adds AbstractTreeViewDropTarget and AbstractGridViewDropTarget. The tree-view needs specialized handling now, and although they could share some code still, it's not worth having another level of inheritance.
  • Modifies the drop-target API to use DragInfo which contains more info about the dragging operation than just the wmDrag.
  • Adds determine_drop_location() to the DropTargetInterface which drop targets can use to determine when dropping means inserting before, after or into.
  • Store the block and region in the view. This is needed unfortunately but shouldn't be an issue since the tree view is recreated on redraws, together with the block.
  • Various smaller tweaks and additions to views as needed.

TODO (outside scope of this change): Increase row height so there is no gap between tree view items, but keep things visually the same otherwise. This reduces flickering while dragging.

No user visible changes expected, these are just the internal API preparations. Modifies the Drop API for views so that tree-views can choose to insert items before, after and into other items. From #109826: <video src="https://projects.blender.org/attachments/a3664b5f-d670-4920-ac33-1701f495f41c" width="400px" controls></video> Note: While there is support for drag-tooltips that can explain how an item will be inserted, there is no drawing yet like in the Outliner, that indicates if an item is inserted before, after or into. There is some work on that but that can be done separately. Changes: - Removes `AbstractViewDropTarget` that was shared between tree- and grid-views, and adds `AbstractTreeViewDropTarget` and `AbstractGridViewDropTarget`. The tree-view needs specialized handling now, and although they could share some code still, it's not worth having another level of inheritance. - Modifies the drop-target API to use `DragInfo` which contains more info about the dragging operation than just the `wmDrag`. - Adds `determine_drop_location()` to the `DropTargetInterface` which drop targets can use to determine when dropping means inserting before, after or into. - Store the block and region in the view. This is needed unfortunately but shouldn't be an issue since the tree view is recreated on redraws, together with the block. - Various smaller tweaks and additions to views as needed. TODO (outside scope of this change): Increase row height so there is no gap between tree view items, but keep things visually the same otherwise. This reduces flickering while dragging.
Julian Eisel added the
Interest
Grease Pencil
Module
User Interface
labels 2023-07-07 16:32:23 +02:00
Julian Eisel added 2 commits 2023-07-07 16:32:33 +02:00
Contributor

Will drag & drop work in every UI list like Vertex Groups & Shape Keys? And I'm interested if there needs to be done something in python to enable this feature on UI lists made in scripts, or will it be automatic for all lists?

Will drag & drop work in every UI list like Vertex Groups & Shape Keys? And I'm interested if there needs to be done something in python to enable this feature on UI lists made in scripts, or will it be automatic for all lists?
Julian Eisel added 1 commit 2023-07-10 11:10:57 +02:00
Julian Eisel added 2 commits 2023-07-10 11:49:15 +02:00
Author
Member

Will drag & drop work in every UI list like Vertex Groups & Shape Keys? And I'm interested if there needs to be done something in python to enable this feature on UI lists made in scripts, or will it be automatic for all lists?

This implements the feature for tree-views, not UI lists. I hope we can replace UI lists with tree-views before too long (or at least make them use tree-views internally) since they are a lot more powerful & flexible.

> Will drag & drop work in every UI list like Vertex Groups & Shape Keys? And I'm interested if there needs to be done something in python to enable this feature on UI lists made in scripts, or will it be automatic for all lists? This implements the feature for tree-views, not UI lists. I hope we can replace UI lists with tree-views before too long (or at least make them use tree-views internally) since they are a lot more powerful & flexible.
Julian Eisel added 1 commit 2023-07-10 12:15:58 +02:00
Julian Eisel requested review from Hans Goudey 2023-07-10 12:55:47 +02:00
Hans Goudey reviewed 2023-07-10 14:39:49 +02:00
@ -72,1 +66,4 @@
protected:
/* Not too great to have these here, but makes things a bit simpler. Views are recreated on
* redraws, so saving these pointers should be save. */
Member

save -> safe

`save` -> `safe`
JulianEisel marked this conversation as resolved
@ -159,0 +169,4 @@
* when dragging over this item. An item can return a drop target for itself via a custom
* implementation of #AbstractGridViewItem::create_drop_target().
*/
class AbstractGridViewItemDropTarget : public DropTargetInterface {
Member

I'm a bit skeptical about the Abstract part of the name personally. I feel like it's not adding too much information that isn't clear with the context anyway, and it makes the name quite a mouthful. But I see some other names with "Abstract" around here.

I'm a bit skeptical about the `Abstract` part of the name personally. I feel like it's not adding too much information that isn't clear with the context anyway, and it makes the name quite a mouthful. But I see some other names with "Abstract" around here.
Author
Member

To me it often matters to make clear that a type must be passed as pointer/reference, to avoid object slicing. In this case it's fine though, since there are pure virtual functions so slicing cannot happen with this type.

Maybe we should figure out guidelines for this.

To me it often matters to make clear that a type must be passed as pointer/reference, to avoid object slicing. In this case it's fine though, since there are pure virtual functions so slicing cannot happen with this type. Maybe we should figure out guidelines for this.
JulianEisel marked this conversation as resolved
@ -70,0 +86,4 @@
* Enable dropping onto/into (#DropLocation::Into), before (#DropLocation::Before) and after
* (#DropLocation::After) the drop target. Typically used for reordering items with nesting
* support. */
Reorder_and_Insert,
Member

I'd stick with camel case here, ReorderInsert or ReorderAndInsert

I'd stick with camel case here, `ReorderInsert` or `ReorderAndInsert`
JulianEisel marked this conversation as resolved
@ -125,0 +127,4 @@
/**
* \param xy: The mouse coordinates in window space.
*/
AbstractTreeViewItem *find_hovered(const int xy[2]);
Member

const int2 & should be a bit nicer here

`const int2 &` should be a bit nicer here
Author
Member

Ah true, have to get used to that still :)

Ah true, have to get used to that still :)
JulianEisel marked this conversation as resolved
@ -320,0 +347,4 @@
AbstractTreeViewItemDropTarget(AbstractTreeView &view,
DropBehavior behavior = DropBehavior::Insert);
std::optional<DropLocation> determine_drop_location(const wmEvent &event) const;
Member

drop_location_calc would probably match the "important words first" naming that's seems to be preferred. No strong opinion personally though.

`drop_location_calc` would probably match the "important words first" naming that's seems to be preferred. No strong opinion personally though.
Author
Member

Personally I prefer determine_drop_location because it reads better and seems easier to memorize. "calc" also sounds a bit like it's calculating some numeric value, maybe "choose" is better even, to denote that possible outcomes are predefined?

Personally I prefer `determine_drop_location` because it reads better and seems easier to memorize. "calc" also sounds a bit like it's calculating some numeric value, maybe "choose" is better even, to denote that possible outcomes are predefined?
JulianEisel marked this conversation as resolved
@ -22,0 +33,4 @@
return false;
}
std::optional<DropLocation> drop_location = drop_target.determine_drop_location(event);
Member

const

`const`
JulianEisel marked this conversation as resolved
@ -31,0 +54,4 @@
return nullptr;
}
std::optional<DropLocation> drop_location = drop_target.determine_drop_location(event);
Member

const

`const`
JulianEisel marked this conversation as resolved
@ -232,0 +273,4 @@
uiButViewItem *hovered_but = hovered_item->view_item_button();
rctf but_win_rect;
ui_block_to_window_rctf(view_.region_, view_.block_, &but_win_rect, &hovered_but->rect);
Member

It seems like it might be worth it to avoid storing ARegion and uiBlock pointers in views if possible. That sort of back-pointer can seem like a reasonable addition one at a time, but after a while they seem to make a huge mess. Could these be passed as arguments here?

It seems like it might be worth it to avoid storing `ARegion` and `uiBlock` pointers in views if possible. That sort of back-pointer can seem like a reasonable addition one at a time, but after a while they seem to make a huge mess. Could these be passed as arguments here?
Author
Member

Not sure, I don't really want to clutter the API with these things just for some simple coordinate conversions. Don't think these pointers are bad usually, at least I don't often encounter real problems with them (even if not nice design wise). But I do have another idea.

Not sure, I don't really want to clutter the API with these things just for some simple coordinate conversions. Don't think these pointers are bad usually, at least I don't often encounter real problems with them (even if not nice design wise). But I do have another idea.
Author
Member

Added AbstractTreeViewItem::get_win_rect() now which makes things a bit cleaner. This now gets the block from the button and the region from context via uiBlock.evil_C. Getting the block from the button seems fine, getting the region this way is a bit meeh. Maybe passing the region is okay, but honestly doesn't seem worth bothering much with rn. Would like to see if we need to access this more often first.

Added `AbstractTreeViewItem::get_win_rect()` now which makes things a bit cleaner. This now gets the block from the button and the region from context via `uiBlock.evil_C`. Getting the block from the button seems fine, getting the region this way is a bit meeh. Maybe passing the region is okay, but honestly doesn't seem worth bothering much with rn. Would like to see if we need to access this more often first.
Member

Yeah, I'd definitely prefer passing the region instead of adding another use of evil_C. Think if it's needed more in the future, then we could think about storing the pointer in the view. For now it's just one case, seems fine to pass it. It makes it clear that it's used anyway, which is sort of nice.

Yeah, I'd definitely prefer passing the region instead of adding another use of `evil_C`. Think if it's needed more in the future, then we could think about storing the pointer in the view. For now it's just one case, seems fine to pass it. It makes it clear that it's used anyway, which is sort of nice.
JulianEisel marked this conversation as resolved
Julian Eisel added 2 commits 2023-07-10 15:16:44 +02:00
Julian Eisel added 1 commit 2023-07-11 11:50:23 +02:00
Julian Eisel added 1 commit 2023-07-11 11:54:39 +02:00
Julian Eisel added 1 commit 2023-07-11 11:56:54 +02:00
Hans Goudey approved these changes 2023-07-11 14:15:45 +02:00
Hans Goudey left a comment
Member

Thanks for making those changes!

Thanks for making those changes!
@ -185,1 +196,4 @@
AbstractTreeView &get_tree_view() const;
/**
* Calculate the view item rectangle from its view-item button, converted to window space.
* Returns an unset optional if there is no view item button for this item.
Member

Nitpick, but "unset" -> "empty" is clearer I think.

Nitpick, but "unset" -> "empty" is clearer I think.
Julian Eisel merged commit 4525527852 into main 2023-07-11 14:30:34 +02:00
Julian Eisel deleted branch temp-tree-view-drag-drop 2023-07-11 14:30:35 +02:00
Sign in to join this conversation.
No reviewers
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
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
EEVEE & Viewport
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
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
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
EEVEE & Viewport
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
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
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
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Reference: blender/blender#109825
No description provided.