Skip to content

Commit

Permalink
backend: embed backend_operations table in backend_base
Browse files Browse the repository at this point in the history
The idea is to allow backend plugins to override backend functions by
modifying this table. Right now, when they do this they are actually
changing a global variable and their change will persist after backend
resets (!). Store the table inside backend_base solves this problem.

Signed-off-by: Yuxuan Shui <[email protected]>
  • Loading branch information
yshui committed May 25, 2024
1 parent 9c4f62c commit bd26302
Show file tree
Hide file tree
Showing 13 changed files with 124 additions and 130 deletions.
20 changes: 11 additions & 9 deletions include/picom/backend.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,7 @@ struct managed_win;
struct ev_loop;
struct backend_operations;

typedef struct backend_base {
struct backend_operations *ops;
struct x_connection *c;
struct ev_loop *loop;

/// Whether the backend can accept new render request at the moment
bool busy;
// ...
} backend_t;
typedef struct backend_base backend_t;

// This mimics OpenGL's ARB_robustness extension, which enables detection of GPU context
// resets.
Expand Down Expand Up @@ -475,6 +467,16 @@ struct backend_operations {
enum device_status (*device_status)(backend_t *backend_data);
};

struct backend_base {
struct backend_operations ops;
struct x_connection *c;
struct ev_loop *loop;

/// Whether the backend can accept new render request at the moment
bool busy;
// ...
};

/// Register a new backend, `major` and `minor` should be the version of the picom backend
/// interface. You should just pass `PICOM_BACKEND_MAJOR` and `PICOM_BACKEND_MINOR` here.
/// `name` is the name of the backend, `init` is the function to initialize the backend,
Expand Down
10 changes: 5 additions & 5 deletions src/backend/backend.c
Original file line number Diff line number Diff line change
Expand Up @@ -118,22 +118,22 @@ bool backend_execute(struct backend_base *backend, image_handle target, unsigned
continue;
}
succeeded =
backend->ops->blit(backend, cmd->origin, target, &cmd->blit);
backend->ops.blit(backend, cmd->origin, target, &cmd->blit);
break;
case BACKEND_COMMAND_COPY_AREA:
if (!pixman_region32_not_empty(cmd->copy_area.region)) {
continue;
}
succeeded = backend->ops->copy_area(backend, cmd->origin, target,
cmd->copy_area.source_image,
cmd->copy_area.region);
succeeded = backend->ops.copy_area(backend, cmd->origin, target,
cmd->copy_area.source_image,
cmd->copy_area.region);
break;
case BACKEND_COMMAND_BLUR:
if (!pixman_region32_not_empty(cmd->blur.target_mask)) {
continue;
}
succeeded =
backend->ops->blur(backend, cmd->origin, target, &cmd->blur);
backend->ops.blur(backend, cmd->origin, target, &cmd->blur);
break;
case BACKEND_COMMAND_INVALID:
default: assert(false);
Expand Down
3 changes: 1 addition & 2 deletions src/backend/backend_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#include <xcb/xcb_image.h>
#include <xcb/xcb_renderutil.h>

#include "backend/backend.h"
#include "backend/backend_common.h"
#include "common.h"
#include "config.h"
Expand Down Expand Up @@ -413,7 +412,7 @@ void init_backend_base(struct backend_base *base, session_t *ps) {
base->c = &ps->c;
base->loop = ps->loop;
base->busy = false;
base->ops = NULL;
base->ops = (struct backend_operations){};
}

