Skip to content

implemented Event::FilesDropped for Windows#1222

Closed
Fytch wants to merge 1 commit intoSFML:masterfrom
Fytch:file_drop
Closed

implemented Event::FilesDropped for Windows#1222
Fytch wants to merge 1 commit intoSFML:masterfrom
Fytch:file_drop

Conversation

@Fytch
Copy link

@Fytch Fytch commented Apr 30, 2017

Disclaimer : this feature is not ready to be merged yet

Related forum thread: https://en.sfml-dev.org/forums/index.php?topic=19096

I have implemented this feature for personal use. I write simple map editors and other small tools with the aid of SFML because I don't want to bring out the big guns like Qt. For something like a map editor, the drag and drop functionality is very handy, especially for textures and materials.

As can be seen in the linked forum thread, I am not the only one wanting this feature. Therefore, I went ahead and created a pull request. This feature is not complete yet but it already works on Windows. I figured that once I take the first step maybe others are more inclined to chime in and collaborate.

Progress:

  • Modify sf::Event to store the relevant information
  • Implement interface in sf::Window (see setFileDroppingEnabled)
  • Implement backend for Windows
  • Implement backend for *nix
  • Implement backend for OSX
  • Add an example or another kind of test

Issues:

  • sf::Event::FileDropEvent is inevitably part of a union, so I couldn't use a vector but had to resort to a raw pointer. Personally, I think this is not a big deal because the usage is still very straight forward:
    for (int i=0; i<event.droppedFiles.count; ++i) event.droppedFiles.files[i];
    But maybe there's a better solution?
  • As sf::Event::FileDropEvent only stores pointers but not the filenames themselves, their lifetime is bound to the window, not to the event. Is there any case in which this window-specific buffer can cause trouble?
  • Am I right in assuming that WindowImplWin32::processEvent doesn't need to be thread-safe? Otherwise the aforementioned buffer would need to be protected by a mutex, ensuring that no concurrent accesses are possible.

Unfortunately, I'm not versed well enough in X11 to implement this feature for Linux, but I have found this which might be handy for anyone wanting to implement this feature.

