Skip to content
Open
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;
using System.Threading;
using Windows.ApplicationModel.DataTransfer;
using Windows.ApplicationModel.DataTransfer.DragDrop;
using Windows.ApplicationModel.DataTransfer.DragDrop.Core;
using Windows.Foundation;
using Windows.Storage.Streams;
using Windows.Win32;
using Windows.Win32.Foundation;
using Windows.Win32.Graphics.Gdi;
using Windows.Win32.System.Com;
using Windows.Win32.System.Ole;
using Windows.Win32.System.SystemServices;
Expand All @@ -20,6 +24,7 @@
using Uno.UI.Hosting;
using Uno.UI.NativeElementHosting;
using IDataObject = Windows.Win32.System.Com.IDataObject;
using Microsoft.UI.Input;

namespace Uno.UI.Runtime.Skia.Win32;

Expand Down Expand Up @@ -149,8 +154,11 @@ unsafe HRESULT IDropTarget.Interface.DragEnter(IDataObject* dataObject, MODIFIER
return medium.u.hGlobal;
});

// Create DragUI for visual feedback during drag operation
var dragUI = CreateDragUIForExternalDrag(dataObject, formatEtcList);

// DROPEFFECT and DataPackageOperation have the same binary representation
var info = new CoreDragInfo(src, package.GetView(), (DataPackageOperation)(*pdwEffect));
var info = new CoreDragInfo(src, package.GetView(), (DataPackageOperation)(*pdwEffect), dragUI);
_coreDragDropManager.DragStarted(info);

*pdwEffect = (DROPEFFECT)_manager.ProcessMoved(src);
Expand Down Expand Up @@ -199,6 +207,199 @@ unsafe HRESULT IDropTarget.Interface.Drop(IDataObject* dataObject, MODIFIERKEYS_

public void StartNativeDrag(CoreDragInfo info, Action<DataPackageOperation> action) => throw new System.NotImplementedException();

private static unsafe DragUI? CreateDragUIForExternalDrag(IDataObject* dataObject, FORMATETC[] formatEtcList)
{
var dragUI = new DragUI(PointerDeviceType.Mouse);

// Check if we have a DIB (Device Independent Bitmap) format
var dibFormat = formatEtcList.FirstOrDefault(f => f.cfFormat == (int)CLIPBOARD_FORMAT.CF_DIB);
if (dibFormat.cfFormat != 0)
Comment on lines +215 to +216
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

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

Using FirstOrDefault on a FORMATETC struct array will return default(FORMATETC) (all zeros) when no match is found. The subsequent check if (dibFormat.cfFormat != 0) assumes that a zero cfFormat indicates "not found", but this could be problematic if FORMATETC with cfFormat=0 is valid.

While unlikely in practice for clipboard formats, a more explicit approach would be safer:

var dibFormatIndex = Array.FindIndex(formatEtcList, f => f.cfFormat == (int)CLIPBOARD_FORMAT.CF_DIB);
if (dibFormatIndex >= 0)
{
    var dibFormat = formatEtcList[dibFormatIndex];
    // ... rest of code
}

The same issue exists for the HDROP format check at line 239.

Copilot uses AI. Check for mistakes.
{
// Try to get the DIB data directly
var hResult = dataObject->GetData(dibFormat, out STGMEDIUM dibMedium);
if (hResult.Succeeded && dibMedium.u.hGlobal != IntPtr.Zero)
Comment on lines +219 to +220
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

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

The code checks hResult.Succeeded but doesn't verify that dibMedium.tymed matches the expected medium type (TYMED_HGLOBAL). According to COM IDataObject conventions, you should verify that the returned medium type matches what was requested in the FORMATETC.

Consider adding:

if (hResult.Succeeded && dibMedium.tymed == (uint)TYMED.TYMED_HGLOBAL && dibMedium.u.hGlobal != IntPtr.Zero)

The same applies to the HDROP format check at line 244.

Copilot uses AI. Check for mistakes.
{
try
{
var unoImage = ConvertDibToUnoBitmapImage(dibMedium.u.hGlobal);
if (unoImage is not null)
{
dragUI.SetContentFromExternalBitmapImage(unoImage);
return dragUI;
}
}
finally
{
PInvoke.ReleaseStgMedium(&dibMedium);
}
}
}

// Check if we have file drop format
var hdropFormat = formatEtcList.FirstOrDefault(f => f.cfFormat == (int)CLIPBOARD_FORMAT.CF_HDROP);
if (hdropFormat.cfFormat != 0)
{
// Try to get the HDROP data directly
var hResult = dataObject->GetData(hdropFormat, out STGMEDIUM hdropMedium);
if (hResult.Succeeded && hdropMedium.u.hGlobal != IntPtr.Zero)
{
try
{
var filePaths = ExtractFilePathsFromHDrop(hdropMedium.u.hGlobal);
var imageFile = filePaths.FirstOrDefault(f => IsImageFile(f));
if (imageFile is not null)
{
try
{
var unoImage = LoadImageFromFile(imageFile);
if (unoImage is not null)
{
dragUI.SetContentFromExternalBitmapImage(unoImage);
return dragUI;
}
}
catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or NotSupportedException or UriFormatException)
{
// If we can't load the image, continue without visual feedback
var logger = typeof(Win32DragDropExtension).Log();
if (logger.IsEnabled(LogLevel.Debug))
{
logger.LogDebug($"Failed to load image thumbnail for drag operation: {ex.Message}");
}
}
}
}
finally
{
PInvoke.ReleaseStgMedium(&hdropMedium);
}
}
}

return dragUI;
}

