search

libsdl-org / SDL

Simple Directmedia Layer
https://libsdl.org
zlib License
9.17k stars 1.72k forks source link

Drag and drop improvements #8559

Open Guldoman opened 9 months ago

Guldoman commented 9 months ago

It would be useful for drag and drop support in SDL3 to get the same treatment of the clipboard support, by allowing specifying the MIME type to accept. Ideally SDL3 would also get support for initiating a drag operation, which would require specifying the supported MIME types, callbacks and drag "icon" as a surface.

For example an application would have to specify:

  1. a callback for "drag movement" operations, which would receive the drag MIME types, and would return whether or not the operation is acceptable;
  2. a callback for "drop" operations, which would receive the drag MIME types, and a callback (on the SDL side) that the application would use to get the content of a specific MIME type and its size.

When initiating a drag operation, the application would have to specify:

  1. supported MIME types;
  2. a callback the drop target will use for getting the actual data, which would receive the chosen MIME type and would return a buffer and its size.
  3. a callback for drag operation cancellation;
  4. a surface to show during the drag.

The use-case for Lite XL would be dragging tabs between different instances, specifying a custom MIME type during drag initiation. The target instance would then see the custom MIME type and accept the drop.

slouken commented 9 months ago

Feel free to submit a draft PR for discussion.

c-smile commented 7 months ago

Conceptually good drag-n-drop cannot be made by using posted events like these. Reason: feedback from drop target (the window) is expected by D&D logic.

Drop target functionality

Drop target is when this window wants to receive data being dragged, either from external process or from itself (D&D inside the app).

Seems like Drop target support needs the same callback mechanism as SDL_SetWindowHitTest :

SDL_SetWindowDropTarget(SDL_Window *window, SDL_DropTargetHandler callback, void *callback_data);

where 

typedef SDL_DropResult (SDLCALL *SDL_DropTargetHandler)(SDL_Window *win,
                                                 SDL_DragOperation operation, 
                                                 const SDL_Point *location,
                                                 const char **mime_types,
                                                 SDL_DropDataAccessor accessor,
                                                 void *callback_data);

enum SDL_DragOperation {
   SDL_DRAG_OVER,  // data is being dragged over the window
   SDL_DRAG_DROP, // data is dropped at the location
}

enum SDL_DropResult {
   SDL_DROP_NONE,         // neither OVER nor DROP is supported at this location and with these mimes
   SDL_DROP_ACCEPTED.  // on OVER - will accept drop here, on DROP - has done it.
}

// data accessor provided by SDL in SDL_DropTargetHandler call.
// this function can be called only inside SDL_DropTargetHandler invocation:

typedef void* (SDLCALL *SDL_DropDataAccessor)(SDL_Window *win,
                                                const char* mime_type, // mime of data to fetch
                                                size_t* data_length, // data length 
                                                void *callback_data); // opaque from SDL_SetWindowDropTarget
// returns pointer to data

Drag source functionality

Drag source - the application/window initiates Drag operation by itself. This functionality requires just one function

SDL_bool SDL_PerformDrag(
    const char** mime_types,   // data_accessor will be able to provide data of these types
    SDL_DropDataAccessor data_accessor, //
    bitmap *drag_icon,  
    SDL_Point  drag_icon_hot_point,   
    void* callback_data); // for data_accessor

// function returns  TRUE if data was accepted by receiver.
slouken commented 7 months ago

This seems reasonable to me. Do you want to draft up a PR that implements this for SDL3?

c-smile commented 7 months ago

Do you want to draft up a PR that implements this for SDL3?

I'll try. I have close implementation in my Sciter but it uses C++ (IDropTarget COM implementation). Is it OK if it will be in .cpp file (C-with-classes)?

slouken commented 7 months ago

The SDL Windows code needs to be all C, to avoid a C++ runtime dependency.

c-smile commented 7 months ago

For the start I can provide mostly C implementation but with IDropTarget implementation itself in C++. IDropTarget/C++ implementation can then be converted to pure C. Not that pretty but possible.

c-smile commented 4 months ago

Here is what I ended up with:

Whole SDL Clipboard (sic!) and Drag-n-drop API is replaced by four basic functions:

