Commit e061a92dc954443cabd33613f7787498ca860d77

Ryan C. Gordon 2018-08-02T16:03:47

Some drag'and'drop improvements. First: disable d'n'd events by default; most apps don't need these at all, and if an app doesn't explicitly handle these, each drop on the window will cause a memory leak if the events are enabled. This follows the guidelines we have for SDL_TEXTINPUT events already. Second: when events are enabled or disabled, signal the video layer, as it might be able to inform the OS, causing UI changes or optimizations (for example, dropping a file icon on a Cocoa app that isn't accepting drops will cause macOS to show a rejection animation instead of the drop operation just vanishing into the ether, X11 might show a different cursor when dragging onto an accepting window, etc). Third: fill in the drop event details in the test library and enable the events in testwm.c for making sure this all works as expected.

diff --git a/src/events/SDL_events.c b/src/events/SDL_events.c
index f2e5b62..c0b9e31 100644
--- a/src/events/SDL_events.c
+++ b/src/events/SDL_events.c
@@ -417,6 +417,8 @@ SDL_StartEventLoop(void)
     SDL_EventState(SDL_TEXTINPUT, SDL_DISABLE);
     SDL_EventState(SDL_TEXTEDITING, SDL_DISABLE);
     SDL_EventState(SDL_SYSWMEVENT, SDL_DISABLE);
+    SDL_EventState(SDL_DROPFILE, SDL_DISABLE);
+    SDL_EventState(SDL_DROPTEXT, SDL_DISABLE);
 
     SDL_AtomicSet(&SDL_EventQ.active, 1);
 
@@ -604,6 +606,10 @@ SDL_FlushEvent(Uint32 type)
 void
 SDL_FlushEvents(Uint32 minType, Uint32 maxType)
 {
+    /* !!! FIXME: we need to manually SDL_free() the strings in TEXTINPUT and
+       drag'n'drop events if we're flushing them without passing them to the
+       app, but I don't know if this is the right place to do that. */
+
     /* Don't look after we've quit */
     if (!SDL_AtomicGet(&SDL_EventQ.active)) {
         return;
@@ -863,6 +869,8 @@ SDL_FilterEvents(SDL_EventFilter filter, void *userdata)
 Uint8
 SDL_EventState(Uint32 type, int state)
 {
+    const SDL_bool isdnd = ((state == SDL_DISABLE) || (state == SDL_ENABLE)) &&
+                           ((type == SDL_DROPFILE) || (type == SDL_DROPTEXT));
     Uint8 current_state;
     Uint8 hi = ((type >> 8) & 0xff);
     Uint8 lo = (type & 0xff);
@@ -898,6 +906,12 @@ SDL_EventState(Uint32 type, int state)
         }
     }
 
+    /* turn off drag'n'drop support if we've disabled the events.
+       This might change some UI details at the OS level. */
+    if (isdnd) {
+        SDL_ToggleDragAndDropSupport();
+    }
+
     return current_state;
 }
 
diff --git a/src/test/SDL_test_common.c b/src/test/SDL_test_common.c
index b7e294a..a51274f 100644
--- a/src/test/SDL_test_common.c
+++ b/src/test/SDL_test_common.c
@@ -1349,7 +1349,18 @@ SDLTest_PrintEvent(SDL_Event * event)
     case SDL_APP_DIDENTERFOREGROUND:
         SDL_Log("SDL EVENT: App entered the foreground");
         break;
-
+    case SDL_DROPBEGIN:
+        SDL_Log("SDL EVENT: Drag and drop beginning");
+        break;
+    case SDL_DROPFILE:
+        SDL_Log("SDL EVENT: Drag and drop file: '%s'", event->drop.file);
+        break;
+    case SDL_DROPTEXT:
+        SDL_Log("SDL EVENT: Drag and drop text: '%s'", event->drop.file);
+        break;
+    case SDL_DROPCOMPLETE:
+        SDL_Log("SDL EVENT: Drag and drop ending");
+        break;
     case SDL_QUIT:
         SDL_Log("SDL EVENT: Quit requested");
         break;
@@ -1744,6 +1755,11 @@ SDLTest_CommonEvent(SDLTest_CommonState * state, SDL_Event * event, int *done)
     case SDL_MOUSEMOTION:
         lastEvent = event->motion;
         break;
+
+    case SDL_DROPFILE:
+    case SDL_DROPTEXT:
+        SDL_free(event->drop.file);
+        break;
     }
 }
 
diff --git a/src/video/SDL_sysvideo.h b/src/video/SDL_sysvideo.h
index 9df71c9..19e1682 100644
--- a/src/video/SDL_sysvideo.h
+++ b/src/video/SDL_sysvideo.h
@@ -304,6 +304,9 @@ struct SDL_VideoDevice
     /* Hit-testing */
     int (*SetWindowHitTest)(SDL_Window * window, SDL_bool enabled);
 