private static bool IsImageFile(string filePath)
{
// Common image formats
// Note: Additional formats can be added here as needed
var extension = Path.GetExtension(filePath).ToLowerInvariant();
return extension is ".png" or ".jpg" or ".jpeg" or ".gif" or ".bmp" or ".tiff" or ".ico";
}
Comment on lines +282 to +288
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

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

The IsImageFile method is duplicated between WpfDragDropExtension.cs and Win32DragDropExtension.cs with identical implementations. Consider extracting this to a shared utility class to reduce code duplication and improve maintainability. The same applies to the LoadImageFromFile method (lines 334-363).

Copilot uses AI. Check for mistakes.

private static unsafe List<string> ExtractFilePathsFromHDrop(HGLOBAL handle)
{
var filePaths = new List<string>();

using var lockDisposable = Win32Helper.GlobalLock(handle, out var firstByte);
if (lockDisposable is null)
{
return filePaths;
}

var hDrop = new Windows.Win32.UI.Shell.HDROP((IntPtr)firstByte);
var filesDropped = PInvoke.DragQueryFile(hDrop, 0xFFFFFFFF, new PWSTR(), 0);

for (uint i = 0; i < filesDropped; i++)
{
var charLength = PInvoke.DragQueryFile(hDrop, i, new PWSTR(), 0);
if (charLength == 0)
{
continue;
}
charLength++; // + 1 for \0

Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

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

The Marshal.AllocHGlobal allocation uses charLength * sizeof(char) but charLength is a uint. For very large values, this multiplication could overflow, leading to a small allocation that causes buffer overruns when DragQueryFile writes to it.

Consider adding a safety check:

if (charLength > int.MaxValue / sizeof(char))
{
    continue; // Skip paths that are too long
}
var buffer = Marshal.AllocHGlobal((int)(charLength * sizeof(char)));

Or cast to long for the multiplication and check the result before casting to IntPtr.

Suggested change
// Prevent integer overflow in allocation
if (charLength > int.MaxValue / sizeof(char))
{
continue; // Skip paths that are too long
}

Copilot uses AI. Check for mistakes.
var buffer = Marshal.AllocHGlobal((IntPtr)(charLength * sizeof(char)));
try
{
var charsWritten = PInvoke.DragQueryFile(hDrop, i, new PWSTR((char*)buffer), charLength);
if (charsWritten > 0)
{
var path = Marshal.PtrToStringUni(buffer);
if (!string.IsNullOrEmpty(path))
{
filePaths.Add(path);
}
}
}
finally
{
Marshal.FreeHGlobal(buffer);
}
}

return filePaths;
}