uint32_t backend_no_quirks(struct backend_base *base attr_unused) {
Expand Down
3 changes: 1 addition & 2 deletions src/backend/backend_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,14 @@

#include <stdbool.h>

#include "backend.h"
#include "config.h"
#include "region.h"

struct session;
struct win;
struct conv;
struct backend_base;
struct backend_operations;
struct x_connection;

struct dual_kawase_params {
/// Number of downsample passes
Expand Down
4 changes: 2 additions & 2 deletions src/backend/driver.c
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ enum driver detect_driver(xcb_connection_t *c, backend_t *backend_data, xcb_wind
free(randr_version);

// If the backend supports driver detection, use that as well
if (backend_data && backend_data->ops->detect_driver) {
ret |= backend_data->ops->detect_driver(backend_data);
if (backend_data && backend_data->ops.detect_driver) {
ret |= backend_data->ops.detect_driver(backend_data);

Check warning on line 87 in src/backend/driver.c

View check run for this annotation

Codecov / codecov/patch

src/backend/driver.c#L87

Added line #L87 was not covered by tests
}
return ret;
}
6 changes: 3 additions & 3 deletions src/backend/dummy/dummy.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ struct dummy_data {
struct dummy_image back_buffer;
};

struct backend_operations dummy_ops;
const struct backend_operations dummy_ops;

struct backend_base *dummy_init(session_t *ps attr_unused, xcb_window_t target attr_unused) {
auto ret = ccalloc(1, struct dummy_data);
init_backend_base(&ret->base, ps);
ret->base.ops = &dummy_ops;
ret->base.ops = dummy_ops;
list_init_head(&ret->non_pixmap_images);
return &ret->base;
}
Expand Down Expand Up @@ -216,7 +216,7 @@ static void dummy_version(struct backend_base * /*base*/, uint64_t *major, uint6
*minor = PICOM_BACKEND_DUMMY_MINOR;

Check warning on line 216 in src/backend/dummy/dummy.c

View check run for this annotation

Codecov / codecov/patch

src/backend/dummy/dummy.c#L214-L216

Added lines #L214 - L216 were not covered by tests
}

struct backend_operations dummy_ops = {
const struct backend_operations dummy_ops = {
.apply_alpha = dummy_apply_alpha,
.back_buffer = dummy_back_buffer,
.blit = dummy_blit,
Expand Down
8 changes: 3 additions & 5 deletions src/backend/gl/egl.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
*/

#include <X11/Xlib-xcb.h>
#include <assert.h>
#include <limits.h>
#include <stdbool.h>
#include <stdlib.h>
#include <string.h>
Expand Down Expand Up @@ -105,7 +103,7 @@ static bool egl_set_swap_interval(int interval, EGLDisplay dpy) {
return eglSwapInterval(dpy, interval);
}

struct backend_operations egl_ops;
const struct backend_operations egl_ops;
/**
* Initialize OpenGL.
*/
Expand Down Expand Up @@ -154,7 +152,7 @@ static backend_t *egl_init(session_t *ps, xcb_window_t target) {

eglext_init(gd->display);
init_backend_base(&gd->gl.base, ps);
gd->gl.base.ops = &egl_ops;
gd->gl.base.ops = egl_ops;
if (!eglext.has_EGL_KHR_image_pixmap) {
log_error("EGL_KHR_image_pixmap not available.");
goto end;
Expand Down Expand Up @@ -355,7 +353,7 @@ static void egl_version(struct backend_base * /*base*/, uint64_t *major, uint64_
*minor = PICOM_BACKEND_EGL_MINOR;

Check warning on line 353 in src/backend/gl/egl.c

View check run for this annotation

Codecov / codecov/patch

src/backend/gl/egl.c#L351-L353

Added lines #L351 - L353 were not covered by tests
}

struct backend_operations egl_ops = {
const struct backend_operations egl_ops = {
.apply_alpha = gl_apply_alpha,
.back_buffer = gl_back_buffer,
.blit = gl_blit,
Expand Down
8 changes: 3 additions & 5 deletions src/backend/gl/glx.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
*/

#include <X11/Xlib-xcb.h>
#include <assert.h>
#include <limits.h>
#include <pixman.h>
#include <stdbool.h>
Expand All @@ -31,7 +30,6 @@
#include "config.h"
#include "log.h"
#include "picom.h"
#include "region.h"
#include "utils.h"
#include "win.h"
#include "x.h"
Expand Down Expand Up @@ -225,7 +223,7 @@ static bool glx_set_swap_interval(int interval, Display *dpy, GLXDrawable drawab
return vsync_enabled;
}

struct backend_operations glx_ops;
const struct backend_operations glx_ops;
/**
* Initialize OpenGL.
*/
Expand All @@ -234,7 +232,7 @@ static backend_t *glx_init(session_t *ps, xcb_window_t target) {
glxext_init(ps->c.dpy, ps->c.screen);
auto gd = ccalloc(1, struct _glx_data);
init_backend_base(&gd->gl.base, ps);
gd->gl.base.ops = &glx_ops;
gd->gl.base.ops = glx_ops;

gd->target_win = target;

Expand Down Expand Up @@ -529,7 +527,7 @@ static void glx_version(struct backend_base * /*base*/, uint64_t *major, uint64_
*minor = PICOM_BACKEND_GLX_MINOR;

Check warning on line 527 in src/backend/gl/glx.c

View check run for this annotation

Codecov / codecov/patch

src/backend/gl/glx.c#L525-L527

Added lines #L525 - L527 were not covered by tests
}

struct backend_operations glx_ops = {
const struct backend_operations glx_ops = {
.apply_alpha = gl_apply_alpha,
.back_buffer = gl_back_buffer,
.bind_pixmap = glx_bind_pixmap,
Expand Down
6 changes: 3 additions & 3 deletions src/backend/xrender/xrender.c
Original file line number Diff line number Diff line change
Expand Up @@ -865,7 +865,7 @@ static void xrender_get_blur_size(void *blur_context, int *width, int *height) {
*width = ctx->resize_width;
*height = ctx->resize_height;
}
struct backend_operations xrender_ops;
const struct backend_operations xrender_ops;
static backend_t *xrender_init(session_t *ps, xcb_window_t target) {
if (ps->o.dithered_present) {
log_warn("\"dithered-present\" is not supported by the xrender backend, "

Check warning on line 871 in src/backend/xrender/xrender.c

View check run for this annotation

Codecov / codecov/patch

src/backend/xrender/xrender.c#L871

Added line #L871 was not covered by tests
Expand All @@ -878,7 +878,7 @@ static backend_t *xrender_init(session_t *ps, xcb_window_t target) {

auto xd = ccalloc(1, struct xrender_data);
init_backend_base(&xd->base, ps);
xd->base.ops = &xrender_ops;
xd->base.ops = xrender_ops;

for (int i = 0; i <= MAX_ALPHA; ++i) {
double o = (double)i / (double)MAX_ALPHA;
Expand Down Expand Up @@ -1032,7 +1032,7 @@ static void xrender_version(struct backend_base * /*base*/, uint64_t *major, uin
*minor = PICOM_BACKEND_XRENDER_MINOR;

Check warning on line 1032 in src/backend/xrender/xrender.c

View check run for this annotation

Codecov / codecov/patch

src/backend/xrender/xrender.c#L1030-L1032

Added lines #L1030 - L1032 were not covered by tests
}

struct backend_operations xrender_ops = {
const struct backend_operations xrender_ops = {
.apply_alpha = xrender_apply_alpha,
.back_buffer = xrender_back_buffer,
.bind_pixmap = xrender_bind_pixmap,
Expand Down
6 changes: 3 additions & 3 deletions src/diagnostic.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,11 @@ void print_diagnostics(session_t *ps, const char *config_file, bool compositor_r
printf(" Cannot initialize backend %s\n", backend_name(i));
continue;
}
if (backend_data->ops->diagnostics) {
if (backend_data->ops.diagnostics) {
printf("\n### Backend: %s\n\n", backend_name(i));
backend_data->ops->diagnostics(backend_data);
backend_data->ops.diagnostics(backend_data);
}
backend_data->ops->deinit(backend_data);
backend_data->ops.deinit(backend_data);
}
}

Expand Down
36 changes: 18 additions & 18 deletions src/picom.c
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ enum vblank_callback_action check_render_finish(struct vblank_event *e attr_unus

struct timespec render_time;
bool completed =
ps->backend_data->ops->last_render_time(ps->backend_data, &render_time);
ps->backend_data->ops.last_render_time(ps->backend_data, &render_time);

Check warning on line 155 in src/picom.c

View check run for this annotation

Codecov / codecov/patch

src/picom.c#L155

Added line #L155 was not covered by tests
if (!completed) {
// Render hasn't completed yet, we can't start another render.
// Check again at the next vblank.
Expand Down Expand Up @@ -575,14 +575,14 @@ static void destroy_backend(session_t *ps) {

HASH_ITER2(ps->shaders, shader) {
if (shader->backend_shader != NULL) {
ps->backend_data->ops->destroy_shader(ps->backend_data,
shader->backend_shader);
ps->backend_data->ops.destroy_shader(ps->backend_data,

Check warning on line 578 in src/picom.c

View check run for this annotation

Codecov / codecov/patch

src/picom.c#L578

Added line #L578 was not covered by tests
shader->backend_shader);
shader->backend_shader = NULL;
}
}

if (ps->backend_data && ps->root_image) {
ps->backend_data->ops->release_image(ps->backend_data, ps->root_image);
ps->backend_data->ops.release_image(ps->backend_data, ps->root_image);

Check warning on line 585 in src/picom.c

View check run for this annotation

Codecov / codecov/patch

src/picom.c#L585

Added line #L585 was not covered by tests
ps->root_image = NULL;
}

Expand All @@ -593,11 +593,11 @@ static void destroy_backend(session_t *ps) {
}
// deinit backend
if (ps->backend_blur_context) {
ps->backend_data->ops->destroy_blur_context(
ps->backend_data->ops.destroy_blur_context(
ps->backend_data, ps->backend_blur_context);
ps->backend_blur_context = NULL;
}
ps->backend_data->ops->deinit(ps->backend_data);
ps->backend_data->ops.deinit(ps->backend_data);
ps->backend_data = NULL;
}
}
Expand Down Expand Up @@ -635,7 +635,7 @@ static bool initialize_blur(session_t *ps) {
enum backend_image_format format = ps->o.dithered_present
? BACKEND_IMAGE_FORMAT_PIXMAP_HIGH
: BACKEND_IMAGE_FORMAT_PIXMAP;
ps->backend_blur_context = ps->backend_data->ops->create_blur_context(
ps->backend_blur_context = ps->backend_data->ops.create_blur_context(
ps->backend_data, ps->o.blur_method, format, args);
return ps->backend_blur_context != NULL;
}
Expand Down Expand Up @@ -674,24 +674,24 @@ static bool initialize_backend(session_t *ps) {
}

// Create shaders
if (!ps->backend_data->ops->create_shader && ps->shaders) {
if (!ps->backend_data->ops.create_shader && ps->shaders) {
log_warn("Shaders are not supported by selected backend %s, "

Check warning on line 678 in src/picom.c

View check run for this annotation

Codecov / codecov/patch

src/picom.c#L678

Added line #L678 was not covered by tests
"they will be ignored",
backend_name(ps->o.backend));
} else {
HASH_ITER2(ps->shaders, shader) {
assert(shader->backend_shader == NULL);
shader->backend_shader = ps->backend_data->ops->create_shader(
shader->backend_shader = ps->backend_data->ops.create_shader(
ps->backend_data, shader->source);
if (shader->backend_shader == NULL) {
log_warn("Failed to create shader for shader "

Check warning on line 687 in src/picom.c

View check run for this annotation

Codecov / codecov/patch

src/picom.c#L683-L687

Added lines #L683 - L687 were not covered by tests
"file %s, this shader will not be used",
shader->key);
} else {
shader->attributes = 0;
if (ps->backend_data->ops->get_shader_attributes) {
if (ps->backend_data->ops.get_shader_attributes) {
shader->attributes =
ps->backend_data->ops->get_shader_attributes(
ps->backend_data->ops.get_shader_attributes(

Check warning on line 694 in src/picom.c

View check run for this annotation

Codecov / codecov/patch

src/picom.c#L692-L694

Added lines #L692 - L694 were not covered by tests
ps->backend_data,
shader->backend_shader);
}
Expand Down Expand Up @@ -719,7 +719,7 @@ static bool initialize_backend(session_t *ps) {
// The old backends binds pixmap lazily, nothing to do here
return true;
err:
ps->backend_data->ops->deinit(ps->backend_data);
ps->backend_data->ops.deinit(ps->backend_data);

Check warning on line 722 in src/picom.c

View check run for this annotation

Codecov / codecov/patch

src/picom.c#L722

Added line #L722 was not covered by tests
ps->backend_data = NULL;
quit(ps);
return false;
Expand All @@ -741,7 +741,7 @@ static void configure_root(session_t *ps) {
// On root window changes
if (!ps->o.use_legacy_backends) {
assert(ps->backend_data);
has_root_change = ps->backend_data->ops->root_change != NULL;
has_root_change = ps->backend_data->ops.root_change != NULL;
} else {
// Old backend can handle root change
has_root_change = true;
Expand Down Expand Up @@ -788,7 +788,7 @@ static void configure_root(session_t *ps) {
#endif
if (has_root_change) {
if (ps->backend_data != NULL) {
ps->backend_data->ops->root_change(ps->backend_data, ps);
ps->backend_data->ops.root_change(ps->backend_data, ps);
}
// Old backend's root_change is not a specific function
} else {
Expand Down Expand Up @@ -1082,7 +1082,7 @@ void root_damaged(session_t *ps) {

if (ps->backend_data) {
if (ps->root_image) {
ps->backend_data->ops->release_image(ps->backend_data, ps->root_image);
ps->backend_data->ops.release_image(ps->backend_data, ps->root_image);

Check warning on line 1085 in src/picom.c

View check run for this annotation

Codecov / codecov/patch

src/picom.c#L1085

Added line #L1085 was not covered by tests
ps->root_image = NULL;
}
auto pixmap = x_get_root_back_pixmap(&ps->c, ps->atoms);
Expand Down Expand Up @@ -1116,7 +1116,7 @@ void root_damaged(session_t *ps) {
: x_get_visual_for_depth(ps->c.screen_info, r->depth);
free(r);

ps->root_image = ps->backend_data->ops->bind_pixmap(
ps->root_image = ps->backend_data->ops.bind_pixmap(

Check warning on line 1119 in src/picom.c

View check run for this annotation

Codecov / codecov/patch

src/picom.c#L1119

Added line #L1119 was not covered by tests
ps->backend_data, pixmap, x_get_visual_info(&ps->c, visual));
ps->root_image_generation += 1;
if (!ps->root_image) {
Expand Down Expand Up @@ -1421,7 +1421,7 @@ static bool redirect_start(session_t *ps) {
if (!ps->o.use_legacy_backends) {
assert(ps->backend_data);
ps->damage_ring.count =
ps->backend_data->ops->max_buffer_age(ps->backend_data);
ps->backend_data->ops.max_buffer_age(ps->backend_data);
ps->layout_manager = layout_manager_new((unsigned)ps->damage_ring.count);
} else {
ps->damage_ring.count = maximum_buffer_age(ps);
Expand All @@ -1435,7 +1435,7 @@ static bool redirect_start(session_t *ps) {

ps->frame_pacing = ps->o.frame_pacing && ps->o.vsync;
if ((ps->o.use_legacy_backends || ps->o.benchmark ||
!ps->backend_data->ops->last_render_time) &&
!ps->backend_data->ops.last_render_time) &&
ps->frame_pacing) {
// Disable frame pacing if we are using a legacy backend or if we are in
// benchmark mode, or if the backend doesn't report render time
Expand Down
Loading

0 comments on commit bd26302

Please sign in to comment.