+    /* Tell window that app enabled drag'n'drop events */
+    void (*AcceptDragAndDrop)(SDL_Window * window, SDL_bool accept);
+
     /* * * */
     /* Data common to all drivers */
     SDL_bool is_dummy;
@@ -454,6 +457,8 @@ extern void SDL_OnApplicationDidEnterBackground(void);
 extern void SDL_OnApplicationWillEnterForeground(void);
 extern void SDL_OnApplicationDidBecomeActive(void);
 
+extern void SDL_ToggleDragAndDropSupport(void);
+
 #endif /* SDL_sysvideo_h_ */
 
 /* vi: set ts=4 sw=4 expandtab: */
diff --git a/src/video/SDL_video.c b/src/video/SDL_video.c
index 761b539..0f44a9c 100644
--- a/src/video/SDL_video.c
+++ b/src/video/SDL_video.c
@@ -1331,9 +1331,43 @@ SDL_UpdateFullscreenMode(SDL_Window * window, SDL_bool fullscreen)
 #define CREATE_FLAGS \
     (SDL_WINDOW_OPENGL | SDL_WINDOW_BORDERLESS | SDL_WINDOW_RESIZABLE | SDL_WINDOW_ALLOW_HIGHDPI | SDL_WINDOW_ALWAYS_ON_TOP | SDL_WINDOW_SKIP_TASKBAR | SDL_WINDOW_POPUP_MENU | SDL_WINDOW_UTILITY | SDL_WINDOW_TOOLTIP | SDL_WINDOW_VULKAN | SDL_WINDOW_MINIMIZED)
 
+static SDL_INLINE SDL_bool
+IsAcceptingDragAndDrop(void)
+{
+    if ((SDL_GetEventState(SDL_DROPFILE) == SDL_ENABLE) ||
+        (SDL_GetEventState(SDL_DROPTEXT) == SDL_ENABLE)) {
+        return SDL_TRUE;
+    }
+    return SDL_FALSE;
+}
+
+/* prepare a newly-created window */
+static SDL_INLINE void
+PrepareDragAndDropSupport(SDL_Window *window)
+{
+    if (_this->AcceptDragAndDrop) {
+        _this->AcceptDragAndDrop(window, IsAcceptingDragAndDrop());
+    }
+}
+
+/* toggle d'n'd for all existing windows. */
+void
+SDL_ToggleDragAndDropSupport(void)
+{
+    if (_this && _this->AcceptDragAndDrop) {
+        const SDL_bool enable = IsAcceptingDragAndDrop();
+        SDL_Window *window;
+        for (window = _this->windows; window; window = window->next) {
+            _this->AcceptDragAndDrop(window, enable);
+        }
+    }
+}
+
 static void
 SDL_FinishWindowCreation(SDL_Window *window, Uint32 flags)
 {
+    PrepareDragAndDropSupport(window);
+
     if (flags & SDL_WINDOW_MAXIMIZED) {
         SDL_MaximizeWindow(window);
     }
@@ -1552,6 +1586,9 @@ SDL_CreateWindowFrom(const void *data)
         SDL_DestroyWindow(window);
         return NULL;
     }
+
+    PrepareDragAndDropSupport(window);
+
     return window;
 }
 
diff --git a/src/video/cocoa/SDL_cocoavideo.m b/src/video/cocoa/SDL_cocoavideo.m
index 545dc1e..9770224 100644
--- a/src/video/cocoa/SDL_cocoavideo.m
+++ b/src/video/cocoa/SDL_cocoavideo.m
@@ -105,6 +105,7 @@ Cocoa_CreateDevice(int devindex)
     device->DestroyWindow = Cocoa_DestroyWindow;
     device->GetWindowWMInfo = Cocoa_GetWindowWMInfo;
     device->SetWindowHitTest = Cocoa_SetWindowHitTest;
+    device->AcceptDragAndDrop = Cocoa_AcceptDragAndDrop;
 
     device->shape_driver.CreateShaper = Cocoa_CreateShaper;
     device->shape_driver.SetWindowShape = Cocoa_SetWindowShape;
diff --git a/src/video/cocoa/SDL_cocoawindow.h b/src/video/cocoa/SDL_cocoawindow.h
index df6f173..2311e3d 100644
--- a/src/video/cocoa/SDL_cocoawindow.h
+++ b/src/video/cocoa/SDL_cocoawindow.h
@@ -148,6 +148,7 @@ extern void Cocoa_SetWindowGrab(_THIS, SDL_Window * window, SDL_bool grabbed);
 extern void Cocoa_DestroyWindow(_THIS, SDL_Window * window);
 extern SDL_bool Cocoa_GetWindowWMInfo(_THIS, SDL_Window * window, struct SDL_SysWMinfo *info);
 extern int Cocoa_SetWindowHitTest(SDL_Window *window, SDL_bool enabled);