private static Microsoft.UI.Xaml.Media.Imaging.BitmapImage? LoadImageFromFile(string filePath)
{
try
{
// Validate file path to prevent potential security issues
if (string.IsNullOrWhiteSpace(filePath) || !File.Exists(filePath))
{
return null;
}

// Load image from file
using var fileStream = File.OpenRead(filePath);
using var memoryStream = new MemoryStream();

// Copy to memory stream for manipulation
fileStream.CopyTo(memoryStream);
memoryStream.Position = 0;

// Create Uno BitmapImage from the stream
var unoBitmap = new Microsoft.UI.Xaml.Media.Imaging.BitmapImage();
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

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

The Win32 LoadImageFromFile method lacks the thumbnail size optimization present in the WPF implementation (WpfDragDropExtension.cs line 249: wpfBitmap.DecodePixelWidth = 96). This could lead to loading full-resolution images into memory for drag thumbnails, which is inefficient.

Consider adding similar size constraints for consistency and performance:

// After creating unoBitmap, set DecodePixelWidth before SetSource
unoBitmap.DecodePixelWidth = 96; // Limit thumbnail size
Suggested change
var unoBitmap = new Microsoft.UI.Xaml.Media.Imaging.BitmapImage();
var unoBitmap = new Microsoft.UI.Xaml.Media.Imaging.BitmapImage();
unoBitmap.DecodePixelWidth = 96; // Limit thumbnail size for performance

Copilot uses AI. Check for mistakes.
unoBitmap.SetSource(memoryStream.AsRandomAccessStream());
Comment on lines +346 to +354
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

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

The LoadImageFromFile method unnecessarily copies the entire file stream into memory. The fileStream.CopyTo(memoryStream) operation is redundant since SetSource can work directly with the FileStream converted to IRandomAccessStream.

Consider using the file stream directly:

using var fileStream = File.OpenRead(filePath);
var unoBitmap = new Microsoft.UI.Xaml.Media.Imaging.BitmapImage();
unoBitmap.SetSource(fileStream.AsRandomAccessStream());

This avoids an unnecessary memory allocation and copy operation, especially beneficial for large image files.

Suggested change
using var memoryStream = new MemoryStream();
// Copy to memory stream for manipulation
fileStream.CopyTo(memoryStream);
memoryStream.Position = 0;
// Create Uno BitmapImage from the stream
var unoBitmap = new Microsoft.UI.Xaml.Media.Imaging.BitmapImage();
unoBitmap.SetSource(memoryStream.AsRandomAccessStream());
// Create Uno BitmapImage from the stream
var unoBitmap = new Microsoft.UI.Xaml.Media.Imaging.BitmapImage();
unoBitmap.SetSource(fileStream.AsRandomAccessStream());

Copilot uses AI. Check for mistakes.

return unoBitmap;
}
catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or NotSupportedException or UriFormatException)
{
// Failed to load image - file might not exist, no access, unsupported format, or invalid path
return null;
}
}