DnD

Installs Drag-n-drop handler on the window ( drop target functionality )

int SDL_SetWindowDropTarget(SDL_Window* window, SDL_DropTargetHandler callback, void* callback_data);

Performs modal drag session ( drag source functionality ): 

SDL_DropResult SDL_PerformDrag(SDL_Window* window,
   SDL_DragMode mode, // SDL_DRAG_COPY or SDL_DRAG_MOVE or both
   SDL_ExchangeDataAccessor* data_accessor, //
   SDL_DragFeedback* feedback);

Clipboard

Get/set clipboard content as a whole:

int SDL_GetClipboardDataEx(SDL_ExchangeDataAccessor** pdacc);
int SDL_SetClipboardDataEx(SDL_ExchangeDataAccessor* dacc);

where SDL_ExchangeDataAccessor is an interface that encapsulates access to corresponding OS entities (e.g. Windows/IDataObject and MacOS/NSPasteboard) and used as in DnD as in Clipboard:

struct SDL_ExchangeDataAccessor {
    Uint32 (SDLCALL* getTypes)(struct SDL_ExchangeDataAccessor* ctx);
    void*  (SDLCALL* getData)(struct SDL_ExchangeDataAccessor* ctx, SDL_ExchangeDataType type, size_t* pDataSize);
    void   (SDLCALL* setData)(struct SDL_ExchangeDataAccessor* ctx, SDL_ExchangeDataType type, void* data, size_t dataSize); /* can be NULL */
    void   (SDLCALL* releaseData)(struct SDL_ExchangeDataAccessor* ctx, SDL_ExchangeDataType type, void* data);
    void   (SDLCALL* releaseThis)(struct SDL_ExchangeDataAccessor* ctx);
};

I believe that these four functions cover most of "exchange" functionality used in modern GUI OSes (clipboard and dnd). And in generic/uniform way.

I have implementation of this for Windows, MacOS, X11 and Wayland and can publish this but not sure how as this affects existing API (SDL_video.h , SDL_clipboard.h plus new SDL_exchange.h). @slouken, please contact me if that is interesting/needed/etc.

For now it supports the following data types:

typedef enum SDL_ExchangeDataType {
    SDL_DATA_TYPE_URI_LIST,  /// SDL_GetExchangeData: utf-8 with '\r\n' URI separators 
    SDL_DATA_TYPE_HTML,      /// SDL_GetExchangeData: utf-8 HTML text (can be a fragment) 
    SDL_DATA_TYPE_JSON,      /// SDL_GetExchangeData: utf-8 JSON text
    SDL_DATA_TYPE_BITMAP,    /// SDL_GetExchangeData: bytes, serialized image (BMP,PNG,JPG,etc) 
    SDL_DATA_TYPE_TEXT,      /// SDL_GetExchangeData: utf-8 plain text
    // note this list may get new values, 32 in total
    SDL_NUM_DATA_TYPE, 
} SDL_ExchangeDataType;
Susko3 commented 4 months ago

I agree that using mime types is not that portable (heck it's not even implemented for window, android. ios), but it does provide a simple way to pass arbitrary data to applications that understand it.

SDL_DATA_TYPE_BITMAP, /// SDL_GetExchangeData: bytes, serialized image (BMP,PNG,JPG,etc)

I'm not quite sure how this would work, from a 'setting data that external apps can access' perspective. How would SDL know a) what image type are you putting on the clipboard, b) how to decode that image and encode it in a platform-specific clipboard image format, as you mention.

At first, both of those tasks seem like a job for SDL_image, which is not part of SDL for a reason.

I think a) can be solved by using SDL_Surface as the platform agnostic format. Then b) would be a necessary evil hidden behind the abstractions.

But if you're giving SDL a surface, you're not really giving it some void* data and its size_t dataSize.

If web custom formats are desired, how would you image adding that to your SDL_ExchangeDataType enum.