+extern void Cocoa_AcceptDragAndDrop(SDL_Window * window, SDL_bool accept);
 
 #endif /* SDL_cocoawindow_h_ */
 
diff --git a/src/video/cocoa/SDL_cocoawindow.m b/src/video/cocoa/SDL_cocoawindow.m
index b1a5b46..0cf7ec3 100644
--- a/src/video/cocoa/SDL_cocoawindow.m
+++ b/src/video/cocoa/SDL_cocoawindow.m
@@ -1354,9 +1354,6 @@ Cocoa_CreateWindow(_THIS, SDL_Window * window)
     [nswindow setContentView:contentView];
     [contentView release];
 
-    /* Allow files and folders to be dragged onto the window by users */
-    [nswindow registerForDraggedTypes:[NSArray arrayWithObject:(NSString *)kUTTypeFileURL]];
-
     if (SetupWindowData(_this, window, nswindow, SDL_TRUE) < 0) {
         [nswindow release];
         return -1;
@@ -1864,6 +1861,17 @@ Cocoa_SetWindowHitTest(SDL_Window * window, SDL_bool enabled)
     return 0;  /* just succeed, the real work is done elsewhere. */
 }
 
+void
+Cocoa_AcceptDragAndDrop(SDL_Window * window, SDL_bool accept)
+{
+    SDL_WindowData *data = (SDL_WindowData *) window->driverdata;
+    if (accept) {
+        [data->nswindow registerForDraggedTypes:[NSArray arrayWithObject:(NSString *)kUTTypeFileURL]];
+    } else {
+        [data->nswindow unregisterDraggedTypes];
+    }
+}
+
 int
 Cocoa_SetWindowOpacity(_THIS, SDL_Window * window, float opacity)
 {
diff --git a/src/video/windows/SDL_windowsvideo.c b/src/video/windows/SDL_windowsvideo.c
index 8d45b72..300bd2f 100644
--- a/src/video/windows/SDL_windowsvideo.c
+++ b/src/video/windows/SDL_windowsvideo.c
@@ -164,6 +164,7 @@ WIN_CreateDevice(int devindex)
     device->DestroyWindowFramebuffer = WIN_DestroyWindowFramebuffer;
     device->OnWindowEnter = WIN_OnWindowEnter;
     device->SetWindowHitTest = WIN_SetWindowHitTest;
+    device->AcceptDragAndDrop = WIN_AcceptDragAndDrop;
 
     device->shape_driver.CreateShaper = Win32_CreateShaper;
     device->shape_driver.SetWindowShape = Win32_SetWindowShape;
diff --git a/src/video/windows/SDL_windowswindow.c b/src/video/windows/SDL_windowswindow.c
index be4d54e..753b736 100644
--- a/src/video/windows/SDL_windowswindow.c
+++ b/src/video/windows/SDL_windowswindow.c
@@ -289,9 +289,6 @@ SetupWindowData(_THIS, SDL_Window * window, HWND hwnd, HWND parent, SDL_bool cre
         videodata->RegisterTouchWindow(hwnd, (TWF_FINETOUCH|TWF_WANTPALM));
     }
 
-    /* Enable dropping files */
-    DragAcceptFiles(hwnd, TRUE);
-
     data->initializing = SDL_FALSE;
 
     /* All done! */
@@ -979,6 +976,13 @@ WIN_SetWindowOpacity(_THIS, SDL_Window * window, float opacity)
     return 0;
 }
 
+void
+WIN_AcceptDragAndDrop(SDL_Window * window, SDL_bool accept)
+{
+    const SDL_WindowData *data = (SDL_WindowData *) window->driverdata;
+    DragAcceptFiles(data->hwnd, accept ? TRUE : FALSE);
+}
+
 #endif /* SDL_VIDEO_DRIVER_WINDOWS */
 
 /* vi: set ts=4 sw=4 expandtab: */