private static unsafe Microsoft.UI.Xaml.Media.Imaging.BitmapImage? ConvertDibToUnoBitmapImage(HGLOBAL handle)
{
try
{
using var lockDisposable = Win32Helper.GlobalLock(handle, out var dib);
if (lockDisposable is null)
{
return null;
}

var memSize = (uint)PInvoke.GlobalSize(handle);
if (memSize <= Marshal.SizeOf<BITMAPINFOHEADER>())
{
return null;
}

// Convert DIB to a stream that can be used by BitmapImage
// Pre-allocate buffer for typical thumbnail size to avoid reallocations
using var memoryStream = new MemoryStream(capacity: 8192);
Comment on lines +382 to +383
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

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

[nitpick] The pre-allocated buffer size of 8192 bytes seems arbitrary for DIB data. Unlike encoded image formats (PNG in WPF implementation), DIB size depends on image dimensions and bit depth. For a 96x96 32-bit image, the DIB would be approximately 36KB (96964 bytes + header), significantly larger than 8KB.

Consider either calculating the appropriate size from the BITMAPINFOHEADER, or using a larger default capacity (e.g., 65536 for typical thumbnail sizes), or omitting the capacity parameter to let MemoryStream grow as needed.

Suggested change
// Pre-allocate buffer for typical thumbnail size to avoid reallocations
using var memoryStream = new MemoryStream(capacity: 8192);
// Pre-allocate buffer for the exact DIB size to avoid reallocations
using var memoryStream = new MemoryStream(capacity: (int)memSize);

Copilot uses AI. Check for mistakes.

// Copy the DIB data to the memory stream
var dibBytes = new Span<byte>(dib, (int)memSize);
Comment on lines +385 to +386
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

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

After writing to memoryStream using memoryStream.Write(dibBytes), the stream position is reset to 0 before calling SetSource. However, given that the DIB-to-BMP conversion is missing (see previous comment on lines 365-401), this position reset won't help because the stream content is still invalid DIB data, not a proper BMP file format.

This line is correct in principle but won't work until the DIB-to-BMP conversion is properly implemented.

Suggested change
// Copy the DIB data to the memory stream
var dibBytes = new Span<byte>(dib, (int)memSize);
// Copy the DIB data to a byte array for manipulation
var dibBytes = new Span<byte>(dib, (int)memSize);
// Read BITMAPINFOHEADER from the start of the DIB
var infoHeader = MemoryMarshal.Read<BITMAPINFOHEADER>(dibBytes);
// Calculate the size of the color table (if any)
int colorTableSize = 0;
if (infoHeader.biBitCount <= 8)
{
// Color table entries = 2^biBitCount unless biClrUsed is set
int colorCount = (int)(infoHeader.biClrUsed != 0 ? infoHeader.biClrUsed : 1u << infoHeader.biBitCount);
colorTableSize = colorCount * 4; // Each color table entry is 4 bytes (RGBQUAD)
}
// The offset to the pixel data in the DIB is the size of BITMAPINFOHEADER + color table
int dibHeaderSize = Marshal.SizeOf<BITMAPINFOHEADER>();
int pixelDataOffset = dibHeaderSize + colorTableSize;
// The BMP file header is 14 bytes
const int BITMAPFILEHEADER_SIZE = 14;
// The file size is the size of the file header + DIB data
uint fileSize = (uint)(BITMAPFILEHEADER_SIZE + memSize);
// Write BITMAPFILEHEADER
using (var writer = new BinaryWriter(memoryStream, System.Text.Encoding.Default, leaveOpen: true))
{
// WORD bfType = 'BM'
writer.Write((ushort)0x4D42);
// DWORD bfSize = file size
writer.Write(fileSize);
// WORD bfReserved1 = 0
writer.Write((ushort)0);
// WORD bfReserved2 = 0
writer.Write((ushort)0);
// DWORD bfOffBits = offset to pixel data (file header + DIB header + color table)
writer.Write((uint)(BITMAPFILEHEADER_SIZE + pixelDataOffset));
}
// Write the DIB data (header + color table + pixels)

Copilot uses AI. Check for mistakes.
memoryStream.Write(dibBytes);
memoryStream.Position = 0;

// Create Uno BitmapImage from the stream
var unoBitmap = new Microsoft.UI.Xaml.Media.Imaging.BitmapImage();
unoBitmap.SetSource(memoryStream.AsRandomAccessStream());

return unoBitmap;
}
catch (Exception ex) when (ex is IOException or NotSupportedException or InvalidOperationException)
{
// Failed to convert bitmap - encoding or stream operations failed
return null;
}
}
Comment on lines +365 to +401
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

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