The drag drop proposal and your clipboard proposal all try to conform all data types to the same generic interface that works with raw buffers. It seems that operating systems support the same common set of data (text, image, other things you've mentioned), but all of them have a different format. And there are also use cases where you want to have your own custom format so that two applications you control can talk to each other.

For example, SDL interprets text/plain as a magic mime type and will translate the UTF-8 text to the OS-native format. Same applies to image/bmp on windows. This kind sucks.

Instead of trying to jam every type of data that can come up into the same generic interface, what if we have use case-specific functions + generic ones. Forcing them into the same callback would be quite hard, so why not have many:

void setClipboardExample(void* imageUserdata, const char* text, void* customUserdata) {
    SDL_Clipboard* clip = SDL_OpenClipboard(); // this would acquire an exclusive lock on the shared OS clipboard
    SDL_EmptyClipboard(clip);

    // the order in which these are called would be used as the
    // priority in the clipboard (if the platform supports it)

    SDL_AddClipboardImage(clip, myImageCallback, imageUserdata); // no cleanup, SDL knows how to free a surface.
    SDL_AddClipboardText(clip, text);  // this can also be changed to a callback if desired
    SDL_AddClipboardCustomData(clip, "example/x.custom", myDataCallback, myDataCleanup, customUserdata);
    SDL_AddClipboardCustomData(clip, "example/x.custom2", myDataCallback, myDataCleanup, customUserdata); // our callback may know how to convert to multiple formats

    SDL_CloseClipboard(clip); // this would set the clipboard
}

SDL_Surface* myImageCallback(void* userdata) {
    // ...
}

void* myDataCallback(void* userdata, const char* mimeType, size_t* size) {
    // ...
    *size = dataSize;
    return data;
}

void myDataCleanup(void* userdata, void* data) {
    // ...
}

It would be easy to provide convenience functions that set up the callbacks for you:

extern DECLSPEC int SDLCALL SDL_SetClipboardImage(SDL_Surface* surface) {
    SDL_Clipboard* clip = SDL_OpenClipboard();
    SDL_AddClipboardImage(clip, identity, surface);
    SDL_CloseClipboard(clip);
}

void* identity(void* data) {
    return data;
}

The same idea could work for getting the data from the clipboard -- open the clipboard to lock it, and then inspect with SDL_HasClipboard{Text,Image,CustomData}(), get what you need, then close when you're done.

This idea also has the benefit when adding features to platforms gradually -- eg. if SDL doesn't support custom data on some platform, SDL_AddClipboardCustomData can just return an error.

Just an alternative approach to consider.

c-smile commented 4 months ago

Each image serialization format has well known signature. For example browsers will open images no matter of their extensions (you can save .png file with .jpg extension and browser will open them happily). My Sciter does the same.

Windows CF_DIB is a .bmp file serialization - SDL has function to convert .bmp to SDL_Surface so to get SDL_Surface is not a problem. Therefore bmp can be used on all platforms as a fallback serialization.

As of data exchange in general...

Practice shows that HTML, JSON, BITMAP and URL LIST ( and probably XML too) are enough in most of SDL related cases. E.g. range of cells in Excel selection is represented as HTML <table> in clipboard (among other things). That is for the exchange with external applications. At least this set is used in Sciter based applications for 10+ years and I had no requests to extend this set.

When DnD and CB is used inside the same app (e.g. to pass the data between widgets) JSON is pretty adequate. Sciter also uses HTML as it has builtin WYSIWYG editor ( <richtext> ). Other SDL-based apps can use HTML if they will want to pass formatted data to MS Office, Libre Office and the like.

As of SDL_OpenClipboard() / SDL_EmptyClipboard(clip) I would strongly advise to do not use those but single transactional clipboard update SDL_SetClipboardDataEx instead - that will reset and populate content of the clipboard. This works on all desktop platforms as for CB as for DnD.

Implementation of SDL_PerformDrag, SDL_GetClipboardDataEx() and SDL_SetClipboardDataEx() on Windows as an example:

extern "C" {

    SDL_DragMode WIN_PerformDrag(SDL_Window* window,
        SDL_DragMode mode,       // SDL_DRAG_COPY or SDL_DRAG_MOVE or both
        SDL_ExchangeDataAccessor* data_accessor, //
        SDL_DragFeedback* feedback)
    {
        DWORD resultDropEffect = 0;

        WIN_DataObject* pdo = new WIN_DataObject(data_accessor, feedback);
        pdo->AddRef();

        HRESULT ret = ::DoDragDrop(pdo, pdo, mode, &resultDropEffect);
        pdo->Release();

        if (SUCCEEDED(ret))
            switch (resultDropEffect) {
            case DROPEFFECT_MOVE: return SDL_DRAG_MOVE;
            case DROPEFFECT_COPY: return SDL_DRAG_COPY;
            }
        return SDL_DRAG_NONE;
    }

    SDL_ExchangeDataAccessor* WIN_GetClipboardDataEx() {
        IDataObject* pdo = nullptr;
        HRESULT hr = ::OleGetClipboard(&pdo);
        if (SUCCEEDED(hr))
            return new WIN_ExchangeDataAccessor(pdo);
        return nullptr;
    }

    SDL_bool WIN_SetClipboardDataEx(SDL_ExchangeDataAccessor* dacc) {
        WIN_DataObject* pdo = new WIN_DataObject(dacc, nullptr);
        pdo->AddRef();
        HRESULT hr = ::OleSetClipboard(pdo);
        pdo->Release();
        return SUCCEEDED(hr);
    }
}

And here is implementation of stock SDL clipboard functions on top of those :

int SDL_SetClipboardText(const char *text)
{
    SDL_VideoDevice *_this = SDL_GetVideoDevice();
    if (!_this) {
        return SDL_SetError("Video subsystem must be initialized to set clipboard text");
    }

    SDL_ExchangeDataAccessor* ped = SDL_CreateWriteableExchangeData();

    if (text && *text)
        ped->setData(ped, SDL_DATA_TYPE_TEXT, text, SDL_strlen(text) + 1);

    int rv = SDL_SetClipboardDataEx(ped);

    ped->releaseThis(ped);

    return rv;
}

char *SDL_GetClipboardText(void)
{
    SDL_VideoDevice* _this = SDL_GetVideoDevice();
    if (!_this) {
        SDL_SetError("Video subsystem must be initialized to check clipboard text");
        return NULL;
    }

    SDL_ExchangeDataAccessor* pda = _this->GetClipboardDataEx();
    size_t sz;
    char* text = pda->getData(pda, SDL_DATA_TYPE_TEXT, &sz);
    pda->releaseThis(pda);

    return text;
}
Susko3 commented 4 months ago

Reading your example application code (SDL_SetClipboardText and SDL_GetClipboardText), we basically have the same idea. You just use function pointers in an SDL_ExchangeDataAccessor * and I have C functions which do the same thing, but to an opaque SDL_Clipboard * struct.

You have SDL_DATA_TYPE_TEXT, SDL_DATA_TYPE_BITMAP, etc. whose underlying data are passed around with void * and size_t. And I have specific functions (SDL_AddClipboardText, SDL_AddClipboardImage) that work with the same idea of specific data types, but use more specific underlying types in the API, such as const char * and SDL_Surface *. The size is implicitly stored by the null terminator of the string, and explicitly in the dimensions of a surface.

I also spot that your SDL_GetClipboardText can return a char * that is not null-terminated, as the SDL_DATA_TYPE_TEXT can be an excerpt from a larger document.

In your approach, I find it interesting that the SDL_ExchangeDataAccessor can return different SDL_ExchangeDataTypes at different times. Not sure if OS platforms support applications deciding that the clipboard data they offered is no longer available. (Or -- who (& when) will ask the SDL_ExchangeDataAccessor to check which formats should be added/removed from the clipboard.)

c-smile commented 4 months ago

but to an opaque SDL_Clipboard * struct.

DnD and Clipboard use the same mechanism to access exchange data (on all platforms), thus I do not see any reason of having special SDL_Clipboard structure.

Unified SDL_ExchangeDataAccessor allows to define exchange data conversions/access in one place - as for DnD as for CB.

SDL_ExchangeDataAccessor is a wrapper around IDataObject on Windows, NSPasteboard on MacOS, etc.

I have specific functions (SDL_AddClipboardText, SDL_AddClipboardImage)

Not a problem, you can have any API on top of SDL_ExchangeDataAccessor

SDL_ExchangeDataAccessor* pda = SDL_CreateWriteableExchangeData();
SDL_ExchangeData_AddText(SDL_ExchangeDataAccessor* pda, const char* text, size_t textLength);
SDL_ExchangeData_AddSurface(SDL_ExchangeDataAccessor* pda, SDL_Surface* psf);

SDL_SetClipboardDataEx(pda); // or 
SDL_PerformDrag(pda,...)

SDL_ExchangeData_Release(pda);

The real value is that all these will still use the same base four functions. Note that each SDL feature has O(N) price in multiplatform space.

c-smile commented 4 months ago

I also spot that your SDL_GetClipboardText can return a char * ...

We can write 

const char* SDL_ExchangeData_GetText(pda, size_t* psize) {
   if(pda) {
      return (const char* )pda->getData(pda,SDL_DATA_TYPE_TEXT,psize);
   }
   return NULL;
}
c-smile commented 4 months ago

Forgot to mention SDL_DropTargetHandler:


typedef enum SDL_DragOperation {
    SDL_DRAG_ENTER, /// drag enters the window
    SDL_DRAG_LEAVE, /// drag left the window
    SDL_DRAG_OVER,  /// data is being dragged over the window
    SDL_DRAG_DROP,  /// data is dropped at the location
} SDL_DragOperation;

typedef SDL_DropResult (SDLCALL* SDL_DropTargetHandler)(
    SDL_Window*         win,
    SDL_DragOperation   operation,
    const SDL_Point*    location,
    SDL_ExchangeDataAccessor* data_accessor,
    void*               callback_data);

extern DECLSPEC int SDLCALL SDL_SetWindowDropTarget(SDL_Window* window, SDL_DropTargetHandler callback, void* callback_data);

SDL_DropTargetHandler is set on window willing to receive dragged data.

Susko3 commented 4 months ago

I like how SDL_ExchangeDataAccessor looks with the public API you've shown.

SDL_CreateWriteableExchangeData and SDL_DestroyExchangeData is probably a better API then open/close clipboard. If the application forgets to free the resource, the former results in a memory leak, while the latter results in the clipboard not working for the entire system (theoretically).

I'm a bit worried for how SDL_GetClipboardData() would work if it returns an SDL_ExchangeDataAccessor * object. If SDL_GetClipboardData opens and closes the clipboard, some other application can modify the clipboard, invalidating our "still valid" SDL_ExchangeDataAccessor. Unless the call to SDL_DestroyExchangeData() would close the clipboard, but that would be kinda strange.

Using SDL_properties could work well for (some part of) the internal representation.

c-smile commented 4 months ago

Depending on OS SDL_GetClipboardDataEx() may use either one of these two approaches:

  1. copy data: copying content of underlying data object at the moment of creation;
  2. generational id: storing version/generational id, so any request for data should compare that id first and if it does not match - reject getData request. See NSPasteboard changeCount for example.

SDL_SetClipboardDataEx() increments/changes generational id by OS means, see NSPasteboard clearContents

Susko3 commented 3 months ago

Depending on OS SDL_GetClipboardDataEx() may use either one of these two approaches:

  1. copy data: copying content of underlying data object at the moment of creation;

  2. generational id: storing version/generational id, so any request for data should compare that id first and if it does not match - reject getData request. See NSPasteboard changeCount for example.

Approach 2. might not work as the data could get invalidated after the check. But if the SDL_ExchangeDataAccessor is valid (on a OS-level) until the call to SDL_DestroyExchangeData(), then it's completely fine. But it does mean that an application that forgets to call SDL_DestroyExchangeData() leaks "global" memory.

Approach 1. sucks because there may be n formats on the clipboard, but the applications will usually be interested in just 1.

Feels like we've explored a lot of options and have better understanding of the problem, I think it's time for a draft implementation.

slouken commented 1 month ago

This is a large enough departure from the simple APIs that SDL normally provides, that we're going to move this out into a future milestone.

slouken commented 3 weeks ago

I'm moving this back into the SDL ABI milestone. Let's get a draft PR going and explore this?

slouken commented 2 weeks ago

No response and we're coming up on ABI lock, so we'll bump this out for now. It doesn't require ABI changes since the existing functionality can be built on the new infrastructure.