From befc3d22cf523e8306d8eff461bb1cbe8a73ec26 Mon Sep 17 00:00:00 2001 From: "Enrico Weigelt, metux IT consult" Date: Fri, 27 Sep 2024 20:45:20 +0200 Subject: [PATCH] dix: add per-screen pixmap destructor mechanism Right now, extension specific pixmap destruction procedures are implemented by wrapping the ScreenRec's DestroyPixmap() proc pointer: the extensions are storing the original pointer in their private data and putting in their own one. On each call, their proc restores the original one, calls it, and switches back again. When multiple extensions doing so, they're forming a kind of daisy chain. (the same is done for lots of other procs) While that approach is looking nice and elegant on the drawing board, it's complicated, dangerous like a chainsaw and makes debugging hard, leading to pretty blurred API borders. It's even getting worse: the proc also has to do ref counting, and only destroy the pixmap if refconter reaching zero - that's all done in the individual screen drivers. Therefore, all extensions must check for refcnt == 1, in order to know when to really act. This commit introduces a simple approach for letting extension hook into the pixmap destruction safely, w/o having to care much about side effects with the call chain. Extensions now can simply register their destructor proc (and an opaque pointer) and get called back - w/o ever having to mess with the ScreenRec's internal structures. Signed-off-by: Enrico Weigelt, metux IT consult --- dix/dispatch.c | 2 ++ dix/dix_priv.h | 16 ++++++++++++++ dix/screen.c | 1 + dix/screen_hooks.c | 11 ++++++++++ dix/screen_hooks_priv.h | 40 ++++++++++++++++++++++++++++++++++ include/scrnintstr.h | 4 ++++ include/xorg-server.h.meson.in | 1 + 7 files changed, 75 insertions(+) diff --git a/dix/dispatch.c b/dix/dispatch.c index c3c0e8270..0f85c66fe 100644 --- a/dix/dispatch.c +++ b/dix/dispatch.c @@ -1447,6 +1447,8 @@ int dixDestroyPixmap(void *value, XID pid) { PixmapPtr pPixmap = (PixmapPtr) value; + if (pPixmap && pPixmap->refcnt == 1) + dixScreenRaisePixmapDestroy(pPixmap); if (pPixmap && pPixmap->drawable.pScreen && pPixmap->drawable.pScreen->DestroyPixmap) return pPixmap->drawable.pScreen->DestroyPixmap(pPixmap); return TRUE; diff --git a/dix/dix_priv.h b/dix/dix_priv.h index 4ca30056e..1ba1fc451 100644 --- a/dix/dix_priv.h +++ b/dix/dix_priv.h @@ -307,6 +307,22 @@ void dixScreenRaiseWindowPosition(WindowPtr pWin, uint32_t x, uint32_t y); */ void dixScreenRaiseClose(ScreenPtr pScreen); +/* + * @brief call screen's PixmapDestroy hook + * @see dixScreenHookPixmapDestroy + * @param pPixmap the pixmap being destroyed + * + * Call the pluggable pixmap destroy pixmap hooks that extensions might have + * registered on the screen. + * Note that it's *only* called, when the pixmap is really being destroyed + * (instead of just unref'ed) + * + * Should only be called by DIX itself, by dixDestroyPixmap() + * It must be called *before* the ScreenRec->DestroyPixmap() is called, any + * only if the reference counter reaches 1. + */ +void dixScreenRaisePixmapDestroy(PixmapPtr pPixmap); + /* * @brief mark event ID as critical * @param event the event to add to the critical events bitmap diff --git a/dix/screen.c b/dix/screen.c index f2da15240..3c3b715a7 100644 --- a/dix/screen.c +++ b/dix/screen.c @@ -23,5 +23,6 @@ void dixFreeScreen(ScreenPtr pScreen) DeleteCallbackList(&pScreen->hookWindowDestroy); DeleteCallbackList(&pScreen->hookWindowPosition); DeleteCallbackList(&pScreen->hookClose); + DeleteCallbackList(&pScreen->hookPixmapDestroy); free(pScreen); } diff --git a/dix/screen_hooks.c b/dix/screen_hooks.c index 42650a50a..64d43ac7e 100644 --- a/dix/screen_hooks.c +++ b/dix/screen_hooks.c @@ -26,6 +26,7 @@ DECLARE_HOOK_PROC(WindowDestroy, hookWindowDestroy, XorgScreenWindowDestroyProcPtr); DECLARE_HOOK_PROC(WindowPosition, hookWindowPosition, XorgScreenWindowPositionProcPtr); DECLARE_HOOK_PROC(Close, hookClose, XorgScreenCloseProcPtr); +DECLARE_HOOK_PROC(PixmapDestroy, hookPixmapDestroy, XorgScreenPixmapDestroyProcPtr); int dixScreenRaiseWindowDestroy(WindowPtr pWin) { @@ -67,3 +68,13 @@ void dixScreenRaiseClose(ScreenPtr pScreen) { if (pScreen->CloseScreen) pScreen->CloseScreen(pScreen); } + +void dixScreenRaisePixmapDestroy(PixmapPtr pPixmap) +{ + if (!pPixmap) + return; + + ScreenPtr pScreen = pPixmap->drawable.pScreen; + CallCallbacks(&pScreen->hookPixmapDestroy, pPixmap); + /* we must not call the original ScreenRec->DestroyPixmap() here */ +} diff --git a/dix/screen_hooks_priv.h b/dix/screen_hooks_priv.h index 00d1606ff..17754f062 100644 --- a/dix/screen_hooks_priv.h +++ b/dix/screen_hooks_priv.h @@ -23,6 +23,7 @@ #include #include "include/callback.h" /* CallbackListPtr */ +#include "include/pixmap.h" /* PixmapPtr */ #include "include/screenint.h" /* ScreenPtr */ #include "include/window.h" /* WindowPtr */ @@ -145,4 +146,43 @@ _X_EXPORT void dixScreenUnhookClose(ScreenPtr pScreen, XorgScreenCloseProcPtr func); +/* prototype of pixmap destroy notification handler */ +typedef void (*XorgScreenPixmapDestroyProcPtr)(CallbackListPtr *pcbl, + ScreenPtr pScreen, + PixmapPtr pPixmap); + +/** + * @brief register a pixmap destroy hook on the given screen + * + * @param pScreen pointer to the screen to register the notify hook into + * @param func pointer to the hook function + * @param arg opaque pointer passed to the hook + * + * When registration fails, the server aborts. + * This hook is called only when the pixmap is really to be destroyed, + * (unlike ScreenRec->DestroyPixmap()) + * + * NOTE: only exported for libglamoregl, not supposed to be used by drivers. + **/ +_X_EXPORT +void dixScreenHookPixmapDestroy(ScreenPtr pScreen, + XorgScreenPixmapDestroyProcPtr func); + +/** + * @brief unregister a pixmap destroy notify hook on the given screen + * + * @param pScreen pointer to the screen to unregister the hook from + * @param func pointer to the hook function + * @param arg opaque pointer passed to the destructor + * + * @see dixScreenHookClose + * + * Unregister a screen close notify hook registered via @ref dixScreenHookPixmapDestroy + * + * NOTE: only exported for libglamoregl, not supposed to be used by drivers. + **/ +_X_EXPORT +void dixScreenUnhookPixmapDestroy(ScreenPtr pScreen, + XorgScreenPixmapDestroyProcPtr func); + #endif /* DIX_SCREEN_HOOKS_H */ diff --git a/include/scrnintstr.h b/include/scrnintstr.h index a0fea9262..df7b2d9cf 100644 --- a/include/scrnintstr.h +++ b/include/scrnintstr.h @@ -679,6 +679,10 @@ typedef struct _Screen { /* additional screen close notify hooks (replaces wrapping CloseScreen) should NOT be touched outside of DIX core */ CallbackListPtr hookClose; + + /* additional pixmap destroy notify hooks (replaces wrapping DestroyPixmap) + should NOT be touched outside of DIX core */ + CallbackListPtr hookPixmapDestroy; } ScreenRec; static inline RegionPtr diff --git a/include/xorg-server.h.meson.in b/include/xorg-server.h.meson.in index 179e31a31..16746cd45 100644 --- a/include/xorg-server.h.meson.in +++ b/include/xorg-server.h.meson.in @@ -218,5 +218,6 @@ #define XORG_API_DIX_SCREEN_HOOK_WINDOW_DESTROY 1 #define XORG_API_DIX_SCREEN_HOOK_WINDOW_POSITION 1 #define XORG_API_DIX_SCREEN_HOOK_CLOSE 1 +#define XORG_API_DIX_SCREEN_HOOK_PIXMAP_DESTROY 1 #endif /* _XORG_SERVER_H_ */