The ConvertDibToUnoBitmapImage method attempts to directly write DIB data to a stream and load it as an image, but DIB format is not a valid standalone image format. DIB (Device Independent Bitmap) requires a BITMAPFILEHEADER to be converted to a BMP file format before it can be loaded by standard image decoders. The current implementation will likely fail when SetSource is called.

To fix this, you need to:

  1. Read the BITMAPINFOHEADER from the DIB
  2. Construct a BITMAPFILEHEADER
  3. Write the file header followed by the DIB data to create a valid BMP stream

Example structure:

// Write BITMAPFILEHEADER
var fileHeader = new BITMAPFILEHEADER { 
    bfType = 0x4D42, // 'BM'
    bfSize = (uint)(Marshal.SizeOf<BITMAPFILEHEADER>() + memSize),
    bfOffBits = (uint)(Marshal.SizeOf<BITMAPFILEHEADER>() + Marshal.SizeOf<BITMAPINFOHEADER>())
};
// Write fileHeader bytes to stream
// Then write DIB data

Copilot uses AI. Check for mistakes.

private readonly struct DragEventSource(Point point, MODIFIERKEYS_FLAGS modifierFlags) : IDragEventSource
{
private static long _nextFrameId;
Expand Down
116 changes: 115 additions & 1 deletion src/Uno.UI.Runtime.Skia.Wpf/Extensions/WpfDragDropExtension.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
using Uno.Foundation.Logging;
using Uno.UI.Hosting;
using Uno.UI.Runtime.Skia.Wpf.Hosting;
using Microsoft.UI.Input;

namespace Uno.UI.Runtime.Skia.Wpf
{
Expand Down Expand Up @@ -53,7 +54,8 @@ private void OnHostDragEnter(object sender, DragEventArgs e)
var src = new DragEventSource(_fakePointerId, e);
var data = ToDataPackage(e.Data);
var allowedOperations = ToDataPackageOperation(e.AllowedEffects);
var info = new CoreDragInfo(src, data.GetView(), allowedOperations);
var dragUI = CreateDragUIForExternalDrag(e.Data);
var info = new CoreDragInfo(src, data.GetView(), allowedOperations, dragUI);

_coreDragDropManager.DragStarted(info);

Expand Down Expand Up @@ -173,6 +175,118 @@ private static DataPackage ToDataPackage(IDataObject src)
return dst;
}

private static DragUI? CreateDragUIForExternalDrag(IDataObject src)
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

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

The method returns a nullable DragUI?, but it always returns a non-null dragUI instance even when no image content is available. This is inconsistent with the nullable return type. Consider either:

  1. Returning null when no content can be loaded (change line 222 to return null;)
  2. Changing the return type to DragUI (non-nullable) if returning an empty DragUI is intentional

The Win32 implementation has the same pattern.

Copilot uses AI. Check for mistakes.
{
var dragUI = new DragUI(PointerDeviceType.Mouse);

// Check if we're dragging an image file that can be shown as a thumbnail
if (src.GetData(DataFormats.Bitmap) is BitmapSource bitmap)
{
// Convert WPF BitmapSource to Uno BitmapImage for DragUI
var unoImage = ConvertBitmapSourceToUnoBitmapImage(bitmap);
if (unoImage is not null)
{
dragUI.SetContentFromExternalBitmapImage(unoImage);
return dragUI;
}
}

// If we have file paths, try to load the first image file as a thumbnail
if (src.GetData(DataFormats.FileDrop) is string[] files && files.Length > 0)
{
var imageFile = files.FirstOrDefault(f => IsImageFile(f));
if (imageFile is not null)
{
try
{
var unoImage = LoadImageFromFile(imageFile);
if (unoImage is not null)
{
dragUI.SetContentFromExternalBitmapImage(unoImage);
return dragUI;
}
}
catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or NotSupportedException or UriFormatException)
{
// If we can't load the image (file not found, no access, unsupported format, or invalid path),
// continue without visual feedback
var logger = typeof(WpfDragDropExtension).Log();
if (logger.IsEnabled(LogLevel.Debug))
{
logger.LogDebug($"Failed to load image thumbnail for drag operation: {ex.Message}");
}
}
}
}

return dragUI;
}

