From 0ed8d0cadfb6fc60ee77cd2c2e24e153175e5547 Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Fri, 2 Feb 2024 17:06:18 +0300 Subject: [PATCH 01/55] picom: post-process and free the corner radius rules list to make conditions based on non-standard atoms in this list work. --- src/picom.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/picom.c b/src/picom.c index 4e7a1b9af5..13dd0a183d 100644 --- a/src/picom.c +++ b/src/picom.c @@ -2249,6 +2249,7 @@ static session_t *session_init(int argc, char **argv, Display *dpy, c2_list_postprocess(ps, ps->o.window_shader_fg_rules) && c2_list_postprocess(ps, ps->o.opacity_rules) && c2_list_postprocess(ps, ps->o.rounded_corners_blacklist) && + c2_list_postprocess(ps, ps->o.corner_radius_rules) && c2_list_postprocess(ps, ps->o.focus_blacklist))) { log_error("Post-processing of conditionals failed, some of your rules " "might not work"); @@ -2664,6 +2665,7 @@ static void session_destroy(session_t *ps) { c2_list_free(&ps->o.paint_blacklist, NULL); c2_list_free(&ps->o.unredir_if_possible_blacklist, NULL); c2_list_free(&ps->o.rounded_corners_blacklist, NULL); + c2_list_free(&ps->o.corner_radius_rules, NULL); c2_list_free(&ps->o.window_shader_fg_rules, free); // Free tracked atom list From eb39426b08a77563fbbe8e74320a3799663b8c47 Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Fri, 2 Feb 2024 23:55:29 +0300 Subject: [PATCH 02/55] backend: gl: inherit image's inner properties in the gl_image_decouple function Image decouple should keep all the image properies from the source image, so shader must be copied. And there are also some internal properties what should be inherited but wasn't. In particular this prevents images from losing their shaders when alpha is applied. Fixes #1174 --- src/backend/gl/gl_common.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/backend/gl/gl_common.c b/src/backend/gl/gl_common.c index 73bfea9446..de1ff73a52 100644 --- a/src/backend/gl/gl_common.c +++ b/src/backend/gl/gl_common.c @@ -1013,9 +1013,11 @@ static inline void gl_image_decouple(backend_t *base, struct backend_image *img) auto new_tex = ccalloc(1, struct gl_texture); new_tex->texture = gl_new_texture(GL_TEXTURE_2D); - new_tex->y_inverted = true; + new_tex->y_inverted = inner->y_inverted; + new_tex->has_alpha = inner->has_alpha; new_tex->height = inner->height; new_tex->width = inner->width; + new_tex->shader = inner->shader; new_tex->refcount = 1; new_tex->user_data = gd->decouple_texture_user_data(base, inner->user_data); From c73840005811ff6d2dbe3bab20d43ebbcad73a1a Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Tue, 6 Feb 2024 12:46:29 +0000 Subject: [PATCH 03/55] backend: clarify what clone_image does Signed-off-by: Yuxuan Shui --- src/backend/backend.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/backend/backend.h b/src/backend/backend.h index 5896e19d64..fba17bbd9d 100644 --- a/src/backend/backend.h +++ b/src/backend/backend.h @@ -335,9 +335,10 @@ struct backend_operations { bool (*image_op)(backend_t *backend_data, enum image_operations op, void *image_data, const region_t *reg_op, const region_t *reg_visible, void *args); - /// Create another instance of the `image_data`. All `image_op` and - /// `set_image_property` calls on the returned image should not affect the - /// original image + /// Create another instance of the `image_data`. The newly created image + /// inherits its content and all image properties from the image being + /// cloned. All `image_op` and `set_image_property` calls on the + /// returned image should not affect the original image. void *(*clone_image)(backend_t *base, const void *image_data, const region_t *reg_visible); From 0d3d18cd88057e39dd7f5ac99c810a23198f9819 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Tue, 6 Feb 2024 13:19:49 +0000 Subject: [PATCH 04/55] backend: gl: remove unused field from gl_win_shader_t Signed-off-by: Yuxuan Shui --- src/backend/gl/gl_common.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/backend/gl/gl_common.h b/src/backend/gl/gl_common.h index a1f396b8d6..9916439432 100644 --- a/src/backend/gl/gl_common.h +++ b/src/backend/gl/gl_common.h @@ -29,8 +29,6 @@ static inline GLint glGetUniformLocationChecked(GLuint p, const char *name) { // Program and uniforms for window shader typedef struct { - UT_hash_handle hh; - uint32_t id; GLuint prog; GLint uniform_opacity; GLint uniform_invert_color; From 9204426caf6a80e858487772c4d24e98582173d5 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Tue, 6 Feb 2024 13:04:10 +0000 Subject: [PATCH 05/55] backend: gl: don't use present shader for decoupling Present shader optionally does dithering, but that's not needed for decoupling. Signed-off-by: Yuxuan Shui --- src/backend/gl/gl_common.c | 30 ++++++++++++++++++++---------- src/backend/gl/gl_common.h | 1 + 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/backend/gl/gl_common.c b/src/backend/gl/gl_common.c index de1ff73a52..22c74ba5af 100644 --- a/src/backend/gl/gl_common.c +++ b/src/backend/gl/gl_common.c @@ -885,24 +885,35 @@ bool gl_init(struct gl_data *gd, session_t *ps) { glUseProgram(0); gd->dithered_present = ps->o.dithered_present; + gd->dummy_prog = + gl_create_program_from_strv((const char *[]){present_vertex_shader, NULL}, + (const char *[]){dummy_frag, NULL}); + if (!gd->dummy_prog) { + log_error("Failed to create the dummy shader"); + return false; + } if (gd->dithered_present) { gd->present_prog = gl_create_program_from_strv( (const char *[]){present_vertex_shader, NULL}, (const char *[]){present_frag, dither_glsl, NULL}); } else { - gd->present_prog = gl_create_program_from_strv( - (const char *[]){present_vertex_shader, NULL}, - (const char *[]){dummy_frag, NULL}); + gd->present_prog = gd->dummy_prog; } if (!gd->present_prog) { log_error("Failed to create the present shader"); return false; } - pml = glGetUniformLocationChecked(gd->present_prog, "projection"); - glUseProgram(gd->present_prog); - glUniform1i(glGetUniformLocationChecked(gd->present_prog, "tex"), 0); + + pml = glGetUniformLocationChecked(gd->dummy_prog, "projection"); + glUseProgram(gd->dummy_prog); + glUniform1i(glGetUniformLocationChecked(gd->dummy_prog, "tex"), 0); glUniformMatrix4fv(pml, 1, false, projection_matrix[0]); - glUseProgram(0); + if (gd->present_prog != gd->dummy_prog) { + pml = glGetUniformLocationChecked(gd->present_prog, "projection"); + glUseProgram(gd->present_prog); + glUniform1i(glGetUniformLocationChecked(gd->present_prog, "tex"), 0); + glUniformMatrix4fv(pml, 1, false, projection_matrix[0]); + } gd->shadow_shader.prog = gl_create_program_from_str(present_vertex_shader, shadow_colorization_frag); @@ -912,7 +923,6 @@ bool gl_init(struct gl_data *gd, session_t *ps) { glUseProgram(gd->shadow_shader.prog); glUniform1i(glGetUniformLocationChecked(gd->shadow_shader.prog, "tex"), 0); glUniformMatrix4fv(pml, 1, false, projection_matrix[0]); - glUseProgram(0); glBindFragDataLocation(gd->shadow_shader.prog, 0, "out_color"); gd->brightness_shader.prog = @@ -925,6 +935,7 @@ bool gl_init(struct gl_data *gd, session_t *ps) { glUseProgram(gd->brightness_shader.prog); glUniform1i(glGetUniformLocationChecked(gd->brightness_shader.prog, "tex"), 0); glUniformMatrix4fv(pml, 1, false, projection_matrix[0]); + glUseProgram(0); // Set up the size and format of the back texture @@ -1026,8 +1037,7 @@ static inline void gl_image_decouple(backend_t *base, struct backend_image *img) GL_BGRA, GL_UNSIGNED_BYTE, NULL); glBindTexture(GL_TEXTURE_2D, 0); - assert(gd->present_prog); - glUseProgram(gd->present_prog); + glUseProgram(gd->dummy_prog); glBindTexture(GL_TEXTURE_2D, inner->texture); GLuint fbo; diff --git a/src/backend/gl/gl_common.h b/src/backend/gl/gl_common.h index a1f396b8d6..cacee38a72 100644 --- a/src/backend/gl/gl_common.h +++ b/src/backend/gl/gl_common.h @@ -112,6 +112,7 @@ struct gl_data { GLuint frame_timing[2]; int current_frame_timing; GLuint present_prog; + GLuint dummy_prog; bool dithered_present; From f265e049a8e409b6601de382ee02c6082cd39f7e Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Tue, 6 Feb 2024 13:16:12 +0000 Subject: [PATCH 06/55] backend: gl: don't leak resources Signed-off-by: Yuxuan Shui --- src/backend/gl/gl_common.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/backend/gl/gl_common.c b/src/backend/gl/gl_common.c index 22c74ba5af..badbd9bb6c 100644 --- a/src/backend/gl/gl_common.c +++ b/src/backend/gl/gl_common.c @@ -989,10 +989,25 @@ void gl_deinit(struct gl_data *gd) { gl_destroy_window_shader(&gd->base, gd->default_shader); gd->default_shader = NULL; } + glDeleteProgram(gd->dummy_prog); + if (gd->present_prog != gd->dummy_prog) { + glDeleteProgram(gd->present_prog); + } + gd->dummy_prog = 0; + gd->present_prog = 0; + + glDeleteProgram(gd->fill_shader.prog); + glDeleteProgram(gd->brightness_shader.prog); + glDeleteProgram(gd->shadow_shader.prog); + gd->fill_shader.prog = 0; + gd->brightness_shader.prog = 0; + gd->shadow_shader.prog = 0; glDeleteTextures(1, &gd->default_mask_texture); glDeleteTextures(1, &gd->back_texture); + glDeleteQueries(2, gd->frame_timing); + gl_check_err(); } From 023103c620749fd5eb1956c255a9cc5fcd6903a4 Mon Sep 17 00:00:00 2001 From: Jose Maldonado aka Yukiteru Date: Fri, 9 Feb 2024 14:16:07 -0400 Subject: [PATCH 07/55] core: use pthread_setschedparam on OpenBSD OpenBSD don't have support for sched_getparam(), sched_setparam(), or sched_setscheduler() functions (yet). In this case, we need use pthead-equivalents for real-time sched for picom. Theses changes add this support. Authored-by: Jose Maldonado aka Yukiteru Signed-off-by: Yuxuan Shui --- src/meson.build | 4 ++++ src/picom.c | 18 +++++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/meson.build b/src/meson.build index a2f9c36c73..b9a5a77171 100644 --- a/src/meson.build +++ b/src/meson.build @@ -86,6 +86,10 @@ elif (host_system == 'freebsd' or host_system == 'netbsd' or cflags += ['-DHAS_KQUEUE'] endif +if host_system == 'openbsd' + deps += [dependency('threads', required: true)] +endif + subdir('backend') picom = executable('picom', srcs, c_args: cflags, diff --git a/src/picom.c b/src/picom.c index 13dd0a183d..c2c17e2824 100644 --- a/src/picom.c +++ b/src/picom.c @@ -35,6 +35,9 @@ #include #include #include +#ifdef __OpenBSD__ +#include +#endif #include #include @@ -2588,17 +2591,30 @@ void set_rr_scheduling(void) { int ret; struct sched_param param; +#ifndef __OpenBSD__ ret = sched_getparam(0, ¶m); +#else + int old_policy; + ret = pthread_getschedparam(pthread_self(), &old_policy, ¶m); +#endif + if (ret != 0) { log_debug("Failed to get old scheduling priority"); return; } param.sched_priority = priority; + +#ifndef __OpenBSD__ ret = sched_setscheduler(0, SCHED_RR, ¶m); +#else + ret = pthread_setschedparam(pthread_self(), SCHED_RR, ¶m); +#endif + if (ret != 0) { log_info("Failed to set real-time scheduling priority to %d. Consider " - "giving picom the CAP_SYS_NICE capability", + "giving picom the CAP_SYS_NICE capability or equivalent " + "support.", priority); return; } From 709f0168d96fa42e40261c65b345c5fa14418300 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Fri, 9 Feb 2024 02:27:58 +0000 Subject: [PATCH 08/55] ci: build on OpenBSD Building on OpenBSD fails currently. Signed-off-by: Yuxuan Shui --- .builds/openbsd.yml | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 .builds/openbsd.yml diff --git a/.builds/openbsd.yml b/.builds/openbsd.yml new file mode 100644 index 0000000000..d73c57692a --- /dev/null +++ b/.builds/openbsd.yml @@ -0,0 +1,23 @@ +image: openbsd/latest +packages: + - libev + - xcb + - meson + - pkgconf + - cmake + - uthash + - libconfig + - dbus + - pcre2 +sources: + - https://github.com/yshui/picom +tasks: + - setup: | + cd picom + CPPFLAGS="-I/usr/local/include" LDFLAGS="-L/usr/local/lib" meson -Dunittest=true build + - build: | + cd picom + ninja -C build + - unittest: | + cd picom + ninja -C build test From dff77aae27cf064941798c066a248283b1996318 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sat, 10 Feb 2024 20:29:39 +0000 Subject: [PATCH 09/55] core: use pthread_setschedparam across the board I think I was trying to avoid introducing pthread as a dependency, but now we are using pthread for SGI_video_sync thread anyway. Let's remove the ifdefs. Signed-off-by: Yuxuan Shui --- src/meson.build | 8 ++------ src/picom.c | 16 ++-------------- 2 files changed, 4 insertions(+), 20 deletions(-) diff --git a/src/meson.build b/src/meson.build index b9a5a77171..dd10eaec3c 100644 --- a/src/meson.build +++ b/src/meson.build @@ -23,7 +23,7 @@ required_xcb_packages = [ # Some XCB packages are here because their versioning differs (see check below). required_packages = [ 'pixman-1', 'x11', 'x11-xcb', 'xcb-image', 'xcb-renderutil', 'xcb-util', - 'xext' + 'xext', 'threads', ] foreach i : required_packages @@ -59,7 +59,7 @@ endif if get_option('opengl') cflags += ['-DCONFIG_OPENGL', '-DGL_GLEXT_PROTOTYPES'] - deps += [dependency('gl', required: true), dependency('egl', required: true), dependency('threads', required:true)] + deps += [dependency('gl', required: true), dependency('egl', required: true)] srcs += [ 'opengl.c' ] endif @@ -86,10 +86,6 @@ elif (host_system == 'freebsd' or host_system == 'netbsd' or cflags += ['-DHAS_KQUEUE'] endif -if host_system == 'openbsd' - deps += [dependency('threads', required: true)] -endif - subdir('backend') picom = executable('picom', srcs, c_args: cflags, diff --git a/src/picom.c b/src/picom.c index c2c17e2824..cdacf75d0c 100644 --- a/src/picom.c +++ b/src/picom.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -35,9 +36,6 @@ #include #include #include -#ifdef __OpenBSD__ -#include -#endif #include #include @@ -2590,14 +2588,8 @@ void set_rr_scheduling(void) { int ret; struct sched_param param; - -#ifndef __OpenBSD__ - ret = sched_getparam(0, ¶m); -#else int old_policy; ret = pthread_getschedparam(pthread_self(), &old_policy, ¶m); -#endif - if (ret != 0) { log_debug("Failed to get old scheduling priority"); return; @@ -2605,12 +2597,7 @@ void set_rr_scheduling(void) { param.sched_priority = priority; -#ifndef __OpenBSD__ - ret = sched_setscheduler(0, SCHED_RR, ¶m); -#else ret = pthread_setschedparam(pthread_self(), SCHED_RR, ¶m); -#endif - if (ret != 0) { log_info("Failed to set real-time scheduling priority to %d. Consider " "giving picom the CAP_SYS_NICE capability or equivalent " @@ -2618,6 +2605,7 @@ void set_rr_scheduling(void) { priority); return; } + log_info("Set real-time scheduling priority to %d", priority); } From fcd51e7373d14881e13ad086304d3511355b5f1b Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sat, 10 Feb 2024 10:57:33 +0000 Subject: [PATCH 10/55] build: add libepoxy Add libepoxy dependency to CI manifest and Nix. For Nix, we need to set shellHook to workaround a NixOS limitation, see: https://github.com/NixOS/nixpkgs/issues/287763 Signed-off-by: Yuxuan Shui --- .builds/freebsd.yml | 1 + .editorconfig | 3 +++ flake.nix | 18 ++++++++++++------ 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/.builds/freebsd.yml b/.builds/freebsd.yml index 41c3d9277b..546df0979a 100644 --- a/.builds/freebsd.yml +++ b/.builds/freebsd.yml @@ -12,6 +12,7 @@ packages: - uthash - libconfig - libglvnd + - libepoxy - dbus - pcre sources: diff --git a/.editorconfig b/.editorconfig index b40d8e8bf5..fea97be835 100644 --- a/.editorconfig +++ b/.editorconfig @@ -3,3 +3,6 @@ root = true indent_style = tab indent_size = 8 max_line_length = 90 +[*.nix] +indent_style = space +indent_size = 2 diff --git a/flake.nix b/flake.nix index 324498e010..414cacc98d 100644 --- a/flake.nix +++ b/flake.nix @@ -24,16 +24,22 @@ overlays = [ overlay ]; in rec { inherit overlay overlays; - defaultPackage = pkgs.picom.overrideAttrs { + defaultPackage = pkgs.picom.overrideAttrs (o: { version = "11"; src = ./.; - }; + buildInputs = o.buildInputs ++ [ pkgs.libepoxy ]; + }); devShell = defaultPackage.overrideAttrs { - buildInputs = defaultPackage.buildInputs ++ [ - pkgs.clang-tools_17 - pkgs.llvmPackages_17.clang-unwrapped.python - ]; + buildInputs = defaultPackage.buildInputs ++ (with pkgs; [ + clang-tools_17 + llvmPackages_17.clang-unwrapped.python + ]); hardeningDisable = [ "fortify" ]; + shellHook = '' + # Workaround a NixOS limitation on sanitizers: + # See: https://github.com/NixOS/nixpkgs/issues/287763 + export LD_LIBRARY_PATH+=":/run/opengl-driver/lib" + ''; }; }); } From eb723eee296e4095cd4f03de5be3ed7048762851 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sat, 10 Feb 2024 20:36:02 +0000 Subject: [PATCH 11/55] backend: gl: use libepoxy There is actually no specification what symbols are exported from a libGL implementation. The (extremely outdated) OpenGL ABI specification says only GL 1.2 functions are guaranteed. Don't know how relevant that is now, but different libGL implementations do export different set of symbols. On Linux we are most likely to be linked with libglvnd, which has everything we need. But on other platforms this is not necessarily the case, for example on OpenBSD we are missing glGetQueryObjectui64v. Use libepoxy so we can outsource this problem and never worry about it ever again. Plus it also saves us from calling GetProcAddress ourselves. Changes other than trivial build fixes I have to make: 1. Can't use eglCreatePlatformWindowSurface/eglGetPlatformDisplay. libepoxy checks for EGL 1.5 when resolving these functions. But without a current context, libepoxy assumes we only have EGL 1.4. This creates a chicken and egg problem - we need a display to call eglGetPlatformDisplay. We have to use the *EXT version instead. Signed-off-by: Yuxuan Shui --- src/backend/gl/egl.c | 64 +++++++------------------------------- src/backend/gl/egl.h | 10 ++---- src/backend/gl/gl_common.c | 3 +- src/backend/gl/gl_common.h | 3 +- src/backend/gl/glx.c | 42 ------------------------- src/backend/gl/glx.h | 27 ++-------------- src/log.c | 29 ++++++----------- src/meson.build | 4 +-- src/opengl.h | 4 +-- src/vblank.c | 9 +----- 10 files changed, 32 insertions(+), 163 deletions(-) diff --git a/src/backend/gl/egl.c b/src/backend/gl/egl.c index ca70359173..d0a1ef3519 100644 --- a/src/backend/gl/egl.c +++ b/src/backend/gl/egl.c @@ -36,12 +36,6 @@ struct egl_data { EGLContext ctx; }; -static PFNGLEGLIMAGETARGETTEXSTORAGEEXTPROC glEGLImageTargetTexStorage = NULL; -static PFNEGLCREATEIMAGEKHRPROC eglCreateImageProc = NULL; -static PFNEGLDESTROYIMAGEKHRPROC eglDestroyImageProc = NULL; -static PFNEGLGETPLATFORMDISPLAYPROC eglGetPlatformDisplayProc = NULL; -static PFNEGLCREATEPLATFORMWINDOWSURFACEPROC eglCreatePlatformWindowSurfaceProc = NULL; - const char *eglGetErrorString(EGLint error) { #define CASE_STR(value) \ case value: return #value; @@ -74,7 +68,7 @@ static void egl_release_image(backend_t *base, struct gl_texture *tex) { struct egl_pixmap *p = tex->user_data; // Release binding if (p->image != EGL_NO_IMAGE) { - eglDestroyImageProc(gd->display, p->image); + eglDestroyImage(gd->display, p->image); p->image = EGL_NO_IMAGE; } @@ -134,18 +128,6 @@ static backend_t *egl_init(session_t *ps, xcb_window_t target) { bool success = false; struct egl_data *gd = NULL; -#define get_proc(name, type) \ - name##Proc = (type)eglGetProcAddress(#name); \ - if (!name##Proc) { \ - log_error("Failed to get EGL function " #name); \ - goto end; \ - } - get_proc(eglCreateImage, PFNEGLCREATEIMAGEKHRPROC); - get_proc(eglDestroyImage, PFNEGLDESTROYIMAGEKHRPROC); - get_proc(eglGetPlatformDisplay, PFNEGLGETPLATFORMDISPLAYPROC); - get_proc(eglCreatePlatformWindowSurface, PFNEGLCREATEPLATFORMWINDOWSURFACEPROC); -#undef get_proc - // Check if we have the X11 platform const char *exts = eglQueryString(EGL_NO_DISPLAY, EGL_EXTENSIONS); if (strstr(exts, "EGL_EXT_platform_x11") == NULL) { @@ -154,12 +136,12 @@ static backend_t *egl_init(session_t *ps, xcb_window_t target) { } gd = ccalloc(1, struct egl_data); - gd->display = eglGetPlatformDisplayProc(EGL_PLATFORM_X11_EXT, ps->c.dpy, - (EGLAttrib[]){ - EGL_PLATFORM_X11_SCREEN_EXT, - ps->c.screen, - EGL_NONE, - }); + gd->display = eglGetPlatformDisplayEXT(EGL_PLATFORM_X11_EXT, ps->c.dpy, + (EGLint[]){ + EGL_PLATFORM_X11_SCREEN_EXT, + ps->c.screen, + EGL_NONE, + }); if (gd->display == EGL_NO_DISPLAY) { log_error("Failed to get EGL display."); goto end; @@ -212,7 +194,7 @@ static backend_t *egl_init(session_t *ps, xcb_window_t target) { // clang-format on gd->target_win = - eglCreatePlatformWindowSurfaceProc(gd->display, config, &target, NULL); + eglCreatePlatformWindowSurfaceEXT(gd->display, config, &target, NULL); if (gd->target_win == EGL_NO_SURFACE) { log_error("Failed to create EGL surface."); goto end; @@ -243,14 +225,6 @@ static backend_t *egl_init(session_t *ps, xcb_window_t target) { goto end; } - glEGLImageTargetTexStorage = - (PFNGLEGLIMAGETARGETTEXSTORAGEEXTPROC)eglGetProcAddress("glEGLImageTargetTexS" - "torageEXT"); - if (glEGLImageTargetTexStorage == NULL) { - log_error("Failed to get glEGLImageTargetTexStorageEXT."); - goto end; - } - gd->gl.decouple_texture_user_data = egl_decouple_user_data; gd->gl.release_user_data = egl_release_image; @@ -302,9 +276,8 @@ egl_bind_pixmap(backend_t *base, xcb_pixmap_t pixmap, struct xvisual_info fmt, b eglpixmap = cmalloc(struct egl_pixmap); eglpixmap->pixmap = pixmap; - eglpixmap->image = - eglCreateImageProc(gd->display, EGL_NO_CONTEXT, EGL_NATIVE_PIXMAP_KHR, - (EGLClientBuffer)(uintptr_t)pixmap, NULL); + eglpixmap->image = eglCreateImage(gd->display, EGL_NO_CONTEXT, EGL_NATIVE_PIXMAP_KHR, + (EGLClientBuffer)(uintptr_t)pixmap, NULL); eglpixmap->owned = owned; if (eglpixmap->image == EGL_NO_IMAGE) { @@ -324,14 +297,14 @@ egl_bind_pixmap(backend_t *base, xcb_pixmap_t pixmap, struct xvisual_info fmt, b wd->dim = 0; wd->inner->refcount = 1; glBindTexture(GL_TEXTURE_2D, inner->texture); - glEGLImageTargetTexStorage(GL_TEXTURE_2D, eglpixmap->image, NULL); + glEGLImageTargetTexStorageEXT(GL_TEXTURE_2D, eglpixmap->image, NULL); glBindTexture(GL_TEXTURE_2D, 0); gl_check_err(); return wd; err: if (eglpixmap && eglpixmap->image) { - eglDestroyImageProc(gd->display, eglpixmap->image); + eglDestroyImage(gd->display, eglpixmap->image); } free(eglpixmap); @@ -422,7 +395,6 @@ struct backend_operations egl_ops = { .max_buffer_age = 5, // Why? }; -PFNEGLGETDISPLAYDRIVERNAMEPROC eglGetDisplayDriverName; /** * Check if a EGL extension exists. */ @@ -472,16 +444,4 @@ void eglext_init(EGLDisplay dpy) { check_ext(EGL_MESA_query_driver); #endif #undef check_ext - - // Checking if the returned function pointer is NULL is not really necessary, - // or maybe not even useful, since eglGetProcAddress might always return - // something. We are doing it just for completeness' sake. - -#ifdef EGL_MESA_query_driver - eglGetDisplayDriverName = - (PFNEGLGETDISPLAYDRIVERNAMEPROC)eglGetProcAddress("eglGetDisplayDriverName"); - if (!eglGetDisplayDriverName) { - eglext.has_EGL_MESA_query_driver = false; - } -#endif } diff --git a/src/backend/gl/egl.h b/src/backend/gl/egl.h index f482032b3e..033e84da37 100644 --- a/src/backend/gl/egl.h +++ b/src/backend/gl/egl.h @@ -1,10 +1,8 @@ // SPDX-License-Identifier: MPL-2.0 // Copyright (c) Yuxuan Shui #pragma once -#include -#include -#include -#include +#include +#include #include #include #include @@ -24,8 +22,4 @@ struct eglext_info { extern struct eglext_info eglext; -#ifdef EGL_MESA_query_driver -extern PFNEGLGETDISPLAYDRIVERNAMEPROC eglGetDisplayDriverName; -#endif - void eglext_init(EGLDisplay); diff --git a/src/backend/gl/gl_common.c b/src/backend/gl/gl_common.c index badbd9bb6c..8e1b93dfa3 100644 --- a/src/backend/gl/gl_common.c +++ b/src/backend/gl/gl_common.c @@ -1,7 +1,6 @@ // SPDX-License-Identifier: MPL-2.0 // Copyright (c) Yuxuan Shui -#include -#include +#include #include #include #include diff --git a/src/backend/gl/gl_common.h b/src/backend/gl/gl_common.h index aff0e0bc4c..8ffec9d265 100644 --- a/src/backend/gl/gl_common.h +++ b/src/backend/gl/gl_common.h @@ -1,8 +1,7 @@ // SPDX-License-Identifier: MPL-2.0 // Copyright (c) Yuxuan Shui #pragma once -#include -#include +#include #include #include diff --git a/src/backend/gl/glx.c b/src/backend/gl/glx.c index 78143a2b77..145804c263 100644 --- a/src/backend/gl/glx.c +++ b/src/backend/gl/glx.c @@ -580,16 +580,6 @@ static inline bool glx_has_extension(Display *dpy, int screen, const char *ext) } struct glxext_info glxext = {0}; -PFNGLXGETVIDEOSYNCSGIPROC glXGetVideoSyncSGI; -PFNGLXWAITVIDEOSYNCSGIPROC glXWaitVideoSyncSGI; -PFNGLXGETSYNCVALUESOMLPROC glXGetSyncValuesOML; -PFNGLXWAITFORMSCOMLPROC glXWaitForMscOML; -PFNGLXSWAPINTERVALEXTPROC glXSwapIntervalEXT; -PFNGLXSWAPINTERVALSGIPROC glXSwapIntervalSGI; -PFNGLXSWAPINTERVALMESAPROC glXSwapIntervalMESA; -PFNGLXBINDTEXIMAGEEXTPROC glXBindTexImageEXT; -PFNGLXRELEASETEXIMAGEEXTPROC glXReleaseTexImageEXT; -PFNGLXCREATECONTEXTATTRIBSARBPROC glXCreateContextAttribsARB; #ifdef GLX_MESA_query_renderer PFNGLXQUERYCURRENTRENDERERINTEGERMESAPROC glXQueryCurrentRendererIntegerMESA; @@ -614,36 +604,4 @@ void glxext_init(Display *dpy, int screen) { check_ext(GLX_MESA_query_renderer); #endif #undef check_ext - -#define lookup(name) ((name) = (__typeof__(name))glXGetProcAddress((GLubyte *)#name)) - // Checking if the returned function pointer is NULL is not really necessary, - // or maybe not even useful, since glXGetProcAddress might always return - // something. We are doing it just for completeness' sake. - if (!lookup(glXGetVideoSyncSGI) || !lookup(glXWaitVideoSyncSGI)) { - glxext.has_GLX_SGI_video_sync = false; - } - if (!lookup(glXSwapIntervalEXT)) { - glxext.has_GLX_EXT_swap_control = false; - } - if (!lookup(glXSwapIntervalMESA)) { - glxext.has_GLX_MESA_swap_control = false; - } - if (!lookup(glXSwapIntervalSGI)) { - glxext.has_GLX_SGI_swap_control = false; - } - if (!lookup(glXWaitForMscOML) || !lookup(glXGetSyncValuesOML)) { - glxext.has_GLX_OML_sync_control = false; - } - if (!lookup(glXBindTexImageEXT) || !lookup(glXReleaseTexImageEXT)) { - glxext.has_GLX_EXT_texture_from_pixmap = false; - } - if (!lookup(glXCreateContextAttribsARB)) { - glxext.has_GLX_ARB_create_context = false; - } -#ifdef GLX_MESA_query_renderer - if (!lookup(glXQueryCurrentRendererIntegerMESA)) { - glxext.has_GLX_MESA_query_renderer = false; - } -#endif -#undef lookup } diff --git a/src/backend/gl/glx.h b/src/backend/gl/glx.h index c7ff281c8b..a7663d946a 100644 --- a/src/backend/gl/glx.h +++ b/src/backend/gl/glx.h @@ -1,17 +1,9 @@ // SPDX-License-Identifier: MPL-2.0 // Copyright (c) Yuxuan Shui #pragma once -#include -// Older version of glx.h defines function prototypes for these extensions... -// Rename them to avoid conflicts -#define glXSwapIntervalMESA glXSwapIntervalMESA_ -#define glXBindTexImageEXT glXBindTexImageEXT_ -#define glXReleaseTexImageEXT glXReleaseTexImageEXT -#include -#undef glXSwapIntervalMESA -#undef glXBindTexImageEXT -#undef glXReleaseTexImageEXT #include +#include +#include #include #include @@ -59,19 +51,4 @@ struct glxext_info { extern struct glxext_info glxext; -extern PFNGLXGETVIDEOSYNCSGIPROC glXGetVideoSyncSGI; -extern PFNGLXWAITVIDEOSYNCSGIPROC glXWaitVideoSyncSGI; -extern PFNGLXGETSYNCVALUESOMLPROC glXGetSyncValuesOML; -extern PFNGLXWAITFORMSCOMLPROC glXWaitForMscOML; -extern PFNGLXSWAPINTERVALEXTPROC glXSwapIntervalEXT; -extern PFNGLXSWAPINTERVALSGIPROC glXSwapIntervalSGI; -extern PFNGLXSWAPINTERVALMESAPROC glXSwapIntervalMESA; -extern PFNGLXBINDTEXIMAGEEXTPROC glXBindTexImageEXT; -extern PFNGLXRELEASETEXIMAGEEXTPROC glXReleaseTexImageEXT; -extern PFNGLXCREATECONTEXTATTRIBSARBPROC glXCreateContextAttribsARB; - -#ifdef GLX_MESA_query_renderer -extern PFNGLXQUERYCURRENTRENDERERINTEGERMESAPROC glXQueryCurrentRendererIntegerMESA; -#endif - void glxext_init(Display *, int screen); diff --git a/src/log.c b/src/log.c index 664b9f41be..ab7c16990b 100644 --- a/src/log.c +++ b/src/log.c @@ -9,7 +9,7 @@ #include #ifdef CONFIG_OPENGL -#include +#include #include "backend/gl/gl_common.h" #include "backend/gl/glx.h" #endif @@ -338,21 +338,14 @@ struct log_target *stderr_logger_new(void) { } #ifdef CONFIG_OPENGL -/// An opengl logger that can be used for logging into opengl debugging tools, -/// such as apitrace -struct gl_string_marker_logger { - struct log_target tgt; - PFNGLSTRINGMARKERGREMEDYPROC gl_string_marker; -}; -static void -gl_string_marker_logger_write(struct log_target *tgt, const char *str, size_t len) { - auto g = (struct gl_string_marker_logger *)tgt; +static void gl_string_marker_logger_write(struct log_target *tgt attr_unused, + const char *str, size_t len) { // strip newlines at the end of the string while (len > 0 && str[len - 1] == '\n') { len--; } - g->gl_string_marker((GLsizei)len, str); + glStringMarkerGREMEDY((GLsizei)len, str); } static const struct log_ops gl_string_marker_logger_ops = { @@ -361,20 +354,16 @@ static const struct log_ops gl_string_marker_logger_ops = { .destroy = logger_trivial_destroy, }; +/// Create an opengl logger that can be used for logging into opengl debugging tools, +/// such as apitrace struct log_target *gl_string_marker_logger_new(void) { if (!gl_has_extension("GL_GREMEDY_string_marker")) { return NULL; } - void *fnptr = glXGetProcAddress((GLubyte *)"glStringMarkerGREMEDY"); - if (!fnptr) { - return NULL; - } - - auto ret = cmalloc(struct gl_string_marker_logger); - ret->tgt.ops = &gl_string_marker_logger_ops; - ret->gl_string_marker = fnptr; - return &ret->tgt; + auto ret = cmalloc(struct log_target); + ret->ops = &gl_string_marker_logger_ops; + return ret; } #else diff --git a/src/meson.build b/src/meson.build index dd10eaec3c..d7108fba7d 100644 --- a/src/meson.build +++ b/src/meson.build @@ -58,8 +58,8 @@ if get_option('vsync_drm') endif if get_option('opengl') - cflags += ['-DCONFIG_OPENGL', '-DGL_GLEXT_PROTOTYPES'] - deps += [dependency('gl', required: true), dependency('egl', required: true)] + cflags += ['-DCONFIG_OPENGL'] + deps += [dependency('epoxy', required: true)] srcs += [ 'opengl.c' ] endif diff --git a/src/opengl.h b/src/opengl.h index cd074ff26e..6f6da068e2 100644 --- a/src/opengl.h +++ b/src/opengl.h @@ -18,9 +18,9 @@ #include "render.h" #include "win.h" -#include -#include #include +#include +#include #include #include #include diff --git a/src/vblank.c b/src/vblank.c index a9f82116f1..b907088136 100644 --- a/src/vblank.c +++ b/src/vblank.c @@ -11,11 +11,11 @@ #ifdef CONFIG_OPENGL // Enable sgi_video_sync_vblank_scheduler -#include #include #include #include #include +#include #include #endif @@ -96,8 +96,6 @@ struct sgi_video_sync_thread_args { pthread_cond_t start_cnd; }; -static PFNGLXWAITVIDEOSYNCSGIPROC glXWaitVideoSyncSGI; - static bool check_sgi_video_sync_extension(Display *dpy, int screen) { const char *glx_ext = glXQueryExtensionsString(dpy, screen); const char *needle = "GLX_SGI_video_sync"; @@ -112,11 +110,6 @@ static bool check_sgi_video_sync_extension(Display *dpy, int screen) { return false; } - glXWaitVideoSyncSGI = (PFNGLXWAITVIDEOSYNCSGIPROC)(void *)glXGetProcAddress( - (const GLubyte *)"glXWaitVideoSyncSGI"); - if (!glXWaitVideoSyncSGI) { - return false; - } return true; } From 755996a42c88483b452e7b9133c8b69c808d3b7a Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sat, 10 Feb 2024 13:28:13 +0000 Subject: [PATCH 12/55] backend: gl: use libepoxy's has_*_extension So we don't need maintain our own version. Signed-off-by: Yuxuan Shui --- src/backend/gl/egl.c | 39 ++++------------------------------ src/backend/gl/gl_common.c | 4 ++-- src/backend/gl/gl_common.h | 20 ------------------ src/backend/gl/glx.c | 43 ++++---------------------------------- src/log.c | 2 +- src/opengl.c | 2 +- 6 files changed, 12 insertions(+), 98 deletions(-) diff --git a/src/backend/gl/egl.c b/src/backend/gl/egl.c index d0a1ef3519..9f3f153ffa 100644 --- a/src/backend/gl/egl.c +++ b/src/backend/gl/egl.c @@ -395,40 +395,6 @@ struct backend_operations egl_ops = { .max_buffer_age = 5, // Why? }; -/** - * Check if a EGL extension exists. - */ -static inline bool egl_has_extension(EGLDisplay dpy, const char *ext) { - const char *egl_exts = eglQueryString(dpy, EGL_EXTENSIONS); - if (!egl_exts) { - log_error("Failed get EGL extension list."); - return false; - } - - auto inlen = strlen(ext); - const char *curr = egl_exts; - bool match = false; - while (curr && !match) { - const char *end = strchr(curr, ' '); - if (!end) { - // Last extension string - match = strcmp(ext, curr) == 0; - } else if (curr + inlen == end) { - // Length match, do match string - match = strncmp(ext, curr, (unsigned long)(end - curr)) == 0; - } - curr = end ? end + 1 : NULL; - } - - if (!match) { - log_info("Missing EGL extension %s.", ext); - } else { - log_info("Found EGL extension %s.", ext); - } - - return match; -} - struct eglext_info eglext = {0}; void eglext_init(EGLDisplay dpy) { @@ -436,7 +402,10 @@ void eglext_init(EGLDisplay dpy) { return; } eglext.initialized = true; -#define check_ext(name) eglext.has_##name = egl_has_extension(dpy, #name) +#define check_ext(name) \ + eglext.has_##name = epoxy_has_egl_extension(dpy, #name); \ + log_info("Extension " #name " - %s", eglext.has_##name ? "present" : "absent") + check_ext(EGL_EXT_buffer_age); check_ext(EGL_EXT_create_context_robustness); check_ext(EGL_KHR_image_pixmap); diff --git a/src/backend/gl/gl_common.c b/src/backend/gl/gl_common.c index 8e1b93dfa3..2ef4170219 100644 --- a/src/backend/gl/gl_common.c +++ b/src/backend/gl/gl_common.c @@ -971,8 +971,8 @@ bool gl_init(struct gl_data *gd, session_t *ps) { } else { gd->is_nvidia = false; } - gd->has_robustness = gl_has_extension("GL_ARB_robustness"); - gd->has_egl_image_storage = gl_has_extension("GL_EXT_EGL_image_storage"); + gd->has_robustness = epoxy_has_gl_extension("GL_ARB_robustness"); + gd->has_egl_image_storage = epoxy_has_gl_extension("GL_EXT_EGL_image_storage"); gl_check_err(); return true; diff --git a/src/backend/gl/gl_common.h b/src/backend/gl/gl_common.h index 8ffec9d265..50bec7c626 100644 --- a/src/backend/gl/gl_common.h +++ b/src/backend/gl/gl_common.h @@ -263,26 +263,6 @@ static inline bool gl_check_fb_complete_(const char *func, int line, GLenum fb) #define gl_check_fb_complete(fb) gl_check_fb_complete_(__func__, __LINE__, (fb)) -/** - * Check if a GL extension exists. - */ -static inline bool gl_has_extension(const char *ext) { - int nexts = 0; - glGetIntegerv(GL_NUM_EXTENSIONS, &nexts); - for (int i = 0; i < nexts || !nexts; i++) { - const char *exti = (const char *)glGetStringi(GL_EXTENSIONS, (GLuint)i); - if (exti == NULL) { - break; - } - if (strcmp(ext, exti) == 0) { - return true; - } - } - gl_clear_err(); - log_info("Missing GL extension %s.", ext); - return false; -} - static const GLuint vert_coord_loc = 0; static const GLuint vert_in_texcoord_loc = 1; diff --git a/src/backend/gl/glx.c b/src/backend/gl/glx.c index 145804c263..af2f891e4f 100644 --- a/src/backend/gl/glx.c +++ b/src/backend/gl/glx.c @@ -545,52 +545,17 @@ struct backend_operations glx_ops = { .max_buffer_age = 5, // Why? }; -/** - * Check if a GLX extension exists. - */ -static inline bool glx_has_extension(Display *dpy, int screen, const char *ext) { - const char *glx_exts = glXQueryExtensionsString(dpy, screen); - if (!glx_exts) { - log_error("Failed get GLX extension list."); - return false; - } - - auto inlen = strlen(ext); - const char *curr = glx_exts; - bool match = false; - while (curr && !match) { - const char *end = strchr(curr, ' '); - if (!end) { - // Last extension string - match = strcmp(ext, curr) == 0; - } else if (curr + inlen == end) { - // Length match, do match string - match = strncmp(ext, curr, (unsigned long)(end - curr)) == 0; - } - curr = end ? end + 1 : NULL; - } - - if (!match) { - log_info("Missing GLX extension %s.", ext); - } else { - log_info("Found GLX extension %s.", ext); - } - - return match; -} - struct glxext_info glxext = {0}; -#ifdef GLX_MESA_query_renderer -PFNGLXQUERYCURRENTRENDERERINTEGERMESAPROC glXQueryCurrentRendererIntegerMESA; -#endif - void glxext_init(Display *dpy, int screen) { if (glxext.initialized) { return; } glxext.initialized = true; -#define check_ext(name) glxext.has_##name = glx_has_extension(dpy, screen, #name) +#define check_ext(name) \ + glxext.has_##name = epoxy_has_glx_extension(dpy, screen, #name); \ + log_info("Extension " #name " - %s", glxext.has_##name ? "present" : "absent") + check_ext(GLX_SGI_video_sync); check_ext(GLX_SGI_swap_control); check_ext(GLX_OML_sync_control); diff --git a/src/log.c b/src/log.c index ab7c16990b..4baa7c7b7a 100644 --- a/src/log.c +++ b/src/log.c @@ -357,7 +357,7 @@ static const struct log_ops gl_string_marker_logger_ops = { /// Create an opengl logger that can be used for logging into opengl debugging tools, /// such as apitrace struct log_target *gl_string_marker_logger_new(void) { - if (!gl_has_extension("GL_GREMEDY_string_marker")) { + if (!epoxy_has_gl_extension("GL_GREMEDY_string_marker")) { return NULL; } diff --git a/src/opengl.c b/src/opengl.c index 7360640ce5..eb40f1b3dd 100644 --- a/src/opengl.c +++ b/src/opengl.c @@ -181,7 +181,7 @@ bool glx_init(session_t *ps, bool need_render) { // must precede FBConfig fetching if (need_render) { psglx->has_texture_non_power_of_two = - gl_has_extension("GL_ARB_texture_non_power_of_two"); + epoxy_has_gl_extension("GL_ARB_texture_non_power_of_two"); } // Render preparations From 642a43acbb5a92f00c31bd7487c07fab59ab18a9 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sat, 10 Feb 2024 13:31:52 +0000 Subject: [PATCH 13/55] Update README.md and CHANGELOG.md Signed-off-by: Yuxuan Shui --- CHANGELOG.md | 5 +++++ README.md | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a11085678b..5fbd447144 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,14 @@ * Allow `corner-radius-rules` to override `corner-radius = 0`. Previously setting corner radius to 0 globally disables rounded corners. (#1170) +## Build changes + +* `picom` now depends on `libepoxy` for OpenGL symbol management. + ## Bug fixes * Workaround a NVIDIA problem that causes high CPU usage after suspend/resume (#1172, #1168) +* Fix building on OpenBSD (#1189, #1188) # v11.1 (2024-Jan-28) diff --git a/README.md b/README.md index d569723817..28790eed45 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ Assuming you already have all the usual building tools installed (e.g. gcc, pyth * pixman * libdbus (optional, disable with the `-Ddbus=false` meson configure flag) * libconfig (optional, disable with the `-Dconfig_file=false` meson configure flag) -* libGL, libEGL (optional, disable with the `-Dopengl=false` meson configure flag) +* libGL, libEGL, libepoxy (optional, disable with the `-Dopengl=false` meson configure flag) * libpcre2 (optional, disable with the `-Dregex=false` meson configure flag) * libev * uthash @@ -49,13 +49,13 @@ Assuming you already have all the usual building tools installed (e.g. gcc, pyth On Debian based distributions (e.g. Ubuntu), the needed packages are ``` -libconfig-dev libdbus-1-dev libegl-dev libev-dev libgl-dev libpcre2-dev libpixman-1-dev libx11-xcb-dev libxcb1-dev libxcb-composite0-dev libxcb-damage0-dev libxcb-dpms0-dev libxcb-glx0-dev libxcb-image0-dev libxcb-present-dev libxcb-randr0-dev libxcb-render0-dev libxcb-render-util0-dev libxcb-shape0-dev libxcb-util-dev libxcb-xfixes0-dev libxext-dev meson ninja-build uthash-dev +libconfig-dev libdbus-1-dev libegl-dev libev-dev libgl-dev libepoxy-dev libpcre2-dev libpixman-1-dev libx11-xcb-dev libxcb1-dev libxcb-composite0-dev libxcb-damage0-dev libxcb-dpms0-dev libxcb-glx0-dev libxcb-image0-dev libxcb-present-dev libxcb-randr0-dev libxcb-render0-dev libxcb-render-util0-dev libxcb-shape0-dev libxcb-util-dev libxcb-xfixes0-dev libxext-dev meson ninja-build uthash-dev ``` On Fedora, the needed packages are ``` -dbus-devel gcc git libconfig-devel libdrm-devel libev-devel libX11-devel libX11-xcb libXext-devel libxcb-devel libGL-devel libEGL-devel meson pcre2-devel pixman-devel uthash-devel xcb-util-image-devel xcb-util-renderutil-devel xorg-x11-proto-devel xcb-util-devel +dbus-devel gcc git libconfig-devel libdrm-devel libev-devel libX11-devel libX11-xcb libXext-devel libxcb-devel libGL-devel libEGL-devel libepoxy-devel meson pcre2-devel pixman-devel uthash-devel xcb-util-image-devel xcb-util-renderutil-devel xorg-x11-proto-devel xcb-util-devel ``` To build the documents, you need `asciidoc` From 037be5cca20366a8bd26fe90865f225a471859b1 Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Sun, 11 Feb 2024 21:33:14 +0300 Subject: [PATCH 14/55] update contributors list --- CONTRIBUTORS | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CONTRIBUTORS b/CONTRIBUTORS index d4373abd99..e203178966 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -34,6 +34,7 @@ hasufell i-c-u-p Ignacio Taranto Istvan Petres +Ivan Malison Jake James Cloos Jamey Sharp @@ -43,6 +44,7 @@ Javeed Shaikh Jerónimo Navarro jialeens Johnny Pribyl +Jose Maldonado aka Yukiteru Keith Packard Kevin Kelley ktprograms @@ -68,6 +70,7 @@ Peter Mattern Phil Blundell Que Quotion Rafael Kitover +Reith Richard Grenville Rytis Karpuska Samuel Hand From baeafb3a3b4ad3de3690929e68245b5cd54b9366 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Tue, 6 Feb 2024 08:56:37 +0000 Subject: [PATCH 15/55] event: tweak ev_reparent_notify Instead of change window attributes back and forth, calculate the evmask and set it just once. And also make sure the request is flushed. Signed-off-by: Yuxuan Shui --- src/event.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/event.c b/src/event.c index 28f67d8554..f59cf9e32f 100644 --- a/src/event.c +++ b/src/event.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MPL-2.0 // Copyright (c) 2019, Yuxuan Shui +#include #include #include @@ -350,19 +351,14 @@ static inline void ev_reparent_notify(session_t *ps, xcb_reparent_notify_event_t } // Reset event mask in case something wrong happens - xcb_change_window_attributes( - ps->c.c, ev->window, XCB_CW_EVENT_MASK, - (const uint32_t[]){determine_evmask(ps, ev->window, WIN_EVMODE_UNKNOWN)}); + uint32_t evmask = determine_evmask(ps, ev->window, WIN_EVMODE_UNKNOWN); if (!wid_has_prop(ps, ev->window, ps->atoms->aWM_STATE)) { log_debug("Window %#010x doesn't have WM_STATE property, it is " "probably not a client window. But we will listen for " "property change in case it gains one.", ev->window); - xcb_change_window_attributes( - ps->c.c, ev->window, XCB_CW_EVENT_MASK, - (const uint32_t[]){determine_evmask(ps, ev->window, WIN_EVMODE_UNKNOWN) | - XCB_EVENT_MASK_PROPERTY_CHANGE}); + evmask |= XCB_EVENT_MASK_PROPERTY_CHANGE; } else { auto w_real_top = find_managed_window_or_parent(ps, ev->parent); if (w_real_top && w_real_top->state != WSTATE_UNMAPPED && @@ -386,6 +382,8 @@ static inline void ev_reparent_notify(session_t *ps, xcb_reparent_notify_event_t } } } + XCB_AWAIT_VOID(xcb_change_window_attributes, ps->c.c, ev->window, + XCB_CW_EVENT_MASK, (const uint32_t[]){evmask}); } } From bdc0943399584275c5bfcf01e84e4c38ecb2d515 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Tue, 6 Feb 2024 09:01:35 +0000 Subject: [PATCH 16/55] event: make sure ev_property_notify flushes its requests Signed-off-by: Yuxuan Shui --- src/event.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/event.c b/src/event.c index f59cf9e32f..5bfcbb03b4 100644 --- a/src/event.c +++ b/src/event.c @@ -473,9 +473,10 @@ static inline void ev_property_notify(session_t *ps, xcb_property_notify_event_t // Check whether it could be a client window if (!find_toplevel(ps, ev->window)) { // Reset event mask anyway - xcb_change_window_attributes(ps->c.c, ev->window, XCB_CW_EVENT_MASK, - (const uint32_t[]){determine_evmask( - ps, ev->window, WIN_EVMODE_UNKNOWN)}); + const uint32_t evmask = + determine_evmask(ps, ev->window, WIN_EVMODE_UNKNOWN); + XCB_AWAIT_VOID(xcb_change_window_attributes, ps->c.c, ev->window, + XCB_CW_EVENT_MASK, (const uint32_t[]){evmask}); auto w_top = find_managed_window_or_parent(ps, ev->window); // ev->window might have not been managed yet, in that case w_top @@ -490,8 +491,8 @@ static inline void ev_property_notify(session_t *ps, xcb_property_notify_event_t // If _NET_WM_WINDOW_TYPE changes... God knows why this would happen, but // there are always some stupid applications. (#144) if (ev->atom == ps->atoms->a_NET_WM_WINDOW_TYPE) { - struct managed_win *w = NULL; - if ((w = find_toplevel(ps, ev->window))) { + struct managed_win *w = find_toplevel(ps, ev->window); + if (w) { win_set_property_stale(w, ev->atom); } } From a5826b6fb0dc4888ce922df5d82151972ff4b247 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Tue, 6 Feb 2024 09:10:04 +0000 Subject: [PATCH 17/55] event: fix dumb bug in repair_win Basically we won't call xcb_damage_subtract if show_all_xerrors is set, which is very bad. Fixes that, and also make sure the damage subtract request is flushed in all branches. Fixes: 1307d9ec709c9fbbe99939d46ad04c57d5e4b501 Signed-off-by: Yuxuan Shui --- src/event.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/event.c b/src/event.c index 5bfcbb03b4..b3cb25a270 100644 --- a/src/event.c +++ b/src/event.c @@ -585,16 +585,28 @@ static inline void repair_win(session_t *ps, struct managed_win *w) { region_t parts; pixman_region32_init(&parts); + // If this is the first time this window is damaged, we would redraw the + // whole window, so we don't need to fetch the damage region. But we still need + // to make sure the X server receives the DamageSubtract request, hence the + // `xcb_request_check` here. + // Otherwise, we fetch the damage regions. That means we will receive a reply + // from the X server, which implies it has received our request. if (!w->ever_damaged) { - win_extents(w, &parts); - if (!ps->o.show_all_xerrors) { - set_ignore_cookie(&ps->c, xcb_damage_subtract(ps->c.c, w->damage, - XCB_NONE, XCB_NONE)); + auto e = xcb_request_check( + ps->c.c, xcb_damage_subtract(ps->c.c, w->damage, XCB_NONE, XCB_NONE)); + if (e) { + if (ps->o.show_all_xerrors) { + x_print_error(e->sequence, e->major_code, e->minor_code, + e->error_code); + } + free(e); } + win_extents(w, &parts); } else { + auto cookie = + xcb_damage_subtract(ps->c.c, w->damage, XCB_NONE, ps->damaged_region); if (!ps->o.show_all_xerrors) { - set_ignore_cookie(&ps->c, xcb_damage_subtract(ps->c.c, w->damage, XCB_NONE, - ps->damaged_region)); + set_ignore_cookie(&ps->c, cookie); } x_fetch_region(&ps->c, ps->damaged_region, &parts); pixman_region32_translate(&parts, w->g.x + w->g.border_width, From 75d0b7ba1e84e5057efaa1e61398f003a8e9e246 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Tue, 6 Feb 2024 09:14:05 +0000 Subject: [PATCH 18/55] core: don't flush X connection before go to sleep See the added comments for details. Fixes #1145 Fixes #1166 Fixes #1040? Signed-off-by: Yuxuan Shui --- src/event.c | 10 ++++++++-- src/picom.c | 32 ++++++++++++++++++++++++-------- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/event.c b/src/event.c index b3cb25a270..a1b93fd476 100644 --- a/src/event.c +++ b/src/event.c @@ -48,8 +48,14 @@ /// When top half finished, we enter the render stage, where no server state should be /// queried. All rendering should be done with our internal knowledge of the server state. /// +/// P.S. There is another reason to avoid sending any request to the server as much as +/// possible. To make sure requests are sent, flushes are needed. And `xcb_flush`/`XFlush` +/// functions may read more events from the server into their queues. This is +/// undesirable, see the comments on `handle_queued_x_events` in picom.c for more details. -// TODO(yshui) the things described above +// TODO(yshui) the things described above. This is mostly done, maybe some of +// the functions here is still making unnecessary queries, we need +// to do some auditing to be sure. /** * Get a window's name from window ID. @@ -590,7 +596,7 @@ static inline void repair_win(session_t *ps, struct managed_win *w) { // to make sure the X server receives the DamageSubtract request, hence the // `xcb_request_check` here. // Otherwise, we fetch the damage regions. That means we will receive a reply - // from the X server, which implies it has received our request. + // from the X server, which implies it has received our DamageSubtract request. if (!w->ever_damaged) { auto e = xcb_request_check( ps->c.c, xcb_damage_subtract(ps->c.c, w->damage, XCB_NONE, XCB_NONE)); diff --git a/src/picom.c b/src/picom.c index cdacf75d0c..55c65d2ce2 100644 --- a/src/picom.c +++ b/src/picom.c @@ -1607,9 +1607,32 @@ static void unredirect(session_t *ps) { log_debug("Screen unredirected."); } -// Handle queued events before we go to sleep +/// Handle queued events before we go to sleep. +/// +/// This function is called by ev_prepare watcher, which is called just before +/// the event loop goes to sleep. X damage events are incremental, which means +/// if we don't handle the ones X server already sent us, we won't get new ones. +/// And if we don't get new ones, we won't render, i.e. we would freeze. libxcb +/// keeps an internal queue of events, so we have to be 100% sure no events are +/// left in that queue before we go to sleep. static void handle_queued_x_events(EV_P attr_unused, ev_prepare *w, int revents attr_unused) { session_t *ps = session_ptr(w, event_check); + // Flush because if we go into sleep when there is still requests in the + // outgoing buffer, they will not be sent for an indefinite amount of + // time. Use XFlush here too, we might still use some Xlib functions + // because OpenGL. + // + // Also note, after we have flushed here, we won't flush again in this + // function before going into sleep. This is because `xcb_flush`/`XFlush` + // may _read_ more events from the server (yes, this is ridiculous, I + // know). And we can't have that, see the comments above this function. + // + // This means if functions called ev_handle need to send some events, + // they need to carefully make sure those events are flushed, one way or + // another. + XFlush(ps->c.dpy); + xcb_flush(ps->c.c); + if (ps->vblank_scheduler) { vblank_handle_x_events(ps->vblank_scheduler); } @@ -1619,13 +1642,6 @@ static void handle_queued_x_events(EV_P attr_unused, ev_prepare *w, int revents ev_handle(ps, ev); free(ev); }; - // Flush because if we go into sleep when there is still - // requests in the outgoing buffer, they will not be sent - // for an indefinite amount of time. - // Use XFlush here too, we might still use some Xlib functions - // because OpenGL. - XFlush(ps->c.dpy); - xcb_flush(ps->c.c); int err = xcb_connection_has_error(ps->c.c); if (err) { log_fatal("X11 server connection broke (error %d)", err); From 0ab3e0740e61849bb230e5f60290905eecfd0c43 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sun, 11 Feb 2024 23:18:47 +0000 Subject: [PATCH 19/55] Update CHANGELOG.md Signed-off-by: Yuxuan Shui --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fbd447144..73f7626f4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ * Workaround a NVIDIA problem that causes high CPU usage after suspend/resume (#1172, #1168) * Fix building on OpenBSD (#1189, #1188) +* Fix occasional freezes (#1040, #1145, #1166) # v11.1 (2024-Jan-28) From a4ec70982c824c172f2958bfe26827be3a68783c Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Sun, 28 Jan 2024 03:46:55 +0300 Subject: [PATCH 20/55] x: add the x_set_region function it sets an x region to a pixman region (cherry picked from commit efb7a1430f2c530c7b9cc0cb6d6d6cff95d8a4d9) Signed-off-by: Yuxuan Shui --- src/x.c | 28 ++++++++++++++++++++++++++++ src/x.h | 3 +++ 2 files changed, 31 insertions(+) diff --git a/src/x.c b/src/x.c index d48ae96ba7..0d00342a43 100644 --- a/src/x.c +++ b/src/x.c @@ -474,6 +474,34 @@ bool x_fetch_region(struct x_connection *c, xcb_xfixes_region_t r, pixman_region return ret; } +bool x_set_region(struct x_connection *c, xcb_xfixes_region_t dst, const region_t *src) { + if (!src || dst == XCB_NONE) { + return false; + } + + int32_t nrects = 0; + const rect_t *rects = pixman_region32_rectangles((region_t *)src, &nrects); + if (!rects || nrects < 1) { + return false; + } + + xcb_rectangle_t *xrects = ccalloc(nrects, xcb_rectangle_t); + for (int32_t i = 0; i < nrects; i++) { + xrects[i] = + (xcb_rectangle_t){.x = to_i16_checked(rects[i].x1), + .y = to_i16_checked(rects[i].y1), + .width = to_u16_checked(rects[i].x2 - rects[i].x1), + .height = to_u16_checked(rects[i].y2 - rects[i].y1)}; + } + + bool success = + XCB_AWAIT_VOID(xcb_xfixes_set_region, c->c, dst, to_u32_checked(nrects), xrects); + + free(xrects); + + return success; +} + uint32_t x_create_region(struct x_connection *c, const region_t *reg) { if (!reg) { return XCB_NONE; diff --git a/src/x.h b/src/x.h index f320e8696e..127ff510ac 100644 --- a/src/x.h +++ b/src/x.h @@ -321,6 +321,9 @@ x_create_picture_with_visual(struct x_connection *, int w, int h, xcb_visualid_t /// Fetch a X region and store it in a pixman region bool x_fetch_region(struct x_connection *, xcb_xfixes_region_t r, region_t *res); +/// Set an X region to a pixman region +bool x_set_region(struct x_connection *c, xcb_xfixes_region_t dst, const region_t *src); + /// Create a X region from a pixman region uint32_t x_create_region(struct x_connection *c, const region_t *reg); From 9392829d84d9b63f411e2af8191d1161e90cb984 Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Sun, 28 Jan 2024 04:21:13 +0300 Subject: [PATCH 21/55] backend: xrender: cache the present region to avoid creating and destroying it every frame (cherry picked from commit 5a1990b236c85f1222098ef147398855cbb3af69) Signed-off-by: Yuxuan Shui --- src/backend/xrender/xrender.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/backend/xrender/xrender.c b/src/backend/xrender/xrender.c index b2ec3f2301..acfe25d4a3 100644 --- a/src/backend/xrender/xrender.c +++ b/src/backend/xrender/xrender.c @@ -56,6 +56,10 @@ typedef struct _xrender_data { int target_width, target_height; xcb_special_event_t *present_event; + + /// Cache an X region to avoid creating and destroying it every frame. A + /// workaround for yshui/picom#1166. + xcb_xfixes_region_t present_region; } xrender_data; struct _xrender_blur_context { @@ -597,6 +601,7 @@ static void deinit(backend_t *backend_data) { xcb_free_pixmap(xd->base.c->c, xd->back_pixmap[i]); } } + x_destroy_region(xd->base.c, xd->present_region); if (xd->present_event) { xcb_unregister_for_special_event(xd->base.c->c, xd->present_event); } @@ -624,16 +629,15 @@ static void present(backend_t *base, const region_t *region) { XCB_NONE, xd->back[xd->curr_back], orig_x, orig_y, 0, 0, orig_x, orig_y, region_width, region_height); - auto xregion = x_create_region(base->c, region); - // Make sure we got reply from PresentPixmap before waiting for events, // to avoid deadlock auto e = xcb_request_check( - base->c->c, xcb_present_pixmap_checked( - xd->base.c->c, xd->target_win, - xd->back_pixmap[xd->curr_back], 0, XCB_NONE, xregion, 0, - 0, XCB_NONE, XCB_NONE, XCB_NONE, 0, 0, 0, 0, 0, NULL)); - x_destroy_region(base->c, xregion); + base->c->c, + xcb_present_pixmap_checked( + base->c->c, xd->target_win, xd->back_pixmap[xd->curr_back], 0, XCB_NONE, + x_set_region(base->c, xd->present_region, region) ? xd->present_region + : XCB_NONE, + 0, 0, XCB_NONE, XCB_NONE, XCB_NONE, 0, 0, 0, 0, 0, NULL)); if (e) { log_error("Failed to present pixmap"); free(e); @@ -929,6 +933,10 @@ static backend_t *backend_xrender_init(session_t *ps, xcb_window_t target) { xd->vsync = false; } + if (xd->vsync) { + xd->present_region = x_create_region(&ps->c, &ps->screen_reg); + } + // We might need to do double buffering for vsync, and buffer 0 and 1 are for // double buffering. int first_buffer_index = xd->vsync ? 0 : 2; From 4f792243c185f4e5b3ccc5da9a9d7c73167d50f8 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Tue, 13 Feb 2024 10:44:38 +0000 Subject: [PATCH 22/55] Update CHANGELOG.md Signed-off-by: Yuxuan Shui --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73f7626f4c..330d69c2db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ * Allow `corner-radius-rules` to override `corner-radius = 0`. Previously setting corner radius to 0 globally disables rounded corners. (#1170) +# v11.2 (2024-Feb-13) + ## Build changes * `picom` now depends on `libepoxy` for OpenGL symbol management. @@ -13,6 +15,9 @@ * Workaround a NVIDIA problem that causes high CPU usage after suspend/resume (#1172, #1168) * Fix building on OpenBSD (#1189, #1188) * Fix occasional freezes (#1040, #1145, #1166) +* Fix `corner-radius-rules` not applying sometimes (#1177) +* Fix window shader not having an effect when frame opacity is enabled (#1174) +* Fix binding root pixmap in case of depth mismatch (#984) # v11.1 (2024-Jan-28) From 466fb4c9e01aebb8e0970d6d38d1dcbbe8e75f4f Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Tue, 13 Feb 2024 23:57:48 +0300 Subject: [PATCH 23/55] ci: update github actions to use node.js 20 --- .github/workflows/codeql-analysis.yml | 8 ++++---- .github/workflows/coding-style-pr.yml | 2 +- .github/workflows/coding-style.yml | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index b1d5987dd9..29ccf790ac 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -18,11 +18,11 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@v3 + uses: actions/checkout@v4 # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL - uses: github/codeql-action/init@v2 + uses: github/codeql-action/init@v3 with: languages: ${{ matrix.language }} @@ -32,7 +32,7 @@ jobs: # Autobuild - name: Autobuild - uses: github/codeql-action/autobuild@v2 + uses: github/codeql-action/autobuild@v3 - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v2 + uses: github/codeql-action/analyze@v3 diff --git a/.github/workflows/coding-style-pr.yml b/.github/workflows/coding-style-pr.yml index 5de4c67fcc..48afce3847 100644 --- a/.github/workflows/coding-style-pr.yml +++ b/.github/workflows/coding-style-pr.yml @@ -6,7 +6,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - run: git fetch --depth=1 origin ${{ github.event.pull_request.base.sha }} - uses: yshui/git-clang-format-lint@v1.14 with: diff --git a/.github/workflows/coding-style.yml b/.github/workflows/coding-style.yml index 49f5379aaa..4eceb944bf 100644 --- a/.github/workflows/coding-style.yml +++ b/.github/workflows/coding-style.yml @@ -6,7 +6,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: fetch-depth: 2 - uses: yshui/git-clang-format-lint@v1.14 From f179119d841094a4e46f1c4f98b0bb9d36201297 Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Wed, 14 Feb 2024 00:19:29 +0300 Subject: [PATCH 24/55] ci: use meson setup instead of meson --- .builds/freebsd.yml | 2 +- .builds/openbsd.yml | 2 +- .circleci/config.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.builds/freebsd.yml b/.builds/freebsd.yml index 546df0979a..47b333544d 100644 --- a/.builds/freebsd.yml +++ b/.builds/freebsd.yml @@ -20,7 +20,7 @@ sources: tasks: - setup: | cd picom - CPPFLAGS="-I/usr/local/include" meson -Dunittest=true build + CPPFLAGS="-I/usr/local/include" meson setup -Dunittest=true build - build: | cd picom ninja -C build diff --git a/.builds/openbsd.yml b/.builds/openbsd.yml index d73c57692a..c8e667e114 100644 --- a/.builds/openbsd.yml +++ b/.builds/openbsd.yml @@ -14,7 +14,7 @@ sources: tasks: - setup: | cd picom - CPPFLAGS="-I/usr/local/include" LDFLAGS="-L/usr/local/lib" meson -Dunittest=true build + CPPFLAGS="-I/usr/local/include" LDFLAGS="-L/usr/local/lib" meson setup -Dunittest=true build - build: | cd picom ninja -C build diff --git a/.circleci/config.yml b/.circleci/config.yml index bed676bf7c..36ecc504b5 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -29,7 +29,7 @@ commands: - ".git" - run: name: config - command: CC=<< parameters.cc >> meson << parameters.build-config >> -Dunittest=true --werror . build + command: CC=<< parameters.cc >> meson setup << parameters.build-config >> -Dunittest=true --werror . build - run: name: build command: ninja -vC build From 53dd8a4e66030d2c7a8103486c74704f217026a1 Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Wed, 14 Feb 2024 02:10:16 +0300 Subject: [PATCH 25/55] fix most of build warnings on openbsd sorry openbsd people, we're not going to use the snprintf function. --- src/backend/backend.c | 2 +- src/config.c | 5 +---- src/picom.c | 8 +++++--- src/vblank.c | 6 +++--- src/win.c | 3 ++- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/backend/backend.c b/src/backend/backend.c index 5120672bc5..f154c91522 100644 --- a/src/backend/backend.c +++ b/src/backend/backend.c @@ -194,7 +194,7 @@ bool paint_all_new(session_t *ps, struct managed_win *const t) { auto after_damage_us = (uint64_t)now.tv_sec * 1000000UL + (uint64_t)now.tv_nsec / 1000; log_trace("Getting damage took %" PRIu64 " us", after_damage_us - after_sync_fence_us); if (ps->next_render > 0) { - log_verbose("Render schedule deviation: %ld us (%s) %" PRIu64 " %ld", + log_verbose("Render schedule deviation: %ld us (%s) %" PRIu64 " %" PRIu64, labs((long)after_damage_us - (long)ps->next_render), after_damage_us < ps->next_render ? "early" : "late", after_damage_us, ps->next_render); diff --git a/src/config.c b/src/config.c index 08d9f62872..63b8fb0c98 100644 --- a/src/config.c +++ b/src/config.c @@ -43,10 +43,7 @@ const char *xdg_config_home(void) { return NULL; } - xdgh = cvalloc(strlen(home) + strlen(default_dir) + 1); - - strcpy(xdgh, home); - strcat(xdgh, default_dir); + xdgh = mstrjoin(home, default_dir); } else { xdgh = strdup(xdgh); } diff --git a/src/picom.c b/src/picom.c index 55c65d2ce2..841c6f32f8 100644 --- a/src/picom.c +++ b/src/picom.c @@ -210,11 +210,13 @@ collect_vblank_interval_statistics(struct vblank_event *e, void *ud) { if (frame_count == 1) { render_statistics_add_vblank_time_sample( &ps->render_stats, frame_time); - log_trace("Frame count %lu, frame time: %d us, ust: " - "%" PRIu64 "", + log_trace("Frame count %" PRIu64 ", frame time: %d us, " + "ust: " + "%" PRIu64, frame_count, frame_time, e->ust); } else { - log_trace("Frame count %lu, frame time: %d us, msc: " + log_trace("Frame count %" PRIu64 ", frame time: %d us, " + "msc: " "%" PRIu64 ", not adding sample.", frame_count, frame_time, e->ust); } diff --git a/src/vblank.c b/src/vblank.c index b907088136..81a6cb9e30 100644 --- a/src/vblank.c +++ b/src/vblank.c @@ -327,7 +327,7 @@ sgi_video_sync_scheduler_callback(EV_P attr_unused, ev_async *w, int attr_unused }; sched->base.vblank_event_requested = false; sched->last_msc = msc; - log_verbose("Received vblank event for msc %lu", event.msc); + log_verbose("Received vblank event for msc %" PRIu64, event.msc); vblank_scheduler_invoke_callbacks(&sched->base, &event); } #endif @@ -411,7 +411,7 @@ static void handle_present_complete_notify(struct present_vblank_scheduler *self auto now_us = (unsigned long)(now.tv_sec * 1000000L + now.tv_nsec / 1000); double delay_sec = 0.0; if (now_us < cne->ust) { - log_trace("The end of this vblank is %lu us into the " + log_trace("The end of this vblank is %" PRIu64 " us into the " "future", cne->ust - now_us); delay_sec = (double)(cne->ust - now_us) / 1000000.0; @@ -566,4 +566,4 @@ bool vblank_handle_x_events(struct vblank_scheduler *self) { return fn(self); } return true; -} \ No newline at end of file +} diff --git a/src/win.c b/src/win.c index c5690b6157..81660c81bd 100644 --- a/src/win.c +++ b/src/win.c @@ -470,7 +470,8 @@ void win_process_update_flags(session_t *ps, struct managed_win *w) { // Whether the window was visible before we process the mapped flag. i.e. // is the window just mapped. bool was_visible = win_is_real_visible(w); - log_trace("Processing flags for window %#010x (%s), was visible: %d, flags: %#lx", + log_trace("Processing flags for window %#010x (%s), was visible: %d, flags: " + "%#" PRIx64, w->base.id, w->name, was_visible, w->flags); if (win_check_flags_all(w, WIN_FLAGS_MAPPED)) { From c6db632d9dc5b33b19fca62116c7d661aec8fed3 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Wed, 14 Feb 2024 17:29:11 +0000 Subject: [PATCH 26/55] ci: update git-clang-format Signed-off-by: Yuxuan Shui --- .github/workflows/coding-style-pr.yml | 2 +- .github/workflows/coding-style.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/coding-style-pr.yml b/.github/workflows/coding-style-pr.yml index 5de4c67fcc..8ca2596c7d 100644 --- a/.github/workflows/coding-style-pr.yml +++ b/.github/workflows/coding-style-pr.yml @@ -8,6 +8,6 @@ jobs: steps: - uses: actions/checkout@v3 - run: git fetch --depth=1 origin ${{ github.event.pull_request.base.sha }} - - uses: yshui/git-clang-format-lint@v1.14 + - uses: yshui/git-clang-format-lint@v1.15 with: base: ${{ github.event.pull_request.base.sha }} diff --git a/.github/workflows/coding-style.yml b/.github/workflows/coding-style.yml index 49f5379aaa..c2f86ad5ae 100644 --- a/.github/workflows/coding-style.yml +++ b/.github/workflows/coding-style.yml @@ -9,6 +9,6 @@ jobs: - uses: actions/checkout@v3 with: fetch-depth: 2 - - uses: yshui/git-clang-format-lint@v1.14 + - uses: yshui/git-clang-format-lint@v1.15 with: base: ${{ github.event.ref }}~1 From b0dfcf4a328703949ecf73689a8603381127c0db Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Wed, 14 Feb 2024 20:22:25 +0300 Subject: [PATCH 27/55] x: remove x_screen_of_display and use xcb_aux_get_screen instead --- src/x.c | 16 ++-------------- src/x.h | 2 -- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/src/x.c b/src/x.c index 0d00342a43..3b782c0aae 100644 --- a/src/x.c +++ b/src/x.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include @@ -94,7 +95,7 @@ void x_connection_init(struct x_connection *c, Display *dpy) { c->previous_xerror_handler = XSetErrorHandler(xerror); c->screen = DefaultScreen(dpy); - c->screen_info = x_screen_of_display(c, c->screen); + c->screen_info = xcb_aux_get_screen(c->c, c->screen); } /** @@ -904,19 +905,6 @@ struct xvisual_info x_get_visual_info(struct x_connection *c, xcb_visualid_t vis }; } -xcb_screen_t *x_screen_of_display(struct x_connection *c, int screen) { - xcb_screen_iterator_t iter; - - iter = xcb_setup_roots_iterator(xcb_get_setup(c->c)); - for (; iter.rem; --screen, xcb_screen_next(&iter)) { - if (screen == 0) { - return iter.data; - } - } - - return NULL; -} - void x_update_monitors(struct x_connection *c, struct x_monitors *m) { x_free_monitor_info(m); diff --git a/src/x.h b/src/x.h index 127ff510ac..898c5f527b 100644 --- a/src/x.h +++ b/src/x.h @@ -414,8 +414,6 @@ xcb_visualid_t x_get_visual_for_depth(struct x_connection *c, uint8_t depth); xcb_render_pictformat_t x_get_pictfmt_for_standard(struct x_connection *c, xcb_pict_standard_t std); -xcb_screen_t *x_screen_of_display(struct x_connection *c, int screen); - /// Populates a `struct x_monitors` with the current monitor configuration. void x_update_monitors(struct x_connection *, struct x_monitors *); /// Free memory allocated for a `struct x_monitors`. From 1aa90f646652b0571ad628c9374fb8e9e4bcd99f Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Wed, 14 Feb 2024 20:30:39 +0300 Subject: [PATCH 28/55] x: remove x_sync and use xcb_aux_sync instead --- src/picom.c | 9 +++++---- src/render.c | 7 ++++--- src/x.h | 11 ----------- 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/src/picom.c b/src/picom.c index 841c6f32f8..c2cac46aaa 100644 --- a/src/picom.c +++ b/src/picom.c @@ -35,6 +35,7 @@ #include #include #include +#include #include #include @@ -1501,7 +1502,7 @@ static bool redirect_start(session_t *ps) { return false; } - x_sync(&ps->c); + xcb_aux_sync(ps->c.c); if (!initialize_backend(ps)) { return false; @@ -1560,7 +1561,7 @@ static bool redirect_start(session_t *ps) { } // Must call XSync() here - x_sync(&ps->c); + xcb_aux_sync(ps->c.c); ps->redirected = true; ps->first_frame = true; @@ -1603,7 +1604,7 @@ static void unredirect(session_t *ps) { } // Must call XSync() here - x_sync(&ps->c); + xcb_aux_sync(ps->c.c); ps->redirected = false; log_debug("Screen unredirected."); @@ -2789,7 +2790,7 @@ static void session_destroy(session_t *ps) { #endif // Flush all events - x_sync(&ps->c); + xcb_aux_sync(ps->c.c); ev_io_stop(ps->loop, &ps->xiow); if (ps->o.legacy_backends) { free_conv((conv *)ps->shadow_context); diff --git a/src/render.c b/src/render.c index 371a9be7d4..9712974c0b 100644 --- a/src/render.c +++ b/src/render.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -1229,7 +1230,7 @@ void paint_all(session_t *ps, struct managed_win *t) { if (ps->o.vsync) { // Make sure all previous requests are processed to achieve best // effect - x_sync(&ps->c); + xcb_aux_sync(ps->c.c); #ifdef CONFIG_OPENGL if (glx_has_context(ps)) { if (ps->o.vsync_use_glfinish) { @@ -1288,7 +1289,7 @@ void paint_all(session_t *ps, struct managed_win *t) { break; #ifdef CONFIG_OPENGL case BKEND_XR_GLX_HYBRID: - x_sync(&ps->c); + xcb_aux_sync(ps->c.c); if (ps->o.vsync_use_glfinish) { glFinish(); } else { @@ -1313,7 +1314,7 @@ void paint_all(session_t *ps, struct managed_win *t) { default: assert(0); } - x_sync(&ps->c); + xcb_aux_sync(ps->c.c); #ifdef CONFIG_OPENGL if (glx_has_context(ps)) { diff --git a/src/x.h b/src/x.h index 898c5f527b..3f1ecff8e8 100644 --- a/src/x.h +++ b/src/x.h @@ -207,17 +207,6 @@ void x_discard_pending(struct x_connection *c, uint32_t sequence); /// This function logs X errors, or aborts the program based on severity of the error. void x_handle_error(struct x_connection *c, xcb_generic_error_t *ev); -/** - * Send a request to X server and get the reply to make sure all previous - * requests are processed, and their replies received - * - * xcb_get_input_focus is used here because it is the same request used by - * libX11 - */ -static inline void x_sync(struct x_connection *c) { - free(xcb_get_input_focus_reply(c->c, xcb_get_input_focus(c->c), NULL)); -} - /** * Get a specific attribute of a window. * From c7591982b636d5332f76c1347c041f95f3846c36 Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Wed, 14 Feb 2024 21:05:36 +0300 Subject: [PATCH 29/55] x: remove x_get_visual_depth and use xcb_aux_get_depth_of_visual instead --- src/backend/gl/glx.c | 4 +++- src/x.c | 26 +++----------------------- src/x.h | 1 - 3 files changed, 6 insertions(+), 25 deletions(-) diff --git a/src/backend/gl/glx.c b/src/backend/gl/glx.c index af2f891e4f..5e3f9407a8 100644 --- a/src/backend/gl/glx.c +++ b/src/backend/gl/glx.c @@ -19,6 +19,7 @@ #include #include #include +#include #include "backend/backend.h" #include "backend/backend_common.h" @@ -114,7 +115,8 @@ struct glx_fbconfig_info *glx_find_fbconfig(struct x_connection *c, struct xvisu int visual; glXGetFBConfigAttribChecked(c->dpy, cfg[i], GLX_VISUAL_ID, &visual); if (m.visual_depth != -1 && - x_get_visual_depth(c, (xcb_visualid_t)visual) != m.visual_depth) { + xcb_aux_get_depth_of_visual(c->screen_info, (xcb_visualid_t)visual) != + m.visual_depth) { // FBConfig and the correspondent X Visual might not have the same // depth. (e.g. 32 bit FBConfig with a 24 bit Visual). This is // quite common, seen in both open source and proprietary drivers. diff --git a/src/x.c b/src/x.c index 3b782c0aae..84cf50e74d 100644 --- a/src/x.c +++ b/src/x.c @@ -346,24 +346,6 @@ x_get_pictfmt_for_standard(struct x_connection *c, xcb_pict_standard_t std) { return pictfmt->id; } -int x_get_visual_depth(struct x_connection *c, xcb_visualid_t visual) { - auto setup = xcb_get_setup(c->c); - for (auto screen = xcb_setup_roots_iterator(setup); screen.rem; - xcb_screen_next(&screen)) { - for (auto depth = xcb_screen_allowed_depths_iterator(screen.data); - depth.rem; xcb_depth_next(&depth)) { - const int len = xcb_depth_visuals_length(depth.data); - const xcb_visualtype_t *visuals = xcb_depth_visuals(depth.data); - for (int i = 0; i < len; i++) { - if (visual == visuals[i].visual_id) { - return depth.data->depth; - } - } - } - } - return -1; -} - xcb_render_picture_t x_create_picture_with_pictfmt_and_pixmap(struct x_connection *c, const xcb_render_pictforminfo_t *pictfmt, @@ -606,9 +588,7 @@ _x_strerror(unsigned long serial, uint8_t major, uint16_t minor, uint8_t error_c const char *name = "Unknown"; #define CASESTRRET(s) \ - case s: \ - name = #s; \ - break + case s: name = #s; break #define CASESTRRET2(s) \ case XCB_##s: name = #s; break @@ -879,8 +859,8 @@ void x_create_convolution_kernel(const conv *kernel, double center, /// Returns {-1, -1, -1, -1, -1, 0} on failure struct xvisual_info x_get_visual_info(struct x_connection *c, xcb_visualid_t visual) { auto pictfmt = x_get_pictform_for_visual(c, visual); - auto depth = x_get_visual_depth(c, visual); - if (!pictfmt || depth == -1) { + auto depth = xcb_aux_get_depth_of_visual(c->screen_info, visual); + if (!pictfmt || depth == 0) { log_error("Invalid visual %#03x", visual); return (struct xvisual_info){-1, -1, -1, -1, -1, 0}; } diff --git a/src/x.h b/src/x.h index 3f1ecff8e8..382383950a 100644 --- a/src/x.h +++ b/src/x.h @@ -265,7 +265,6 @@ bool wid_get_text_prop(session_t *ps, xcb_window_t wid, xcb_atom_t prop, char ** const xcb_render_pictforminfo_t * x_get_pictform_for_visual(struct x_connection *, xcb_visualid_t); -int x_get_visual_depth(struct x_connection *, xcb_visualid_t); xcb_render_picture_t x_create_picture_with_pictfmt_and_pixmap(struct x_connection *, From 23b0c5a1d5aaf93336756c45b47b55c2a0a2b3ca Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Wed, 14 Feb 2024 21:11:31 +0300 Subject: [PATCH 30/55] x: don't require an entire struct x_connection in x_get_visual_for_depth inspired by the xcb-util's xcb_aux_get_depth_of_visual function implementation --- src/picom.c | 2 +- src/render.c | 2 +- src/x.c | 14 +++++--------- src/x.h | 2 +- 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/picom.c b/src/picom.c index c2cac46aaa..e968c6561d 100644 --- a/src/picom.c +++ b/src/picom.c @@ -1188,7 +1188,7 @@ void root_damaged(session_t *ps) { xcb_visualid_t visual = r->depth == ps->c.screen_info->root_depth ? ps->c.screen_info->root_visual - : x_get_visual_for_depth(&ps->c, r->depth); + : x_get_visual_for_depth(ps->c.screen_info, r->depth); free(r); ps->root_image = ps->backend_data->ops->bind_pixmap( diff --git a/src/render.c b/src/render.c index 9712974c0b..9321a8e77c 100644 --- a/src/render.c +++ b/src/render.c @@ -624,7 +624,7 @@ static bool get_root_tile(session_t *ps) { } else { visual = r->depth == ps->c.screen_info->root_depth ? ps->c.screen_info->root_visual - : x_get_visual_for_depth(&ps->c, r->depth); + : x_get_visual_for_depth(ps->c.screen_info, r->depth); free(r); } diff --git a/src/x.c b/src/x.c index 84cf50e74d..f33142b279 100644 --- a/src/x.c +++ b/src/x.c @@ -322,15 +322,11 @@ xcb_visualid_t x_get_visual_for_standard(struct x_connection *c, xcb_pict_standa return x_get_visual_for_pictfmt(g_pictfmts, pictfmt->id); } -xcb_visualid_t x_get_visual_for_depth(struct x_connection *c, uint8_t depth) { - xcb_screen_iterator_t screen_it = xcb_setup_roots_iterator(xcb_get_setup(c->c)); - for (; screen_it.rem; xcb_screen_next(&screen_it)) { - xcb_depth_iterator_t depth_it = - xcb_screen_allowed_depths_iterator(screen_it.data); - for (; depth_it.rem; xcb_depth_next(&depth_it)) { - if (depth_it.data->depth == depth) { - return xcb_depth_visuals_iterator(depth_it.data).data->visual_id; - } +xcb_visualid_t x_get_visual_for_depth(xcb_screen_t *screen, uint8_t depth) { + xcb_depth_iterator_t depth_it = xcb_screen_allowed_depths_iterator(screen); + for (; depth_it.rem; xcb_depth_next(&depth_it)) { + if (depth_it.data->depth == depth) { + return xcb_depth_visuals_iterator(depth_it.data).data->visual_id; } } diff --git a/src/x.h b/src/x.h index 382383950a..cd4805db41 100644 --- a/src/x.h +++ b/src/x.h @@ -397,7 +397,7 @@ struct xvisual_info x_get_visual_info(struct x_connection *c, xcb_visualid_t vis xcb_visualid_t x_get_visual_for_standard(struct x_connection *c, xcb_pict_standard_t std); -xcb_visualid_t x_get_visual_for_depth(struct x_connection *c, uint8_t depth); +xcb_visualid_t x_get_visual_for_depth(xcb_screen_t *screen, uint8_t depth); xcb_render_pictformat_t x_get_pictfmt_for_standard(struct x_connection *c, xcb_pict_standard_t std); From e948b743631ac9c38ec35fed4d13d8d9e9088191 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sat, 10 Feb 2024 14:00:53 +0000 Subject: [PATCH 31/55] backend: gl: remove an unused type Signed-off-by: Yuxuan Shui --- src/backend/gl/glx.h | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/backend/gl/glx.h b/src/backend/gl/glx.h index a7663d946a..86701023a0 100644 --- a/src/backend/gl/glx.h +++ b/src/backend/gl/glx.h @@ -19,20 +19,6 @@ struct glx_fbconfig_info { int y_inverted; }; -/// The search criteria for glx_find_fbconfig -struct glx_fbconfig_criteria { - /// Bit width of the red component - int red_size; - /// Bit width of the green component - int green_size; - /// Bit width of the blue component - int blue_size; - /// Bit width of the alpha component - int alpha_size; - /// The depth of X visual - int visual_depth; -}; - struct glx_fbconfig_info *glx_find_fbconfig(struct x_connection *, struct xvisual_info); struct glxext_info { From e8d42885fa85b1ceacdc11dbc4e7dd6ff37b4b5f Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sat, 10 Feb 2024 14:19:48 +0000 Subject: [PATCH 32/55] backend: gl: don't force fbconfig info on to heap It's fairly small, so it's reasonable to put it on the stack. Signed-off-by: Yuxuan Shui --- src/backend/gl/glx.c | 35 +++++++++++++++++------------------ src/backend/gl/glx.h | 3 ++- src/common.h | 2 +- src/opengl.c | 2 +- src/opengl.h | 5 +---- src/picom.c | 5 ----- src/render.c | 29 +++++++++++++++-------------- src/render.h | 2 +- 8 files changed, 38 insertions(+), 45 deletions(-) diff --git a/src/backend/gl/glx.c b/src/backend/gl/glx.c index af2f891e4f..c86ccb5547 100644 --- a/src/backend/gl/glx.c +++ b/src/backend/gl/glx.c @@ -54,10 +54,13 @@ struct _glx_data { } \ } while (0) -struct glx_fbconfig_info *glx_find_fbconfig(struct x_connection *c, struct xvisual_info m) { +bool glx_find_fbconfig(struct x_connection *c, struct xvisual_info m, + struct glx_fbconfig_info *info) { log_debug("Looking for FBConfig for RGBA%d%d%d%d, depth %d", m.red_size, m.blue_size, m.green_size, m.alpha_size, m.visual_depth); + info->cfg = NULL; + int ncfg; // clang-format off GLXFBConfig *cfg = @@ -143,16 +146,13 @@ struct glx_fbconfig_info *glx_find_fbconfig(struct x_connection *c, struct xvisu min_cost = depthbuf + stencil + bufsize * (doublebuf + 1); } free(cfg); - if (!found) { - return NULL; + if (found) { + info->cfg = ret; + info->texture_tgts = texture_tgts; + info->texture_fmt = texture_fmt; + info->y_inverted = y_inverted; } - - auto info = cmalloc(struct glx_fbconfig_info); - info->cfg = ret; - info->texture_tgts = texture_tgts; - info->texture_fmt = texture_fmt; - info->y_inverted = y_inverted; - return info; + return found; } /** @@ -394,8 +394,8 @@ glx_bind_pixmap(backend_t *base, xcb_pixmap_t pixmap, struct xvisual_info fmt, b wd->inner = (struct backend_image_inner_base *)inner; free(r); - auto fbcfg = glx_find_fbconfig(base->c, fmt); - if (!fbcfg) { + struct glx_fbconfig_info fbcfg; + if (!glx_find_fbconfig(base->c, fmt, &fbcfg)) { log_error("Couldn't find FBConfig with requested visual %x", fmt.visual); goto err; } @@ -403,29 +403,28 @@ glx_bind_pixmap(backend_t *base, xcb_pixmap_t pixmap, struct xvisual_info fmt, b // Choose a suitable texture target for our pixmap. // Refer to GLX_EXT_texture_om_pixmap spec to see what are the mean // of the bits in texture_tgts - if (!(fbcfg->texture_tgts & GLX_TEXTURE_2D_BIT_EXT)) { + if (!(fbcfg.texture_tgts & GLX_TEXTURE_2D_BIT_EXT)) { log_error("Cannot bind pixmap to GL_TEXTURE_2D, giving up"); goto err; } log_debug("depth %d, rgba %d", fmt.visual_depth, - (fbcfg->texture_fmt == GLX_TEXTURE_FORMAT_RGBA_EXT)); + (fbcfg.texture_fmt == GLX_TEXTURE_FORMAT_RGBA_EXT)); GLint attrs[] = { GLX_TEXTURE_FORMAT_EXT, - fbcfg->texture_fmt, + fbcfg.texture_fmt, GLX_TEXTURE_TARGET_EXT, GLX_TEXTURE_2D_EXT, 0, }; - inner->y_inverted = fbcfg->y_inverted; + inner->y_inverted = fbcfg.y_inverted; glxpixmap = cmalloc(struct _glx_pixmap); glxpixmap->pixmap = pixmap; - glxpixmap->glpixmap = glXCreatePixmap(base->c->dpy, fbcfg->cfg, pixmap, attrs); + glxpixmap->glpixmap = glXCreatePixmap(base->c->dpy, fbcfg.cfg, pixmap, attrs); glxpixmap->owned = owned; - free(fbcfg); if (!glxpixmap->glpixmap) { log_error("Failed to create glpixmap for pixmap %#010x", pixmap); diff --git a/src/backend/gl/glx.h b/src/backend/gl/glx.h index 86701023a0..7167936542 100644 --- a/src/backend/gl/glx.h +++ b/src/backend/gl/glx.h @@ -19,7 +19,8 @@ struct glx_fbconfig_info { int y_inverted; }; -struct glx_fbconfig_info *glx_find_fbconfig(struct x_connection *, struct xvisual_info); +bool glx_find_fbconfig(struct x_connection *c, struct xvisual_info m, + struct glx_fbconfig_info *info); struct glxext_info { bool initialized; diff --git a/src/common.h b/src/common.h index 6ba9b4f8a1..35316e1057 100644 --- a/src/common.h +++ b/src/common.h @@ -204,7 +204,7 @@ typedef struct session { /// Custom GLX program used for painting window. // XXX should be in struct glx_session glx_prog_main_t glx_prog_win; - struct glx_fbconfig_info *argb_fbconfig; + struct glx_fbconfig_info argb_fbconfig; #endif /// Sync fence to sync draw operations xcb_sync_fence_t sync_fence; diff --git a/src/opengl.c b/src/opengl.c index eb40f1b3dd..26679b3773 100644 --- a/src/opengl.c +++ b/src/opengl.c @@ -279,7 +279,7 @@ void glx_destroy(session_t *ps) { free(ps->psglx); ps->psglx = NULL; - ps->argb_fbconfig = NULL; + ps->argb_fbconfig = (struct glx_fbconfig_info){0}; } /** diff --git a/src/opengl.h b/src/opengl.h index 6f6da068e2..5bf92ed438 100644 --- a/src/opengl.h +++ b/src/opengl.h @@ -228,10 +228,7 @@ static inline void free_texture(session_t *ps, glx_texture_t **pptex) { */ static inline void free_paint_glx(session_t *ps, paint_t *ppaint) { free_texture(ps, &ppaint->ptex); -#ifdef CONFIG_OPENGL - free(ppaint->fbcfg); -#endif - ppaint->fbcfg = NULL; + ppaint->fbcfg = (struct glx_fbconfig_info){0}; } /** diff --git a/src/picom.c b/src/picom.c index 841c6f32f8..ff355480e8 100644 --- a/src/picom.c +++ b/src/picom.c @@ -2640,11 +2640,6 @@ static void session_destroy(session_t *ps) { unredirect(ps); } -#ifdef CONFIG_OPENGL - free(ps->argb_fbconfig); - ps->argb_fbconfig = NULL; -#endif - file_watch_destroy(ps->loop, ps->file_watch_handle); ps->file_watch_handle = NULL; diff --git a/src/render.c b/src/render.c index 371a9be7d4..a76f1e904c 100644 --- a/src/render.c +++ b/src/render.c @@ -55,19 +55,20 @@ static inline bool paint_bind_tex(session_t *ps, paint_t *ppaint, int wid, int h struct glx_fbconfig_info *fbcfg; if (!visual) { assert(depth == 32); - if (!ps->argb_fbconfig) { - ps->argb_fbconfig = glx_find_fbconfig( - &ps->c, (struct xvisual_info){.red_size = 8, - .green_size = 8, - .blue_size = 8, - .alpha_size = 8, - .visual_depth = 32}); + if (!ps->argb_fbconfig.cfg) { + glx_find_fbconfig(&ps->c, + (struct xvisual_info){.red_size = 8, + .green_size = 8, + .blue_size = 8, + .alpha_size = 8, + .visual_depth = 32}, + &ps->argb_fbconfig); } - if (!ps->argb_fbconfig) { + if (!ps->argb_fbconfig.cfg) { log_error("Failed to find appropriate FBConfig for 32 bit depth"); return false; } - fbcfg = ps->argb_fbconfig; + fbcfg = &ps->argb_fbconfig; } else { auto m = x_get_visual_info(&ps->c, visual); if (m.visual_depth < 0) { @@ -79,14 +80,14 @@ static inline bool paint_bind_tex(session_t *ps, paint_t *ppaint, int wid, int h return false; } - if (!ppaint->fbcfg) { - ppaint->fbcfg = glx_find_fbconfig(&ps->c, m); + if (!ppaint->fbcfg.cfg) { + glx_find_fbconfig(&ps->c, m, &ppaint->fbcfg); } - if (!ppaint->fbcfg) { + if (!ppaint->fbcfg.cfg) { log_error("Failed to find appropriate FBConfig for X pixmap"); return false; } - fbcfg = ppaint->fbcfg; + fbcfg = &ppaint->fbcfg; } if (force || !glx_tex_bound(ppaint->ptex, ppaint->pixmap)) { @@ -1528,7 +1529,7 @@ void deinit_render(session_t *ps) { free_root_tile(ps); #ifdef CONFIG_OPENGL - free(ps->root_tile_paint.fbcfg); + ps->root_tile_paint.fbcfg = (struct glx_fbconfig_info){0}; if (bkend_use_glx(ps)) { glx_destroy(ps); } diff --git a/src/render.h b/src/render.h index 4e0c7a8162..62258f0edb 100644 --- a/src/render.h +++ b/src/render.h @@ -21,7 +21,7 @@ typedef struct paint { xcb_render_picture_t pict; glx_texture_t *ptex; #ifdef CONFIG_OPENGL - struct glx_fbconfig_info *fbcfg; + struct glx_fbconfig_info fbcfg; #endif } paint_t; From 241d7f1d035524025df3186ac6cce51026245577 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sat, 10 Feb 2024 14:32:08 +0000 Subject: [PATCH 33/55] backend: glx: cache GLX FBConfigs This should marginally speed up pixmap binding for the glx backend (we don't need FBConfigs for egl). Fix a long running complaint in #381 (unrelated issue, but there is complaint in there about glXChooseFBConfig being called whenever we bind a new pixmap). Signed-off-by: Yuxuan Shui --- src/backend/gl/glx.c | 52 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 11 deletions(-) diff --git a/src/backend/gl/glx.c b/src/backend/gl/glx.c index c86ccb5547..215f7b2327 100644 --- a/src/backend/gl/glx.c +++ b/src/backend/gl/glx.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -44,6 +45,13 @@ struct _glx_data { struct gl_data gl; xcb_window_t target_win; GLXContext ctx; + struct glx_fbconfig_cache *cached_fbconfigs; +}; + +struct glx_fbconfig_cache { + UT_hash_handle hh; + struct xvisual_info visual_info; + struct glx_fbconfig_info info; }; #define glXGetFBConfigAttribChecked(a, b, attr, c) \ @@ -56,8 +64,8 @@ struct _glx_data { bool glx_find_fbconfig(struct x_connection *c, struct xvisual_info m, struct glx_fbconfig_info *info) { - log_debug("Looking for FBConfig for RGBA%d%d%d%d, depth %d", m.red_size, - m.blue_size, m.green_size, m.alpha_size, m.visual_depth); + log_debug("Looking for FBConfig for RGBA%d%d%d%d, depth: %d, visual id: %#x", m.red_size, + m.blue_size, m.green_size, m.alpha_size, m.visual_depth, m.visual); info->cfg = NULL; @@ -197,6 +205,12 @@ void glx_deinit(backend_t *base) { gd->ctx = 0; } + struct glx_fbconfig_cache *cached_fbconfig = NULL, *tmp = NULL; + HASH_ITER(hh, gd->cached_fbconfigs, cached_fbconfig, tmp) { + HASH_DEL(gd->cached_fbconfigs, cached_fbconfig); + free(cached_fbconfig); + } + free(gd); } @@ -367,6 +381,7 @@ static backend_t *glx_init(session_t *ps, xcb_window_t target) { static void * glx_bind_pixmap(backend_t *base, xcb_pixmap_t pixmap, struct xvisual_info fmt, bool owned) { struct _glx_pixmap *glxpixmap = NULL; + auto gd = (struct _glx_data *)base; // Retrieve pixmap parameters, if they aren't provided if (fmt.visual_depth > OPENGL_MAX_DEPTH) { log_error("Requested depth %d higher than max possible depth %d.", @@ -394,36 +409,51 @@ glx_bind_pixmap(backend_t *base, xcb_pixmap_t pixmap, struct xvisual_info fmt, b wd->inner = (struct backend_image_inner_base *)inner; free(r); - struct glx_fbconfig_info fbcfg; - if (!glx_find_fbconfig(base->c, fmt, &fbcfg)) { - log_error("Couldn't find FBConfig with requested visual %x", fmt.visual); - goto err; + struct glx_fbconfig_cache *cached_fbconfig = NULL; + HASH_FIND(hh, gd->cached_fbconfigs, &fmt, sizeof(fmt), cached_fbconfig); + if (!cached_fbconfig) { + struct glx_fbconfig_info fbconfig; + if (!glx_find_fbconfig(base->c, fmt, &fbconfig)) { + log_error("Couldn't find FBConfig with requested visual %#x", + fmt.visual); + goto err; + } + cached_fbconfig = cmalloc(struct glx_fbconfig_cache); + cached_fbconfig->visual_info = fmt; + cached_fbconfig->info = fbconfig; + HASH_ADD(hh, gd->cached_fbconfigs, visual_info, sizeof(fmt), cached_fbconfig); + } else { + log_debug("Found cached FBConfig for RGBA%d%d%d%d, depth: %d, visual id: " + "%#x", + fmt.red_size, fmt.blue_size, fmt.green_size, fmt.alpha_size, + fmt.visual_depth, fmt.visual); } + struct glx_fbconfig_info *fbconfig = &cached_fbconfig->info; // Choose a suitable texture target for our pixmap. // Refer to GLX_EXT_texture_om_pixmap spec to see what are the mean // of the bits in texture_tgts - if (!(fbcfg.texture_tgts & GLX_TEXTURE_2D_BIT_EXT)) { + if (!(fbconfig->texture_tgts & GLX_TEXTURE_2D_BIT_EXT)) { log_error("Cannot bind pixmap to GL_TEXTURE_2D, giving up"); goto err; } log_debug("depth %d, rgba %d", fmt.visual_depth, - (fbcfg.texture_fmt == GLX_TEXTURE_FORMAT_RGBA_EXT)); + (fbconfig->texture_fmt == GLX_TEXTURE_FORMAT_RGBA_EXT)); GLint attrs[] = { GLX_TEXTURE_FORMAT_EXT, - fbcfg.texture_fmt, + fbconfig->texture_fmt, GLX_TEXTURE_TARGET_EXT, GLX_TEXTURE_2D_EXT, 0, }; - inner->y_inverted = fbcfg.y_inverted; + inner->y_inverted = fbconfig->y_inverted; glxpixmap = cmalloc(struct _glx_pixmap); glxpixmap->pixmap = pixmap; - glxpixmap->glpixmap = glXCreatePixmap(base->c->dpy, fbcfg.cfg, pixmap, attrs); + glxpixmap->glpixmap = glXCreatePixmap(base->c->dpy, fbconfig->cfg, pixmap, attrs); glxpixmap->owned = owned; if (!glxpixmap->glpixmap) { From 85bb56e8a6f056fa11937453c70503b28396f7df Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Wed, 14 Feb 2024 18:26:50 +0000 Subject: [PATCH 34/55] Update CHANGELOG.md Signed-off-by: Yuxuan Shui --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 330d69c2db..c993ddefe0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ * Allow `corner-radius-rules` to override `corner-radius = 0`. Previously setting corner radius to 0 globally disables rounded corners. (#1170) +## Notable changes + +* Marginally improve performance when resizing/opening/closing windows. (#1190) + # v11.2 (2024-Feb-13) ## Build changes From 5b81ea2c5805deab4033c5fdcce8a6d6c285e400 Mon Sep 17 00:00:00 2001 From: Maxim Solovyov Date: Thu, 15 Feb 2024 00:03:22 +0300 Subject: [PATCH 35/55] ci: build with --werror on freebsd and openbsd --- .builds/freebsd.yml | 2 +- .builds/openbsd.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.builds/freebsd.yml b/.builds/freebsd.yml index 47b333544d..baa81ab6e4 100644 --- a/.builds/freebsd.yml +++ b/.builds/freebsd.yml @@ -20,7 +20,7 @@ sources: tasks: - setup: | cd picom - CPPFLAGS="-I/usr/local/include" meson setup -Dunittest=true build + CPPFLAGS="-I/usr/local/include" meson setup -Dunittest=true --werror build - build: | cd picom ninja -C build diff --git a/.builds/openbsd.yml b/.builds/openbsd.yml index c8e667e114..1b82536a59 100644 --- a/.builds/openbsd.yml +++ b/.builds/openbsd.yml @@ -14,7 +14,7 @@ sources: tasks: - setup: | cd picom - CPPFLAGS="-I/usr/local/include" LDFLAGS="-L/usr/local/lib" meson setup -Dunittest=true build + CPPFLAGS="-I/usr/local/include" LDFLAGS="-L/usr/local/lib" meson setup -Dunittest=true --werror build - build: | cd picom ninja -C build From 84b9ff31487b302a389e87382a860f025497af1e Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Wed, 14 Feb 2024 23:15:26 +0000 Subject: [PATCH 36/55] x: support getting winprop_t items as atoms Signed-off-by: Yuxuan Shui --- src/x.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/x.h b/src/x.h index cd4805db41..f465b22a22 100644 --- a/src/x.h +++ b/src/x.h @@ -27,6 +27,7 @@ typedef struct winprop { int16_t *p16; int32_t *p32; uint32_t *c32; // 32bit cardinal + xcb_atom_t *atom; }; unsigned long nitems; xcb_atom_t type; From 613d179f2d66f040c7843fb4c25a725396ec9d23 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Wed, 14 Feb 2024 18:38:20 +0000 Subject: [PATCH 37/55] win: cache the EWMH fullscreen property Signed-off-by: Yuxuan Shui --- src/event.c | 7 ++++++ src/win.c | 67 +++++++++++++++++++++++++++++------------------------ src/win.h | 2 ++ 3 files changed, 46 insertions(+), 30 deletions(-) diff --git a/src/event.c b/src/event.c index a1b93fd476..17374c3f1e 100644 --- a/src/event.c +++ b/src/event.c @@ -565,6 +565,13 @@ static inline void ev_property_notify(session_t *ps, xcb_property_notify_event_t } } + if (!ps->o.no_ewmh_fullscreen && ev->atom == ps->atoms->a_NET_WM_STATE) { + auto w = find_toplevel(ps, ev->window); + if (w) { + win_set_property_stale(w, ev->atom); + } + } + // Check for other atoms we are tracking for (latom_t *platom = ps->track_atom_lst; platom; platom = platom->next) { if (platom->atom == ev->atom) { diff --git a/src/win.c b/src/win.c index 81660c81bd..0a1bd8119a 100644 --- a/src/win.c +++ b/src/win.c @@ -73,6 +73,11 @@ static void win_update_frame_extents(session_t *ps, struct managed_win *w, xcb_window_t client); static void win_update_prop_shadow_raw(session_t *ps, struct managed_win *w); static void win_update_prop_shadow(session_t *ps, struct managed_win *w); +/** + * Update window EWMH fullscreen state. + */ +bool win_update_prop_fullscreen(struct x_connection *c, const struct atom *atoms, + struct managed_win *w); /** * Update leader of a window. */ @@ -457,6 +462,12 @@ static void win_update_properties(session_t *ps, struct managed_win *w) { win_update_prop_shadow(ps, w); } + if (win_fetch_and_unset_property_stale(w, ps->atoms->a_NET_WM_STATE)) { + if (win_update_prop_fullscreen(&ps->c, ps->atoms, w)) { + win_set_flags(w, WIN_FLAGS_FACTOR_CHANGED); + } + } + if (win_fetch_and_unset_property_stale(w, ps->atoms->aWM_CLIENT_LEADER) || win_fetch_and_unset_property_stale(w, ps->atoms->aWM_TRANSIENT_FOR)) { win_update_leader(ps, w); @@ -1012,6 +1023,30 @@ void win_update_prop_shadow(session_t *ps, struct managed_win *w) { } } +/** + * Update window EWMH fullscreen state. + */ +bool win_update_prop_fullscreen(struct x_connection *c, const struct atom *atoms, + struct managed_win *w) { + auto prop = x_get_prop(c, w->client_win, atoms->a_NET_WM_STATE, 12, XCB_ATOM_ATOM, 0); + if (!prop.nitems) { + return false; + } + + bool is_fullscreen = false; + for (uint32_t i = 0; i < prop.nitems; i++) { + if (prop.atom[i] == atoms->a_NET_WM_STATE_FULLSCREEN) { + is_fullscreen = true; + break; + } + } + free_winprop(&prop); + + bool changed = w->is_ewmh_fullscreen != is_fullscreen; + w->is_ewmh_fullscreen = is_fullscreen; + return changed; +} + static void win_determine_clip_shadow_above(session_t *ps, struct managed_win *w) { bool should_crop = (ps->o.wintype_option[w->window_type].clip_shadow_above || c2_match(ps, w, ps->o.shadow_clip_list, NULL)); @@ -1723,6 +1758,7 @@ struct win *fill_win(session_t *ps, struct win *w) { ps->atoms->a_NET_WM_NAME, ps->atoms->aWM_CLASS, ps->atoms->aWM_WINDOW_ROLE, ps->atoms->a_COMPTON_SHADOW, ps->atoms->aWM_CLIENT_LEADER, ps->atoms->aWM_TRANSIENT_FOR, + ps->atoms->a_NET_WM_STATE, }; win_set_properties_stale(new, init_stale_props, ARR_SIZE(init_stale_props)); @@ -2717,34 +2753,6 @@ static inline bool rect_is_fullscreen(const session_t *ps, int x, int y, int wid return (x <= 0 && y <= 0 && (x + wid) >= ps->root_width && (y + hei) >= ps->root_height); } -/** - * Check if a window is full-screen using EWMH - * - * TODO(yshui) cache this property - */ -static inline bool -win_is_fullscreen_xcb(xcb_connection_t *c, const struct atom *a, const xcb_window_t w) { - xcb_get_property_cookie_t prop = - xcb_get_property(c, 0, w, a->a_NET_WM_STATE, XCB_ATOM_ATOM, 0, 12); - xcb_get_property_reply_t *reply = xcb_get_property_reply(c, prop, NULL); - if (!reply) { - return false; - } - - if (reply->length) { - xcb_atom_t *val = xcb_get_property_value(reply); - for (uint32_t i = 0; i < reply->length; i++) { - if (val[i] != a->a_NET_WM_STATE_FULLSCREEN) { - continue; - } - free(reply); - return true; - } - } - free(reply); - return false; -} - /// Set flags on a window. Some sanity checks are performed void win_set_flags(struct managed_win *w, uint64_t flags) { log_debug("Set flags %" PRIu64 " to window %#010x (%s)", flags, w->base.id, w->name); @@ -2830,8 +2838,7 @@ bool win_check_flags_all(struct managed_win *w, uint64_t flags) { * It's not using w->border_size for performance measures. */ bool win_is_fullscreen(const session_t *ps, const struct managed_win *w) { - if (!ps->o.no_ewmh_fullscreen && - win_is_fullscreen_xcb(ps->c.c, ps->atoms, w->client_win)) { + if (!ps->o.no_ewmh_fullscreen && w->is_ewmh_fullscreen) { return true; } return rect_is_fullscreen(ps, w->g.x, w->g.y, w->widthb, w->heightb) && diff --git a/src/win.h b/src/win.h index c45c7ef88c..acc96c030a 100644 --- a/src/win.h +++ b/src/win.h @@ -199,6 +199,8 @@ struct managed_win { char *class_general; /// WM_WINDOW_ROLE value of the window. char *role; + /// Whether the window sets the EWMH fullscreen property. + bool is_ewmh_fullscreen; // Opacity-related members /// Current window opacity. From 05b1fbff9e40bc3d4e295d5e4320c55c8e7cd3bd Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Wed, 14 Feb 2024 18:54:04 +0000 Subject: [PATCH 38/55] win: remember calculated fullscreen state for window Signed-off-by: Yuxuan Shui --- src/c2.c | 4 +--- src/picom.c | 11 ++++++++++- src/win.c | 28 ++++++++++++++-------------- src/win.h | 13 ++++++------- 4 files changed, 31 insertions(+), 25 deletions(-) diff --git a/src/c2.c b/src/c2.c index 7f1f80e0f7..daee18f7d4 100644 --- a/src/c2.c +++ b/src/c2.c @@ -1380,9 +1380,7 @@ static inline void c2_match_once_leaf(session_t *ps, const struct managed_win *w case C2_L_PWIDTHB: predef_target = w->widthb; break; case C2_L_PHEIGHTB: predef_target = w->heightb; break; case C2_L_PBDW: predef_target = w->g.border_width; break; - case C2_L_PFULLSCREEN: - predef_target = win_is_fullscreen(ps, w); - break; + case C2_L_PFULLSCREEN: predef_target = w->is_fullscreen; break; case C2_L_POVREDIR: predef_target = w->a.override_redirect; break; case C2_L_PARGB: predef_target = win_has_alpha(w); break; case C2_L_PFOCUSED: diff --git a/src/picom.c b/src/picom.c index 6c89e97789..c024dd4e7c 100644 --- a/src/picom.c +++ b/src/picom.c @@ -791,6 +791,8 @@ static bool initialize_backend(session_t *ps) { /// Handle configure event of the root window static void configure_root(session_t *ps) { + // TODO(yshui) re-initializing backend should be done outside of the + // critical section. Probably set a flag and do it in draw_callback_impl. auto r = XCB_AWAIT(xcb_get_geometry, ps->c.c, ps->c.screen_info->root); if (!r) { log_fatal("Failed to fetch root geometry"); @@ -830,6 +832,13 @@ static void configure_root(session_t *ps) { top_w->reg_ignore_valid = false; } + // Whether a window is fullscreen depends on the new screen + // size. So we need to refresh the fullscreen state of all + // windows. + win_stack_foreach_managed(w, &ps->window_stack) { + win_update_is_fullscreen(ps, w); + } + if (ps->redirected) { for (int i = 0; i < ps->ndamage; i++) { pixman_region32_clear(&ps->damage_ring[i]); @@ -1072,7 +1081,7 @@ static bool paint_preprocess(session_t *ps, bool *fade_running, bool *animation, // is not correctly set. if (ps->o.unredir_if_possible && is_highest) { if (w->mode == WMODE_SOLID && !ps->o.force_win_blend && - win_is_fullscreen(ps, w) && !w->unredir_if_possible_excluded) { + w->is_fullscreen && !w->unredir_if_possible_excluded) { unredir_possible = true; } } diff --git a/src/win.c b/src/win.c index 0a1bd8119a..aafbe024c4 100644 --- a/src/win.c +++ b/src/win.c @@ -520,6 +520,9 @@ void win_process_update_flags(session_t *ps, struct managed_win *w) { // Update window geometry w->g = w->pending_g; + // Whether a window is fullscreen changes based on its geometry + win_update_is_fullscreen(ps, w); + if (win_check_flags_all(w, WIN_FLAGS_SIZE_STALE)) { win_on_win_size_change(ps, w); win_update_bounding_shape(ps, w); @@ -1187,7 +1190,7 @@ static void win_determine_rounded_corners(session_t *ps, struct managed_win *w) // Don't round full screen windows & excluded windows, // unless we find a corner override in corner_radius_rules - if (!radius_override && ((w && win_is_fullscreen(ps, w)) || + if (!radius_override && ((w && w->is_fullscreen) || c2_match(ps, w, ps->o.rounded_corners_blacklist, NULL))) { w->corner_radius = 0; log_debug("Not rounding corners for window %#010x", w->base.id); @@ -1254,9 +1257,10 @@ void win_update_opacity_rule(session_t *ps, struct managed_win *w) { */ void win_on_factor_change(session_t *ps, struct managed_win *w) { log_debug("Window %#010x (%s) factor change", w->base.id, w->name); - // Focus needs to be updated first, as other rules might depend on the - // focused state of the window + // Focus and is_fullscreen needs to be updated first, as other rules might depend + // on the focused state of the window win_update_focused(ps, w); + win_update_is_fullscreen(ps, w); win_determine_shadow(ps, w); win_determine_clip_shadow_above(ps, w); @@ -2746,13 +2750,6 @@ struct managed_win *find_managed_window_or_parent(session_t *ps, xcb_window_t wi return (struct managed_win *)w; } -/** - * Check if a rectangle includes the whole screen. - */ -static inline bool rect_is_fullscreen(const session_t *ps, int x, int y, int wid, int hei) { - return (x <= 0 && y <= 0 && (x + wid) >= ps->root_width && (y + hei) >= ps->root_height); -} - /// Set flags on a window. Some sanity checks are performed void win_set_flags(struct managed_win *w, uint64_t flags) { log_debug("Set flags %" PRIu64 " to window %#010x (%s)", flags, w->base.id, w->name); @@ -2837,12 +2834,15 @@ bool win_check_flags_all(struct managed_win *w, uint64_t flags) { * * It's not using w->border_size for performance measures. */ -bool win_is_fullscreen(const session_t *ps, const struct managed_win *w) { +void win_update_is_fullscreen(const session_t *ps, struct managed_win *w) { if (!ps->o.no_ewmh_fullscreen && w->is_ewmh_fullscreen) { - return true; + w->is_fullscreen = true; + return; } - return rect_is_fullscreen(ps, w->g.x, w->g.y, w->widthb, w->heightb) && - (!w->bounding_shaped || w->rounded_corners); + w->is_fullscreen = w->g.x <= 0 && w->g.y <= 0 && + (w->g.x + w->widthb) >= ps->root_width && + (w->g.y + w->heightb) >= ps->root_height && + (!w->bounding_shaped || w->rounded_corners); } /** diff --git a/src/win.h b/src/win.h index acc96c030a..f98cb9a86d 100644 --- a/src/win.h +++ b/src/win.h @@ -201,6 +201,10 @@ struct managed_win { char *role; /// Whether the window sets the EWMH fullscreen property. bool is_ewmh_fullscreen; + /// Whether the window should be considered fullscreen. Based on + /// `is_ewmh_fullscreen`, or the windows spatial relation with the + /// root window. Which one is used is determined by user configuration. + bool is_fullscreen; // Opacity-related members /// Current window opacity. @@ -351,6 +355,8 @@ void win_update_monitor(struct x_monitors *monitors, struct managed_win *mw); */ // XXX was win_border_size void win_update_bounding_shape(session_t *ps, struct managed_win *w); +/// Recheck if a window is fullscreen +void win_update_is_fullscreen(const session_t *ps, struct managed_win *w); /** * Check if a window has BYPASS_COMPOSITOR property set */ @@ -426,13 +432,6 @@ struct managed_win *find_toplevel(session_t *ps, xcb_window_t id); */ struct managed_win *find_managed_window_or_parent(session_t *ps, xcb_window_t wid); -/** - * Check if a window is a fullscreen window. - * - * It's not using w->border_size for performance measures. - */ -bool attr_pure win_is_fullscreen(const session_t *ps, const struct managed_win *w); - /** * Check if a window is focused, without using any focus rules or forced focus settings */ From f509008cb957802b953779647a5c6506da410424 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Wed, 14 Feb 2024 23:30:05 +0000 Subject: [PATCH 39/55] win: don't call win_on_factor_change in win_update_bounding_shape Later in win_process_update_flags, we check for WIN_FLAGS_FACTOR_CHANGED and will call win_on_factor_change if needed. So in win_update_bounding_shape we just need to set that flag. Otherwise we call win_on_factor_change multiple times unnecessarily. Signed-off-by: Yuxuan Shui --- src/win.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/win.c b/src/win.c index aafbe024c4..76f085d43c 100644 --- a/src/win.c +++ b/src/win.c @@ -2046,14 +2046,12 @@ void win_update_bounding_shape(session_t *ps, struct managed_win *w) { // Window shape changed, we should free old wpaint and shadow pict // log_trace("free out dated pict"); - win_set_flags(w, WIN_FLAGS_IMAGES_STALE); + win_set_flags(w, WIN_FLAGS_IMAGES_STALE | WIN_FLAGS_FACTOR_CHANGED); win_release_mask(ps->backend_data, w); ps->pending_updates = true; free_paint(ps, &w->paint); free_paint(ps, &w->shadow_paint); - - win_on_factor_change(ps, w); } /** From 7e7c2b0cef9c54f2aa1ca987963c97ee38d2fcc8 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Thu, 15 Feb 2024 00:07:31 +0000 Subject: [PATCH 40/55] win: store focused state in struct managed_win So we don't need the whole session_t just to check if a window is focused. Signed-off-by: Yuxuan Shui --- src/c2.c | 4 +--- src/dbus.c | 4 ++-- src/win.c | 27 +++++++++++++++------------ src/win.h | 4 +++- 4 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/c2.c b/src/c2.c index daee18f7d4..614c456cba 100644 --- a/src/c2.c +++ b/src/c2.c @@ -1383,9 +1383,7 @@ static inline void c2_match_once_leaf(session_t *ps, const struct managed_win *w case C2_L_PFULLSCREEN: predef_target = w->is_fullscreen; break; case C2_L_POVREDIR: predef_target = w->a.override_redirect; break; case C2_L_PARGB: predef_target = win_has_alpha(w); break; - case C2_L_PFOCUSED: - predef_target = win_is_focused_raw(ps, w); - break; + case C2_L_PFOCUSED: predef_target = win_is_focused_raw(w); break; case C2_L_PWMWIN: predef_target = w->wmwin; break; case C2_L_PBSHAPED: predef_target = w->bounding_shaped; break; case C2_L_PROUNDED: predef_target = w->rounded_corners; break; diff --git a/src/dbus.c b/src/dbus.c index 861d424e26..f16695b793 100644 --- a/src/dbus.c +++ b/src/dbus.c @@ -906,7 +906,7 @@ cdbus_process_window_property_get(session_t *ps, DBusMessage *msg, cdbus_window_ } if (!strcmp("RawFocused", target)) { cdbus_reply(ps, msg, cdbus_append_bool_variant, - (bool[]){win_is_focused_raw(ps, w)}); + (bool[]){win_is_focused_raw(w)}); return true; } @@ -976,7 +976,7 @@ static bool cdbus_process_win_get(session_t *ps, DBusMessage *msg) { cdbus_m_win_get_do(wmwin, cdbus_reply_bool); cdbus_m_win_get_do(leader, cdbus_reply_wid); if (!strcmp("focused_raw", target)) { - cdbus_reply_bool(ps, msg, win_is_focused_raw(ps, w)); + cdbus_reply_bool(ps, msg, win_is_focused_raw(w)); return true; } cdbus_m_win_get_do(fade_force, cdbus_reply_enum); diff --git a/src/win.c b/src/win.c index 76f085d43c..a966022741 100644 --- a/src/win.c +++ b/src/win.c @@ -139,10 +139,10 @@ static inline bool attr_pure win_is_real_visible(const struct managed_win *w) { * Update focused state of a window. */ static void win_update_focused(session_t *ps, struct managed_win *w) { - if (UNSET != w->focused_force) { + if (w->focused_force != UNSET) { w->focused = w->focused_force; } else { - w->focused = win_is_focused_raw(ps, w); + w->focused = win_is_focused_raw(w); // Use wintype_focus, and treat WM windows and override-redirected // windows specially @@ -210,7 +210,7 @@ static inline bool group_is_focused(session_t *ps, xcb_window_t leader) { continue; } auto mw = (struct managed_win *)w; - if (win_get_leader(ps, mw) == leader && win_is_focused_raw(ps, mw)) { + if (win_get_leader(ps, mw) == leader && win_is_focused_raw(mw)) { return true; } } @@ -843,7 +843,7 @@ double win_calc_opacity_target(session_t *ps, const struct managed_win *w) { } else { // Respect active_opacity only when the window is physically // focused - if (win_is_focused_raw(ps, w)) { + if (win_is_focused_raw(w)) { opacity = ps->o.active_opacity; } else if (!w->focused) { // Respect inactive_opacity in some cases @@ -1792,7 +1792,7 @@ static inline void win_set_leader(session_t *ps, struct managed_win *w, xcb_wind // Update the old and new window group and active_leader if the // window could affect their state. xcb_window_t cache_leader = win_get_leader(ps, w); - if (win_is_focused_raw(ps, w) && cache_leader_old != cache_leader) { + if (win_is_focused_raw(w) && cache_leader_old != cache_leader) { ps->active_leader = cache_leader; group_on_factor_change(ps, cache_leader_old); @@ -1905,7 +1905,7 @@ static void win_on_focus_change(session_t *ps, struct managed_win *w) { xcb_window_t leader = win_get_leader(ps, w); // If the window gets focused, replace the old active_leader - if (win_is_focused_raw(ps, w) && leader != ps->active_leader) { + if (win_is_focused_raw(w) && leader != ps->active_leader) { xcb_window_t active_leader_old = ps->active_leader; ps->active_leader = leader; @@ -1914,7 +1914,7 @@ static void win_on_focus_change(session_t *ps, struct managed_win *w) { group_on_factor_change(ps, leader); } // If the group get unfocused, remove it from active_leader - else if (!win_is_focused_raw(ps, w) && leader && + else if (!win_is_focused_raw(w) && leader && leader == ps->active_leader && !group_is_focused(ps, leader)) { ps->active_leader = XCB_NONE; group_on_factor_change(ps, leader); @@ -1927,7 +1927,7 @@ static void win_on_focus_change(session_t *ps, struct managed_win *w) { #ifdef CONFIG_DBUS // Send D-Bus signal if (ps->o.dbus) { - if (win_is_focused_raw(ps, w)) { + if (win_is_focused_raw(w)) { cdbus_ev_win_focusin(ps, &w->base); } else { cdbus_ev_win_focusout(ps, &w->base); @@ -1945,15 +1945,18 @@ void win_set_focused(session_t *ps, struct managed_win *w) { return; } - if (win_is_focused_raw(ps, w)) { + if (w->is_ewmh_focused) { + assert(ps->active_win == w); return; } auto old_active_win = ps->active_win; ps->active_win = w; - assert(win_is_focused_raw(ps, w)); + w->is_ewmh_focused = true; if (old_active_win) { + assert(old_active_win->is_ewmh_focused); + old_active_win->is_ewmh_focused = false; win_on_focus_change(ps, old_active_win); } win_on_focus_change(ps, w); @@ -2866,8 +2869,8 @@ bool win_is_bypassing_compositor(const session_t *ps, const struct managed_win * * Check if a window is focused, without using any focus rules or forced focus * settings */ -bool win_is_focused_raw(const session_t *ps, const struct managed_win *w) { - return w->a.map_state == XCB_MAP_STATE_VIEWABLE && ps->active_win == w; +bool win_is_focused_raw(const struct managed_win *w) { + return w->a.map_state == XCB_MAP_STATE_VIEWABLE && w->is_ewmh_focused; } // Find the managed window immediately below `i` in the window stack diff --git a/src/win.h b/src/win.h index f98cb9a86d..93f10b151c 100644 --- a/src/win.h +++ b/src/win.h @@ -205,6 +205,8 @@ struct managed_win { /// `is_ewmh_fullscreen`, or the windows spatial relation with the /// root window. Which one is used is determined by user configuration. bool is_fullscreen; + /// Whether the window is the EWMH active window. + bool is_ewmh_focused; // Opacity-related members /// Current window opacity. @@ -435,7 +437,7 @@ struct managed_win *find_managed_window_or_parent(session_t *ps, xcb_window_t wi /** * Check if a window is focused, without using any focus rules or forced focus settings */ -bool attr_pure win_is_focused_raw(const session_t *ps, const struct managed_win *w); +bool attr_pure win_is_focused_raw(const struct managed_win *w); /// check if window has ARGB visual bool attr_pure win_has_alpha(const struct managed_win *w); From 216bfefd9ffba46d98e9bc1aaf9d6c10047a688f Mon Sep 17 00:00:00 2001 From: Lenivaya Date: Thu, 15 Feb 2024 13:03:11 +0200 Subject: [PATCH 41/55] fix: add libepoxy to default nix overlay --- flake.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flake.nix b/flake.nix index 414cacc98d..943c45e9d9 100644 --- a/flake.nix +++ b/flake.nix @@ -13,7 +13,7 @@ picom = super.picom.overrideAttrs (oldAttrs: rec { pname = "picom"; buildInputs = [ - self.pcre2 self.xorg.xcbutil + self.pcre2 self.xorg.xcbutil self.libepoxy ] ++ self.lib.remove self.xorg.libXinerama ( self.lib.remove self.pcre oldAttrs.buildInputs ); From 42782689e850ac7403a229d4f103822e909b64ad Mon Sep 17 00:00:00 2001 From: Lenivaya Date: Thu, 15 Feb 2024 13:37:12 +0200 Subject: [PATCH 42/55] chore: deduplication and reformat of flake.nix --- flake.nix | 84 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 50 insertions(+), 34 deletions(-) diff --git a/flake.nix b/flake.nix index 943c45e9d9..59956ac6dc 100644 --- a/flake.nix +++ b/flake.nix @@ -7,39 +7,55 @@ }; }; outputs = { - self, flake-utils, nixpkgs, git-ignore-nix, ... - }: flake-utils.lib.eachDefaultSystem (system: let - overlay = self: super: { - picom = super.picom.overrideAttrs (oldAttrs: rec { - pname = "picom"; - buildInputs = [ - self.pcre2 self.xorg.xcbutil self.libepoxy - ] ++ self.lib.remove self.xorg.libXinerama ( - self.lib.remove self.pcre oldAttrs.buildInputs - ); - src = git-ignore-nix.lib.gitignoreSource ./.; - }); - }; - pkgs = import nixpkgs { inherit system overlays; config.allowBroken = true; }; - overlays = [ overlay ]; - in rec { - inherit overlay overlays; - defaultPackage = pkgs.picom.overrideAttrs (o: { - version = "11"; - src = ./.; - buildInputs = o.buildInputs ++ [ pkgs.libepoxy ]; + self, + flake-utils, + nixpkgs, + git-ignore-nix, + ... + }: + flake-utils.lib.eachDefaultSystem (system: let + overlay = self: super: { + picom = super.picom.overrideAttrs (oldAttrs: rec { + version = "11"; + pname = "picom"; + buildInputs = + [ + self.pcre2 + self.xorg.xcbutil + self.libepoxy + ] + ++ self.lib.remove self.xorg.libXinerama ( + self.lib.remove self.pcre oldAttrs.buildInputs + ); + src = git-ignore-nix.lib.gitignoreSource ./.; + }); + }; + + pkgs = import nixpkgs { + inherit system overlays; + config.allowBroken = true; + }; + + overlays = [overlay]; + in rec { + inherit + overlay + overlays + ; + defaultPackage = pkgs.picom; + devShell = defaultPackage.overrideAttrs { + buildInputs = + defaultPackage.buildInputs + ++ (with pkgs; [ + clang-tools_17 + llvmPackages_17.clang-unwrapped.python + ]); + hardeningDisable = ["fortify"]; + shellHook = '' + # Workaround a NixOS limitation on sanitizers: + # See: https://github.com/NixOS/nixpkgs/issues/287763 + export LD_LIBRARY_PATH+=":/run/opengl-driver/lib" + ''; + }; }); - devShell = defaultPackage.overrideAttrs { - buildInputs = defaultPackage.buildInputs ++ (with pkgs; [ - clang-tools_17 - llvmPackages_17.clang-unwrapped.python - ]); - hardeningDisable = [ "fortify" ]; - shellHook = '' - # Workaround a NixOS limitation on sanitizers: - # See: https://github.com/NixOS/nixpkgs/issues/287763 - export LD_LIBRARY_PATH+=":/run/opengl-driver/lib" - ''; - }; - }); } From 2d98518b7d5ce3a6a688aa434447cba800f57b87 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sat, 10 Feb 2024 08:20:10 +0000 Subject: [PATCH 43/55] backend: give images a type It's quite confusing what should be passed into what because too many things are `void *`. So give images a type to make things a bit clearer. Because of C's limited type system, we lose the ability to annotate them as nonnull or const, well you win some you lose some. Also while doing this I noticed error handling around this is a bit lacking. Co-authored-by: Maxim Solovyov Signed-off-by: Yuxuan Shui --- src/backend/backend.h | 87 ++++++++++++++++++++++------------- src/backend/backend_common.c | 28 +++++------ src/backend/backend_common.h | 12 ++--- src/backend/dummy/dummy.c | 54 +++++++++++----------- src/backend/gl/blur.c | 15 +++--- src/backend/gl/egl.c | 4 +- src/backend/gl/gl_common.c | 37 +++++++-------- src/backend/gl/gl_common.h | 31 +++++++------ src/backend/gl/glx.c | 4 +- src/backend/xrender/xrender.c | 43 +++++++++-------- src/common.h | 2 +- src/win.h | 6 +-- 12 files changed, 177 insertions(+), 146 deletions(-) diff --git a/src/backend/backend.h b/src/backend/backend.h index fba17bbd9d..556803c16a 100644 --- a/src/backend/backend.h +++ b/src/backend/backend.h @@ -115,6 +115,10 @@ struct dual_kawase_blur_args { int strength; }; +typedef struct { + // Intentionally left blank +} *image_handle; + struct backend_operations { // =========== Initialization =========== @@ -167,26 +171,27 @@ struct backend_operations { * Paint the content of an image onto the rendering buffer. * * @param backend_data the backend data - * @param image_data the image to paint + * @param image the image to paint, cannot be NULL * @param dst_x, dst_y the top left corner of the image in the target * @param mask the mask image, the top left of the mask is aligned with - * the top left of the image + * the top left of the image. Optional, can be + * NULL. * @param reg_paint the clip region, in target coordinates * @param reg_visible the visible region, in target coordinates */ - void (*compose)(backend_t *backend_data, void *image_data, coord_t image_dst, - void *mask, coord_t mask_dst, const region_t *reg_paint, - const region_t *reg_visible); + void (*compose)(backend_t *backend_data, image_handle image, coord_t image_dst, + image_handle mask, coord_t mask_dst, const region_t *reg_paint, + const region_t *reg_visible) attr_nonnull(1, 2, 6, 7); /// Fill rectangle of the rendering buffer, mostly for debug purposes, optional. void (*fill)(backend_t *backend_data, struct color, const region_t *clip); /// Blur a given region of the rendering buffer. /// - /// The blur is limited by `mask`. `mask_dst` specifies the top left corner of the - /// mask is. - bool (*blur)(backend_t *backend_data, double opacity, void *blur_ctx, void *mask, - coord_t mask_dst, const region_t *reg_blur, + /// The blur can be limited by `mask`. `mask_dst` specifies the top left corner of + /// the mask. `mask` can be NULL. + bool (*blur)(backend_t *backend_data, double opacity, void *blur_ctx, + image_handle mask, coord_t mask_dst, const region_t *reg_blur, const region_t *reg_visible) attr_nonnull(1, 3, 6, 7); /// Update part of the back buffer with the rendering buffer, then present the @@ -202,13 +207,15 @@ struct backend_operations { * Bind a X pixmap to the backend's internal image data structure. * * @param backend_data backend data - * @param pixmap X pixmap to bind - * @param fmt information of the pixmap's visual - * @param owned whether the ownership of the pixmap is transferred to the backend - * @return backend internal data structure bound with this pixmap + * @param pixmap X pixmap to bind + * @param fmt information of the pixmap's visual + * @param owned whether the ownership of the pixmap is transferred to the + * backend. + * @return backend specific image handle for the pixmap. May be + * NULL. */ - void *(*bind_pixmap)(backend_t *backend_data, xcb_pixmap_t pixmap, - struct xvisual_info fmt, bool owned); + image_handle (*bind_pixmap)(backend_t *backend_data, xcb_pixmap_t pixmap, + struct xvisual_info fmt, bool owned); /// Create a shadow context for rendering shadows with radius `radius`. /// Default implementation: default_create_shadow_context @@ -224,17 +231,23 @@ struct backend_operations { /// shadow context is created. /// Default implementation: default_render_shadow /// + /// @return the shadow image, may be NULL. + /// /// Required. - void *(*render_shadow)(backend_t *backend_data, int width, int height, - struct backend_shadow_context *ctx, struct color color); + image_handle (*render_shadow)(backend_t *backend_data, int width, int height, + struct backend_shadow_context *ctx, struct color color); /// Create a shadow by blurring a mask. `size` is the size of the blur. The /// backend can use whichever blur method is the fastest. The shadow produced /// shoule be consistent with `render_shadow`. /// + /// @param mask the input mask, must not be NULL. + /// @return the shadow image, may be NULL. + /// /// Optional. - void *(*shadow_from_mask)(backend_t *backend_data, void *mask, - struct backend_shadow_context *ctx, struct color color); + image_handle (*shadow_from_mask)(backend_t *backend_data, image_handle mask, + struct backend_shadow_context *ctx, + struct color color); /// Create a mask image from region `reg`. This region can be used to create /// shadow, or used as a mask for composing. When used as a mask, it should mask @@ -245,13 +258,18 @@ struct backend_operations { /// and outside of the mask. Corner radius should exclude the corners from the /// mask. Corner radius should be applied before the inversion. /// + /// @return the mask image, may be NULL. + /// /// Required. - void *(*make_mask)(backend_t *backend_data, geometry_t size, const region_t *reg); + image_handle (*make_mask)(backend_t *backend_data, geometry_t size, + const region_t *reg); // ============ Resource management =========== /// Free resources associated with an image data structure - void (*release_image)(backend_t *backend_data, void *img_data) attr_nonnull(1, 2); + /// + /// @param image the image to be released, cannot be NULL. + void (*release_image)(backend_t *backend_data, image_handle image) attr_nonnull(1, 2); /// Create a shader object from a shader source. /// @@ -276,7 +294,9 @@ struct backend_operations { /// This function is needed because some backend might change the content of the /// window (e.g. when using a custom shader with the glx backend), so only the /// backend knows if an image is transparent. - bool (*is_image_transparent)(backend_t *backend_data, void *image_data) + /// + /// @param image the image to be checked, must not be NULL. + bool (*is_image_transparent)(backend_t *backend_data, image_handle image) attr_nonnull(1, 2); /// Get the age of the buffer content we are currently rendering on top @@ -311,36 +331,39 @@ struct backend_operations { * * @param backend_data backend data * @param prop the property to change - * @param image_data an image data structure returned by the backend + * @param image an image handle, cannot be NULL. * @param args property value - * @return whether the operation is successful + * @return whether the operation is successful */ bool (*set_image_property)(backend_t *backend_data, enum image_properties prop, - void *image_data, void *args); + image_handle image, void *args) attr_nonnull(1, 3); /** * Manipulate an image. Image properties are untouched. * * @param backend_data backend data * @param op the operation to perform - * @param image_data an image data structure returned by the backend + * @param image an image handle, cannot be NULL. * @param reg_op the clip region, define the part of the image to be * operated on. * @param reg_visible define the part of the image that will eventually * be visible on target. this is a hint to the backend * for optimization purposes. * @param args extra arguments, operation specific - * @return whether the operation is successful + * @return whether the operation is successful */ - bool (*image_op)(backend_t *backend_data, enum image_operations op, void *image_data, - const region_t *reg_op, const region_t *reg_visible, void *args); + bool (*image_op)(backend_t *backend_data, enum image_operations op, + image_handle image, const region_t *reg_op, + const region_t *reg_visible, void *args) attr_nonnull(1, 3, 4, 5); - /// Create another instance of the `image_data`. The newly created image + /// Create another instance of the `image`. The newly created image /// inherits its content and all image properties from the image being /// cloned. All `image_op` and `set_image_property` calls on the /// returned image should not affect the original image. - void *(*clone_image)(backend_t *base, const void *image_data, - const region_t *reg_visible); + /// + /// @param image the image to be cloned, must not be NULL. + image_handle (*clone_image)(backend_t *base, image_handle image, + const region_t *reg_visible) attr_nonnull_all; /// Create a blur context that can be used to call `blur` void *(*create_blur_context)(backend_t *base, enum blur_method, void *args); diff --git a/src/backend/backend_common.c b/src/backend/backend_common.c index eef214593b..5a1b785e60 100644 --- a/src/backend/backend_common.c +++ b/src/backend/backend_common.c @@ -293,8 +293,8 @@ bool build_shadow(struct x_connection *c, double opacity, const int width, return false; } -void *default_render_shadow(backend_t *backend_data, int width, int height, - struct backend_shadow_context *sctx, struct color color) { +image_handle default_render_shadow(backend_t *backend_data, int width, int height, + struct backend_shadow_context *sctx, struct color color) { const conv *kernel = (void *)sctx; xcb_render_picture_t shadow_pixel = solid_picture(backend_data->c, true, 1, color.red, color.green, color.blue); @@ -308,7 +308,7 @@ void *default_render_shadow(backend_t *backend_data, int width, int height, } auto visual = x_get_visual_for_standard(backend_data->c, XCB_PICT_STANDARD_ARGB_32); - void *ret = backend_data->ops->bind_pixmap( + auto ret = backend_data->ops->bind_pixmap( backend_data, shadow, x_get_visual_info(backend_data->c, visual), true); x_free_picture(backend_data->c, pict); x_free_picture(backend_data->c, shadow_pixel); @@ -316,16 +316,16 @@ void *default_render_shadow(backend_t *backend_data, int width, int height, } /// Implement render_shadow with shadow_from_mask -void * +image_handle backend_render_shadow_from_mask(backend_t *backend_data, int width, int height, struct backend_shadow_context *sctx, struct color color) { region_t reg; pixman_region32_init_rect(®, 0, 0, (unsigned int)width, (unsigned int)height); - void *mask = backend_data->ops->make_mask( + auto mask = backend_data->ops->make_mask( backend_data, (geometry_t){.width = width, .height = height}, ®); pixman_region32_fini(®); - void *shadow = backend_data->ops->shadow_from_mask(backend_data, mask, sctx, color); + auto shadow = backend_data->ops->shadow_from_mask(backend_data, mask, sctx, color); backend_data->ops->release_image(backend_data, mask); return shadow; } @@ -458,17 +458,17 @@ struct dual_kawase_params *generate_dual_kawase_params(void *args) { return params; } -void *default_clone_image(backend_t *base attr_unused, const void *image_data, - const region_t *reg_visible attr_unused) { +image_handle default_clone_image(backend_t *base attr_unused, image_handle image, + const region_t *reg_visible attr_unused) { auto new_img = ccalloc(1, struct backend_image); - *new_img = *(struct backend_image *)image_data; + *new_img = *(struct backend_image *)image; new_img->inner->refcount++; - return new_img; + return (image_handle)new_img; } bool default_set_image_property(backend_t *base attr_unused, enum image_properties op, - void *image_data, void *arg) { - struct backend_image *tex = image_data; + image_handle image, void *arg) { + auto tex = (struct backend_image *)image; int *iargs = arg; bool *bargs = arg; double *dargs = arg; @@ -490,8 +490,8 @@ bool default_set_image_property(backend_t *base attr_unused, enum image_properti return true; } -bool default_is_image_transparent(backend_t *base attr_unused, void *image_data) { - struct backend_image *img = image_data; +bool default_is_image_transparent(backend_t *base attr_unused, image_handle image) { + auto img = (struct backend_image *)image; return img->opacity < 1 || img->inner->has_alpha; } diff --git a/src/backend/backend_common.h b/src/backend/backend_common.h index da9ccd0235..20357c09bb 100644 --- a/src/backend/backend_common.h +++ b/src/backend/backend_common.h @@ -54,11 +54,11 @@ solid_picture(struct x_connection *, bool argb, double a, double r, double g, do xcb_image_t *make_shadow(struct x_connection *c, const conv *kernel, double opacity, int width, int height); -void *default_render_shadow(backend_t *backend_data, int width, int height, - struct backend_shadow_context *sctx, struct color color); +image_handle default_render_shadow(backend_t *backend_data, int width, int height, + struct backend_shadow_context *sctx, struct color color); /// Implement `render_shadow` with `shadow_from_mask`. -void * +image_handle backend_render_shadow_from_mask(backend_t *backend_data, int width, int height, struct backend_shadow_context *sctx, struct color color); struct backend_shadow_context * @@ -72,8 +72,8 @@ void init_backend_base(struct backend_base *base, session_t *ps); struct conv **generate_blur_kernel(enum blur_method method, void *args, int *kernel_count); struct dual_kawase_params *generate_dual_kawase_params(void *args); -void *default_clone_image(backend_t *base, const void *image_data, const region_t *reg); -bool default_is_image_transparent(backend_t *base attr_unused, void *image_data); +image_handle default_clone_image(backend_t *base, image_handle image, const region_t *reg); +bool default_is_image_transparent(backend_t *base attr_unused, image_handle image); bool default_set_image_property(backend_t *base attr_unused, enum image_properties op, - void *image_data, void *arg); + image_handle image, void *arg); struct backend_image *default_new_backend_image(int w, int h); diff --git a/src/backend/dummy/dummy.c b/src/backend/dummy/dummy.c index 3e57c03955..bab7c931e4 100644 --- a/src/backend/dummy/dummy.c +++ b/src/backend/dummy/dummy.c @@ -50,8 +50,9 @@ void dummy_deinit(struct backend_base *data) { free(dummy); } -static void dummy_check_image(struct backend_base *base, const struct dummy_image *img) { +static void dummy_check_image(struct backend_base *base, image_handle image) { auto dummy = (struct dummy_data *)base; + auto img = (struct dummy_image *)image; if (img == (struct dummy_image *)&dummy->mask) { return; } @@ -64,13 +65,13 @@ static void dummy_check_image(struct backend_base *base, const struct dummy_imag assert(*tmp->refcount > 0); } -void dummy_compose(struct backend_base *base, void *image, coord_t dst attr_unused, - void *mask attr_unused, coord_t mask_dst attr_unused, +void dummy_compose(struct backend_base *base, image_handle image, coord_t dst attr_unused, + image_handle mask attr_unused, coord_t mask_dst attr_unused, const region_t *reg_paint attr_unused, const region_t *reg_visible attr_unused) { auto dummy attr_unused = (struct dummy_data *)base; dummy_check_image(base, image); - assert(mask == NULL || mask == &dummy->mask); + assert(mask == NULL || (struct backend_image *)mask == &dummy->mask); } void dummy_fill(struct backend_base *backend_data attr_unused, struct color c attr_unused, @@ -78,20 +79,20 @@ void dummy_fill(struct backend_base *backend_data attr_unused, struct color c at } bool dummy_blur(struct backend_base *backend_data attr_unused, double opacity attr_unused, - void *blur_ctx attr_unused, void *mask attr_unused, + void *blur_ctx attr_unused, image_handle mask attr_unused, coord_t mask_dst attr_unused, const region_t *reg_blur attr_unused, const region_t *reg_visible attr_unused) { return true; } -void *dummy_bind_pixmap(struct backend_base *base, xcb_pixmap_t pixmap, - struct xvisual_info fmt, bool owned) { +image_handle dummy_bind_pixmap(struct backend_base *base, xcb_pixmap_t pixmap, + struct xvisual_info fmt, bool owned) { auto dummy = (struct dummy_data *)base; struct dummy_image *img = NULL; HASH_FIND_INT(dummy->images, &pixmap, img); if (img) { (*img->refcount)++; - return img; + return (image_handle)img; } img = ccalloc(1, struct dummy_image); @@ -102,12 +103,12 @@ void *dummy_bind_pixmap(struct backend_base *base, xcb_pixmap_t pixmap, img->owned = owned; HASH_ADD_INT(dummy->images, pixmap, img); - return (void *)img; + return (image_handle)img; } -void dummy_release_image(backend_t *base, void *image) { +void dummy_release_image(backend_t *base, image_handle image) { auto dummy = (struct dummy_data *)base; - if (image == &dummy->mask) { + if ((struct backend_image *)image == &dummy->mask) { return; } auto img = (struct dummy_image *)image; @@ -123,10 +124,9 @@ void dummy_release_image(backend_t *base, void *image) { } } -bool dummy_is_image_transparent(struct backend_base *base, void *image) { - auto img = (struct dummy_image *)image; - dummy_check_image(base, img); - return img->transparent; +bool dummy_is_image_transparent(struct backend_base *base, image_handle image) { + dummy_check_image(base, image); + return ((struct dummy_image *)image)->transparent; } int dummy_buffer_age(struct backend_base *base attr_unused) { @@ -134,29 +134,31 @@ int dummy_buffer_age(struct backend_base *base attr_unused) { } bool dummy_image_op(struct backend_base *base, enum image_operations op attr_unused, - void *image, const region_t *reg_op attr_unused, + image_handle image, const region_t *reg_op attr_unused, const region_t *reg_visible attr_unused, void *args attr_unused) { dummy_check_image(base, image); return true; } -void *dummy_make_mask(struct backend_base *base, geometry_t size attr_unused, - const region_t *reg attr_unused) { - return &(((struct dummy_data *)base)->mask); +image_handle dummy_make_mask(struct backend_base *base, geometry_t size attr_unused, + const region_t *reg attr_unused) { + auto dummy = (struct dummy_data *)base; + auto mask = &dummy->mask; + return (image_handle)mask; } bool dummy_set_image_property(struct backend_base *base, enum image_properties prop attr_unused, - void *image, void *arg attr_unused) { + image_handle image, void *arg attr_unused) { dummy_check_image(base, image); return true; } -void *dummy_clone_image(struct backend_base *base, const void *image, - const region_t *reg_visible attr_unused) { - auto img = (const struct dummy_image *)image; - dummy_check_image(base, img); - (*img->refcount)++; - return (void *)img; +image_handle dummy_clone_image(struct backend_base *base, image_handle image, + const region_t *reg_visible attr_unused) { + dummy_check_image(base, image); + auto image_impl = (struct dummy_image *)image; + (*image_impl->refcount)++; + return image; } void *dummy_create_blur_context(struct backend_base *base attr_unused, diff --git a/src/backend/gl/blur.c b/src/backend/gl/blur.c index 328756fc30..acdea76185 100644 --- a/src/backend/gl/blur.c +++ b/src/backend/gl/blur.c @@ -256,10 +256,11 @@ bool gl_dual_kawase_blur(double opacity, struct gl_blur_context *bctx, const rec return true; } -bool gl_blur_impl(double opacity, struct gl_blur_context *bctx, void *mask, coord_t mask_dst, - const region_t *reg_blur, const region_t *reg_visible attr_unused, - GLuint source_texture, geometry_t source_size, GLuint target_fbo, - GLuint default_mask, bool high_precision) { +bool gl_blur_impl(double opacity, struct gl_blur_context *bctx, + struct backend_image *mask, coord_t mask_dst, const region_t *reg_blur, + const region_t *reg_visible attr_unused, GLuint source_texture, + geometry_t source_size, GLuint target_fbo, GLuint default_mask, + bool high_precision) { bool ret = false; if (source_size.width != bctx->fb_width || source_size.height != bctx->fb_height) { @@ -400,12 +401,12 @@ bool gl_blur_impl(double opacity, struct gl_blur_context *bctx, void *mask, coor return ret; } -bool gl_blur(backend_t *base, double opacity, void *ctx, void *mask, coord_t mask_dst, +bool gl_blur(backend_t *base, double opacity, void *ctx, image_handle mask, coord_t mask_dst, const region_t *reg_blur, const region_t *reg_visible attr_unused) { auto gd = (struct gl_data *)base; auto bctx = (struct gl_blur_context *)ctx; - return gl_blur_impl(opacity, bctx, mask, mask_dst, reg_blur, reg_visible, - gd->back_texture, + return gl_blur_impl(opacity, bctx, (struct backend_image *)mask, mask_dst, + reg_blur, reg_visible, gd->back_texture, (geometry_t){.width = gd->width, .height = gd->height}, gd->back_fbo, gd->default_mask_texture, gd->dithered_present); } diff --git a/src/backend/gl/egl.c b/src/backend/gl/egl.c index 9f3f153ffa..082a15d14f 100644 --- a/src/backend/gl/egl.c +++ b/src/backend/gl/egl.c @@ -249,7 +249,7 @@ static backend_t *egl_init(session_t *ps, xcb_window_t target) { return &gd->gl.base; } -static void * +static image_handle egl_bind_pixmap(backend_t *base, xcb_pixmap_t pixmap, struct xvisual_info fmt, bool owned) { struct egl_data *gd = (void *)base; struct egl_pixmap *eglpixmap = NULL; @@ -301,7 +301,7 @@ egl_bind_pixmap(backend_t *base, xcb_pixmap_t pixmap, struct xvisual_info fmt, b glBindTexture(GL_TEXTURE_2D, 0); gl_check_err(); - return wd; + return (image_handle)wd; err: if (eglpixmap && eglpixmap->image) { eglDestroyImage(gd->display, eglpixmap->image); diff --git a/src/backend/gl/gl_common.c b/src/backend/gl/gl_common.c index 2ef4170219..7480c78e51 100644 --- a/src/backend/gl/gl_common.c +++ b/src/backend/gl/gl_common.c @@ -551,11 +551,12 @@ void x_rect_to_coords(int nrects, const rect_t *rects, coord_t image_dst, } // TODO(yshui) make use of reg_visible -void gl_compose(backend_t *base, void *image_data, coord_t image_dst, void *mask, - coord_t mask_dst, const region_t *reg_tgt, +void gl_compose(backend_t *base, image_handle image, coord_t image_dst, + image_handle mask_, coord_t mask_dst, const region_t *reg_tgt, const region_t *reg_visible attr_unused) { auto gd = (struct gl_data *)base; - struct backend_image *img = image_data; + auto img = (struct backend_image *)image; + auto mask = (struct backend_image *)mask_; auto inner = (struct gl_texture *)img->inner; // Painting @@ -709,7 +710,7 @@ void gl_fill(backend_t *base, struct color c, const region_t *clip) { return _gl_fill(base, c, clip, gd->back_fbo, gd->height, true); } -void *gl_make_mask(backend_t *base, geometry_t size, const region_t *reg) { +image_handle gl_make_mask(backend_t *base, geometry_t size, const region_t *reg) { auto tex = ccalloc(1, struct gl_texture); auto img = default_new_backend_image(size.width, size.height); tex->width = size.width; @@ -740,7 +741,7 @@ void *gl_make_mask(backend_t *base, geometry_t size, const region_t *reg) { glBlendFunc(GL_ONE, GL_ONE_MINUS_SRC_ALPHA); glBindFramebuffer(GL_DRAW_FRAMEBUFFER, 0); glDeleteFramebuffers(1, &fbo); - return img; + return (image_handle)img; } static void gl_release_image_inner(backend_t *base, struct gl_texture *inner) { @@ -756,8 +757,8 @@ static void gl_release_image_inner(backend_t *base, struct gl_texture *inner) { gl_check_err(); } -void gl_release_image(backend_t *base, void *image_data) { - struct backend_image *wd = image_data; +void gl_release_image(backend_t *base, image_handle image) { + auto wd = (struct backend_image *)image; auto inner = (struct gl_texture *)wd->inner; inner->refcount--; assert(inner->refcount >= 0); @@ -1221,9 +1222,9 @@ bool gl_last_render_time(backend_t *base, struct timespec *ts) { return true; } -bool gl_image_op(backend_t *base, enum image_operations op, void *image_data, +bool gl_image_op(backend_t *base, enum image_operations op, image_handle image, const region_t *reg_op, const region_t *reg_visible attr_unused, void *arg) { - struct backend_image *tex = image_data; + auto tex = (struct backend_image *)image; switch (op) { case IMAGE_OP_APPLY_ALPHA: gl_image_decouple(base, tex); @@ -1236,12 +1237,12 @@ bool gl_image_op(backend_t *base, enum image_operations op, void *image_data, } bool gl_set_image_property(backend_t *backend_data, enum image_properties prop, - void *image_data, void *args) { + image_handle image, void *args) { if (prop != IMAGE_PROPERTY_CUSTOM_SHADER) { - return default_set_image_property(backend_data, prop, image_data, args); + return default_set_image_property(backend_data, prop, image, args); } - struct backend_image *img = image_data; + auto img = (struct backend_image *)image; auto inner = (struct gl_texture *)img->inner; inner->shader = args; return true; @@ -1280,12 +1281,12 @@ void gl_destroy_shadow_context(backend_t *base attr_unused, struct backend_shado free(ctx_); } -void *gl_shadow_from_mask(backend_t *base, void *mask, - struct backend_shadow_context *sctx, struct color color) { +image_handle gl_shadow_from_mask(backend_t *base, image_handle mask_, + struct backend_shadow_context *sctx, struct color color) { log_debug("Create shadow from mask"); auto gd = (struct gl_data *)base; - auto img = (struct backend_image *)mask; - auto inner = (struct gl_texture *)img->inner; + auto mask = (struct backend_image *)mask_; + auto inner = (struct gl_texture *)mask->inner; auto gsctx = (struct gl_shadow_context *)sctx; int radius = (int)gsctx->radius; @@ -1315,7 +1316,7 @@ void *gl_shadow_from_mask(backend_t *base, void *mask, glFramebufferTexture2D(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, source_texture, 0); glDrawBuffer(GL_COLOR_ATTACHMENT0); - if (img->color_inverted) { + if (mask->color_inverted) { // If the mask is inverted, clear the source_texture to white, so the // "outside" of the mask would be correct glClearColor(1, 1, 1, 1); @@ -1424,7 +1425,7 @@ void *gl_shadow_from_mask(backend_t *base, void *mask, glBindFramebuffer(GL_DRAW_FRAMEBUFFER, 0); glDeleteFramebuffers(1, &fbo); gl_check_err(); - return new_img; + return (image_handle)new_img; } enum device_status gl_device_status(backend_t *base) { diff --git a/src/backend/gl/gl_common.h b/src/backend/gl/gl_common.h index 50bec7c626..cd2310cb43 100644 --- a/src/backend/gl/gl_common.h +++ b/src/backend/gl/gl_common.h @@ -6,6 +6,7 @@ #include #include "backend/backend.h" +#include "backend/backend_common.h" #include "log.h" #include "region.h" @@ -143,13 +144,13 @@ void *gl_create_window_shader(backend_t *backend_data, const char *source); void gl_destroy_window_shader(backend_t *backend_data, void *shader); uint64_t gl_get_shader_attributes(backend_t *backend_data, void *shader); bool gl_set_image_property(backend_t *backend_data, enum image_properties prop, - void *image_data, void *args); + image_handle image, void *args); bool gl_last_render_time(backend_t *backend_data, struct timespec *time); /** * @brief Render a region with texture data. */ -void gl_compose(backend_t *, void *image_data, coord_t image_dst, void *mask, +void gl_compose(backend_t *, image_handle image, coord_t image_dst, image_handle mask, coord_t mask_dst, const region_t *reg_tgt, const region_t *reg_visible); void gl_resize(struct gl_data *, int width, int height); @@ -159,32 +160,32 @@ void gl_deinit(struct gl_data *gd); GLuint gl_new_texture(GLenum target); -bool gl_image_op(backend_t *base, enum image_operations op, void *image_data, +bool gl_image_op(backend_t *base, enum image_operations op, image_handle image, const region_t *reg_op, const region_t *reg_visible, void *arg); -void gl_release_image(backend_t *base, void *image_data); -void *gl_make_mask(backend_t *base, geometry_t size, const region_t *reg); +void gl_release_image(backend_t *base, image_handle image); +image_handle gl_make_mask(backend_t *base, geometry_t size, const region_t *reg); -void *gl_clone(backend_t *base, const void *image_data, const region_t *reg_visible); +image_handle gl_clone(backend_t *base, image_handle image, const region_t *reg_visible); -bool gl_blur(backend_t *base, double opacity, void *ctx, void *mask, coord_t mask_dst, - const region_t *reg_blur, const region_t *reg_visible); -bool gl_blur_impl(double opacity, struct gl_blur_context *bctx, void *mask, coord_t mask_dst, - const region_t *reg_blur, const region_t *reg_visible attr_unused, - GLuint source_texture, geometry_t source_size, GLuint target_fbo, - GLuint default_mask, bool high_precision); +bool gl_blur(backend_t *base, double opacity, void *ctx, image_handle mask, + coord_t mask_dst, const region_t *reg_blur, const region_t *reg_visible); +bool gl_blur_impl(double opacity, struct gl_blur_context *bctx, + struct backend_image *mask, coord_t mask_dst, const region_t *reg_blur, + const region_t *reg_visible attr_unused, GLuint source_texture, + geometry_t source_size, GLuint target_fbo, GLuint default_mask, + bool high_precision); void *gl_create_blur_context(backend_t *base, enum blur_method, void *args); void gl_destroy_blur_context(backend_t *base, void *ctx); struct backend_shadow_context *gl_create_shadow_context(backend_t *base, double radius); void gl_destroy_shadow_context(backend_t *base attr_unused, struct backend_shadow_context *ctx); -void *gl_shadow_from_mask(backend_t *base, void *mask, - struct backend_shadow_context *sctx, struct color color); +image_handle gl_shadow_from_mask(backend_t *base, image_handle mask, + struct backend_shadow_context *sctx, struct color color); void gl_get_blur_size(void *blur_context, int *width, int *height); void gl_fill(backend_t *base, struct color, const region_t *clip); void gl_present(backend_t *base, const region_t *); -bool gl_read_pixel(backend_t *base, void *image_data, int x, int y, struct color *output); enum device_status gl_device_status(backend_t *base); /** diff --git a/src/backend/gl/glx.c b/src/backend/gl/glx.c index 1047af91ba..a985e3fc67 100644 --- a/src/backend/gl/glx.c +++ b/src/backend/gl/glx.c @@ -380,7 +380,7 @@ static backend_t *glx_init(session_t *ps, xcb_window_t target) { return &gd->gl.base; } -static void * +static image_handle glx_bind_pixmap(backend_t *base, xcb_pixmap_t pixmap, struct xvisual_info fmt, bool owned) { struct _glx_pixmap *glxpixmap = NULL; auto gd = (struct _glx_data *)base; @@ -475,7 +475,7 @@ glx_bind_pixmap(backend_t *base, xcb_pixmap_t pixmap, struct xvisual_info fmt, b glBindTexture(GL_TEXTURE_2D, 0); gl_check_err(); - return wd; + return (image_handle)wd; err: if (glxpixmap && glxpixmap->glpixmap) { glXDestroyPixmap(base->c->dpy, glxpixmap->glpixmap); diff --git a/src/backend/xrender/xrender.c b/src/backend/xrender/xrender.c index acfe25d4a3..72d5ece694 100644 --- a/src/backend/xrender/xrender.c +++ b/src/backend/xrender/xrender.c @@ -359,10 +359,12 @@ compose_impl(struct _xrender_data *xd, struct xrender_image *xrimg, coord_t dst, pixman_region32_fini(®); } -static void compose(backend_t *base, void *img_data, coord_t dst, void *mask, coord_t mask_dst, - const region_t *reg_paint, const region_t *reg_visible) { +static void compose(backend_t *base, image_handle image_, coord_t dst, image_handle mask_, + coord_t mask_dst, const region_t *reg_paint, const region_t *reg_visible) { struct _xrender_data *xd = (void *)base; - return compose_impl(xd, img_data, dst, mask, mask_dst, reg_paint, reg_visible, + auto image = (struct xrender_image *)image_; + auto mask = (struct xrender_image *)mask_; + return compose_impl(xd, image, dst, mask, mask_dst, reg_paint, reg_visible, xd->back[2]); } @@ -384,9 +386,10 @@ static void fill(backend_t *base, struct color c, const region_t *clip) { .height = to_u16_checked(extent->y2 - extent->y1)}}); } -static bool blur(backend_t *backend_data, double opacity, void *ctx_, void *mask, +static bool blur(backend_t *backend_data, double opacity, void *ctx_, image_handle mask_, coord_t mask_dst, const region_t *reg_blur, const region_t *reg_visible) { - struct _xrender_blur_context *bctx = ctx_; + auto bctx = (struct _xrender_blur_context *)ctx_; + auto mask = (struct xrender_image *)mask_; if (bctx->method == BLUR_METHOD_NONE) { return true; } @@ -517,7 +520,7 @@ static bool blur(backend_t *backend_data, double opacity, void *ctx_, void *mask return true; } -static void * +static image_handle bind_pixmap(backend_t *base, xcb_pixmap_t pixmap, struct xvisual_info fmt, bool owned) { xcb_generic_error_t *e; auto r = xcb_get_geometry_reply(base->c->c, xcb_get_geometry(base->c->c, pixmap), &e); @@ -552,7 +555,7 @@ bind_pixmap(backend_t *base, xcb_pixmap_t pixmap, struct xvisual_info fmt, bool free(img); return NULL; } - return img; + return (image_handle)img; } static void release_image_inner(backend_t *base, struct _xrender_image_data_inner *inner) { x_free_picture(base->c, inner->pict); @@ -576,13 +579,13 @@ release_rounded_corner_cache(backend_t *base, struct xrender_rounded_rectangle_c } } -static void release_image(backend_t *base, void *image) { - struct xrender_image *img = image; +static void release_image(backend_t *base, image_handle image) { + auto img = (struct xrender_image *)image; release_rounded_corner_cache(base, img->rounded_rectangle); img->rounded_rectangle = NULL; img->base.inner->refcount -= 1; if (img->base.inner->refcount == 0) { - release_image_inner(base, (void *)img->base.inner); + release_image_inner(base, (struct _xrender_image_data_inner *)img->base.inner); } free(img); } @@ -711,7 +714,7 @@ new_inner(backend_t *base, int w, int h, xcb_visualid_t visual, uint8_t depth) { return new_inner; } -static void *make_mask(backend_t *base, geometry_t size, const region_t *reg) { +static image_handle make_mask(backend_t *base, geometry_t size, const region_t *reg) { struct _xrender_data *xd = (void *)base; // Give the mask a 1 pixel wide border to emulate the clamp to border behavior of // OpenGL textures. @@ -754,7 +757,7 @@ static void *make_mask(backend_t *base, geometry_t size, const region_t *reg) { img->base.dim = 0; img->base.inner = (struct backend_image_inner_base *)inner; img->rounded_rectangle = NULL; - return img; + return (image_handle)img; } static bool decouple_image(backend_t *base, struct backend_image *img, const region_t *reg) { @@ -782,10 +785,10 @@ static bool decouple_image(backend_t *base, struct backend_image *img, const reg return true; } -static bool image_op(backend_t *base, enum image_operations op, void *image, +static bool image_op(backend_t *base, enum image_operations op, image_handle image, const region_t *reg_op, const region_t *reg_visible, void *arg) { struct _xrender_data *xd = (void *)base; - struct backend_image *img = image; + auto img = (struct backend_image *)image; region_t reg; double *dargs = arg; @@ -964,19 +967,19 @@ static backend_t *backend_xrender_init(session_t *ps, xcb_window_t target) { return NULL; } -void *clone_image(backend_t *base attr_unused, const void *image_data, - const region_t *reg_visible attr_unused) { +image_handle clone_image(backend_t *base attr_unused, image_handle image, + const region_t *reg_visible attr_unused) { auto new_img = ccalloc(1, struct xrender_image); - *new_img = *(struct xrender_image *)image_data; + *new_img = *(struct xrender_image *)image; new_img->base.inner->refcount++; if (new_img->rounded_rectangle) { new_img->rounded_rectangle->refcount++; } - return new_img; + return (image_handle)new_img; } -static bool -set_image_property(backend_t *base, enum image_properties op, void *image, void *args) { +static bool set_image_property(backend_t *base, enum image_properties op, + image_handle image, void *args) { auto xrimg = (struct xrender_image *)image; if (op == IMAGE_PROPERTY_CORNER_RADIUS && ((double *)args)[0] != xrimg->base.corner_radius) { diff --git a/src/common.h b/src/common.h index 35316e1057..f92803ae1d 100644 --- a/src/common.h +++ b/src/common.h @@ -186,7 +186,7 @@ typedef struct session { /// Picture of the root window background. paint_t root_tile_paint; /// The backend data the root pixmap bound to - void *root_image; + image_handle root_image; /// A region of the size of the screen. region_t screen_reg; /// Picture of root window. Destination of painting in no-DBE painting diff --git a/src/win.h b/src/win.h index 93f10b151c..bd3288067f 100644 --- a/src/win.h +++ b/src/win.h @@ -100,9 +100,9 @@ struct managed_win { struct win base; /// backend data attached to this window. Only available when /// `state` is not UNMAPPED - void *win_image; - void *shadow_image; - void *mask_image; + image_handle win_image; + image_handle shadow_image; + image_handle mask_image; /// Pointer to the next higher window to paint. struct managed_win *prev_trans; /// Number of windows above this window From 8d0284da1b7558a25db26d568db74da507582aba Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Tue, 13 Feb 2024 23:52:12 +0000 Subject: [PATCH 44/55] compiler: abort in debug build if unreachable() is reached Signed-off-by: Yuxuan Shui --- src/compiler.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/compiler.h b/src/compiler.h index 544daa559e..63e337fc1b 100644 --- a/src/compiler.h +++ b/src/compiler.h @@ -105,9 +105,9 @@ #ifndef unreachable # if defined(__GNUC__) || defined(__clang__) -# define unreachable() __builtin_unreachable() +# define unreachable() assert(false); __builtin_unreachable() # else -# define unreachable() do {} while(0) +# define unreachable() assert(false); do {} while(0) # endif #endif From bb097730c7694021cb4c4dbe068dc2ce31faac9f Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Wed, 14 Feb 2024 16:26:40 +0000 Subject: [PATCH 45/55] test.h: import upstream updates Signed-off-by: Yuxuan Shui --- subprojects/test.h/test.h | 54 +++++++++++++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/subprojects/test.h/test.h b/subprojects/test.h/test.h index 71522e13a4..3aa722022e 100644 --- a/subprojects/test.h/test.h +++ b/subprojects/test.h/test.h @@ -58,6 +58,14 @@ struct test_file_metadata __attribute__((weak)) * test_file_head; } \ } while (0) +#define TEST_NOTEQUAL(a, b) \ + do { \ + if ((a) == (b)) { \ + SET_FAILURE(#a " == " #b, false); \ + return; \ + } \ + } while (0) + #define TEST_TRUE(a) \ do { \ if (!(a)) { \ @@ -69,11 +77,13 @@ struct test_file_metadata __attribute__((weak)) * test_file_head; #define TEST_STREQUAL(a, b) \ do { \ if (strcmp(a, b) != 0) { \ - const char *part2 = " != " #b; \ - size_t len = strlen(a) + strlen(part2) + 3; \ - char *buf = malloc(len); \ - snprintf(buf, len, "\"%s\"%s", a, part2); \ - SET_FAILURE(buf, true); \ + const char *test_strequal__part2 = " != " #b; \ + size_t test_strequal__len = \ + strlen(a) + strlen(test_strequal__part2) + 3; \ + char *test_strequal__buf = malloc(test_strequal__len); \ + snprintf(test_strequal__buf, test_strequal__len, "\"%s\"%s", a, \ + test_strequal__part2); \ + SET_FAILURE(test_strequal__buf, true); \ return; \ } \ } while (0) @@ -81,11 +91,27 @@ struct test_file_metadata __attribute__((weak)) * test_file_head; #define TEST_STRNEQUAL(a, b, len) \ do { \ if (strncmp(a, b, len) != 0) { \ - const char *part2 = " != " #b; \ - size_t len2 = len + strlen(part2) + 3; \ - char *buf = malloc(len2); \ - snprintf(buf, len2, "\"%.*s\"%s", (int)len, a, part2); \ - SET_FAILURE(buf, true); \ + const char *test_strnequal__part2 = " != " #b; \ + size_t test_strnequal__len2 = \ + len + strlen(test_strnequal__part2) + 3; \ + char *test_strnequal__buf = malloc(test_strnequal__len2); \ + snprintf(test_strnequal__buf, test_strnequal__len2, \ + "\"%.*s\"%s", (int)len, a, test_strnequal__part2); \ + SET_FAILURE(test_strnequal__buf, true); \ + return; \ + } \ + } while (0) + +#define TEST_STREQUAL3(str, expected, len) \ + do { \ + if (len != strlen(expected) || strncmp(str, expected, len) != 0) { \ + const char *test_strequal3__part2 = " != " #expected; \ + size_t test_strequal3__len2 = \ + len + strlen(test_strequal3__part2) + 3; \ + char *test_strequal3__buf = malloc(test_strequal3__len2); \ + snprintf(test_strequal3__buf, test_strequal3__len2, \ + "\"%.*s\"%s", (int)len, str, test_strequal3__part2); \ + SET_FAILURE(test_strequal3__buf, true); \ return; \ } \ } while (0) @@ -199,6 +225,9 @@ static inline void __attribute__((constructor(102))) run_tests(void) { #define TEST_EQUAL(a, b) \ (void)(a); \ (void)(b) +#define TEST_NOTEQUAL(a, b) \ + (void)(a); \ + (void)(b) #define TEST_TRUE(a) (void)(a) #define TEST_STREQUAL(a, b) \ (void)(a); \ @@ -207,5 +236,8 @@ static inline void __attribute__((constructor(102))) run_tests(void) { (void)(a); \ (void)(b); \ (void)(len) - +#define TEST_STREQUAL3(str, expected, len) \ + (void)(str); \ + (void)(expected); \ + (void)(len) #endif From 0dcca2228e3e215623ee5db960dd653fc9522794 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Thu, 15 Feb 2024 20:05:09 +0000 Subject: [PATCH 46/55] cache: slight refactor Added a pure version of `cache_get` which does not change the cache, and renamed the old `cache_get` to `cache_get_or_fetch`. Remove unused `cache_set`, and remove prefix underscore from function names. Signed-off-by: Yuxuan Shui --- src/atom.c | 3 ++- src/atom.h | 2 +- src/cache.c | 25 ++++++++++++------------- src/cache.h | 14 ++++++-------- 4 files changed, 21 insertions(+), 23 deletions(-) diff --git a/src/atom.c b/src/atom.c index 0272dc8364..bf94232e4f 100644 --- a/src/atom.c +++ b/src/atom.c @@ -29,7 +29,8 @@ static inline void *atom_getter(void *ud, const char *atom_name, int *err) { struct atom *init_atoms(xcb_connection_t *c) { auto atoms = ccalloc(1, struct atom); atoms->c = new_cache((void *)c, atom_getter, NULL); -#define ATOM_GET(x) atoms->a##x = (xcb_atom_t)(intptr_t)cache_get(atoms->c, #x, NULL) +#define ATOM_GET(x) \ + atoms->a##x = (xcb_atom_t)(intptr_t)cache_get_or_fetch(atoms->c, #x, NULL) LIST_APPLY(ATOM_GET, SEP_COLON, ATOM_LIST1); LIST_APPLY(ATOM_GET, SEP_COLON, ATOM_LIST2); #undef ATOM_GET diff --git a/src/atom.h b/src/atom.h index 5ea7701c4b..f984e665b6 100644 --- a/src/atom.h +++ b/src/atom.h @@ -61,7 +61,7 @@ struct atom { struct atom *init_atoms(xcb_connection_t *); static inline xcb_atom_t get_atom(struct atom *a, const char *key) { - return (xcb_atom_t)(intptr_t)cache_get(a->c, key, NULL); + return (xcb_atom_t)(intptr_t)cache_get_or_fetch(a->c, key, NULL); } static inline void destroy_atoms(struct atom *a) { diff --git a/src/cache.c b/src/cache.c index 1ffb31c5df..8c5d91d64b 100644 --- a/src/cache.c +++ b/src/cache.c @@ -17,20 +17,19 @@ struct cache { struct cache_entry *entries; }; -void cache_set(struct cache *c, const char *key, void *data) { - struct cache_entry *e = NULL; +static inline struct cache_entry *cache_get_entry(struct cache *c, const char *key) { + struct cache_entry *e; HASH_FIND_STR(c->entries, key, e); - CHECK(!e); + return e; +} - e = ccalloc(1, struct cache_entry); - e->key = strdup(key); - e->value = data; - HASH_ADD_STR(c->entries, key, e); +void *cache_get(struct cache *c, const char *key) { + struct cache_entry *e = cache_get_entry(c, key); + return e ? e->value : NULL; } -void *cache_get(struct cache *c, const char *key, int *err) { - struct cache_entry *e; - HASH_FIND_STR(c->entries, key, e); +void *cache_get_or_fetch(struct cache *c, const char *key, int *err) { + struct cache_entry *e = cache_get_entry(c, key); if (e) { return e->value; } @@ -54,7 +53,7 @@ void *cache_get(struct cache *c, const char *key, int *err) { return e->value; } -static inline void _cache_invalidate(struct cache *c, struct cache_entry *e) { +static inline void cache_invalidate_impl(struct cache *c, struct cache_entry *e) { if (c->free) { c->free(c->user_data, e->value); } @@ -68,14 +67,14 @@ void cache_invalidate(struct cache *c, const char *key) { HASH_FIND_STR(c->entries, key, e); if (e) { - _cache_invalidate(c, e); + cache_invalidate_impl(c, e); } } void cache_invalidate_all(struct cache *c) { struct cache_entry *e, *tmpe; HASH_ITER(hh, c->entries, e, tmpe) { - _cache_invalidate(c, e); + cache_invalidate_impl(c, e); } } diff --git a/src/cache.h b/src/cache.h index 3ca054f0fc..2ac1a01b49 100644 --- a/src/cache.h +++ b/src/cache.h @@ -11,9 +11,13 @@ typedef void (*cache_free_t)(void *user_data, void *data); /// `user_data` will be passed to `getter` and `f` when they are called. struct cache *new_cache(void *user_data, cache_getter_t getter, cache_free_t f); -/// Fetch a value from the cache. If the value doesn't present in the cache yet, the +/// Get a value from the cache. If the value doesn't present in the cache yet, the /// getter will be called, and the returned value will be stored into the cache. -void *cache_get(struct cache *, const char *key, int *err); +void *cache_get_or_fetch(struct cache *, const char *key, int *err); + +/// Get a value from the cache. If the value doesn't present in the cache, NULL will be +/// returned. +void *cache_get(struct cache *, const char *key); /// Invalidate a value in the cache. void cache_invalidate(struct cache *, const char *key); @@ -24,9 +28,3 @@ void cache_invalidate_all(struct cache *); /// Invalidate all values in the cache and free it. Returns the user data passed to /// `new_cache` void *cache_free(struct cache *); - -/// Insert a key-value pair into the cache. Only used for internal testing. Takes -/// ownership of `data` -/// -/// If `key` already exists in the cache, this function will abort the program. -void cache_set(struct cache *c, const char *key, void *data); From 071b77c49f6df5e19cba3eb3d2e225a7021a96ca Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Thu, 15 Feb 2024 20:11:42 +0000 Subject: [PATCH 47/55] atom: remove an unnecessary use of get_atom Replace get_atom("COMPTON_VERSION") with a fixed atom. Signed-off-by: Yuxuan Shui --- src/atom.h | 1 + src/picom.c | 9 ++++----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/atom.h b/src/atom.h index f984e665b6..29374d94d1 100644 --- a/src/atom.h +++ b/src/atom.h @@ -23,6 +23,7 @@ WM_CLIENT_MACHINE, \ _NET_ACTIVE_WINDOW, \ _COMPTON_SHADOW, \ + COMPTON_VERSION, \ _NET_WM_WINDOW_TYPE, \ _XROOTPMAP_ID, \ ESETROOT_PMAP_ID, \ diff --git a/src/picom.c b/src/picom.c index c024dd4e7c..dc075c13d9 100644 --- a/src/picom.c +++ b/src/picom.c @@ -1327,11 +1327,10 @@ static int register_cm(session_t *ps) { } // Set COMPTON_VERSION - e = xcb_request_check( - ps->c.c, xcb_change_property_checked( - ps->c.c, XCB_PROP_MODE_REPLACE, ps->reg_win, - get_atom(ps->atoms, "COMPTON_VERSION"), XCB_ATOM_STRING, 8, - (uint32_t)strlen(PICOM_VERSION), PICOM_VERSION)); + e = xcb_request_check(ps->c.c, xcb_change_property_checked( + ps->c.c, XCB_PROP_MODE_REPLACE, ps->reg_win, + ps->atoms->aCOMPTON_VERSION, XCB_ATOM_STRING, 8, + (uint32_t)strlen(PICOM_VERSION), PICOM_VERSION)); if (e) { log_error_x_error(e, "Failed to set COMPTON_VERSION."); free(e); From 580aef939f3f9a91dd1bbd23eec825981c6fa93b Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Thu, 15 Feb 2024 20:12:20 +0000 Subject: [PATCH 48/55] atom: add get_atom_cached Add a version of get_atom that does not query the X server. Signed-off-by: Yuxuan Shui --- src/atom.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/atom.h b/src/atom.h index 29374d94d1..91c28290ca 100644 --- a/src/atom.h +++ b/src/atom.h @@ -65,6 +65,10 @@ static inline xcb_atom_t get_atom(struct atom *a, const char *key) { return (xcb_atom_t)(intptr_t)cache_get_or_fetch(a->c, key, NULL); } +static inline xcb_atom_t get_atom_cached(struct atom *a, const char *key) { + return (xcb_atom_t)(intptr_t)cache_get(a->c, key); +} + static inline void destroy_atoms(struct atom *a) { cache_free(a->c); free(a); From 6315faed20ce31a60015ba051239d386ea355158 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Thu, 15 Feb 2024 21:08:48 +0000 Subject: [PATCH 49/55] cache: redesign cache uses an invasive design now, a la list.h and uthash. This get rid of the ugly integer to pointer cache, and gives us freedom of what we can put into the cache. Signed-off-by: Yuxuan Shui --- src/atom.c | 38 ++++++++++++++++------ src/atom.h | 26 +++++++++++---- src/cache.c | 92 ++++++++++++++++------------------------------------- src/cache.h | 48 ++++++++++++++++++---------- src/list.h | 13 +------- src/utils.h | 14 ++++++++ 6 files changed, 122 insertions(+), 109 deletions(-) diff --git a/src/atom.c b/src/atom.c index bf94232e4f..76fc57f4f8 100644 --- a/src/atom.c +++ b/src/atom.c @@ -2,14 +2,19 @@ #include #include "atom.h" +#include "cache.h" #include "common.h" -#include "utils.h" +#include "compiler.h" #include "log.h" +#include "utils.h" -static inline void *atom_getter(void *ud, const char *atom_name, int *err) { - xcb_connection_t *c = ud; +static inline int +atom_getter(struct cache *cache, const char *atom_name, struct cache_handle **value) { + struct atom *atoms = container_of(cache, struct atom, c); xcb_intern_atom_reply_t *reply = xcb_intern_atom_reply( - c, xcb_intern_atom(c, 0, to_u16_checked(strlen(atom_name)), atom_name), NULL); + atoms->conn, + xcb_intern_atom(atoms->conn, 0, to_u16_checked(strlen(atom_name)), atom_name), + NULL); xcb_atom_t atom = XCB_NONE; if (reply) { @@ -18,9 +23,19 @@ static inline void *atom_getter(void *ud, const char *atom_name, int *err) { free(reply); } else { log_error("Failed to intern atoms"); - *err = 1; + return -1; } - return (void *)(intptr_t)atom; + + struct atom_entry *entry = ccalloc(1, struct atom_entry); + entry->atom = atom; + *value = &entry->entry; + return 0; +} + +static inline void +atom_entry_free(struct cache *cache attr_unused, struct cache_handle *handle) { + struct atom_entry *entry = cache_entry(handle, struct atom_entry, entry); + free(entry); } /** @@ -28,11 +43,16 @@ static inline void *atom_getter(void *ud, const char *atom_name, int *err) { */ struct atom *init_atoms(xcb_connection_t *c) { auto atoms = ccalloc(1, struct atom); - atoms->c = new_cache((void *)c, atom_getter, NULL); -#define ATOM_GET(x) \ - atoms->a##x = (xcb_atom_t)(intptr_t)cache_get_or_fetch(atoms->c, #x, NULL) + atoms->conn = c; + cache_init(&atoms->c, atom_getter); +#define ATOM_GET(x) atoms->a##x = get_atom(atoms, #x) LIST_APPLY(ATOM_GET, SEP_COLON, ATOM_LIST1); LIST_APPLY(ATOM_GET, SEP_COLON, ATOM_LIST2); #undef ATOM_GET return atoms; } + +void destroy_atoms(struct atom *a) { + cache_invalidate_all(&a->c, atom_entry_free); + free(a); +} diff --git a/src/atom.h b/src/atom.h index 91c28290ca..678733172d 100644 --- a/src/atom.h +++ b/src/atom.h @@ -4,6 +4,7 @@ #include #include "cache.h" +#include "log.h" #include "meta.h" // clang-format off @@ -54,22 +55,33 @@ #define ATOM_DEF(x) xcb_atom_t a##x struct atom { - struct cache *c; + xcb_connection_t *conn; + struct cache c; LIST_APPLY(ATOM_DEF, SEP_COLON, ATOM_LIST1); LIST_APPLY(ATOM_DEF, SEP_COLON, ATOM_LIST2); }; +struct atom_entry { + struct cache_handle entry; + xcb_atom_t atom; +}; + +/// Create a new atom object with a xcb connection, note that this atom object will hold a +/// reference to the connection, so the caller must keep the connection alive until the +/// atom object is destroyed. struct atom *init_atoms(xcb_connection_t *); static inline xcb_atom_t get_atom(struct atom *a, const char *key) { - return (xcb_atom_t)(intptr_t)cache_get_or_fetch(a->c, key, NULL); + struct cache_handle *entry = NULL; + if (cache_get_or_fetch(&a->c, key, &entry) < 0) { + log_error("Failed to get atom %s", key); + return XCB_NONE; + } + return cache_entry(entry, struct atom_entry, entry)->atom; } static inline xcb_atom_t get_atom_cached(struct atom *a, const char *key) { - return (xcb_atom_t)(intptr_t)cache_get(a->c, key); + return cache_entry(cache_get(&a->c, key), struct atom_entry, entry)->atom; } -static inline void destroy_atoms(struct atom *a) { - cache_free(a->c); - free(a); -} +void destroy_atoms(struct atom *a); diff --git a/src/cache.c b/src/cache.c index 8c5d91d64b..d25a22fbec 100644 --- a/src/cache.c +++ b/src/cache.c @@ -1,94 +1,56 @@ #include -#include "compiler.h" -#include "utils.h" #include "cache.h" -struct cache_entry { - char *key; - void *value; - UT_hash_handle hh; -}; - -struct cache { - cache_getter_t getter; - cache_free_t free; - void *user_data; - struct cache_entry *entries; -}; - -static inline struct cache_entry *cache_get_entry(struct cache *c, const char *key) { - struct cache_entry *e; +struct cache_handle *cache_get(struct cache *c, const char *key) { + struct cache_handle *e; HASH_FIND_STR(c->entries, key, e); return e; } -void *cache_get(struct cache *c, const char *key) { - struct cache_entry *e = cache_get_entry(c, key); - return e ? e->value : NULL; -} - -void *cache_get_or_fetch(struct cache *c, const char *key, int *err) { - struct cache_entry *e = cache_get_entry(c, key); - if (e) { - return e->value; +int cache_get_or_fetch(struct cache *c, const char *key, struct cache_handle **value) { + *value = cache_get(c, key); + if (*value) { + return 0; } - int tmperr; - if (!err) { - err = &tmperr; + int err = c->getter(c, key, value); + assert(err <= 0); + if (err < 0) { + return err; } + (*value)->key = strdup(key); - *err = 0; - e = ccalloc(1, struct cache_entry); - e->key = strdup(key); - e->value = c->getter(c->user_data, key, err); - if (*err) { - free(e->key); - free(e); - return NULL; - } - - HASH_ADD_STR(c->entries, key, e); - return e->value; + HASH_ADD_STR(c->entries, key, *value); + return 1; } -static inline void cache_invalidate_impl(struct cache *c, struct cache_entry *e) { - if (c->free) { - c->free(c->user_data, e->value); - } +static inline void +cache_invalidate_impl(struct cache *c, struct cache_handle *e, cache_free_t free_fn) { free(e->key); HASH_DEL(c->entries, e); - free(e); + if (free_fn) { + free_fn(c, e); + } } -void cache_invalidate(struct cache *c, const char *key) { - struct cache_entry *e; +void cache_invalidate(struct cache *c, const char *key, cache_free_t free_fn) { + struct cache_handle *e; HASH_FIND_STR(c->entries, key, e); if (e) { - cache_invalidate_impl(c, e); + cache_invalidate_impl(c, e, free_fn); } } -void cache_invalidate_all(struct cache *c) { - struct cache_entry *e, *tmpe; +void cache_invalidate_all(struct cache *c, cache_free_t free_fn) { + struct cache_handle *e, *tmpe; HASH_ITER(hh, c->entries, e, tmpe) { - cache_invalidate_impl(c, e); + cache_invalidate_impl(c, e, free_fn); } } -void *cache_free(struct cache *c) { - void *ret = c->user_data; - cache_invalidate_all(c); - free(c); - return ret; -} - -struct cache *new_cache(void *ud, cache_getter_t getter, cache_free_t f) { - auto c = ccalloc(1, struct cache); - c->user_data = ud; - c->getter = getter; - c->free = f; - return c; +void cache_init(struct cache *cache, cache_getter_t getter) { + cache->getter = getter; + cache->entries = NULL; } diff --git a/src/cache.h b/src/cache.h index 2ac1a01b49..60609190a1 100644 --- a/src/cache.h +++ b/src/cache.h @@ -1,30 +1,46 @@ #pragma once +#include +#include "utils.h" + +#define cache_entry(ptr, type, member) container_of(ptr, type, member) + struct cache; +struct cache_handle; + +/// User-provided function to fetch a value for the cache, when it's not present. +/// Should return 0 if the value is fetched successfully, and a negative number if the +/// value cannot be fetched. Getter doesn't need to initialize fields of `struct +/// cache_handle`. +typedef int (*cache_getter_t)(struct cache *, const char *key, struct cache_handle **value); +typedef void (*cache_free_t)(struct cache *, struct cache_handle *value); -typedef void *(*cache_getter_t)(void *user_data, const char *key, int *err); -typedef void (*cache_free_t)(void *user_data, void *data); +struct cache { + cache_getter_t getter; + struct cache_handle *entries; +}; -/// Create a cache with `getter`, and a free function `f` which is used to free the cache -/// value when they are invalidated. -/// -/// `user_data` will be passed to `getter` and `f` when they are called. -struct cache *new_cache(void *user_data, cache_getter_t getter, cache_free_t f); +struct cache_handle { + char *key; + UT_hash_handle hh; +}; + +/// Initialize a cache with `getter` +void cache_init(struct cache *cache, cache_getter_t getter); /// Get a value from the cache. If the value doesn't present in the cache yet, the /// getter will be called, and the returned value will be stored into the cache. -void *cache_get_or_fetch(struct cache *, const char *key, int *err); +/// Returns 0 if the value is already present in the cache, 1 if the value is fetched +/// successfully, and a negative number if the value cannot be fetched. +int cache_get_or_fetch(struct cache *, const char *key, struct cache_handle **value); /// Get a value from the cache. If the value doesn't present in the cache, NULL will be /// returned. -void *cache_get(struct cache *, const char *key); +struct cache_handle *cache_get(struct cache *, const char *key); /// Invalidate a value in the cache. -void cache_invalidate(struct cache *, const char *key); - -/// Invalidate all values in the cache. -void cache_invalidate_all(struct cache *); +void cache_invalidate(struct cache *, const char *key, cache_free_t free_fn); -/// Invalidate all values in the cache and free it. Returns the user data passed to -/// `new_cache` -void *cache_free(struct cache *); +/// Invalidate all values in the cache. After this call, `struct cache` holds no allocated +/// memory, and can be discarded. +void cache_invalidate_all(struct cache *, cache_free_t free_fn); diff --git a/src/list.h b/src/list.h index 19e2c2c277..d63a764611 100644 --- a/src/list.h +++ b/src/list.h @@ -2,18 +2,7 @@ #include #include -/** - * container_of - cast a member of a structure out to the containing structure - * @ptr: the pointer to the member. - * @type: the type of the container struct this is embedded in. - * @member: the name of the member within the struct. - * - */ -#define container_of(ptr, type, member) \ - ({ \ - const __typeof__(((type *)0)->member) *__mptr = (ptr); \ - (type *)((char *)__mptr - offsetof(type, member)); \ - }) +#include "utils.h" struct list_node { struct list_node *next, *prev; diff --git a/src/utils.h b/src/utils.h index 73cb71127e..5a42d99a90 100644 --- a/src/utils.h +++ b/src/utils.h @@ -117,6 +117,20 @@ safe_isnan(double a) { ASSERT_IN_RANGE(__to_tmp, 0, max); \ (uint32_t) __to_tmp; \ }) + +/** + * container_of - cast a member of a structure out to the containing structure + * @ptr: the pointer to the member. + * @type: the type of the container struct this is embedded in. + * @member: the name of the member within the struct. + * + */ +#define container_of(ptr, type, member) \ + ({ \ + const __typeof__(((type *)0)->member) *__mptr = (ptr); \ + (type *)((char *)__mptr - offsetof(type, member)); \ + }) + /** * Normalize an int value to a specific range. * From a93bbc30e5d22a497e3158a810457c3b054003fd Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Fri, 16 Feb 2024 00:31:53 +0000 Subject: [PATCH 50/55] atom: get_atom now requires explicit xcb_connection_t This is to make the access to X server more explicit, and make managing the lifetime of xcb_connection_t a bit easier. Signed-off-by: Yuxuan Shui --- src/atom.c | 24 +++++++++++++++--------- src/atom.h | 18 ++++-------------- src/c2.c | 2 +- src/cache.c | 10 +++------- src/cache.h | 12 ++++++------ src/picom.c | 2 +- 6 files changed, 30 insertions(+), 38 deletions(-) diff --git a/src/atom.c b/src/atom.c index 76fc57f4f8..8ef699e876 100644 --- a/src/atom.c +++ b/src/atom.c @@ -8,13 +8,11 @@ #include "log.h" #include "utils.h" -static inline int -atom_getter(struct cache *cache, const char *atom_name, struct cache_handle **value) { - struct atom *atoms = container_of(cache, struct atom, c); +static inline int atom_getter(struct cache *cache attr_unused, const char *atom_name, + struct cache_handle **value, void *user_data) { + xcb_connection_t *c = user_data; xcb_intern_atom_reply_t *reply = xcb_intern_atom_reply( - atoms->conn, - xcb_intern_atom(atoms->conn, 0, to_u16_checked(strlen(atom_name)), atom_name), - NULL); + c, xcb_intern_atom(c, 0, to_u16_checked(strlen(atom_name)), atom_name), NULL); xcb_atom_t atom = XCB_NONE; if (reply) { @@ -38,14 +36,22 @@ atom_entry_free(struct cache *cache attr_unused, struct cache_handle *handle) { free(entry); } +xcb_atom_t get_atom(struct atom *a, const char *key, xcb_connection_t *c) { + struct cache_handle *entry = NULL; + if (cache_get_or_fetch(&a->c, key, &entry, c, atom_getter) < 0) { + log_error("Failed to get atom %s", key); + return XCB_NONE; + } + return cache_entry(entry, struct atom_entry, entry)->atom; +} + /** * Create a new atom structure and fetch all predefined atoms */ struct atom *init_atoms(xcb_connection_t *c) { auto atoms = ccalloc(1, struct atom); - atoms->conn = c; - cache_init(&atoms->c, atom_getter); -#define ATOM_GET(x) atoms->a##x = get_atom(atoms, #x) + atoms->c = CACHE_INIT; +#define ATOM_GET(x) atoms->a##x = get_atom(atoms, #x, c) LIST_APPLY(ATOM_GET, SEP_COLON, ATOM_LIST1); LIST_APPLY(ATOM_GET, SEP_COLON, ATOM_LIST2); #undef ATOM_GET diff --git a/src/atom.h b/src/atom.h index 678733172d..d5a41d585f 100644 --- a/src/atom.h +++ b/src/atom.h @@ -55,7 +55,6 @@ #define ATOM_DEF(x) xcb_atom_t a##x struct atom { - xcb_connection_t *conn; struct cache c; LIST_APPLY(ATOM_DEF, SEP_COLON, ATOM_LIST1); LIST_APPLY(ATOM_DEF, SEP_COLON, ATOM_LIST2); @@ -66,20 +65,11 @@ struct atom_entry { xcb_atom_t atom; }; -/// Create a new atom object with a xcb connection, note that this atom object will hold a -/// reference to the connection, so the caller must keep the connection alive until the -/// atom object is destroyed. -struct atom *init_atoms(xcb_connection_t *); - -static inline xcb_atom_t get_atom(struct atom *a, const char *key) { - struct cache_handle *entry = NULL; - if (cache_get_or_fetch(&a->c, key, &entry) < 0) { - log_error("Failed to get atom %s", key); - return XCB_NONE; - } - return cache_entry(entry, struct atom_entry, entry)->atom; -} +/// Create a new atom object with a xcb connection. `struct atom` does not hold +/// a reference to the connection. +struct atom *init_atoms(xcb_connection_t *c); +xcb_atom_t get_atom(struct atom *a, const char *key, xcb_connection_t *c); static inline xcb_atom_t get_atom_cached(struct atom *a, const char *key) { return cache_entry(cache_get(&a->c, key), struct atom_entry, entry)->atom; } diff --git a/src/c2.c b/src/c2.c index 614c456cba..140d274652 100644 --- a/src/c2.c +++ b/src/c2.c @@ -1047,7 +1047,7 @@ static bool c2_l_postprocess(session_t *ps, c2_l_t *pleaf) { // Get target atom if it's not a predefined one if (pleaf->predef == C2_L_PUNDEFINED) { - pleaf->tgtatom = get_atom(ps->atoms, pleaf->tgt); + pleaf->tgtatom = get_atom(ps->atoms, pleaf->tgt, ps->c.c); if (!pleaf->tgtatom) { log_error("Failed to get atom for target \"%s\".", pleaf->tgt); return false; diff --git a/src/cache.c b/src/cache.c index d25a22fbec..e297c4fefa 100644 --- a/src/cache.c +++ b/src/cache.c @@ -8,13 +8,14 @@ struct cache_handle *cache_get(struct cache *c, const char *key) { return e; } -int cache_get_or_fetch(struct cache *c, const char *key, struct cache_handle **value) { +int cache_get_or_fetch(struct cache *c, const char *key, struct cache_handle **value, + void *user_data, cache_getter_t getter) { *value = cache_get(c, key); if (*value) { return 0; } - int err = c->getter(c, key, value); + int err = getter(c, key, value, user_data); assert(err <= 0); if (err < 0) { return err; @@ -49,8 +50,3 @@ void cache_invalidate_all(struct cache *c, cache_free_t free_fn) { cache_invalidate_impl(c, e, free_fn); } } - -void cache_init(struct cache *cache, cache_getter_t getter) { - cache->getter = getter; - cache->entries = NULL; -} diff --git a/src/cache.h b/src/cache.h index 60609190a1..a50ec4bc21 100644 --- a/src/cache.h +++ b/src/cache.h @@ -12,27 +12,27 @@ struct cache_handle; /// Should return 0 if the value is fetched successfully, and a negative number if the /// value cannot be fetched. Getter doesn't need to initialize fields of `struct /// cache_handle`. -typedef int (*cache_getter_t)(struct cache *, const char *key, struct cache_handle **value); +typedef int (*cache_getter_t)(struct cache *, const char *key, + struct cache_handle **value, void *user_data); typedef void (*cache_free_t)(struct cache *, struct cache_handle *value); struct cache { - cache_getter_t getter; struct cache_handle *entries; }; +static const struct cache CACHE_INIT = {NULL}; + struct cache_handle { char *key; UT_hash_handle hh; }; -/// Initialize a cache with `getter` -void cache_init(struct cache *cache, cache_getter_t getter); - /// Get a value from the cache. If the value doesn't present in the cache yet, the /// getter will be called, and the returned value will be stored into the cache. /// Returns 0 if the value is already present in the cache, 1 if the value is fetched /// successfully, and a negative number if the value cannot be fetched. -int cache_get_or_fetch(struct cache *, const char *key, struct cache_handle **value); +int cache_get_or_fetch(struct cache *, const char *key, struct cache_handle **value, + void *user_data, cache_getter_t getter); /// Get a value from the cache. If the value doesn't present in the cache, NULL will be /// returned. diff --git a/src/picom.c b/src/picom.c index dc075c13d9..578935146a 100644 --- a/src/picom.c +++ b/src/picom.c @@ -1346,7 +1346,7 @@ static int register_cm(session_t *ps) { log_fatal("Failed to allocate memory"); return -1; } - atom = get_atom(ps->atoms, buf); + atom = get_atom(ps->atoms, buf, ps->c.c); free(buf); xcb_get_selection_owner_reply_t *reply = xcb_get_selection_owner_reply( From 96de4f07ca1cb3c21293fbce9e25c1a5129a4159 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Fri, 16 Feb 2024 00:42:22 +0000 Subject: [PATCH 51/55] cache: remove unused function cache_invalidate Signed-off-by: Yuxuan Shui --- src/cache.c | 9 --------- src/cache.h | 3 --- 2 files changed, 12 deletions(-) diff --git a/src/cache.c b/src/cache.c index e297c4fefa..e4f2927f70 100644 --- a/src/cache.c +++ b/src/cache.c @@ -35,15 +35,6 @@ cache_invalidate_impl(struct cache *c, struct cache_handle *e, cache_free_t free } } -void cache_invalidate(struct cache *c, const char *key, cache_free_t free_fn) { - struct cache_handle *e; - HASH_FIND_STR(c->entries, key, e); - - if (e) { - cache_invalidate_impl(c, e, free_fn); - } -} - void cache_invalidate_all(struct cache *c, cache_free_t free_fn) { struct cache_handle *e, *tmpe; HASH_ITER(hh, c->entries, e, tmpe) { diff --git a/src/cache.h b/src/cache.h index a50ec4bc21..8c022cf44a 100644 --- a/src/cache.h +++ b/src/cache.h @@ -38,9 +38,6 @@ int cache_get_or_fetch(struct cache *, const char *key, struct cache_handle **va /// returned. struct cache_handle *cache_get(struct cache *, const char *key); -/// Invalidate a value in the cache. -void cache_invalidate(struct cache *, const char *key, cache_free_t free_fn); - /// Invalidate all values in the cache. After this call, `struct cache` holds no allocated /// memory, and can be discarded. void cache_invalidate_all(struct cache *, cache_free_t free_fn); From 9175489f65875534ee0c1d47268b0f22729ee895 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Fri, 16 Feb 2024 12:41:27 +0000 Subject: [PATCH 52/55] utils: try to avoid variable shadowing in some macros Signed-off-by: Yuxuan Shui --- src/utils.h | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/utils.h b/src/utils.h index 5a42d99a90..22da6c29b0 100644 --- a/src/utils.h +++ b/src/utils.h @@ -58,11 +58,11 @@ safe_isnan(double a) { /// being always true or false. #define ASSERT_IN_RANGE(var, lower, upper) \ do { \ - auto __tmp attr_unused = (var); \ + auto __assert_in_range_tmp attr_unused = (var); \ _Pragma("GCC diagnostic push"); \ _Pragma("GCC diagnostic ignored \"-Wtype-limits\""); \ - assert(__tmp >= lower); \ - assert(__tmp <= upper); \ + assert(__assert_in_range_tmp >= lower); \ + assert(__assert_in_range_tmp <= upper); \ _Pragma("GCC diagnostic pop"); \ } while (0) @@ -112,9 +112,10 @@ safe_isnan(double a) { #define to_u32_checked(val) \ ({ \ auto __to_tmp = (val); \ - int64_t max attr_unused = UINT32_MAX; /* silence clang tautological \ - comparison warning*/ \ - ASSERT_IN_RANGE(__to_tmp, 0, max); \ + int64_t __to_u32_max attr_unused = UINT32_MAX; /* silence clang \ + tautological \ + comparison warning */ \ + ASSERT_IN_RANGE(__to_tmp, 0, __to_u32_max); \ (uint32_t) __to_tmp; \ }) From 2b52edd9e385a2d36965af73ce722032a7f94c0c Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Fri, 16 Feb 2024 01:00:50 +0000 Subject: [PATCH 53/55] cache: hide details of struct atom_entry Signed-off-by: Yuxuan Shui --- src/atom.c | 9 +++++++++ src/atom.h | 13 ++----------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/atom.c b/src/atom.c index 8ef699e876..fdcf73a94b 100644 --- a/src/atom.c +++ b/src/atom.c @@ -8,6 +8,11 @@ #include "log.h" #include "utils.h" +struct atom_entry { + struct cache_handle entry; + xcb_atom_t atom; +}; + static inline int atom_getter(struct cache *cache attr_unused, const char *atom_name, struct cache_handle **value, void *user_data) { xcb_connection_t *c = user_data; @@ -45,6 +50,10 @@ xcb_atom_t get_atom(struct atom *a, const char *key, xcb_connection_t *c) { return cache_entry(entry, struct atom_entry, entry)->atom; } +xcb_atom_t get_atom_cached(struct atom *a, const char *key) { + return cache_entry(cache_get(&a->c, key), struct atom_entry, entry)->atom; +} + /** * Create a new atom structure and fetch all predefined atoms */ diff --git a/src/atom.h b/src/atom.h index d5a41d585f..0cebe3a090 100644 --- a/src/atom.h +++ b/src/atom.h @@ -1,10 +1,7 @@ #pragma once -#include - #include #include "cache.h" -#include "log.h" #include "meta.h" // clang-format off @@ -54,24 +51,18 @@ #define ATOM_DEF(x) xcb_atom_t a##x +struct atom_entry; struct atom { struct cache c; LIST_APPLY(ATOM_DEF, SEP_COLON, ATOM_LIST1); LIST_APPLY(ATOM_DEF, SEP_COLON, ATOM_LIST2); }; -struct atom_entry { - struct cache_handle entry; - xcb_atom_t atom; -}; - /// Create a new atom object with a xcb connection. `struct atom` does not hold /// a reference to the connection. struct atom *init_atoms(xcb_connection_t *c); xcb_atom_t get_atom(struct atom *a, const char *key, xcb_connection_t *c); -static inline xcb_atom_t get_atom_cached(struct atom *a, const char *key) { - return cache_entry(cache_get(&a->c, key), struct atom_entry, entry)->atom; -} +xcb_atom_t get_atom_cached(struct atom *a, const char *key); void destroy_atoms(struct atom *a); From 7c15a1438a6723b15646880454680c158786c353 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Fri, 16 Feb 2024 15:42:21 +0000 Subject: [PATCH 54/55] chore: tweak .clang-tidy 1. Increase cognitive complexity threshold to 50. The default value of 25 marks a lot of functions, which is too noisy to be useful. 2. Ignore int8_t in signed char to integer conversion check. Signed-off-by: Yuxuan Shui --- .clang-tidy | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.clang-tidy b/.clang-tidy index e46c0873f2..57d71dc561 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -23,5 +23,9 @@ CheckOptions: value: 255.0;1.0; - key: readability-function-cognitive-complexity.IgnoreMacros value: true + - key: readability-function-cognitive-complexity.Threshold + value: 50 - key: readability-function-cognitive-complexity.DescribeBasicIncrements value: true + - key: bugprone-signed-char-misuse.CharTypdefsToIgnore + value: int8_t From b99c7db73eb62204568c21d9542dce5388d4d356 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Wed, 14 Feb 2024 18:02:14 +0000 Subject: [PATCH 55/55] x: wid_get_text_prop doesn't need the whole session_t Signed-off-by: Yuxuan Shui --- src/c2.c | 3 ++- src/win.c | 12 ++++++++---- src/x.c | 12 +++++------- src/x.h | 4 ++-- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/c2.c b/src/c2.c index 140d274652..e039821f6d 100644 --- a/src/c2.c +++ b/src/c2.c @@ -1509,7 +1509,8 @@ static inline void c2_match_once_leaf(session_t *ps, const struct managed_win *w else { char **strlst = NULL; int nstr = 0; - if (wid_get_text_prop(ps, wid, pleaf->tgtatom, &strlst, &nstr)) { + if (wid_get_text_prop(&ps->c, ps->atoms, wid, pleaf->tgtatom, + &strlst, &nstr)) { if (pleaf->index < 0 && nstr > 0 && strlen(strlst[0]) > 0) { ntargets = to_u32_checked(nstr); targets = (const char **)strlst; diff --git a/src/win.c b/src/win.c index a966022741..9a0e193efd 100644 --- a/src/win.c +++ b/src/win.c @@ -658,12 +658,14 @@ int win_update_name(session_t *ps, struct managed_win *w) { return 0; } - if (!(wid_get_text_prop(ps, w->client_win, ps->atoms->a_NET_WM_NAME, &strlst, &nstr))) { + if (!(wid_get_text_prop(&ps->c, ps->atoms, w->client_win, + ps->atoms->a_NET_WM_NAME, &strlst, &nstr))) { log_debug("(%#010x): _NET_WM_NAME unset, falling back to " "WM_NAME.", w->client_win); - if (!wid_get_text_prop(ps, w->client_win, ps->atoms->aWM_NAME, &strlst, &nstr)) { + if (!wid_get_text_prop(&ps->c, ps->atoms, w->client_win, + ps->atoms->aWM_NAME, &strlst, &nstr)) { log_debug("Unsetting window name for %#010x", w->client_win); free(w->name); w->name = NULL; @@ -690,7 +692,8 @@ static int win_update_role(session_t *ps, struct managed_win *w) { char **strlst = NULL; int nstr = 0; - if (!wid_get_text_prop(ps, w->client_win, ps->atoms->aWM_WINDOW_ROLE, &strlst, &nstr)) { + if (!wid_get_text_prop(&ps->c, ps->atoms, w->client_win, + ps->atoms->aWM_WINDOW_ROLE, &strlst, &nstr)) { return -1; } @@ -1876,7 +1879,8 @@ bool win_update_class(session_t *ps, struct managed_win *w) { w->class_general = NULL; // Retrieve the property string list - if (!wid_get_text_prop(ps, w->client_win, ps->atoms->aWM_CLASS, &strlst, &nstr)) { + if (!wid_get_text_prop(&ps->c, ps->atoms, w->client_win, ps->atoms->aWM_CLASS, + &strlst, &nstr)) { return false; } diff --git a/src/x.c b/src/x.c index f33142b279..a5f25ab23f 100644 --- a/src/x.c +++ b/src/x.c @@ -182,10 +182,9 @@ xcb_window_t wid_get_prop_window(struct x_connection *c, xcb_window_t wid, xcb_a /** * Get the value of a text property of a window. */ -bool wid_get_text_prop(session_t *ps, xcb_window_t wid, xcb_atom_t prop, char ***pstrlst, - int *pnstr) { - assert(ps->server_grabbed); - auto prop_info = x_get_prop_info(&ps->c, wid, prop); +bool wid_get_text_prop(struct x_connection *c, struct atom *atoms, xcb_window_t wid, + xcb_atom_t prop, char ***pstrlst, int *pnstr) { + auto prop_info = x_get_prop_info(c, wid, prop); auto type = prop_info.type; auto format = prop_info.format; auto length = prop_info.length; @@ -194,8 +193,7 @@ bool wid_get_text_prop(session_t *ps, xcb_window_t wid, xcb_atom_t prop, char ** return false; } - if (type != XCB_ATOM_STRING && type != ps->atoms->aUTF8_STRING && - type != ps->atoms->aC_STRING) { + if (type != XCB_ATOM_STRING && type != atoms->aUTF8_STRING && type != atoms->aC_STRING) { log_warn("Text property %d of window %#010x has unsupported type: %d", prop, wid, type); return false; @@ -210,7 +208,7 @@ bool wid_get_text_prop(session_t *ps, xcb_window_t wid, xcb_atom_t prop, char ** xcb_generic_error_t *e = NULL; auto word_count = (length + 4 - 1) / 4; auto r = xcb_get_property_reply( - ps->c.c, xcb_get_property(ps->c.c, 0, wid, prop, type, 0, word_count), &e); + c->c, xcb_get_property(c->c, 0, wid, prop, type, 0, word_count), &e); if (!r) { log_debug_x_error(e, "Failed to get window property for %#010x", wid); free(e); diff --git a/src/x.h b/src/x.h index f465b22a22..78cd55d241 100644 --- a/src/x.h +++ b/src/x.h @@ -261,8 +261,8 @@ xcb_window_t wid_get_prop_window(struct x_connection *c, xcb_window_t wid, xcb_a * array * @param[out] pnstr Number of strings in the array */ -bool wid_get_text_prop(session_t *ps, xcb_window_t wid, xcb_atom_t prop, char ***pstrlst, - int *pnstr); +bool wid_get_text_prop(struct x_connection *c, struct atom *atoms, xcb_window_t wid, + xcb_atom_t prop, char ***pstrlst, int *pnstr); const xcb_render_pictforminfo_t * x_get_pictform_for_visual(struct x_connection *, xcb_visualid_t);