diff --git a/src/video/windows/SDL_windowswindow.h b/src/video/windows/SDL_windowswindow.h
index 0325abb..0ed8759 100644
--- a/src/video/windows/SDL_windowswindow.h
+++ b/src/video/windows/SDL_windowswindow.h
@@ -78,6 +78,7 @@ extern SDL_bool WIN_GetWindowWMInfo(_THIS, SDL_Window * window,
 extern void WIN_OnWindowEnter(_THIS, SDL_Window * window);
 extern void WIN_UpdateClipCursor(SDL_Window *window);
 extern int WIN_SetWindowHitTest(SDL_Window *window, SDL_bool enabled);
+extern void WIN_AcceptDragAndDrop(SDL_Window * window, SDL_bool accept);
 
 #endif /* SDL_windowswindow_h_ */
 
diff --git a/src/video/x11/SDL_x11video.c b/src/video/x11/SDL_x11video.c
index b8f8edf..b3b1a70 100644
--- a/src/video/x11/SDL_x11video.c
+++ b/src/video/x11/SDL_x11video.c
@@ -260,6 +260,7 @@ X11_CreateDevice(int devindex)
     device->DestroyWindowFramebuffer = X11_DestroyWindowFramebuffer;
     device->GetWindowWMInfo = X11_GetWindowWMInfo;
     device->SetWindowHitTest = X11_SetWindowHitTest;
+    device->AcceptDragAndDrop = X11_AcceptDragAndDrop;
 
     device->shape_driver.CreateShaper = X11_CreateShaper;
     device->shape_driver.SetWindowShape = X11_SetWindowShape;
diff --git a/src/video/x11/SDL_x11window.c b/src/video/x11/SDL_x11window.c
index be03aa6..0a254b0 100644
--- a/src/video/x11/SDL_x11window.c
+++ b/src/video/x11/SDL_x11window.c
@@ -390,7 +390,6 @@ X11_CreateWindow(_THIS, SDL_Window * window)
     const char *wintype_name = NULL;
     long compositor = 1;
     Atom _NET_WM_PID;
-    Atom XdndAware, xdnd_version = 5;
     long fevent = 0;
 
 #if SDL_VIDEO_OPENGL_GLX || SDL_VIDEO_OPENGL_EGL
@@ -651,11 +650,6 @@ X11_CreateWindow(_THIS, SDL_Window * window)
                  PropertyChangeMask | StructureNotifyMask |
                  KeymapStateMask | fevent));
 
-    XdndAware = X11_XInternAtom(display, "XdndAware", False);
-    X11_XChangeProperty(display, w, XdndAware, XA_ATOM, 32,
-                 PropModeReplace,
-                 (unsigned char*)&xdnd_version, 1);
-
     X11_XFlush(display);
 
     return 0;
@@ -1604,6 +1598,22 @@ X11_SetWindowHitTest(SDL_Window *window, SDL_bool enabled)
     return 0;  /* just succeed, the real work is done elsewhere. */
 }
 
+void
+X11_AcceptDragAndDrop(SDL_Window * window, SDL_bool accept)
+{
+    SDL_WindowData *data = (SDL_WindowData *) window->driverdata;
+    Display *display = data->videodata->display;
+    Atom XdndAware = X11_XInternAtom(display, "XdndAware", False);
+
+    if (accept) {
+        Atom xdnd_version = 5;
+        X11_XChangeProperty(display, data->xwindow, XdndAware, XA_ATOM, 32,
+                     PropModeReplace, (unsigned char*)&xdnd_version, 1);
+    } else {
+        X11_XDeleteProperty(display, data->xwindow, XdndAware);
+    }
+}
+
 #endif /* SDL_VIDEO_DRIVER_X11 */
 
 /* vi: set ts=4 sw=4 expandtab: */
diff --git a/src/video/x11/SDL_x11window.h b/src/video/x11/SDL_x11window.h
index 7c4c6c5..6ee8016 100644
--- a/src/video/x11/SDL_x11window.h
+++ b/src/video/x11/SDL_x11window.h
@@ -104,6 +104,7 @@ extern void X11_DestroyWindow(_THIS, SDL_Window * window);
 extern SDL_bool X11_GetWindowWMInfo(_THIS, SDL_Window * window,
                                     struct SDL_SysWMinfo *info);
 extern int X11_SetWindowHitTest(SDL_Window *window, SDL_bool enabled);
+extern void X11_AcceptDragAndDrop(SDL_Window * window, SDL_bool accept);
 
 #endif /* SDL_x11window_h_ */
 
diff --git a/test/testwm2.c b/test/testwm2.c
index 74a0fe0..5da3873 100644
--- a/test/testwm2.c
+++ b/test/testwm2.c
@@ -146,6 +146,9 @@ main(int argc, char *argv[])
         quit(2);
     }
 
+    SDL_EventState(SDL_DROPFILE, SDL_ENABLE);
+    SDL_EventState(SDL_DROPTEXT, SDL_ENABLE);
+
     for (i = 0; i < state->num_windows; ++i) {
         SDL_Renderer *renderer = state->renderers[i];
         SDL_SetRenderDrawColor(renderer, 0xA0, 0xA0, 0xA0, 0xFF);