Opened 22 months ago

Closed 22 months ago

Last modified 5 months ago

#14697 closed enhancement (fixed)

Native drag image support (MSW)

Reported by: PeterO Owned by:
Priority: low Milestone:
Component: wxMSW Version: 2.9.4
Keywords: dnd drag image native docs-needed Cc:
Blocked By: Blocking:
Patch: yes

Description

Add support to wxDataObject, wxDropTarget, wxDropSource to be able to create a native drag image. Added wxDropSourceHelper which can create an drag image based on a wxImage or on the image provided by the associated window. This helper can be initalized explicitly before the drag operation or implicitly by using the new corresponding functions in wxDropSource.
For MSW the dataobject has been adjusted to accommodate the auxilary formats which the operating system uses for its own purposes (drag image bits and other administration).

Attachments (5)

native_drag_image_support.patch download (19.5 KB) - added by PeterO 22 months ago.
Native drag image support
native_drag_image_support_v2.patch download (20.8 KB) - added by PeterO 22 months ago.
native_drag_image_support_v2.2.patch download (79 bytes) - added by PeterO 22 months ago.
native_drag_image_support_v3.patch download (27.5 KB) - added by PeterO 22 months ago.
dropsrc-setdragimage.diff download (8.0 KB) - added by vadz 22 months ago.
VZ's modified version of wxDropSource part of the patch

Download all attachments as: .zip

Change History (20)

comment:1 Changed 22 months ago by vadz

  • Keywords docs-needed added
  • Priority changed from normal to low
  • Status changed from new to confirmed

Thanks, this would be useful to have but we really need some documentation and an example of using this in the dnd sample, otherwise nobody would even know about it.

I also wonder about the whole idea of "aux data" -- what does it all mean and what is it for? It would be nice to have some more in depth explanation of it in the comments.

Also, a couple of minor remarks:

  1. Why does wxDropSourceHelper need to be public? AFAICS it's only used in the implementation code.
  2. It would be better to avoid including shlobj.h from a public header, forward declaring IDropTargetHelper should be enough.
  3. DATASTORAGES is a really bad name, we don't use all caps for the type names.
  4. It should also use wxVector<> instead of std::vector<> (nothing in the rest of the code should need any changes).
  5. It seems inconsistent to pass an image to CreateDragImageFromImage() but not a window to CreateDragImageFromWindow().
  6. It also looks like it CreateDragImageFromImage() should take a wxCursor in the first place because it already is a wxBitmap with a hot spot. Using wxImage here is rather strange because if you already have a wxCursor, wxIcon or wxBitmap you'd need to convert it to wxImage only to convert it back in this code.
  7. It would be nice to use style consistent with the rest of the files you modify, i.e.
    • Put spaces around if conditions.
    • Avoid blank single line comments.

Thanks in advance!

comment:2 Changed 22 months ago by PeterO