private static bool IsImageFile(string filePath)
{
// Common image formats supported by WPF BitmapImage
// Note: Additional formats can be added here as needed
var extension = Path.GetExtension(filePath).ToLowerInvariant();
return extension is ".png" or ".jpg" or ".jpeg" or ".gif" or ".bmp" or ".tiff" or ".ico";
}

private static Microsoft.UI.Xaml.Media.Imaging.BitmapImage? LoadImageFromFile(string filePath)
{
try
{
// Validate file path to prevent potential security issues
if (string.IsNullOrWhiteSpace(filePath) || !File.Exists(filePath))
{
return null;
}

// Load the WPF bitmap
var wpfBitmap = new System.Windows.Media.Imaging.BitmapImage();
wpfBitmap.BeginInit();
wpfBitmap.CacheOption = BitmapCacheOption.OnLoad;
wpfBitmap.CreateOptions = BitmapCreateOptions.None;
wpfBitmap.UriSource = new Uri(filePath, UriKind.Absolute);
wpfBitmap.DecodePixelWidth = 96; // Limit thumbnail size
wpfBitmap.EndInit();
wpfBitmap.Freeze();

// Convert to Uno BitmapImage
return ConvertBitmapSourceToUnoBitmapImage(wpfBitmap);
}
catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or NotSupportedException or UriFormatException)
{
// Failed to load image - file might not exist, no access, unsupported format, or invalid path
return null;
}
}

private static Microsoft.UI.Xaml.Media.Imaging.BitmapImage? ConvertBitmapSourceToUnoBitmapImage(BitmapSource wpfBitmap)
{
try
{
// Encode the WPF bitmap to a stream
// Note: The using statement disposes the MemoryStream after SetSource() completes.
// This is safe because SetSource() reads and copies the stream data synchronously.
// Pre-allocate buffer for typical thumbnail size to avoid reallocations
using var memoryStream = new MemoryStream(capacity: 8192);
var encoder = new PngBitmapEncoder();
encoder.Frames.Add(BitmapFrame.Create(wpfBitmap));
encoder.Save(memoryStream);
memoryStream.Position = 0;

// Create Uno BitmapImage from the stream
var unoBitmap = new Microsoft.UI.Xaml.Media.Imaging.BitmapImage();
unoBitmap.SetSource(memoryStream.AsRandomAccessStream());

return unoBitmap;
}
catch (Exception ex) when (ex is IOException or NotSupportedException or InvalidOperationException)
{
// Failed to convert bitmap - encoding or stream operations failed
return null;
}
}

private static async Task<DataObject> ToDataObject(DataPackageView src, CancellationToken ct)
{
var dst = new DataObject();
Expand Down
8 changes: 8 additions & 0 deletions src/Uno.UI/UI/Xaml/DragDrop/DragUI.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ internal DragUI(PointerDeviceType pointerDeviceType)

internal Point? Anchor { get; set; }

internal bool IsExternalContent { get; set; }

public void SetContentFromBitmapImage(BitmapImage bitmapImage)
=> SetContentFromBitmapImage(bitmapImage, default);

Expand All @@ -32,5 +34,11 @@ public void SetContentFromBitmapImage(BitmapImage bitmapImage, Point anchorPoint
Content = bitmapImage;
Anchor = anchorPoint;
}

internal void SetContentFromExternalBitmapImage(BitmapImage bitmapImage)
{
Content = bitmapImage;
IsExternalContent = true;
}
}
}
Loading
Loading