From 29ff0d30ad059d7d2554311c33eca513b9445abf Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Mon, 5 Jul 2021 15:17:29 +0300 Subject: [PATCH] x11: Fix BadDamage warning The XDamageDestroy has a weird requirement saying that it can be called as long as the X11 window is still valid. On the other hand, one could argue that it is more intuitive if the damage handle becomes inert if the associated window is destroyed. Unfortunately, that's not the case and as git history shows, it's an easy way to shoot yourself in the foot, we had the exact warning many years ago. The problem with the XDamageDestroy API is that it is simply unreliable given the asynchronous nature of communication between kwin and xorg. Anyway, with X11 sunsetting, let's destroy the damage handle only when the X11 window is unmapped and not bother too much about it. --- src/surfaceitem_x11.cpp | 3 +-- src/toplevel.cpp | 9 ++++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/surfaceitem_x11.cpp b/src/surfaceitem_x11.cpp index 4ffc775702..26a10d5022 100644 --- a/src/surfaceitem_x11.cpp +++ b/src/surfaceitem_x11.cpp @@ -19,8 +19,6 @@ SurfaceItemX11::SurfaceItemX11(Scene::Window *window, Item *parent) connect(toplevel, &Toplevel::bufferGeometryChanged, this, &SurfaceItemX11::handleBufferGeometryChanged); - connect(toplevel, &Toplevel::markedAsZombie, - this, &SurfaceItemX11::destroyDamage); connect(toplevel, &Toplevel::geometryShapeChanged, this, &SurfaceItemX11::discardQuads); @@ -33,6 +31,7 @@ SurfaceItemX11::SurfaceItemX11(Scene::Window *window, Item *parent) SurfaceItemX11::~SurfaceItemX11() { + // destroyDamage() will be called by the associated Toplevel. } void SurfaceItemX11::preprocess() diff --git a/src/toplevel.cpp b/src/toplevel.cpp index 7ada48d145..bc8b09dc90 100644 --- a/src/toplevel.cpp +++ b/src/toplevel.cpp @@ -20,6 +20,7 @@ #include "screens.h" #include "shadow.h" #include "shadowitem.h" +#include "surfaceitem_x11.h" #include "windowitem.h" #include "workspace.h" @@ -276,8 +277,14 @@ bool Toplevel::setupCompositing() return true; } -void Toplevel::finishCompositing(ReleaseReason) +void Toplevel::finishCompositing(ReleaseReason releaseReason) { + // If the X11 window has been destroyed, avoid calling XDamageDestroy. + if (releaseReason != ReleaseReason::Destroyed) { + if (SurfaceItemX11 *item = qobject_cast(surfaceItem())) { + item->destroyDamage(); + } + } if (effect_window && effect_window->window() == this) { // otherwise it's already passed to Deleted, don't free data deleteEffectWindow(); }