Hereby some additional documentation (I don't know exactly where I have to place this in the comments):

Explanation:

wxDataObject contains the data we want to transfer: text, bitmap etc. To add data to the data object we use SetData which stores the data in a specific format. To be able to show a drag image while dragging, even beyond the borders of our program, the operating system has to store the image somewhere accessible and in a format known to all clients. Therefore Windows saves the data image into the data object in a format called 'DragImageBits'. Besides that Windows may save other data in specific formats for its own housekeeping and extended functionality (eg. DragSourceHelperFlags, InShellDragLoop, DragContext, IsComputingImage, IsShowingLayered etc). These formats aren't very well documented.). 
So additional to our own SetData calls to set the content data, the system uses the same SetData to saves its own data. wxDataObject checks if the format of the data set by SetData is accepted. The Windows formats are unknown and therefore rejected. But to be able to support the native drag images we have to accept these formats as well. So if the data hasn't a format set by the user (the format of the content: text, bitmap etc) we assume that it's system data and save it in the data object as well. So now wxDataObject will contain content data (text, bitmap) and system data (drag image etc). If data is requested from the data object in GetData, a check is made first if the data is system data else we continue to check the content formats.

Usage:

To be able to accept drag images (Windows drag images as well, eg. folder from Explorer) nothing has to be done. wxDropTarget handles all operations/requests and will show the image.

To be able to create a drag image on starting a drag-drop operation, one have to call wxDropSource::CreateDragImageFromWindow/Bitmap before calling DoDragDrop.

Responding to your other questions:

  1. dnd sample: The new patch contains a adjusted dnd sample as well. The window show native window images from outside. Text and shape which will be dragged from the window will be shown in/outside as well.
  2. aux data: The aux data is thus the system data. I've changed the name 'AuxData' to 'SystemData' if that covers the load more clearly.
  3. wxDropSourceHelper: I removed it completely. The corresponding functions in wxDropSource will now do the job.
  4. shlobj.h: Has forward declared IDropTargetHelper and moved shlobj.h to the cpp file.
  5. DATASTORAGES: Is called Data now.
  6. std::vector<>: Changed to wxVector<>
  7. CreateDragImageFromImage: Changed name to CreateDragImageFromBitmap and accepting now a wxBitmap.
  8. CreateDragImageFromWindow: Added window argument.
  9. Style consistent: Adjusted but maybe I overlooked something.

Changed 22 months ago by PeterO

Native drag image support

Changed 22 months ago by PeterO

Changed 22 months ago by PeterO

comment:3 Changed 22 months ago by vadz

Thanks, I understand it much better now.

But I see 2 problems in the dnd sample:

  1. If I now drag anything from it, I get a lot of memory leaks on exit. I didn't have time to debug them but this really should be fixed.
  2. This is much more minor, but the drag image for the text is rather ugly because of the use of "colour key" which replaces some of the pixels in the text itself with background colour. SHDRAGIMAGE documentation states "Turn off antialiasing when drawing text. Otherwise, artifacts could occur at the edges, between the text color and the color key." but we don't provide a simple way to do it. So perhaps we shouldn't be using the colour key at all instead?

Other than that we also need the documentation for the new CreateDragImageFrom{Bitmap,Window}() methods in interface/wx/dnd.h, could you please add them there and put "@since 2.9.5" in their description?

Thanks in advance!

comment:4 Changed 22 months ago by vadz

Ah, and one other thing: the patch uses CopyStgMedium() which requires linking with urlmon.lib that we don't currently use and so linking simply fails when using it as is. We probably don't want to require linking with urlmon.lib as this would require people to update all their projects when upgrading to 2.9 so if we really need this function we should load it dynamically. But perhaps an even simpler solution would be to avoid using it and just call AddRef() on STGMEDIUM::IStream or IStorage member as I think the only types of storage medium we can have here are TYMED_ISTREAM or TYMED_ISTORAGE, don't we?

Changed 22 months ago by PeterO

comment:5 Changed 22 months ago by PeterO

I applied your suggestions:

  1. Regarding the dnd sample: The memory leak has been solved and the quality of the text drag image improved.
  2. I've adjusted the documentation.
  3. I've created a local implementation CopyStgMedium, so urlmon.lib is not necessary anmore.

Patch v3 contains it all.

Regards,

Peter

comment:6 follow-up: Changed 22 months ago by vadz

Sorry for the delay, I've finally looked at this patch again and will apply its wxDropTarget part soon with only minor changes. The most important of them is that I did not make the various drag image support functions virtual, I really don't think this makes much sense and I'd rather not make them part of public API at all to be able to change them in the future.

I'm much more ambivalent about the wxDropSource part though because:

  1. It's not clear how does this combine with the existing wxDropSource::SetCursor(), perhaps it would be better to extend the existing function to cover this functionality instead of adding a new one? They do seem rather similar.
  2. Related: it doesn't look like it's ever going to be possible to implement something so MSW-specific for the other platforms. Again, SetCursor() interface is much more reasonable from portability point of view.
  3. Using a window for drag image doesn't seem to work well for me, the "window" image is always empty and doesn't represent the actual window contents at all. Any idea why?

Anyhow, I'm attaching my modified version of the patch but I think it needs more work before being applied and probably some API rethink.

Changed 22 months ago by vadz

VZ's modified version of wxDropSource part of the patch

comment:7 Changed 22 months ago by VZ

(In [72668]) Display system-provided drag images during drag-and-drop in wxMSW.

This is especially useful when dragging files from Explorer as it provides
big, informative drag images for them that can be easily displayed using
Windows shell support for them.

See #14697.

comment:8 in reply to: ↑ 6 Changed 22 months ago by PeterO

About remark nr. 3:

The Microsoft documentation explains that on invoking DragSourceHelper::InitializeFromWindow, the window receives a DI_GETDRAGIMAGE message whereby lparam holds a pointer to an SHDRAGIMAGE structure. The handler should fill the structure with the drag image bitmap information. (See http://msdn.microsoft.com/en-us/library/windows/desktop/bb762036(v=vs.85).aspx).

So a window has to have a handler that fills the structure with bitmap info. Just a basic wxWindow (or derived window) will do nothing. But wxListCtrl/wxTreeCtrl do have them. They are based on the native Windows list/tree view which handles this event and filling the structure with an appropriate bitmap based on the dragged items.

comment:9 follow-up: Changed 22 months ago by VZ

(In [72673]) Provide stand-in IDropTargetHelper definition to fix VC6 build.

VC6 SDK doesn't define this interface, so do it ourselves to fix its build
after the changes of r72668.

See #14697.

comment:10 Changed 22 months ago by VZ

  • Resolution set to fixed
  • Status changed from confirmed to closed

(In [72674]) Make GetClippingBox() work for wxPrinterDC in wxGTK.

GetClippingBox() implementation relies on wxDCImpl::m_clip[XY][12] being
updated in DoSetClippingRegion() but this wasn't done here. Fix this by adding
the code to do this to the base class version of this method and calling it
from wxGtkPrinterDCImpl.

Also, refactor wxGCDCImpl to reuse the same code instead of duplicating it.

Closes #14697.

comment:11 Changed 22 months ago by vadz

  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to PeterO:

About remark nr. 3:

The Microsoft documentation explains that on invoking DragSourceHelper::InitializeFromWindow, the window receives a DI_GETDRAGIMAGE message whereby lparam holds a pointer to an SHDRAGIMAGE structure. The handler should fill the structure with the drag image bitmap information. (See http://msdn.microsoft.com/en-us/library/windows/desktop/bb762036(v=vs.85).aspx).

So a window has to have a handler that fills the structure with bitmap info. Just a basic wxWindow (or derived window) will do nothing. But wxListCtrl/wxTreeCtrl do have them. They are based on the native Windows list/tree view which handles this event and filling the structure with an appropriate bitmap based on the dragged items.

I see, thanks, I didn't know about this message. So actually we'd also need to have a matching event (or maybe MSW-only virtual callback) to allow providing it if we do this...

P.S. Sorry for the above commit message with a wrong ticket number, it was a typo and is unrelated to this ticket.

comment:12 in reply to: ↑ 9 Changed 22 months ago by chowette

Replying to VZ:

(In [72673]) Provide stand-in IDropTargetHelper definition to fix VC6 build.

VC6 SDK doesn't define this interface, so do it ourselves to fix its build
after the changes of r72668.

See #14697.

Unfortunately, VC6 is also missing declaration of CLSID_DragDropHelper
see http://buildbot.tt-solutions.com/wx/builders/XPSP2%20VC6%20wxMSW%20trunk%20release/builds/2772/steps/compile/logs/stdio

comment:13 Changed 22 months ago by VZ

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [72692]) Define CLSID_DragDropHelper ourselves to fix VC6 build.

VC6 SDK doesn't define CLSID_DragDropHelper constant neither, so do it
ourselves too to complete the changes of r72673.

Closes #14697.

comment:14 Changed 5 months ago by VZ

(In [76076]) Don't accept data in unsupported format in wxMSW dnd code.

We wrongly pretended to accept the data in formats which we didn't actually
accept and showed misleading cursors to the user.

Fix this by partially reverting some of the changes of r72668 (see #14697).

Closes #16042.

comment:15 Changed 5 months ago by VZ

(In [76078]) Don't accept data in unsupported format in wxMSW dnd code.

We wrongly pretended to accept the data in formats which we didn't actually
accept and showed misleading cursors to the user.

Fix this by partially reverting some of the changes of r72668 (see #14697).

Closes #16042.

Note: See TracTickets for help on using tickets.