Hi, I don't want to disturb you much but I would really appreciate some pointers with regards to winex11.
I came across a pretty frustrating bug on buffered xorg based window managers where drawing of custom cursors that exceed XQueryBestSize end up in draws with artifacts with nvidia proprietary (visually looks like a bounded overflow draw), with mesa/intel you get flickering in some situations on xfce but not as crazy, (likely rapid switching between hw cursor <-> sw cursor or what have you).
Anyways, i'm no X expert but I really tried looking into the issue and see which level of stack should the issue fall on/be addressed at. Well.. It's not obvious to me. Xlib / X protocol technically doesn't impose any policy on cursor limits but it does expose things like XQueryBestSize. For example on xfce/xlibre/nvidia machine:
```
❯ xdpyinfo | grep -i cursor
largest cursor: 256x256
```
And so there is one game on steam that creates a cursor of 384x384 in 1440p and that ends up blowing up and turning into a visual mess. Of course, that cursor is not 384x384, it just creates a buffer of 384x384 because the game for some reason does like 4x internal scaling or something (96dpi x 4 I assume).
Basically it's not that easy to really nail where the discrepancy is. x11/xorg server does 'technically' have path that switches hw cursor to sw cursor but it doesn't seem to work or is unstable on window manager/graphics driver basis? Like I said earlier, I experienced this artifact on both xlibre/xfce and freedesktop xorg/gnome on nvidia and mesa. The reason I'm mentioning this is that it seems that it doesn't seem to be a problem of just one window manager and also I'm still uncertain whether patching this on xorg server is either valid approach or easy.
So from a protocol perspective I don't know whether or not technically the rendering artifacts are a fault of X protocol or server entirely or not. I think in part but the issue is that from a protocol perspective there is no real strictness about this so...
I do figure that userspace apps that interact with create cursor can basically take advantage of protocol facilities that result in stable drawing.
The only problem being is that I found two ways to address this issue, in wine in particular. either:
1. pass smaller BITMAP dimensions to NtGdiCreateDIBSection (cursor initializer?) which win seems to auto downscale the cursor to those dimensions ? (looks correct in my use-case but the cursor is visibly smaller)
2. clamp/trim/crop the larger cursor buffer into smaller one that fits XQueryBestSize. (looks 1:1 to windows in my use-case)
The problem is that visually for my use-case 2 gives correct visual behavior but I think trimming is likely wrong approach if it were default. As you can see from the diff I'm gating both methods atm but what I would love to know if:
- it's acceptable to have something like this be handled by winex11.drv at all or do you think this is not wine's problem at all? I would say that it would be good if wine respected XQueryBestSize but I'm not certain yet on what the right shape of the hot path should be.
- does this diff look sensible or is this wrong approach in general? In terms of naming I'm pretty sure it's wrong, I just don't know what the right names should be. scale cursor is technically also an implicit lie (it doesn't resample, it just rewrites dims right - it just happend to give that resulting effect when I was testing)...
If you think it's better for me to create a merge/draft request lmk, I just figured to just ask around first before jumping into something more formal:
```
diff --git a/dlls/winex11.drv/mouse.c b/dlls/winex11.drv/mouse.c
index e1a5e70a6fe..899990af266 100644
--- a/dlls/winex11.drv/mouse.c
+++ b/dlls/winex11.drv/mouse.c
@@ -673,7 +673,8 @@ static void send_mouse_input( HWND hwnd, Window window, unsigned int state, INPU
static XcursorImage *create_xcursor_frame( HDC hdc, const ICONINFOEXW *iinfo, HANDLE icon,
HBITMAP hbmColor, unsigned char *color_bits, int color_size,
HBITMAP hbmMask, unsigned char *mask_bits, int mask_size,
- int width, int height, int istep )
+ int width, int height, int istep,
+ int x_width, int x_height)
{
XcursorImage *image, *ret = NULL;
DWORD delay_jiffies, num_steps;
@@ -681,15 +682,29 @@ static XcursorImage *create_xcursor_frame( HDC hdc, const ICONINFOEXW *iinfo, HA
BOOL has_alpha = FALSE;
XcursorPixel *ptr;
- image = pXcursorImageCreate( width, height );
+ image = pXcursorImageCreate( x_width, x_height );
if (!image)
{
ERR("X11 failed to produce a cursor frame!\n");
return NULL;
}
- image->xhot = iinfo->xHotspot;
- image->yhot = iinfo->yHotspot;
+ if (!x_width || !x_height || !width || !height)
+ {
+ return NULL;
+ }
+
+ if(iinfo->xHotspot > x_width) {
+ image->xhot = (XcursorDim)x_width - 1;
+ } else {
+ image->xhot = iinfo->xHotspot;
+ }
+
+ if(iinfo->yHotspot > y_width) {
+ image->yhot = (XcursorDim)y_width - 1;
+ } else {
+ image->yhot = iinfo->yHotspot;
+ }
image->delay = 100; /* fallback delay, 100 ms */
if (NtUserGetCursorFrameInfo(icon, istep, &delay_jiffies, &num_steps) != 0)
@@ -705,10 +720,18 @@ static XcursorImage *create_xcursor_frame( HDC hdc, const ICONINFOEXW *iinfo, HA
TRACE("Could not draw frame %d (walk past end of frames).\n", istep);
goto cleanup;
}
- memcpy( image->pixels, color_bits, color_size );
+
+ if (x_width >= width && x_height >= height) {
+ memcpy( image->pixels, color_bits, color_size );
+ } else {
+ for (y = 0; y < x_height; y++)
+ memcpy( image->pixels + y * x_width,
+ (XcursorPixel *)color_bits + y * width,
+ x_width * sizeof(XcursorPixel) );
+ }
/* check if the cursor frame was drawn with an alpha channel */
- for (i = 0, ptr = image->pixels; i < width * height; i++, ptr++)
+ for (i = 0, ptr = image->pixels; i < x_width * x_height; i++, ptr++)
if ((has_alpha = (*ptr & 0xff000000) != 0)) break;
/* if no alpha channel was drawn then generate it from the mask */
@@ -725,8 +748,8 @@ static XcursorImage *create_xcursor_frame( HDC hdc, const ICONINFOEXW *iinfo, HA
goto cleanup;
}
/* use the buffer to directly modify the XcursorImage alpha channel */
- for (y = 0, ptr = image->pixels; y < height; y++)
- for (x = 0; x < width; x++, ptr++)
+ for (y = 0, ptr = image->pixels; y < x_height; y++)
+ for (x = 0; x < x_width; x++, ptr++)
if (!((mask_bits[y * width_bytes + x / 8] << (x % 8)) & 0x80))
*ptr |= 0xff000000;
}
@@ -752,6 +775,27 @@ static Cursor create_xcursor_cursor( HDC hdc, const ICONINFOEXW *iinfo, HANDLE i
XcursorImages *images;
XcursorImage **imgs;
Cursor cursor = 0;
+ unsigned int x_width = 0;
+ unsigned int x_height = 0;
+ int w_width = width;
+ int w_height = height;
+
+ /* Clamp requested cursor frame dimensions to max best dimensions
+ * available by x to avoid hwcursor overflows (or forced sw cursor switch)
+ * with oversized cursors on draw */
+ if (scale_cursor_to_best_size ^ clamp_cursor_to_best_size) {
+ XQueryBestCursor( gdi_display, DefaultRootWindow( gdi_display ),
+ w_width, w_height, &x_width, &x_height );
+
+ /* Aggressively reassign cursor dimensions to max available */
+ if(scale_cursor_to_best_size) {
+ if (width > (int)x_width) w_width = (int)x_width;
+ if (height > (int)x_height) w_height = (int)x_height;
+ }
+ } else {
+ x_width = w_width;
+ x_height = w_height;
+ }
/* Retrieve the number of frames to render */
if (!NtUserGetCursorFrameInfo(icon, 0, &delay_jiffies, &nFrames)) return 0;
@@ -760,8 +804,8 @@ static Cursor create_xcursor_cursor( HDC hdc, const ICONINFOEXW *iinfo, HANDLE i
/* Allocate all of the resources necessary to obtain a cursor frame */
if (!(info = malloc( FIELD_OFFSET( BITMAPINFO, bmiColors[256] )))) goto cleanup;
info->bmiHeader.biSize = sizeof(BITMAPINFOHEADER);
- info->bmiHeader.biWidth = width;
- info->bmiHeader.biHeight = -height;
+ info->bmiHeader.biWidth = w_width;
+ info->bmiHeader.biHeight = -w_height;
info->bmiHeader.biPlanes = 1;
info->bmiHeader.biCompression = BI_RGB;
info->bmiHeader.biXPelsPerMeter = 0;
@@ -769,7 +813,7 @@ static Cursor create_xcursor_cursor( HDC hdc, const ICONINFOEXW *iinfo, HANDLE i
info->bmiHeader.biClrUsed = 0;
info->bmiHeader.biClrImportant = 0;
info->bmiHeader.biBitCount = 32;
- color_size = width * height * 4;
+ color_size = w_width * w_height * 4;
info->bmiHeader.biSizeImage = color_size;
hbmColor = NtGdiCreateDIBSection( hdc, NULL, 0, info, DIB_RGB_COLORS, 0, 0, 0, (void **)&color_bits );
if (!hbmColor)
@@ -787,7 +831,7 @@ static Cursor create_xcursor_cursor( HDC hdc, const ICONINFOEXW *iinfo, HANDLE i
info->bmiColors[1].rgbBlue = 0xff;
info->bmiColors[1].rgbReserved = 0;
- mask_size = ((width + 31) / 32 * 4) * height; /* width_bytes * height */
+ mask_size = ((width + 31) / 32 * 4) * w_height; /* width_bytes * height */
info->bmiHeader.biSizeImage = mask_size;
hbmMask = NtGdiCreateDIBSection( hdc, NULL, 0, info, DIB_RGB_COLORS, 0, 0, 0, (void **)&mask_bits );
if (!hbmMask)
@@ -802,7 +846,7 @@ static Cursor create_xcursor_cursor( HDC hdc, const ICONINFOEXW *iinfo, HANDLE i
imgs[i] = create_xcursor_frame( hdc, iinfo, icon,
hbmColor, color_bits, color_size,
hbmMask, mask_bits, mask_size,
- width, height, i );
+ w_width, w_height, i, (int)x_width, (int)x_height );
if (!imgs[i]) goto cleanup;
}
diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h
index 9436f35e1ca..cdcdf95f5c0 100644
--- a/dlls/winex11.drv/x11drv.h
+++ b/dlls/winex11.drv/x11drv.h
@@ -478,6 +478,8 @@ extern BOOL usexcomposite;
extern BOOL use_xfixes;
extern BOOL managed_mode;
extern BOOL private_color_map;
+extern BOOL clamp_cursor_to_best_size;
+extern BOOL scale_cursor_to_best_size;
extern int primary_monitor;
extern int copy_default_colors;
extern int alloc_system_colors;
diff --git a/dlls/winex11.drv/x11drv_main.c b/dlls/winex11.drv/x11drv_main.c
index 81008f45c33..0b1c87fef91 100644
--- a/dlls/winex11.drv/x11drv_main.c
+++ b/dlls/winex11.drv/x11drv_main.c
@@ -81,6 +81,8 @@ int primary_monitor = 0;
BOOL client_side_graphics = TRUE;
BOOL client_side_with_render = TRUE;
BOOL shape_layered_windows = TRUE;
+BOOL clamp_cursor_to_best_size = FALSE;
+BOOL scale_cursor_to_best_size = FALSE;
int copy_default_colors = 128;
int alloc_system_colors = 256;
int xrender_error_base = 0;
@@ -514,6 +516,12 @@ static void setup_options(void)
if (!get_config_key( hkey, appkey, "AllocSystemColors", buffer, sizeof(buffer) ))
alloc_system_colors = wcstol( buffer, NULL, 0 );
+ if (!get_config_key( hkey, appkey, "ClampCursorToBestSize", buffer, sizeof(buffer) ))
+ clamp_cursor_to_best_size = IS_OPTION_TRUE( buffer[0] );
+
+ if (!get_config_key( hkey, appkey, "ScaleCursorToBestSize", buffer, sizeof(buffer) ))
+ scale_cursor_to_best_size = IS_OPTION_TRUE( buffer[0] );
+
get_config_key( hkey, appkey, "InputStyle", input_style, sizeof(input_style) );
NtClose( appkey );
```
