Add support to Drag and Drop to FileHandlers #116047
No reviewers
Labels
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
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#116047
Loading…
Reference in New Issue
No description provided.
Delete Branch "guishe/blender:fh-dnd-support"
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?
Added support to Drag and Drop to file handlers, part of #111242.
If file handlers are registered with an import operator they
can now be invoked with drag and drop path data.
To make it works, import
operatos
must have declared afilepath
string property if supports handling one fileat the time, if they support multiple files it needs a
directory
string property and afiles
collection properties.Multiple
FileHandlers
could be valid for handle drop path data,when this happens a menu is shown so user can choose what to do
with the drop data.
If the operator only supports to handle one file
at the time, the operator should have a
FILE_PATH
StringProperty
calledfilepath
If the operator supports multiple files, the operator should
have a
DIR_PATH
StringProperty
calleddirectory
and a
OperatorFileListElement
CollectionProperty
calledfiles
Some examples of file handlers for testing
Add support to Drag and Drop to file handlersto Add support to Drag and Drop to FileHandlersCan you add a
doc/python_api/examples/bpy.types.FileHandler.py
with a simple example operator and file handler? With code comments explaining the required operator properties.Generally works alright given your two examples here. Just a few comments from what I've seen so far.
@ -0,0 +49,4 @@
return {'RUNNING_MODAL'}
class Curve_FH_text_import(bpy.types.FileHandler):
Should be
CURVE_FH_text_import
(here and a few other places etc.) to prevent "Warning: 'Curve_FH_text_import' doesn't have upper case alpha-numeric prefix"@ -0,0 +64,4 @@
return {'RUNNING_MODAL'}
class Shader_FH_script_import(bpy.types.FileHandler):
Adjust the casing of the name of this FileHandler too.
@ -21,6 +21,7 @@ set(INC
../sequencer
../shader_fx
../simulation
../windowmanager
This creates a circular dependency as windowmanager also depends on blenkernel. There's many such circular refs already but I would hate to add another one. I think I would keep the use of the WM API to just the
io_drop_import_file.cc
file to side step the organizational issue.@ -60,0 +141,4 @@
return WM_operatortype_find(import_operator, quiet);
}
static const char *property_type_str(PropertyType prop_type)
Feels a bit wrong for something like this to be defined here rather than under a RNA_ api. Will let others comment further on it if they wish though.
Moved to
rna_access
I found that this can prevent a crash in the file browser
@ -60,0 +245,4 @@
{
BLI_assert(get_import_operator());
PointerRNA props;
wmOperatorType *op = get_import_operator();
By convention use
ot
instead of op.@ -60,0 +283,4 @@
"' files : CollectionProperty(type = OperatorFileListElement)'"),
idname,
op->idname);
CLOG_WARN(&FH_LOG, message.c_str());
I definitely appreciate messages that explain the actual problem :) Generally though, to prevent "documentation" from being in more than one place, I would instead direct the user to the API docs rather than explaining the single/multi file difference here. Maybe just indicate that expected properties were not found and see the api documentation for details. What does everyone think about that?
Sounds good to me. Something like:
@ -0,0 +45,4 @@
static int wm_drop_import_file_exec(bContext *C, wmOperator *op)
{
auto paths = drop_import_file_paths(op);
if (paths.size() < 1) {
paths.empty()
@ -0,0 +50,4 @@
}
auto file_handlers = BKE_file_handlers_poll_file_drop(C, paths);
if (file_handlers.size() == 0) {
file_handlers.empty()
@ -0,0 +166,4 @@
void ED_dropbox_drop_import_file()
{
ListBase *lb = WM_dropboxmap_find("Window", 0, 0);
WM_dropbox_add(lb,
Wondering if maybe we can just place this inside
ED_keymap_screen
as that's where the .blend file drag-drop box is added. Should useSPACE_EMPTY
andRGN_TYPE_WINDOW
values instead of 0 too.had to duplicate
drop_import_file_poll_file_handlers
betweenio_drop_import_file.cc
and
screen_ops.cc
, only has the additional check for file handlers with valid operator.this was on
BKE_file_handlers_poll_file_drop
but removing thewm
dependency frombke
dont know the right place to validate thisI didn't notice that my suggestion would cause this duplication. In that case I think it's fine to keep the WM_dropbox_add/ED_dropbox_drop_import_file call inside the "io_drop_import_file.cc" file as you originally had it.
Other than that, things look fine.
Looks ok from my side now.
There is a way to make this even more user friendly
but that involves using a same popup for
drop_to_import
operator and aselected file handler's operator and some tricks
Attached video and patch
@brecht @deadpin can take a look? maybe good enough to redo a revision
I would prefer to merge the simple version of this pull request, and then refine it in a separate step.
@deadpin, do you think this could be committed since we both approved it?
As for the design in the video, I think this is getting too complex. The user already chose the files they wanted to drag, they don't have to pick some subset again on dropping. They also don't need to see or change the directory again.
The way I would present it is that, you're going to import all the files you dragged in, there's no changing that. If there are multiple file types, it could show a separate expandable panels for each, with a parent panel per file type. But still with a single button to import everything, not a separate import button for each.
I think it's more complex as the patch is now in terms of UX.
For example, if an importer only supports one file for call, the user has to drag and drop multiple times, and even worse, if the input parameters are the same, the user has to configure the settings again and again.
Also current state forces to update operators, so that they do something different if they are invoked with a path (this to avoid call a
fileselect
window for example)This would requiere addon maintainers to update the addond.
In how is the video, the user would just need to register a file handler if the addon owner not longer updates the addon.
As proposed in the video all is automatically selected.
If the user only drops
n
images all will be selected, only in the case where user would wants to import a subset with different settings it will require to the user to filter.It would not work as well if several importers can use the same files.
My preference would also be to iterate on further ideas separately if possible.
For importers that only support one file per call, it would be good to try to change those regardless of drag-drop support. Script authors who use those operators would also appreciate being able to just provide a list of files to import as well.
As a last resort we could loop over the files ourselves and repeatedly call the operator to give the user the illusion that multiple files are supported (all calls would use the same settings). Would that help unify the experience in a meaningful way for the implementation?
The main benefit of the latest demo seems to be the ability to change settings for just a subset of the dropped files. Unsure if we really need to worry about that part.
Would drag-dropping multiple file types at once, say .obj and .usd, still be possible outside of this new UI prototype? That, to me, might be a more compelling scenario to support.
We could consider making multi file support in operators a requirement for registering them as file handlers. Maybe it's better to make that requirement now rather than dragging along support for single file import operators for a long time.
I don't think this aspect should be interactive. To me drag & drop is supposed to be a simple operation. If you want more fine grained control you can drag & drop multiple times.
I would more imagine like an expanded enum at the top, with short names for the file types. There you can switch between the settings for them. And then an import all button at the button.
In a future unified "Import .." operator in the file menu, I think it should work the same way. You can set the settings for each file type in the sidebar, but the file dialog would still have a single "Import" button to import all and dismiss the dialog.
There can still be an enum to switch between the importer for a given file extensions. But to be honest, I'd also be fine not supporting that at all and always using one file handler for import. Or leaving it for future work.
What I want to avoid is somehow make it the user's responsibility to go through all the file types and not forget any file types, or import the same files multiple times.
For me to be interactive does not harm the user's workflow. If user just wanna import single file the user just drop a single file and perform a simple operation.
But if the user just wanna pick files and let blender tell him can do let him and choose, its nice to have this, and not to have to pick and drop again and again.
I think it is better to compare how the workflow is improved with some videos
@guishe I agree it's better to support importing multiple file types by dragging once. What I'm saying is that the UI for doing that should be simpler than your current implementation.
@guishe We'd like to get the initial work in this PR committed for 4.1. Iteration on the UI and other convenience aspects can then happen on top of that and would be easier to track and review. Does that sound ok? If so I'll plan on committing this sometime tomorrow once I give it another quick test locally.
That's fine with me!
I just got a little too excited about the improvements I propose #116047 (comment), but #116724 it's still in a very early stage.
Actually, can you merge in main on your side. Han's commit conflicts a little bit.
@blender-bot build