////////////////////////////////////////////////////////////
struct FileDropEvent
{
unsigned int count; ///< Number of dropped files
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather see a std::vector<string> here, which would also make count obsolete.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the point above:

sf::Event::FileDropEvent is inevitably part of a union, so I couldn't use a vector but had to resort to a raw pointer. ...

Copy link
Author

@Fytch Fytch Apr 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All event data is stored in a union, so using std::vector<string> is not possible. Since C++11, it would theoretically be allowed using placement new and manual destructor invocations. Alternatively, there's also std::variant/boost::variant (which is essentially a wrapper of the method described).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right, the destructor getting broken data. Damn. :(

Uint16 m_surrogate; ///< First half of the surrogate pair, in case we're receiving a Unicode character in two events
bool m_mouseInside; ///< Mouse is inside the window?
bool m_fullscreen; ///< Is the window fullscreen?
bool m_cursorGrabbed; ///< Is the mouse cursor trapped?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this member even needed? The list of files is in the event after all.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the event only stores a pointer to the buffer. See the first two bullet points in my issues list.

{
// This is the fallback for unsupported platforms.
// Trying to enable file dropping returns a bad value
// indicating an error, whereas disabling it is fine.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is confusing. Why would it return !enabled? This indicates a success, if you fail to disable it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, disabling a non-existent function is always possible. Only enabling it would be troublesome because it - well - doesn't exist.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, okay, from that perspective it makes sense. But I'm not 100% sure how it should react considering it's not implemented/available. You're making it fail silently.

Copy link
Author

@Fytch Fytch Apr 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, it doesn't fail silently. Disabling it is indistinguishable from it not being implemented, so why generate an error?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I can live with that when disabling.

break;

// Expand the buffer list by one vector
FilesType& files = *m_droppedFiles.insert(m_droppedFiles.end(), FilesType());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't make any sense either. First you append an element, then resize? Or did I miss something? And why the reference?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m_droppedFiles is a std::list of std::vectors of sf::Strings. First append to the list, then resize the newly appended vector.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah,, right, should probably read it twice next time. So you're appending a new list for every drag&drop operation. Too bad we can't free them unless we assume that it's only valid till the next event is polled.

Copy link
Author

@Fytch Fytch Apr 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. I append a new vector to the list for every drop operation. I decided this way because I would rather waste some Kilobytes than get nasty SIGSEGVs. User friendliness is more important to me.

@mantognini
Copy link
Member

Question: do Windows/Linux support drag operation for other kinds of data (e.g. text, image, ...)? Mac does, that's why I'm asking. It actually works like a clipboard for that. So, like in #1221, we can start by only supporting one kind of data (i.e. paths) but it'd be nice to have a rough idea how things could be extended to support other data formats.

In that respect, maybe a different API would be good. With 3.x we can think about using std::variant-like. But if we want to have something in 2.x, maybe something like this could be more flexible:

struct Data {
  template <T> T get();
  bool isPath(); // if true then get<String> is legal
  bool isImage(); // if true then get<Image> is legal
  // ...

  // or maybe something like that:
  struct Kind { Path, Image, ... };
  Kind getKind();
};


/* implementation defined */ Cookie;

struct DropEvent {
  Cookie cookie;
};

struct Window {
  // Let's say the Data for a Cookie is valid at least until
  // the next display/popEvent/whatever makes sense.
  Data getDropData(Cookie);

  void enableDropData(Kind kind, bool enable);
};

What do you think?

@spacechase0
Copy link
Contributor

It looks like Linux does? It sends a MIME type with the data, from what I can tell here and the code from the link at the end of the issue text (although I'll admit I didn't understand most from either places).

Windows does seem to, since I just dragged highlighted text from Firefox directory to Visual Studio and it put the text in the editor. I can also drag an image directly from Firefox into MS Paint. (I'm lost trying to find documentation on how this works though. It seems related to clipboard formats? From what I read here, Windows can actually give multiple formats at once.)

The API just proposed can't support Windows having multiple types at once, so if that is chosen SFML would need to decide what is best and only give that (or generate an event for each supported type, but that doesn't sound good either).

@OvermindDL1
Copy link

Linux supports multiple formats as well. A lot of software wrongly just chooses the first in the list, which causes things to not work properly quite often (try dragging some selected text to Dolphin for example, the first entry is a pointer to the text rather than the text itself (a later option), and it writes 4/8 bytes (the pointer itself) to a new file. Always give the option to specify the formats and give the list of all available on linux.

@manuel5975p
Copy link

Has there been any progress? Fytch's idea seems outstanding.

@Matthieu-Sauer
Copy link

A bit late here, but instead of storing a raw pointer (not very user friendly) why not wrapping it into a custom class, which will manage it properly without any risks?
(and maybe then implement a forEach function or use iterators to iterate in the list, just like a std::vector or a std::list would, (we could even use std::list maybe? since I don't see a reason not to have FIFO behavior while reading the content of any drag'n drop event)

I don't know how much could be implemented of hat I said, or even if it is relevant since I am not a maintainer of the library, just a random user sorry in advance

@eXpl0it3r eXpl0it3r linked an issue Aug 28, 2025 that may be closed by this pull request
@eXpl0it3r eXpl0it3r added this to the 3.2 milestone Aug 28, 2025
@github-project-automation github-project-automation bot moved this to Planned in SFML 3.2.0 Aug 28, 2025
@JonnyPtn
Copy link
Contributor

Think this is superseded by #2265 so can be closed

@eXpl0it3r
Copy link
Member

Superseded by #2265

@eXpl0it3r eXpl0it3r closed this Nov 26, 2025
@eXpl0it3r eXpl0it3r moved this from Planned to Done in SFML 3.2.0 Nov 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Add Support for Drag & Drop Events

10 participants