From 48da8c40405c3901df87b6388128ca2d73a4f656 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Wed, 17 Aug 2022 12:53:35 +1000 Subject: Fix T98462: Save Screenshot (glReadPixels) fails under Wayland Use an off-screen buffer for the screen-shot operator. Reading from the front-buffer immediately after calling swap-buffers failed for GHOST/Wayland in some cases. While EGL can request to preserve the front-buffer while drawing, this isn't always supported. So workaround the problem by avoiding use of the front-buffer entirely. --- source/blender/editors/screen/screendump.c | 3 +-- source/blender/windowmanager/WM_api.h | 17 +++++++++++++ source/blender/windowmanager/intern/wm_draw.c | 33 +++++++++++++++++++++++++ source/blender/windowmanager/intern/wm_window.c | 3 +++ 4 files changed, 54 insertions(+), 2 deletions(-) diff --git a/source/blender/editors/screen/screendump.c b/source/blender/editors/screen/screendump.c index 5464d0a347d..38a9d8ba7ab 100644 --- a/source/blender/editors/screen/screendump.c +++ b/source/blender/editors/screen/screendump.c @@ -54,13 +54,12 @@ static int screenshot_data_create(bContext *C, wmOperator *op, ScrArea *area) { int dumprect_size[2]; - wmWindowManager *wm = CTX_wm_manager(C); wmWindow *win = CTX_wm_window(C); /* do redraw so we don't show popups/menus */ WM_redraw_windows(C); - uint *dumprect = WM_window_pixels_read(wm, win, dumprect_size); + uint *dumprect = WM_window_pixels_read_offscreen(C, win, dumprect_size); if (dumprect) { ScreenshotData *scd = MEM_callocN(sizeof(ScreenshotData), "screenshot"); diff --git a/source/blender/windowmanager/WM_api.h b/source/blender/windowmanager/WM_api.h index 44c5b86857d..0393be93bb5 100644 --- a/source/blender/windowmanager/WM_api.h +++ b/source/blender/windowmanager/WM_api.h @@ -134,7 +134,24 @@ void WM_window_pixel_sample_read(const wmWindowManager *wm, const int pos[2], float r_col[3]); +/** + * Read pixels from the front-buffer (fast). + * + * \note Internally this depends on the front-buffer state, + * for a slower but more reliable method of reading pixels, use #WM_window_pixels_read_offscreen. + * Fast pixel access may be preferred for file-save thumbnails. + * + * \warning Drawing (swap-buffers) immediately before calling this function causes + * the front-buffer state to be invalid under some EGL configurations. + */ uint *WM_window_pixels_read(struct wmWindowManager *wm, struct wmWindow *win, int r_size[2]); +/** + * Draw the window & read pixels from an off-screen buffer (slower than #WM_window_pixels_read). + * + * \note This is needed because the state of the front-buffer may be damaged + * (see in-line code comments for details). + */ +uint *WM_window_pixels_read_offscreen(struct bContext *C, struct wmWindow *win, int r_size[2]); /** * Support for native pixel size diff --git a/source/blender/windowmanager/intern/wm_draw.c b/source/blender/windowmanager/intern/wm_draw.c index 1bb405d1abc..48743c2649f 100644 --- a/source/blender/windowmanager/intern/wm_draw.c +++ b/source/blender/windowmanager/intern/wm_draw.c @@ -1191,6 +1191,39 @@ static void wm_draw_surface(bContext *C, wmSurface *surface) wm_surface_clear_drawable(); } +uint *WM_window_pixels_read_offscreen(bContext *C, wmWindow *win, int r_size[2]) +{ + /* NOTE(@campbellbarton): There is a problem reading the windows front-buffer after redrawing + * the window in some cases (typically to clear UI elements such as menus or search popup). + * With EGL `eglSurfaceAttrib(..)` may support setting the `EGL_SWAP_BEHAVIOR` attribute to + * `EGL_BUFFER_PRESERVED` however not all implementations support this. + * Requesting the ability with `EGL_SWAP_BEHAVIOR_PRESERVED_BIT` can even cause the EGL context + * not to initialize at all. + * Confusingly there are some cases where this *does* work, depending on the state of the window + * and prior calls to swap-buffers, however ensuring the state exactly as needed to satisfy a + * particular GPU back-end is fragile, see T98462. + * + * So provide an alternative to #WM_window_pixels_read that avoids using the front-buffer. */ + + /* Draw into an off-screen buffer and read it's contents. */ + r_size[0] = WM_window_pixels_x(win); + r_size[1] = WM_window_pixels_y(win); + + GPUOffScreen *offscreen = GPU_offscreen_create(r_size[0], r_size[1], false, GPU_RGBA8, NULL); + if (UNLIKELY(!offscreen)) { + return NULL; + } + + const uint rect_len = r_size[0] * r_size[1]; + uint *rect = MEM_mallocN(sizeof(*rect) * rect_len, __func__); + GPU_offscreen_bind(offscreen, false); + wm_draw_window_onscreen(C, win, -1); + GPU_offscreen_unbind(offscreen, false); + GPU_offscreen_read_pixels(offscreen, GPU_DATA_UBYTE, rect); + GPU_offscreen_free(offscreen); + return rect; +} + /** \} */ /* -------------------------------------------------------------------- */ diff --git a/source/blender/windowmanager/intern/wm_window.c b/source/blender/windowmanager/intern/wm_window.c index a4dba7145bd..723b606251e 100644 --- a/source/blender/windowmanager/intern/wm_window.c +++ b/source/blender/windowmanager/intern/wm_window.c @@ -1951,6 +1951,9 @@ void WM_window_pixel_sample_read(const wmWindowManager *wm, uint *WM_window_pixels_read(wmWindowManager *wm, wmWindow *win, int r_size[2]) { + /* WARNING: Reading from the front-buffer immediately after drawing may fail, + * for a slower but more reliable version of this function #WM_window_pixels_read_offscreen + * should be preferred. See it's comments for details on why it's needed, see also T98462. */ bool setup_context = wm->windrawable != win; if (setup_context) { -- cgit v1.2.3