griefith/test
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

UI: Refactor path dropping so logic doesn't depend on icons

Browse Source 
No behavior change intended.

Many file drag & drop handlers used the icon assigned for dragging to
determine what type of data is dragged. This is fragile, for example
changing an icon would break drag & drop (!). This happened a few times,
e.g. see 3788003cda. It's also causing problems with #104830, which
changes how file browser drag data is handled.

Instead use the file extension to determine the file type.
This commit is contained in:
Julian Eisel
2023-02-27 16:31:31 +01:00
parent 1d4abfb537
commit 17e92711d3
17 changed files with 151 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
source/blender/editors/include/UI_interface.h
View File

@@ -1819,16 +1819,22 @@ void UI_but_drag_set_asset(uiBut *but,
struct ImBuf *imb,
float scale);
void UI_but_drag_set_rna(uiBut *but, struct PointerRNA *ptr);
void UI_but_drag_set_path(uiBut *but, const char *path, bool use_free);
/**
* Enable dragging a path from this button.
* \param path: The path to drag. The passed string may be destructed, button keeps a copy.
*/
void UI_but_drag_set_path(uiBut *but, const char *path);
void UI_but_drag_set_name(uiBut *but, const char *name);
/**
* Value from button itself.
*/
void UI_but_drag_set_value(uiBut *but);
/** Sets #UI_BUT_DRAG_FULL_BUT so the full button can be dragged. */
void UI_but_drag_set_image(
uiBut *but, const char *path, int icon, struct ImBuf *imb, float scale, bool use_free);
/**
* Sets #UI_BUT_DRAG_FULL_BUT so the full button can be dragged.
* \param path: The path to drag. The passed string may be destructed, button keeps a copy.
*/
void UI_but_drag_set_image(uiBut *but, const char *path, int icon, struct ImBuf *imb, float scale);
/* Panels
*

22
source/blender/editors/interface/interface_drag.cc
View File

@@ -64,17 +64,14 @@ void UI_but_drag_set_rna(uiBut *but, PointerRNA *ptr)
but->dragpoin = (void *)ptr;
}
void UI_but_drag_set_path(uiBut *but, const char *path, const bool use_free)
void UI_but_drag_set_path(uiBut *but, const char *path)
{
but->dragtype = WM_DRAG_PATH;
if (but->dragflag & UI_BUT_DRAGPOIN_FREE) {
WM_drag_data_free(but->dragtype, but->dragpoin);
but->dragflag &= ~UI_BUT_DRAGPOIN_FREE;
}
but->dragpoin = (void *)path;
if (use_free) {
but->dragflag |= UI_BUT_DRAGPOIN_FREE;
}
but->dragpoin = WM_drag_create_path_data(path);
but->dragflag |= UI_BUT_DRAGPOIN_FREE;
}
void UI_but_drag_set_name(uiBut *but, const char *name)
@@ -92,19 +89,10 @@ void UI_but_drag_set_value(uiBut *but)
but->dragtype = WM_DRAG_VALUE;
}
void UI_but_drag_set_image(
uiBut *but, const char *path, int icon, struct ImBuf *imb, float scale, const bool use_free)
void UI_but_drag_set_image(uiBut *but, const char *path, int icon, struct ImBuf *imb, float scale)
{
but->dragtype = WM_DRAG_PATH;
ui_def_but_icon(but, icon, 0); /* no flag UI_HAS_ICON, so icon doesn't draw in button */
if (but->dragflag & UI_BUT_DRAGPOIN_FREE) {
WM_drag_data_free(but->dragtype, but->dragpoin);
but->dragflag &= ~UI_BUT_DRAGPOIN_FREE;
}
but->dragpoin = (void *)path;
if (use_free) {
but->dragflag |= UI_BUT_DRAGPOIN_FREE;
}
UI_but_drag_set_path(but, path);
UI_but_drag_attach_image(but, imb, scale);
}

5
source/blender/editors/screen/screen_ops.c
View File

