From cef1bd0639058ce4cfa34f862f88b38b12d2a58f Mon Sep 17 00:00:00 2001 From: Manuel Alfayate Corchete Date: Fri, 8 Jan 2021 13:14:42 +0100 Subject: [PATCH] [KMS/DRM] Prevent creating another default cursor everytime a window is created. Other fixes and cleanups. --- src/video/kmsdrm/SDL_kmsdrmmouse.c | 172 +++++++------- src/video/kmsdrm/SDL_kmsdrmopengles.c | 3 +- src/video/kmsdrm/SDL_kmsdrmvideo.c | 218 +++--------------- src/video/kmsdrm/SDL_kmsdrmvideo.h | 4 +- .../kmsdrm_legacy/SDL_kmsdrm_legacy_mouse.c | 73 +++--- .../kmsdrm_legacy/SDL_kmsdrm_legacy_video.c | 214 ++--------------- .../kmsdrm_legacy/SDL_kmsdrm_legacy_video.h | 5 +- 7 files changed, 173 insertions(+), 516 deletions(-) diff --git a/src/video/kmsdrm/SDL_kmsdrmmouse.c b/src/video/kmsdrm/SDL_kmsdrmmouse.c index a8b7a0405..1546c7c2b 100644 --- a/src/video/kmsdrm/SDL_kmsdrmmouse.c +++ b/src/video/kmsdrm/SDL_kmsdrmmouse.c @@ -40,6 +40,21 @@ static void KMSDRM_FreeCursor(SDL_Cursor * cursor); static void KMSDRM_WarpMouse(SDL_Window * window, int x, int y); static int KMSDRM_WarpMouseGlobal(int x, int y); +/**************************************************************************************/ +/* BEFORE CODING ANYTHING MOUSE/CURSOR RELATED, REMEMBER THIS. */ +/* How does SDL manage cursors internally? First, mouse =! cursor. The mouse can have */ +/* many cursors in mouse->cursors. */ +/* -SDL tells us to create a cursor with KMSDRM_CreateCursor(). It can create many */ +/* cursosr with this, not only one. */ +/* -SDL stores those cursors in a cursors array, in mouse->cursors. */ +/* -Whenever it wants (or the programmer wants) takes a cursor from that array */ +/* and shows it on screen with KMSDRM_ShowCursor(). */ +/* KMSDRM_ShowCursor() simply shows or hides the cursor it receives: it does NOT */ +/* mind if it's mouse->cur_cursor, etc. */ +/* -If KMSDRM_ShowCursor() returns succesfully, that cursor becomes mouse->cur_cursor */ +/* and mouse->cursor_shown is 1. */ +/**************************************************************************************/ + /**********************************/ /* Atomic helper functions block. */ /**********************************/ @@ -98,8 +113,9 @@ KMSDRM_CreateDefaultCursor(void) return SDL_CreateCursor(default_cdata, default_cmask, DEFAULT_CWIDTH, DEFAULT_CHEIGHT, DEFAULT_CHOTX, DEFAULT_CHOTY); } -/* This simply gets the cursor soft-buffer ready. We don't copy it to a GBO BO until ShowCursor() - because the cusor GBM BO (living in dispata) is destroyed and recreated when we recreate windows, etc. */ +/* This simply gets the cursor soft-buffer ready. We don't copy it to a GBO BO + until ShowCursor() because the cusor GBM BO (living in dispata) is destroyed + and recreated when we recreate windows, etc. */ static SDL_Cursor * KMSDRM_CreateCursor(SDL_Surface * surface, int hot_x, int hot_y) { @@ -204,20 +220,16 @@ KMSDRM_InitCursor() KMSDRM_ShowCursor(mouse->cur_cursor); } -/* Show the specified cursor, or hide if cursor is NULL. - cur_cursor is the current cursor, and cursor is the new cursor. - A cursor is displayed on a display, so we have to add a pointer to dispdata - to the driverdata - */ +/* Show the specified cursor, or hide if cursor is NULL or has no focus. */ static int KMSDRM_ShowCursor(SDL_Cursor * cursor) { SDL_VideoDevice *video_device = SDL_GetVideoDevice(); - //SDL_VideoData *viddata = ((SDL_VideoData *)dev->driverdata); + //SDL_VideoData *viddata = ((SDL_VideoData *)video_device->driverdata); + SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0); SDL_Mouse *mouse; KMSDRM_CursorData *curdata; - SDL_VideoDisplay *display = NULL; - SDL_DisplayData *dispdata = NULL; + KMSDRM_FBInfo *fb; KMSDRM_PlaneInfo info = {0}; @@ -234,41 +246,28 @@ KMSDRM_ShowCursor(SDL_Cursor * cursor) return SDL_SetError("No mouse."); } - if (mouse->focus) { - display = SDL_GetDisplayForWindow(mouse->focus); - if (display) { - dispdata = (SDL_DisplayData*) display->driverdata; + /*********************************************************/ + /* Hide cursor if it's NULL or it has no focus(=winwow). */ + /*********************************************************/ + if (!cursor || !mouse->focus) { + if (dispdata->cursor_plane) { + /* Hide the drm cursor with no more considerations because + SDL_VideoQuit() takes us here after disabling the mouse + so there is no mouse->cur_cursor by now. */ + info.plane = dispdata->cursor_plane; + /* The rest of the members are zeroed, so this takes away the cursor + from the cursor plane. */ + drm_atomic_set_plane_props(&info); + if (drm_atomic_commit(video_device, SDL_TRUE, SDL_FALSE)) { + ret = SDL_SetError("Failed atomic commit in KMSDRM_ShowCursor."); + } } - } - - /**********************************/ - /* if cursor == NULL, HIDE cursor */ - /**********************************/ - if (!cursor) { - /* Hide CURRENT cursor, a cursor that is already on screen - and SDL is stored in mouse->cur_cursor. */ - if (mouse->cur_cursor && mouse->cur_cursor->driverdata) { - if (dispdata && dispdata->cursor_plane) { - info.plane = dispdata->cursor_plane; - /* The rest of the members are zeroed. */ - drm_atomic_set_plane_props(&info); - if (drm_atomic_commit(display->device, SDL_TRUE, SDL_FALSE)) - return SDL_SetError("Failed atomic commit in KMSDRM_ShowCursor."); - } - return 0; - } - return SDL_SetError("Couldn't find cursor to hide."); + return ret; } /************************************************/ /* If cursor != NULL, DO show cursor on display */ /************************************************/ - if (!display) { - return SDL_SetError("Could not get display for mouse."); - } - if (!dispdata) { - return SDL_SetError("Could not get display driverdata."); - } if (!dispdata->cursor_plane) { return SDL_SetError("Hardware cursor plane not initialized."); } @@ -279,7 +278,8 @@ KMSDRM_ShowCursor(SDL_Cursor * cursor) return SDL_SetError("Cursor not initialized properly."); } - /* Prepare a buffer we can dump to our GBM BO (different size, alpha premultiplication...) */ + /* Prepare a buffer we can dump to our GBM BO (different + size, alpha premultiplication...) */ bo_stride = KMSDRM_gbm_bo_get_stride(dispdata->cursor_bo); bufsize = bo_stride * curdata->h; @@ -315,8 +315,8 @@ KMSDRM_ShowCursor(SDL_Cursor * cursor) info.plane = dispdata->cursor_plane; info.crtc_id = dispdata->crtc->crtc->crtc_id; info.fb_id = fb->fb_id; - info.src_w = curdata->w; - info.src_h = curdata->h; + info.src_w = dispdata->cursor_w; + info.src_h = dispdata->cursor_h; info.crtc_x = mouse->x - curdata->hot_x; info.crtc_y = mouse->y - curdata->hot_y; info.crtc_w = curdata->w; @@ -324,7 +324,7 @@ KMSDRM_ShowCursor(SDL_Cursor * cursor) drm_atomic_set_plane_props(&info); - if (drm_atomic_commit(display->device, SDL_TRUE, SDL_FALSE)) { + if (drm_atomic_commit(video_device, SDL_TRUE, SDL_FALSE)) { ret = SDL_SetError("Failed atomic commit in KMSDRM_ShowCursor."); goto cleanup; } @@ -339,8 +339,7 @@ cleanup: return ret; } -/* We have destroyed the cursor by now, in KMSDRM_DestroyCursor. - This is only for freeing the SDL_cursor.*/ +/* This is only for freeing the SDL_cursor.*/ static void KMSDRM_FreeCursor(SDL_Cursor * cursor) { @@ -398,13 +397,40 @@ KMSDRM_WarpMouseGlobal(int x, int y) return 0; } +/* UNDO WHAT WE DID IN KMSDRM_InitMouse(). */ +void +KMSDRM_DeinitMouse(_THIS) +{ + SDL_VideoDevice *video_device = SDL_GetVideoDevice(); + SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0); + KMSDRM_PlaneInfo info = {0}; + SDL_Mouse *mouse = SDL_GetMouse(); + + /* 1- Destroy the curso GBM BO. */ + if (video_device && dispdata->cursor_bo) { + /* Unsethe the cursor BO from the cursor plane. + (The other members of the plane info are zeroed). */ + info.plane = dispdata->cursor_plane; + drm_atomic_set_plane_props(&info); + /* Wait until the cursor is unset from the cursor plane + before destroying it's BO. */ + if (drm_atomic_commit(video_device, SDL_TRUE, SDL_FALSE)) { + SDL_SetError("Failed atomic commit in KMSDRM_DenitMouse."); + } + /* ..and finally destroy the cursor DRM BO! */ + KMSDRM_gbm_bo_destroy(dispdata->cursor_bo); + dispdata->cursor_bo = NULL; + } + + /* 2- Free the cursor plane, on which the cursor was being shown. */ + if (dispdata->cursor_plane) { + free_plane(&dispdata->cursor_plane); + } +} + void KMSDRM_InitMouse(_THIS) { - /* FIXME: Using UDEV it should be possible to scan all mice - * but there's no point in doing so as there's no multimice support...yet! - */ - SDL_VideoDevice *dev = SDL_GetVideoDevice(); SDL_VideoData *viddata = ((SDL_VideoData *)dev->driverdata); SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0); @@ -420,7 +446,7 @@ KMSDRM_InitMouse(_THIS) /***************************************************************************/ /* REMEMBER TO BE SURE OF UNDOING ALL THESE STEPS PROPERLY BEFORE CALLING */ /* gbm_device_destroy, OR YOU WON'T BE ABLE TO CREATE A NEW ONE (ERROR -13 */ - /* ON gbm_create_device). */ + /* on gbm_create_device). */ /***************************************************************************/ /* 1- Init cursor plane, if we haven't yet. */ @@ -460,8 +486,16 @@ KMSDRM_InitMouse(_THIS) } } - /* SDL expects to set the default cursor on screen when we init the mouse. */ - SDL_SetDefaultCursor(KMSDRM_CreateDefaultCursor()); + /* SDL expects to set the default cursor on screen when we init the mouse, + but since we have moved the KMSDRM_InitMouse() call to KMSDRM_CreateWindow(), + we end up calling KMSDRM_InitMouse() every time we create a window, so we + have to prevent this from being done every time a new window is created. + If we don't, new default cursors would stack up on mouse->cursors and SDL + would have to hide and delete them at quit, not to mention the memory leak... */ + if(dispdata->set_default_cursor_pending) { + SDL_SetDefaultCursor(KMSDRM_CreateDefaultCursor()); + dispdata->set_default_cursor_pending = SDL_FALSE; + } return; @@ -472,40 +506,6 @@ cleanup: } } -void -KMSDRM_DeinitMouse(_THIS) -{ - SDL_VideoDevice *video_device = SDL_GetVideoDevice(); - SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0); - KMSDRM_PlaneInfo info = {0}; - - /*******************************************/ - /* UNDO WHAT WE DID IN KMSDRM_InitMouse(). */ - /*******************************************/ - - /* 1- Destroy the curso GBM BO. */ - if (video_device && dispdata->cursor_bo) { - /* Unsethe the cursor BO from the cursor plane. - (The other members of the plane info are zeroed). */ - info.plane = dispdata->cursor_plane; - drm_atomic_set_plane_props(&info); - /* Wait until the cursor is unset from the cursor plane - before destroying it's BO. */ - if (drm_atomic_commit(video_device, SDL_TRUE, SDL_FALSE)) { - SDL_SetError("Failed atomic commit in KMSDRM_DenitMouse."); - } - /* ..and finally destroy the cursor DRM BO! */ - KMSDRM_gbm_bo_destroy(dispdata->cursor_bo); - dispdata->cursor_bo = NULL; - } - - /* 2- Free the cursor plane, on which the cursor was being shown. */ - if (dispdata->cursor_plane) { - free_plane(&dispdata->cursor_plane); - } - -} - /* This is called when a mouse motion event occurs */ static void KMSDRM_MoveCursor(SDL_Cursor * cursor) diff --git a/src/video/kmsdrm/SDL_kmsdrmopengles.c b/src/video/kmsdrm/SDL_kmsdrmopengles.c index 788a03b9a..91916de46 100644 --- a/src/video/kmsdrm/SDL_kmsdrmopengles.c +++ b/src/video/kmsdrm/SDL_kmsdrmopengles.c @@ -287,7 +287,8 @@ KMSDRM_GLES_SwapWindowDoubleBuffered(_THIS, SDL_Window * window) /**********************************************************************************/ /* In double-buffer mode, atomic_commit will always be synchronous/blocking (ie: */ - /* won't return until the requested changes are really done). */ /* Also, there's no need to fence KMS or the GPU, because we won't be entering */ + /* won't return until the requested changes are really done). */ + /* Also, there's no need to fence KMS or the GPU, because we won't be entering */ /* game loop again (hence not building or executing a new cmdstring) until */ /* pageflip is done, so we don't need to protect the KMS/GPU access to the buffer.*/ /**********************************************************************************/ diff --git a/src/video/kmsdrm/SDL_kmsdrmvideo.c b/src/video/kmsdrm/SDL_kmsdrmvideo.c index b68646c7f..c94f8b215 100644 --- a/src/video/kmsdrm/SDL_kmsdrmvideo.c +++ b/src/video/kmsdrm/SDL_kmsdrmvideo.c @@ -57,6 +57,8 @@ #define KMSDRM_DRI_PATH "/dev/dri/" + + static int set_client_caps (int fd) { if (KMSDRM_drmSetClientCap(fd, DRM_CLIENT_CAP_ATOMIC, 1)) { @@ -164,161 +166,6 @@ get_driindex(void) return -ENOENT; } -#if 0 - -/**********************/ -/* DUMB BUFFER Block. */ -/**********************/ - -/* Create a dumb buffer, mmap the dumb buffer and fill it with pixels, */ -/* then create a KMS framebuffer wrapping the dumb buffer. */ -static dumb_buffer *KMSDRM_CreateDumbBuffer(_THIS) -{ - SDL_VideoData *viddata = ((SDL_VideoData *)_this->driverdata); - SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0); - - struct drm_mode_create_dumb create; - struct drm_mode_map_dumb map; - struct drm_mode_destroy_dumb destroy; - - dumb_buffer *ret = SDL_calloc(1, sizeof(*ret)); - if (!ret) { - SDL_OutOfMemory(); - return NULL; - } - - /* - * The create ioctl uses the combination of depth and bpp to infer - * a format; 24/32 refers to DRM_FORMAT_XRGB8888 as defined in - * the drm_fourcc.h header. These arguments are the same as given - * to drmModeAddFB, which has since been superseded by - * drmModeAddFB2 as the latter takes an explicit format token. - * - * We only specify these arguments; the driver calculates the - * pitch (also known as stride or row length) and total buffer size - * for us, also returning us the GEM handle. - */ - create = (struct drm_mode_create_dumb) { - .width = dispdata->mode.hdisplay, - .height = dispdata->mode.vdisplay, - .bpp = 32, - }; - - if (KMSDRM_drmIoctl(viddata->drm_fd, DRM_IOCTL_MODE_CREATE_DUMB, &create)) { - SDL_SetError("failed to create dumb buffer\n"); - goto err; - } - - ret->gem_handles[0] = create.handle; - ret->format = DRM_FORMAT_XRGB8888; - ret->modifier = DRM_FORMAT_MOD_LINEAR; - ret->width = create.width; - ret->height = create.height; - ret->pitches[0] = create.pitch; - - /* - * In order to map the buffer, we call an ioctl specific to the buffer - * type, which returns us a fake offset to use with the mmap syscall. - * mmap itself then works as you expect. - * - * Note this means it is not possible to map arbitrary offsets of - * buffers without specifically requesting it from the kernel. - */ - map = (struct drm_mode_map_dumb) { - .handle = ret->gem_handles[0], - }; - - if (KMSDRM_drmIoctl(viddata->drm_fd, DRM_IOCTL_MODE_MAP_DUMB, &map)) { - SDL_SetError("failed to get mmap offset for the dumb buffer."); - goto err_dumb; - } - - ret->dumb.mem = mmap(NULL, create.size, PROT_WRITE, MAP_SHARED, - viddata->drm_fd, map.offset); - - if (ret->dumb.mem == MAP_FAILED) { - SDL_SetError("failed to get mmap offset for the dumb buffer."); - goto err_dumb; - } - ret->dumb.size = create.size; - - return ret; - -err_dumb: - destroy = (struct drm_mode_destroy_dumb) { .handle = create.handle }; - KMSDRM_drmIoctl(viddata->drm_fd, DRM_IOCTL_MODE_DESTROY_DUMB, &destroy); -err: - SDL_free(ret); - return NULL; -} - -static void -KMSDRM_DestroyDumbBuffer(_THIS, dumb_buffer **buffer) -{ - SDL_VideoData *viddata = ((SDL_VideoData *)_this->driverdata); - - struct drm_mode_destroy_dumb destroy = { - .handle = (*buffer)->gem_handles[0], - }; - - KMSDRM_drmModeRmFB(viddata->drm_fd, (*buffer)->fb_id); - - munmap((*buffer)->dumb.mem, (*buffer)->dumb.size); - KMSDRM_drmIoctl(viddata->drm_fd, DRM_IOCTL_MODE_DESTROY_DUMB, &destroy); - free(*buffer); - *buffer = NULL; -} - -/* Using the CPU mapping, fill the dumb buffer with black pixels. */ -static void -KMSDRM_FillDumbBuffer(dumb_buffer *buffer) -{ - unsigned int x, y; - for (y = 0; y < buffer->height; y++) { - uint32_t *pix = (uint32_t *) ((uint8_t *) buffer->dumb.mem + (y * buffer->pitches[0])); - for (x = 0; x < buffer->width; x++) { - *pix++ = (0x00000000); - } - } -} - -static dumb_buffer *KMSDRM_CreateBuffer(_THIS) -{ - dumb_buffer *ret; - int err; - - SDL_VideoData *viddata = ((SDL_VideoData *)_this->driverdata); - - ret = KMSDRM_CreateDumbBuffer(_this); - - if (!ret) - return NULL; - - /* - * Wrap our GEM buffer in a KMS framebuffer, so we can then attach it - * to a plane. Here's where we get out fb_id! - */ - err = KMSDRM_drmModeAddFB2(viddata->drm_fd, ret->width, ret->height, - ret->format, ret->gem_handles, ret->pitches, - ret->offsets, &ret->fb_id, 0); - - if (err != 0 || ret->fb_id == 0) { - SDL_SetError("Failed AddFB2 on dumb buffer\n"); - goto err; - } - return ret; - -err: - KMSDRM_DestroyDumbBuffer(_this, &ret); - return NULL; -} - -/***************************/ -/* DUMB BUFFER Block ends. */ -/***************************/ - -#endif - /*********************************/ /* Atomic helper functions block */ /*********************************/ @@ -1161,20 +1008,8 @@ int KMSDRM_DisplayDataInit (_THIS, SDL_DisplayData *dispdata) { /* Figure out the default mode to be set. */ dispdata->mode = crtc->mode; - /* Find the connector's preferred mode, to be used in case the current mode - is not valid, or if restoring the current mode fails. - We can always count on the preferred mode! */ - for (i = 0; i < connector->count_modes; i++) { - if (connector->modes[i].type & DRM_MODE_TYPE_PREFERRED) { - dispdata->preferred_mode = connector->modes[i]; - } - } - - /* If the current CRTC's mode isn't valid, select the preferred - mode of the connector. */ - if (crtc->mode_valid == 0) { - dispdata->mode = dispdata->preferred_mode; - } + /* Save the original mode for restoration on quit. */ + dispdata->original_mode = dispdata->mode; if (dispdata->mode.hdisplay == 0 || dispdata->mode.vdisplay == 0 ) { ret = SDL_SetError("Couldn't get a valid connector videomode."); @@ -1360,15 +1195,17 @@ KMSDRM_DestroySurfaces(_THIS, SDL_Window *window) #if 1 /************************************************************/ /* Make the display plane point to the original TTY buffer. */ + /* We have to configure it's input and output scaling */ + /* parameters accordingly. */ /************************************************************/ plane_info.plane = dispdata->display_plane; plane_info.crtc_id = dispdata->crtc->crtc->crtc_id; plane_info.fb_id = dispdata->crtc->crtc->buffer_id; - plane_info.src_w = dispdata->mode.hdisplay; - plane_info.src_h = dispdata->mode.vdisplay; - plane_info.crtc_w = dispdata->mode.hdisplay; - plane_info.crtc_h = dispdata->mode.vdisplay; + plane_info.src_w = dispdata->original_mode.hdisplay; + plane_info.src_h = dispdata->original_mode.vdisplay; + plane_info.crtc_w = dispdata->original_mode.hdisplay; + plane_info.crtc_h = dispdata->original_mode.vdisplay; drm_atomic_set_plane_props(&plane_info); @@ -1426,9 +1263,16 @@ KMSDRM_CreateSurfaces(_THIS, SDL_Window * window) int ret = 0; - if (((window->flags & SDL_WINDOW_FULLSCREEN_DESKTOP) == SDL_WINDOW_FULLSCREEN_DESKTOP) || - ((window->flags & SDL_WINDOW_FULLSCREEN) == SDL_WINDOW_FULLSCREEN)) { + /* If the current window already has surfaces, destroy them before creating other. + This is mainly for ReconfigureWindow(), where we simply call CreateSurfaces() + for regenerating a window's surfaces. */ + if (windata->gs) { + KMSDRM_DestroySurfaces(_this, window); + } + if ( ((window->flags & SDL_WINDOW_FULLSCREEN_DESKTOP) == SDL_WINDOW_FULLSCREEN_DESKTOP) + || ((window->flags & SDL_WINDOW_FULLSCREEN) == SDL_WINDOW_FULLSCREEN)) + { width = dispdata->mode.hdisplay; height = dispdata->mode.vdisplay; } else { @@ -1436,11 +1280,14 @@ KMSDRM_CreateSurfaces(_THIS, SDL_Window * window) height = window->h; } - if (!KMSDRM_gbm_device_is_format_supported(viddata->gbm_dev, surface_fmt, surface_flags)) { - SDL_LogWarn(SDL_LOG_CATEGORY_VIDEO, "GBM surface format not supported. Trying anyway."); + if (!KMSDRM_gbm_device_is_format_supported(viddata->gbm_dev, + surface_fmt, surface_flags)) { + SDL_LogWarn(SDL_LOG_CATEGORY_VIDEO, + "GBM surface format not supported. Trying anyway."); } - windata->gs = KMSDRM_gbm_surface_create(viddata->gbm_dev, width, height, surface_fmt, surface_flags); + windata->gs = KMSDRM_gbm_surface_create(viddata->gbm_dev, + width, height, surface_fmt, surface_flags); if (!windata->gs) { return SDL_SetError("Could not create GBM surface"); @@ -1562,8 +1409,10 @@ KMSDRM_ReconfigureWindow( _THIS, SDL_Window * window) { windata->output_x = 0; } else { - /* Normal non-fullscreen windows are scaled using the CRTC, - so get output (CRTC) size and position, for AR correction. */ + /* Normal non-fullscreen windows are scaled using the PRIMARY PLANE, so here we store: + input size (ie: the size of the window buffers), + output size (ie: th mode configured on the CRTC), an X position to compensate for AR correction. + These are used when we set the PRIMARY PLANE props in SwapWindow() */ ratio = (float)window->w / (float)window->h; windata->src_w = window->w; windata->src_h = window->h; @@ -1637,6 +1486,13 @@ KMSDRM_VideoInit(_THIS) SDL_EVDEV_Init(); #endif + /* Since we create and show the default cursor on KMSDRM_InitMouse() and + we call KMSDRM_InitMouse() everytime we create a new window, we have + to be sure to create and show the default cursor only the first time. + If we don't, new default cursors would stack up on mouse->cursors and SDL + would have to hide and delete them at quit, not to mention the memory leak... */ + dispdata->set_default_cursor_pending = SDL_TRUE; + viddata->video_init = SDL_TRUE; cleanup: @@ -1821,7 +1677,7 @@ KMSDRM_CreateWindow(_THIS, SDL_Window * window) so we do it here. */ KMSDRM_InitMouse(_this); - /* Since we take cursor buffer way from the cursor plane and + /* Since we take cursor buffer away from the cursor plane and destroy the cursor GBM BO when we destroy a window, we must also manually re-show the cursor on screen, if necessary, when we create a window. */ diff --git a/src/video/kmsdrm/SDL_kmsdrmvideo.h b/src/video/kmsdrm/SDL_kmsdrmvideo.h index cc7a1f988..da08a9290 100644 --- a/src/video/kmsdrm/SDL_kmsdrmvideo.h +++ b/src/video/kmsdrm/SDL_kmsdrmvideo.h @@ -83,7 +83,7 @@ typedef struct connector { typedef struct SDL_DisplayData { drmModeModeInfo mode; - drmModeModeInfo preferred_mode; + drmModeModeInfo original_mode; plane *display_plane; plane *cursor_plane; @@ -110,6 +110,8 @@ typedef struct SDL_DisplayData struct gbm_bo *cursor_bo; uint64_t cursor_w, cursor_h; + SDL_bool set_default_cursor_pending; + } SDL_DisplayData; /* Driverdata info that gives KMSDRM-side support and substance to the SDL_Window. */ diff --git a/src/video/kmsdrm_legacy/SDL_kmsdrm_legacy_mouse.c b/src/video/kmsdrm_legacy/SDL_kmsdrm_legacy_mouse.c index 7fa725908..e84c9ad9a 100644 --- a/src/video/kmsdrm_legacy/SDL_kmsdrm_legacy_mouse.c +++ b/src/video/kmsdrm_legacy/SDL_kmsdrm_legacy_mouse.c @@ -38,6 +38,21 @@ static void KMSDRM_LEGACY_FreeCursor(SDL_Cursor * cursor); static void KMSDRM_LEGACY_WarpMouse(SDL_Window * window, int x, int y); static int KMSDRM_LEGACY_WarpMouseGlobal(int x, int y); +/**************************************************************************************/ +/* BEFORE CODING ANYTHING MOUSE/CURSOR RELATED, REMEMBER THIS. */ +/* How does SDL manage cursors internally? First, mouse =! cursor. The mouse can have */ +/* many cursors in mouse->cursors. */ +/* -SDL tells us to create a cursor with KMSDRM_CreateCursor(). It can create many */ +/* cursosr with this, not only one. */ +/* -SDL stores those cursors in a cursors array, in mouse->cursors. */ +/* -Whenever it wants (or the programmer wants) takes a cursor from that array */ +/* and shows it on screen with KMSDRM_ShowCursor(). */ +/* KMSDRM_ShowCursor() simply shows or hides the cursor it receives: it does NOT */ +/* mind if it's mouse->cur_cursor, etc. */ +/* -If KMSDRM_ShowCursor() returns succesfully, that cursor becomes mouse->cur_cursor */ +/* and mouse->cursor_shown is 1. */ +/**************************************************************************************/ + static SDL_Cursor * KMSDRM_LEGACY_CreateDefaultCursor(void) { @@ -68,43 +83,6 @@ void legacy_alpha_premultiply_ARGB8888 (uint32_t *pixel) { (*pixel) = (((uint32_t)A << 24) | ((uint32_t)R << 16) | ((uint32_t)G << 8)) | ((uint32_t)B << 0); } -/* Evaluate if a given cursor size is supported or not. - Notably, current Intel gfx only support 64x64 and up. */ -static SDL_bool -KMSDRM_LEGACY_IsCursorSizeSupported (int w, int h, uint32_t bo_format) { - - SDL_VideoDevice *dev = SDL_GetVideoDevice(); - SDL_VideoData *viddata = ((SDL_VideoData *)dev->driverdata); - SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0); - - int ret; - uint32_t bo_handle; - struct gbm_bo *bo = KMSDRM_LEGACY_gbm_bo_create(viddata->gbm_dev, w, h, bo_format, - GBM_BO_USE_CURSOR | GBM_BO_USE_WRITE); - - if (!bo) { - SDL_SetError("Could not create GBM cursor BO width size %dx%d for size testing", w, h); - goto cleanup; - } - - bo_handle = KMSDRM_LEGACY_gbm_bo_get_handle(bo).u32; - ret = KMSDRM_LEGACY_drmModeSetCursor(viddata->drm_fd, dispdata->crtc->crtc_id, bo_handle, w, h); - - if (ret) { - goto cleanup; - } - else { - KMSDRM_LEGACY_gbm_bo_destroy(bo); - return SDL_TRUE; - } - -cleanup: - if (bo) { - KMSDRM_LEGACY_gbm_bo_destroy(bo); - } - return SDL_FALSE; -} - /* This simply gets the cursor soft-buffer ready. We don't copy it to a GBO BO until ShowCursor() because the cusor GBM BO (living in dispata) is destroyed and recreated when we recreate windows, etc. */ @@ -212,7 +190,7 @@ KMSDRM_LEGACY_InitCursor() KMSDRM_LEGACY_ShowCursor(mouse->cur_cursor); } -/* Show the specified cursor, or hide if cursor is NULL. */ +/* Show the specified cursor, or hide if cursor is NULL or has no focus. */ static int KMSDRM_LEGACY_ShowCursor(SDL_Cursor * cursor) { @@ -255,10 +233,6 @@ KMSDRM_LEGACY_ShowCursor(SDL_Cursor * cursor) /************************************************/ /* If cursor != NULL, DO show cursor on display */ /************************************************/ - if (!dispdata) { - return SDL_SetError("Could not get display driverdata."); - } - curdata = (KMSDRM_LEGACY_CursorData *) cursor->driverdata; if (!curdata || !dispdata->cursor_bo) { @@ -318,8 +292,7 @@ cleanup: return ret; } -/* We have destroyed the cursor by now, in KMSDRM_DestroyCursor. - This is only for freeing the SDL_cursor.*/ +/* This is only for freeing the SDL_cursor.*/ static void KMSDRM_LEGACY_FreeCursor(SDL_Cursor * cursor) { @@ -450,8 +423,16 @@ KMSDRM_LEGACY_InitMouse(_THIS) } } - /* SDL expects to set the default cursor on screen when we init the mouse. */ - SDL_SetDefaultCursor(KMSDRM_LEGACY_CreateDefaultCursor()); + /* SDL expects to set the default cursor on screen when we init the mouse, + but since we have moved the KMSDRM_InitMouse() call to KMSDRM_CreateWindow(), + we end up calling KMSDRM_InitMouse() every time we create a window, so we + have to prevent this from being done every time a new window is created. + If we don't, new default cursors would stack up on mouse->cursors and SDL + would have to hide and delete them at quit, not to mention the memory leak... */ + if(dispdata->set_default_cursor_pending) { + SDL_SetDefaultCursor(KMSDRM_LEGACY_CreateDefaultCursor()); + dispdata->set_default_cursor_pending = SDL_FALSE; + } return; diff --git a/src/video/kmsdrm_legacy/SDL_kmsdrm_legacy_video.c b/src/video/kmsdrm_legacy/SDL_kmsdrm_legacy_video.c index b7607a357..17be847ad 100644 --- a/src/video/kmsdrm_legacy/SDL_kmsdrm_legacy_video.c +++ b/src/video/kmsdrm_legacy/SDL_kmsdrm_legacy_video.c @@ -62,145 +62,6 @@ #define KMSDRM_LEGACY_DRI_CARDPATHFMT "/dev/dri/card%d" #endif -#if 0 -/**************************************************************************************/ -/* UNUSED function because any plane compatible with the CRTC we chose is good enough */ -/* for us, whatever it's type is. Keep it here for documentation purposes. */ -/* It cold be needed sometime in the future, too. */ -/**************************************************************************************/ - -SDL_bool KMSDRM_IsPlanePrimary (_THIS, drmModePlane *plane) { - - SDL_VideoData *viddata = (SDL_VideoData *)_this->driverdata; - SDL_bool ret = SDL_FALSE; - int j; - - /* Find out if it's a primary plane. */ - drmModeObjectProperties *plane_props = - KMSDRM_LEGACY_drmModeObjectGetProperties(viddata->drm_fd, - plane->plane_id, DRM_MODE_OBJECT_ANY); - - for (j = 0; (j < plane_props->count_props); j++) { - - drmModePropertyRes *prop = KMSDRM_LEGACY_drmModeGetProperty(viddata->drm_fd, - plane_props->props[j]); - - if ((strcmp(prop->name, "type") == 0) && - (plane_props->prop_values[j] == DRM_PLANE_TYPE_PRIMARY)) - { - ret = SDL_TRUE; - } - - KMSDRM_LEGACY_drmModeFreeProperty(prop); - - } - - KMSDRM_LEGACY_drmModeFreeObjectProperties(plane_props); - - return ret; -} -#endif - -#if 0 -/***********************************************/ -/* Use these functions if you ever need info */ -/* about the available planes on your machine. */ -/***********************************************/ - -void print_plane_info(_THIS, drmModePlanePtr plane) -{ - char *plane_type; - drmModeRes *resources; - uint32_t type = 0; - SDL_VideoData *viddata = ((SDL_VideoData *)_this->driverdata); - int i; - - drmModeObjectPropertiesPtr props = KMSDRM_LEGACY_drmModeObjectGetProperties(viddata->drm_fd, - plane->plane_id, DRM_MODE_OBJECT_PLANE); - - /* Search the plane props for the plane type. */ - for (i = 0; i < props->count_props; i++) { - drmModePropertyPtr p = KMSDRM_LEGACY_drmModeGetProperty(viddata->drm_fd, props->props[i]); - if ((strcmp(p->name, "type") == 0)) { - type = props->prop_values[i]; - } - - KMSDRM_LEGACY_drmModeFreeProperty(p); - } - - switch (type) { - case DRM_PLANE_TYPE_OVERLAY: - plane_type = "overlay"; - break; - - case DRM_PLANE_TYPE_PRIMARY: - plane_type = "primary"; - break; - - case DRM_PLANE_TYPE_CURSOR: - plane_type = "cursor"; - break; - } - - - /* Remember that to present a plane on screen, it has to be - connected to a CRTC so the CRTC scans it, - scales it, etc... and presents it on screen. */ - - /* Now we look for the CRTCs supported by the plane. */ - resources = KMSDRM_LEGACY_drmModeGetResources(viddata->drm_fd); - if (!resources) - return; - - printf("--PLANE ID: %d\nPLANE TYPE: %s\nCRTC READING THIS PLANE: %d\nCRTCS SUPPORTED BY THIS PLANE: ", plane->plane_id, plane_type, plane->crtc_id); - for (i = 0; i < resources->count_crtcs; i++) { - if (plane->possible_crtcs & (1 << i)) { - uint32_t crtc_id = resources->crtcs[i]; - printf ("%d", crtc_id); - break; - } - } - - printf ("\n\n"); -} - -void get_planes_info(_THIS) -{ - drmModePlaneResPtr plane_resources; - uint32_t i; - - SDL_VideoData *viddata = ((SDL_VideoData *)_this->driverdata); - SDL_DisplayData *dispdata = (SDL_DisplayData *)SDL_GetDisplayDriverData(0); - - plane_resources = KMSDRM_LEGACY_drmModeGetPlaneResources(viddata->drm_fd); - if (!plane_resources) { - printf("drmModeGetPlaneResources failed: %s\n", strerror(errno)); - return; - } - - printf("--Number of planes found: %d-- \n", plane_resources->count_planes); - printf("--Usable CRTC that we have chosen: %d-- \n", dispdata->crtc->crtc_id); - - /* Iterate on all the available planes. */ - for (i = 0; (i < plane_resources->count_planes); i++) { - - uint32_t plane_id = plane_resources->planes[i]; - - drmModePlanePtr plane = KMSDRM_LEGACY_drmModeGetPlane(viddata->drm_fd, plane_id); - if (!plane) { - printf("drmModeGetPlane(%u) failed: %s\n", plane_id, strerror(errno)); - continue; - } - - /* Print plane info. */ - print_plane_info(_this, plane); - KMSDRM_LEGACY_drmModeFreePlane(plane); - } - - KMSDRM_LEGACY_drmModeFreePlaneResources(plane_resources); -} -#endif - static int check_modestting(int devindex) { @@ -545,13 +406,10 @@ int KMSDRM_LEGACY_DisplayDataInit (_THIS, SDL_DisplayData *dispdata) { SDL_VideoData *viddata = ((SDL_VideoData *)_this->driverdata); drmModeRes *resources = NULL; - drmModePlaneRes *plane_resources = NULL; drmModeEncoder *encoder = NULL; drmModeConnector *connector = NULL; drmModeCrtc *crtc = NULL; - uint32_t crtc_index = 0; - int ret = 0; unsigned i,j; @@ -560,8 +418,6 @@ int KMSDRM_LEGACY_DisplayDataInit (_THIS, SDL_DisplayData *dispdata) { dispdata->cursor_bo = NULL; - dispdata->plane_id = 0; - /* Open /dev/dri/cardNN (/dev/drmN if on OpenBSD) */ SDL_snprintf(viddata->devpath, sizeof(viddata->devpath), KMSDRM_LEGACY_DRI_CARDPATHFMT, viddata->devindex); @@ -662,7 +518,6 @@ int KMSDRM_LEGACY_DisplayDataInit (_THIS, SDL_DisplayData *dispdata) { for (i = 0; i < resources->count_crtcs; i++) { if (encoder->possible_crtcs & (1 << i)) { encoder->crtc_id = resources->crtcs[i]; - crtc_index = i; crtc = KMSDRM_LEGACY_drmModeGetCrtc(viddata->drm_fd, encoder->crtc_id); break; } @@ -680,57 +535,13 @@ int KMSDRM_LEGACY_DisplayDataInit (_THIS, SDL_DisplayData *dispdata) { /* Save the original mode for restoration on quit. */ dispdata->original_mode = dispdata->mode; - /* Find the connector's preferred mode, to be used in case the current mode - is not valid, or if restoring the current mode fails. */ - for (i = 0; i < connector->count_modes; i++) { - if (connector->modes[i].type & DRM_MODE_TYPE_PREFERRED) { - dispdata->preferred_mode = connector->modes[i]; - } - } - - /* If the current CRTC's mode isn't valid, select the preferred - mode of the connector. */ - if (crtc->mode_valid == 0) { - dispdata->mode = dispdata->preferred_mode; - } - if (dispdata->mode.hdisplay == 0 || dispdata->mode.vdisplay == 0 ) { ret = SDL_SetError("Couldn't get a valid connector videomode."); goto cleanup; } - /*******************************************************/ - /* Look for a plane that can be connected to our CRTC. */ - /*******************************************************/ - plane_resources = KMSDRM_LEGACY_drmModeGetPlaneResources(viddata->drm_fd); - - for (i = 0; (i < plane_resources->count_planes); i++) { - - drmModePlane *plane = KMSDRM_LEGACY_drmModeGetPlane(viddata->drm_fd, - plane_resources->planes[i]); - - /* 1 - Does this plane support our CRTC? - 2 - Is this plane unused or used by our CRTC? Both possibilities are good. - We don't mind if it's primary or overlay. */ - if ((plane->possible_crtcs & (1 << crtc_index)) && - (plane->crtc_id == crtc->crtc_id || plane->crtc_id == 0 )) - { - dispdata->plane_id = plane->plane_id; - break; - } - - KMSDRM_LEGACY_drmModeFreePlane(plane); - } - - KMSDRM_LEGACY_drmModeFreePlaneResources(plane_resources); - - if (!dispdata->plane_id) { - ret = SDL_SetError("Could not locate a primary plane compatible with active CRTC."); - goto cleanup; - } - - /* Store the connector and crtc for future use. These and the plane_id is - all we keep from this function, and these are just structs, inoffensive to VK. */ + /* Store the connector and crtc for future use. These are all we keep + from this function, and these are just structs, inoffensive to VK. */ dispdata->connector = connector; dispdata->crtc = crtc; @@ -900,15 +711,15 @@ KMSDRM_LEGACY_CreateSurfaces(_THIS, SDL_Window * window) int ret = 0; /* If the current window already has surfaces, destroy them before creating other. - This is mainly for ReconfigureWindow, where we simply call CreateSurfaces() + This is mainly for ReconfigureWindow(), where we simply call CreateSurfaces() for regenerating a window's surfaces. */ if (windata->gs) { KMSDRM_LEGACY_DestroySurfaces(_this, window); } - if (((window->flags & SDL_WINDOW_FULLSCREEN_DESKTOP) == SDL_WINDOW_FULLSCREEN_DESKTOP) || - ((window->flags & SDL_WINDOW_FULLSCREEN) == SDL_WINDOW_FULLSCREEN)) { - + if ( ((window->flags & SDL_WINDOW_FULLSCREEN_DESKTOP) == SDL_WINDOW_FULLSCREEN_DESKTOP) + || ((window->flags & SDL_WINDOW_FULLSCREEN) == SDL_WINDOW_FULLSCREEN)) + { width = dispdata->mode.hdisplay; height = dispdata->mode.vdisplay; } else { @@ -1001,6 +812,13 @@ KMSDRM_LEGACY_VideoInit(_THIS) SDL_EVDEV_Init(); #endif + /* Since we create and show the default cursor on KMSDRM_InitMouse() and + we call KMSDRM_InitMouse() everytime we create a new window, we have + to be sure to create and show the default cursor only the first time. + If we don't, new default cursors would stack up on mouse->cursors and SDL + would have to hide and delete them at quit, not to mention the memory leak... */ + dispdata->set_default_cursor_pending = SDL_TRUE; + viddata->video_init = SDL_TRUE; cleanup: @@ -1285,9 +1103,9 @@ KMSDRM_LEGACY_CreateWindow(_THIS, SDL_Window * window) /* LEGACY-only hardware, so never use drmModeSetPlane(). */ /***************************************************************************/ - ret = KMSDRM_LEGACY_drmModeSetCrtc(viddata->drm_fd, dispdata->crtc->crtc_id, - /*fb_info->fb_id*/ -1, 0, 0, &dispdata->connector->connector_id, 1, - &dispdata->mode); + ret = KMSDRM_LEGACY_drmModeSetCrtc(viddata->drm_fd, dispdata->crtc->crtc_id, + /*fb_info->fb_id*/ -1, 0, 0, &dispdata->connector->connector_id, 1, + &dispdata->mode); if (ret) { SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Could not set CRTC"); diff --git a/src/video/kmsdrm_legacy/SDL_kmsdrm_legacy_video.h b/src/video/kmsdrm_legacy/SDL_kmsdrm_legacy_video.h index 53a30eab4..e1b7d0af9 100644 --- a/src/video/kmsdrm_legacy/SDL_kmsdrm_legacy_video.h +++ b/src/video/kmsdrm_legacy/SDL_kmsdrm_legacy_video.h @@ -62,12 +62,9 @@ typedef struct SDL_DisplayData drmModeCrtc *crtc; drmModeModeInfo mode; drmModeModeInfo original_mode; - drmModeModeInfo preferred_mode; drmModeCrtc *saved_crtc; /* CRTC to restore on quit */ - uint32_t plane_id; /* ID of the primary plane used by the CRTC */ - SDL_bool gbm_init; /* DRM & GBM cursor stuff lives here, not in an SDL_Cursor's driverdata struct, @@ -79,6 +76,8 @@ typedef struct SDL_DisplayData SDL_bool modeset_pending; + SDL_bool set_default_cursor_pending; + } SDL_DisplayData; typedef struct SDL_WindowData