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 ); ```