@@ -5749,7 +5749,8 @@ static void keymap_modal_set(wmKeyConfig *keyconf)
static bool blend_file_drop_poll(bContext *UNUSED(C), wmDrag *drag, const wmEvent *UNUSED(event))
{
if (drag->type == WM_DRAG_PATH) {
if (ELEM(drag->icon, ICON_FILE_BLEND, ICON_FILE_BACKUP, ICON_BLENDER)) {
const eFileSel_File_Types file_type = WM_drag_get_path_file_type(drag);
if (ELEM(file_type, FILE_TYPE_BLENDER, FILE_TYPE_BLENDER_BACKUP)) {
return true;
}
}
@@ -5759,7 +5760,7 @@ static bool blend_file_drop_poll(bContext *UNUSED(C), wmDrag *drag, const wmEven
static void blend_file_drop_copy(bContext *UNUSED(C), wmDrag *drag, wmDropBox *drop)
{
/* copy drag path to properties */
RNA_string_set(drop->ptr, "filepath", drag->path);
RNA_string_set(drop->ptr, "filepath", WM_drag_get_path(drag));
}
void ED_keymap_screen(wmKeyConfig *keyconf)

6
source/blender/editors/space_clip/space_clip.c
View File

@@ -590,8 +590,8 @@ static int /*eContextResult*/ clip_context(const bContext *C,
static bool clip_drop_poll(bContext *UNUSED(C), wmDrag *drag, const wmEvent *UNUSED(event))
{
if (drag->type == WM_DRAG_PATH) {
/* rule might not work? */
if (ELEM(drag->icon, 0, ICON_FILE_IMAGE, ICON_FILE_MOVIE, ICON_FILE_BLANK)) {
const eFileSel_File_Types file_type = WM_drag_get_path_file_type(drag);
if (ELEM(file_type, 0, FILE_TYPE_IMAGE, FILE_TYPE_MOVIE)) {
return true;
}
}
@@ -604,7 +604,7 @@ static void clip_drop_copy(bContext *UNUSED(C), wmDrag *drag, wmDropBox *drop)
PointerRNA itemptr;
char dir[FILE_MAX], file[FILE_MAX];
BLI_split_dirfile(drag->path, dir, file, sizeof(dir), sizeof(file));
BLI_split_dirfile(WM_drag_get_path(drag), dir, file, sizeof(dir), sizeof(file));
RNA_string_set(drop->ptr, "directory", dir);

2
source/blender/editors/space_console/space_console.c
View File

@@ -174,7 +174,7 @@ static bool path_drop_poll(bContext *UNUSED(C), wmDrag *drag, const wmEvent *UNU
static void path_drop_copy(bContext *UNUSED(C), wmDrag *drag, wmDropBox *drop)
{
char pathname[FILE_MAX + 2];
BLI_snprintf(pathname, sizeof(pathname), "\"%s\"", drag->path);
BLI_snprintf(pathname, sizeof(pathname), "\"%s\"", WM_drag_get_path(drag));
RNA_string_set(drop->ptr, "text", pathname);
}

4
source/blender/editors/space_file/file_draw.c
View File

@@ -161,11 +161,11 @@ static void file_but_enable_drag(uiBut *but,
}
}
else if (preview_image) {
UI_but_drag_set_image(but, BLI_strdup(path), icon, preview_image, scale, true);
UI_but_drag_set_image(but, path, icon, preview_image, scale);
}
else {
/* path is no more static, cannot give it directly to but... */
UI_but_drag_set_path(but, BLI_strdup(path), true);
UI_but_drag_set_path(but, path);
}
}

3
source/blender/editors/space_file/filelist.cc
View File

@@ -2620,6 +2620,9 @@ static bool file_is_blend_backup(const char *str)
int ED_path_extension_type(const char *path)
{
/* ATTENTION: Never return OR'ed bit-flags here, always return a single enum value! Some code
* using this may do `ELEM()`-like checks. */
if (BLO_has_bfile_extension(path)) {
return FILE_TYPE_BLENDER;
}

2
source/blender/editors/space_file/space_file.c
View File

@@ -849,7 +849,7 @@ static bool filepath_drop_poll(bContext *C, wmDrag *drag, const wmEvent *UNUSED(
static void filepath_drop_copy(bContext *UNUSED(C), wmDrag *drag, wmDropBox *drop)
{
RNA_string_set(drop->ptr, "filepath", drag->path);
RNA_string_set(drop->ptr, "filepath", WM_drag_get_path(drag));
}
/* region dropbox definition */

6
source/blender/editors/space_image/space_image.c
View File

@@ -255,8 +255,8 @@ static bool image_drop_poll(bContext *C, wmDrag *drag, const wmEvent *event)
return false;
}
if (drag->type == WM_DRAG_PATH) {
/* rule might not work? */
if (ELEM(drag->icon, 0, ICON_FILE_IMAGE, ICON_FILE_MOVIE, ICON_FILE_BLANK)) {
const eFileSel_File_Types file_type = WM_drag_get_path_file_type(drag);
if (ELEM(file_type, 0, FILE_TYPE_IMAGE, FILE_TYPE_MOVIE)) {
return true;
}
}
@@ -266,7 +266,7 @@ static bool image_drop_poll(bContext *C, wmDrag *drag, const wmEvent *event)
static void image_drop_copy(bContext *UNUSED(C), wmDrag *drag, wmDropBox *drop)
{
/* copy drag path to properties */
RNA_string_set(drop->ptr, "filepath", drag->path);
RNA_string_set(drop->ptr, "filepath", WM_drag_get_path(drag));
}
/* area+region dropbox definition */

13
source/blender/editors/space_node/space_node.cc
View File

@@ -670,8 +670,9 @@ static bool node_collection_drop_poll(bContext * /*C*/, wmDrag *drag, const wmEv
static bool node_ima_drop_poll(bContext * /*C*/, wmDrag *drag, const wmEvent * /*event*/)
{
if (drag->type == WM_DRAG_PATH) {
/* rule might not work? */
return ELEM(drag->icon, 0, ICON_FILE_IMAGE, ICON_FILE_MOVIE);
const eFileSel_File_Types file_type = static_cast<eFileSel_File_Types>(
WM_drag_get_path_file_type(drag));
return ELEM(file_type, 0, FILE_TYPE_IMAGE, FILE_TYPE_MOVIE);
}
return WM_drag_is_ID_type(drag, ID_IM);
}
@@ -702,10 +703,14 @@ static void node_id_path_drop_copy(bContext * /*C*/, wmDrag *drag, wmDropBox *dr
if (id) {
RNA_int_set(drop->ptr, "session_uuid", int(id->session_uuid));
RNA_struct_property_unset(drop->ptr, "filepath");
return;
}
else if (drag->path[0]) {
RNA_string_set(drop->ptr, "filepath", drag->path);
const char *path = WM_drag_get_path(drag);
if (path) {
RNA_string_set(drop->ptr, "filepath", path);
RNA_struct_property_unset(drop->ptr, "name");
return;
}
}

30
source/blender/editors/space_sequencer/sequencer_drag_drop.c
View File

@@ -78,7 +78,8 @@ static void generic_poll_operations(const wmEvent *event, uint8_t type)
static bool image_drop_poll(bContext *UNUSED(C), wmDrag *drag, const wmEvent *event)
{
if (drag->type == WM_DRAG_PATH) {
if (ELEM(drag->icon, ICON_FILE_IMAGE, ICON_FILE_BLANK)) { /* Rule might not work? */
const eFileSel_File_Types file_type = WM_drag_get_path_file_type(drag);
if (ELEM(file_type, 0, FILE_TYPE_IMAGE)) {
generic_poll_operations(event, TH_SEQ_IMAGE);
return true;
}
@@ -95,7 +96,8 @@ static bool image_drop_poll(bContext *UNUSED(C), wmDrag *drag, const wmEvent *ev
static bool is_movie(wmDrag *drag)
{
if (drag->type == WM_DRAG_PATH) {
if (ELEM(drag->icon, ICON_FILE_MOVIE, ICON_FILE_BLANK)) { /* Rule might not work? */
const eFileSel_File_Types file_type = WM_drag_get_path_file_type(drag);
if (ELEM(file_type, 0, FILE_TYPE_MOVIE)) {
return true;
}
}
@@ -118,7 +120,8 @@ static bool movie_drop_poll(bContext *UNUSED(C), wmDrag *drag, const wmEvent *ev
static bool is_sound(wmDrag *drag)
{
if (drag->type == WM_DRAG_PATH) {
if (ELEM(drag->icon, ICON_FILE_SOUND, ICON_FILE_BLANK)) { /* Rule might not work? */
const eFileSel_File_Types file_type = WM_drag_get_path_file_type(drag);
if (ELEM(file_type, 0, FILE_TYPE_SOUND)) {
return true;
}
}
@@ -240,17 +243,21 @@ static void sequencer_drop_copy(bContext *C, wmDrag *drag, wmDropBox *drop)
RNA_string_set(drop->ptr, "filepath", sound->filepath);
RNA_struct_property_unset(drop->ptr, "name");
}
return;
}
const char *path = WM_drag_get_path(drag);
/* Path dropped. */
else if (drag->path[0]) {
if (path) {
if (RNA_struct_find_property(drop->ptr, "filepath")) {
RNA_string_set(drop->ptr, "filepath", drag->path);
RNA_string_set(drop->ptr, "filepath", path);
}
if (RNA_struct_find_property(drop->ptr, "directory")) {
PointerRNA itemptr;
char dir[FILE_MAX], file[FILE_MAX];
BLI_split_dirfile(drag->path, dir, file, sizeof(dir), sizeof(file));
BLI_split_dirfile(path, dir, file, sizeof(dir), sizeof(file));
RNA_string_set(drop->ptr, "directory", dir);
@@ -328,7 +335,7 @@ static void get_drag_path(wmDrag *drag, char r_path[FILE_MAX])
BLI_path_abs(r_path, BKE_main_blendfile_path_from_global());
}
else {
BLI_strncpy(r_path, drag->path, FILE_MAX);
BLI_strncpy(r_path, WM_drag_get_path(drag), FILE_MAX);
}
}
@@ -682,7 +689,8 @@ static bool image_drop_preview_poll(bContext *UNUSED(C),
const wmEvent *UNUSED(event))
{
if (drag->type == WM_DRAG_PATH) {
if (ELEM(drag->icon, ICON_FILE_IMAGE, ICON_FILE_BLANK)) { /* Rule might not work? */
const eFileSel_File_Types file_type = WM_drag_get_path_file_type(drag);
if (ELEM(file_type, 0, FILE_TYPE_IMAGE)) {
return true;
}
}
@@ -695,7 +703,8 @@ static bool movie_drop_preview_poll(bContext *UNUSED(C),
const wmEvent *UNUSED(event))
{
if (drag->type == WM_DRAG_PATH) {
if (ELEM(drag->icon, 0, ICON_FILE_MOVIE, ICON_FILE_BLANK)) { /* Rule might not work? */
const eFileSel_File_Types file_type = WM_drag_get_path_file_type(drag);
if (ELEM(file_type, 0, FILE_TYPE_MOVIE)) {
return true;
}
}
@@ -708,7 +717,8 @@ static bool sound_drop_preview_poll(bContext *UNUSED(C),
const wmEvent *UNUSED(event))
{
if (drag->type == WM_DRAG_PATH) {
if (ELEM(drag->icon, ICON_FILE_SOUND, ICON_FILE_BLANK)) { /* Rule might not work? */
const eFileSel_File_Types file_type = WM_drag_get_path_file_type(drag);
if (ELEM(file_type, 0, FILE_TYPE_SOUND)) {
return true;
}
}

6
source/blender/editors/space_text/space_text.c
View File

@@ -303,8 +303,8 @@ static void text_cursor(wmWindow *win, ScrArea *area, ARegion *region)
static bool text_drop_poll(bContext *UNUSED(C), wmDrag *drag, const wmEvent *UNUSED(event))
{
if (drag->type == WM_DRAG_PATH) {
/* rule might not work? */
if (ELEM(drag->icon, ICON_FILE_SCRIPT, ICON_FILE_TEXT, ICON_FILE_BLANK)) {
const eFileSel_File_Types file_type = WM_drag_get_path_file_type(drag);
if (ELEM(file_type, 0, FILE_TYPE_PYSCRIPT, FILE_TYPE_TEXT)) {
return true;
}
}
@@ -314,7 +314,7 @@ static bool text_drop_poll(bContext *UNUSED(C), wmDrag *drag, const wmEvent *UNU
static void text_drop_copy(bContext *UNUSED(C), wmDrag *drag, wmDropBox *drop)
{
/* copy drag path to properties */
RNA_string_set(drop->ptr, "filepath", drag->path);
RNA_string_set(drop->ptr, "filepath", WM_drag_get_path(drag));
}
static bool text_drop_paste_poll(bContext *UNUSED(C), wmDrag *drag, const wmEvent *UNUSED(event))

13
source/blender/editors/space_view3d/space_view3d.cc
View File

@@ -647,8 +647,8 @@ static bool view3d_ima_drop_poll(bContext *C, wmDrag *drag, const wmEvent *event
return false;
}
if (drag->type == WM_DRAG_PATH) {
/* rule might not work? */
return ELEM(drag->icon, 0, ICON_FILE_IMAGE, ICON_FILE_MOVIE);
const eFileSel_File_Types file_type = eFileSel_File_Types(WM_drag_get_path_file_type(drag));
return (ELEM(file_type, 0, FILE_TYPE_IMAGE, FILE_TYPE_MOVIE));
}
return WM_drag_is_ID_type(drag, ID_IM);
@@ -700,7 +700,8 @@ static bool view3d_ima_empty_drop_poll(bContext *C, wmDrag *drag, const wmEvent
static bool view3d_volume_drop_poll(bContext * /*C*/, wmDrag *drag, const wmEvent * /*event*/)
{
return (drag->type == WM_DRAG_PATH) && (drag->icon == ICON_FILE_VOLUME);
const eFileSel_File_Types file_type = eFileSel_File_Types(WM_drag_get_path_file_type(drag));
return (drag->type == WM_DRAG_PATH) && (file_type == FILE_TYPE_VOLUME);
}
static bool view3d_geometry_nodes_drop_poll(bContext *C, wmDrag *drag, const wmEvent *event)
@@ -902,9 +903,11 @@ static void view3d_id_path_drop_copy(bContext * /*C*/, wmDrag *drag, wmDropBox *
if (id) {
WM_operator_properties_id_lookup_set_from_id(drop->ptr, id);
RNA_struct_property_unset(drop->ptr, "filepath");
return;
}
else if (drag->path[0]) {
RNA_string_set(drop->ptr, "filepath", drag->path);
const char *path = WM_drag_get_path(drag);
if (path) {
RNA_string_set(drop->ptr, "filepath", path);
RNA_struct_property_unset(drop->ptr, "image");
}
}

13
source/blender/windowmanager/WM_api.h
View File

@@ -1379,6 +1379,19 @@ const ListBase *WM_drag_asset_list_get(const wmDrag *drag);
const char *WM_drag_get_item_name(struct wmDrag *drag);
/* Path drag and drop. */
/**
* \param path: The path to drag. Value will be copied into the drag data so the passed string may
* be destructed.
*/
wmDragPath *WM_drag_create_path_data(const char *path);
const char *WM_drag_get_path(const wmDrag *drag);
/**
* Note that even though the enum return type uses bit-flags, this should never have multiple
* type-bits set, so `ELEM()` like comparison is possible.
*/
int /* eFileSel_File_Types */ WM_drag_get_path_file_type(const wmDrag *drag);
/* Set OpenGL viewport and scissor */
void wmViewport(const struct rcti *winrct);
void wmPartialViewport(rcti *drawrct, const rcti *winrct, const rcti *partialrct);

8
source/blender/windowmanager/WM_types.h
View File

@@ -1107,6 +1107,13 @@ typedef struct wmDragAssetListItem {
bool is_external;
} wmDragAssetListItem;
typedef struct wmDragPath {
char *path;
/* Note that even though the enum type uses bit-flags, this should never have multiple type-bits
* set, so `ELEM()` like comparison is possible. */
int file_type; /* eFileSel_File_Types */
} wmDragPath;
typedef char *(*WMDropboxTooltipFunc)(struct bContext *,
struct wmDrag *,
const int xy[2],
@@ -1144,7 +1151,6 @@ typedef struct wmDrag {
/** See 'WM_DRAG_' defines above. */
int type;
void *poin;
char path[1024]; /* FILE_MAX */
double value;
/** If no icon but imbuf should be drawn around cursor. */

71
source/blender/windowmanager/intern/wm_dragdrop.cc
View File

@@ -37,6 +37,7 @@
#include "BLO_readfile.h"
#include "ED_asset.h"
#include "ED_fileselect.h"
#include "ED_screen.h"
#include "GPU_shader.h"
@@ -61,6 +62,7 @@
static ListBase dropboxes = {nullptr, nullptr};
static void wm_drag_free_asset_data(wmDragAsset **asset_data);
static void wm_drag_free_path_data(wmDragPath **path_data);
/* drop box maps are stored global for now */
/* these are part of blender's UI/space specs, and not like keymaps */
@@ -187,11 +189,8 @@ wmDrag *WM_drag_data_create(bContext *C, int icon, int type, void *poin, double
drag->type = type;
switch (type) {
case WM_DRAG_PATH:
BLI_strncpy(drag->path, static_cast<const char *>(poin), FILE_MAX);
/* As the path is being copied, free it immediately as `drag` won't "own" the data. */
if (flags & WM_DRAG_FREE_DATA) {
MEM_freeN(poin);
}
drag->poin = poin;
drag->flags |= WM_DRAG_FREE_DATA;
break;
case WM_DRAG_ID:
if (poin) {
@@ -297,12 +296,20 @@ void WM_drag_data_free(int dragtype, void *poin)
}
/* Not too nice, could become a callback. */
if (dragtype == WM_DRAG_ASSET) {
wmDragAsset *asset_data = static_cast<wmDragAsset *>(poin);
wm_drag_free_asset_data(&asset_data);
}
else {
MEM_freeN(poin);
switch (dragtype) {
case WM_DRAG_ASSET: {
wmDragAsset *asset_data = static_cast<wmDragAsset *>(poin);
wm_drag_free_asset_data(&asset_data);
break;
}
case WM_DRAG_PATH: {
wmDragPath *path_data = static_cast<wmDragPath *>(poin);
wm_drag_free_path_data(&path_data);
break;
}
default:
MEM_freeN(poin);
break;
}
}
@@ -743,6 +750,41 @@ const ListBase *WM_drag_asset_list_get(const wmDrag *drag)
return &drag->asset_items;
}
wmDragPath *WM_drag_create_path_data(const char *path)
{
wmDragPath *path_data = MEM_new<wmDragPath>("wmDragPath");
path_data->path = BLI_strdup(path);
path_data->file_type = ED_path_extension_type(path);
return path_data;
}
static void wm_drag_free_path_data(wmDragPath **path_data)
{
MEM_freeN((*path_data)->path);
MEM_delete(*path_data);
*path_data = nullptr;
}
const char *WM_drag_get_path(const wmDrag *drag)
{
if (drag->type != WM_DRAG_PATH) {
return NULL;
}
const wmDragPath *path_data = static_cast<const wmDragPath *>(drag->poin);
return path_data->path;
}
int WM_drag_get_path_file_type(const wmDrag *drag)
{
if (drag->type != WM_DRAG_PATH) {
return 0;
}
const wmDragPath *path_data = static_cast<const wmDragPath *>(drag->poin);
return path_data->file_type;
}
/* ************** draw ***************** */
static void wm_drop_operator_draw(const char *name, int x, int y)
@@ -792,9 +834,12 @@ const char *WM_drag_get_item_name(wmDrag *drag)
const wmDragAsset *asset_drag = WM_drag_get_asset_data(drag, 0);
return asset_drag->name;
}
case WM_DRAG_PATH:
case WM_DRAG_PATH: {
const wmDragPath *path_drag_data = static_cast<const wmDragPath *>(drag->poin);
return path_drag_data->path;
}
case WM_DRAG_NAME:
return drag->path;
return static_cast<const char *>(drag->poin);
}
return "";
}

4
source/blender/windowmanager/intern/wm_window.c
View File

@@ -1445,8 +1445,8 @@ static bool ghost_event_proc(GHOST_EventHandle evt, GHOST_TUserDataPtr C_void_pt
printf("drop file %s\n", stra->strings[a]);
/* try to get icon type from extension */
int icon = ED_file_extension_icon((char *)stra->strings[a]);
WM_event_start_drag(C, icon, WM_DRAG_PATH, stra->strings[a], 0.0, WM_DRAG_NOP);
wmDragPath *path_data = WM_drag_create_path_data((char *)stra->strings[a]);
WM_event_start_drag(C, icon, WM_DRAG_PATH, path_data, 0.0, WM_DRAG_NOP);
/* void poin should point to string, it makes a copy */
break; /* only one drop element supported now */
}