Commit 000f3d83 authored by Vlad Zahorodnii's avatar Vlad Zahorodnii
Browse files

Ensure that Workspace::deletedRemoved() is emitted while there's scene window

Effects may perform cleanup when a deleted window is removed. If that
happens and the SceneWindow is accessed, kwin may crash.

The Scene processes Workspace::deletedRemoved() before effects.

In order to fix dereferencing null pointer, this change makes the Window
destroy its associated SceneWindow.
parent 892059cd
Pipeline #169087 passed with stage
in 24 minutes and 51 seconds
......@@ -503,15 +503,6 @@ void Compositor::stop()
effects = nullptr;
if (Workspace::self()) {
for (X11Window *window : Workspace::self()->clientList()) {
m_scene->removeToplevel(window);
}
for (Unmanaged *window : Workspace::self()->unmanagedList()) {
m_scene->removeToplevel(window);
}
for (InternalWindow *window : workspace()->internalWindows()) {
m_scene->removeToplevel(window);
}
for (X11Window *window : Workspace::self()->clientList()) {
window->finishCompositing();
}
......@@ -531,10 +522,6 @@ void Compositor::stop()
}
if (waylandServer()) {
const QList<Window *> toRemoveTopLevel = waylandServer()->windows();
for (Window *window : toRemoveTopLevel) {
m_scene->removeToplevel(window);
}
const QList<Window *> toFinishCompositing = waylandServer()->windows();
for (Window *window : toFinishCompositing) {
window->finishCompositing();
......
......@@ -48,6 +48,7 @@ Deleted::~Deleted()
workspace()->removeDeleted(this);
}
deleteEffectWindow();
deleteSceneWindow();
deleteShadow();
}
......
......@@ -53,14 +53,13 @@ void WindowScreenCastSource::render(GLFramebuffer *target)
projectionMatrix.ortho(geometry.x(), geometry.x() + geometry.width(),
geometry.y(), geometry.y() + geometry.height(), -1, 1);
EffectWindowImpl *effectWindow = m_window->effectWindow();
WindowPaintData data(effectWindow);
WindowPaintData data(m_window->effectWindow());
data.setProjectionMatrix(projectionMatrix);
GLFramebuffer::pushFramebuffer(target);
glClearColor(0.0, 0.0, 0.0, 0.0);
glClear(GL_COLOR_BUFFER_BIT);
effectWindow->sceneWindow()->performPaint(Scene::PAINT_WINDOW_TRANSFORMED, infiniteRegion(), data);
m_window->sceneWindow()->performPaint(Scene::PAINT_WINDOW_TRANSFORMED, infiniteRegion(), data);
GLFramebuffer::popFramebuffer();
}
......
......@@ -143,13 +143,10 @@ Scene::Scene(QObject *parent)
Scene::~Scene()
{
Q_ASSERT(m_windows.isEmpty());
}
void Scene::initialize()
{
connect(workspace(), &Workspace::deletedRemoved, this, &Scene::removeToplevel);
connect(workspace(), &Workspace::currentActivityChanged, this, &Scene::addRepaintFull);
connect(workspace(), &Workspace::currentDesktopChanged, this, &Scene::addRepaintFull);
connect(workspace(), &Workspace::stackingOrderChanged, this, &Scene::addRepaintFull);
......@@ -411,7 +408,7 @@ void Scene::postPaint()
const std::chrono::milliseconds frameTime =
std::chrono::duration_cast<std::chrono::milliseconds>(painted_screen->renderLoop()->lastPresentationTimestamp());
for (SceneWindow *window : std::as_const(m_windows)) {
for (SceneWindow *window : std::as_const(stacking_order)) {
Window *toplevel = window->window();
if (!toplevel->isOnOutput(painted_screen)) {
continue;
......@@ -555,37 +552,6 @@ void Scene::paintSimpleScreen(int, const QRegion &region)
}
}
void Scene::addToplevel(Window *c)
{
Q_ASSERT(!m_windows.contains(c));
SceneWindow *w = createWindow(c);
m_windows[c] = w;
connect(c, &Window::windowClosed, this, &Scene::windowClosed);
c->effectWindow()->setSceneWindow(w);
}
void Scene::removeToplevel(Window *toplevel)
{
Q_ASSERT(m_windows.contains(toplevel));
delete m_windows.take(toplevel);
toplevel->effectWindow()->setSceneWindow(nullptr);
}
void Scene::windowClosed(Window *toplevel, Deleted *deleted)
{
if (!deleted) {
removeToplevel(toplevel);
return;
}
Q_ASSERT(m_windows.contains(toplevel));
auto window = m_windows.take(toplevel);
window->updateToplevel(deleted);
m_windows[deleted] = window;
}
void Scene::createStackingOrder()
{
// Create a list of all windows in the stacking order
......@@ -616,8 +582,8 @@ void Scene::createStackingOrder()
// TODO: cache the stacking_order in case it has not changed
for (Window *c : std::as_const(windows)) {
Q_ASSERT(m_windows.contains(c));
stacking_order.append(m_windows[c]);
Q_ASSERT(c->sceneWindow());
stacking_order.append(c->sceneWindow());
}
}
......@@ -730,9 +696,9 @@ SceneWindow::~SceneWindow()
{
}
void SceneWindow::updateToplevel(Deleted *deleted)
void SceneWindow::setToplevel(Window *window)
{
toplevel = deleted;
toplevel = window;
}
void SceneWindow::referencePreviousPixmap()
......
......@@ -121,15 +121,7 @@ public:
* @param toplevel The window to be added.
* @note You can add a toplevel to scene only once.
*/
void addToplevel(Window *toplevel);
/**
* Removes the Window from the Scene.
*
* @param toplevel The window to be removed.
* @note You can remove a toplevel from the scene only once.
*/
void removeToplevel(Window *toplevel);
virtual SceneWindow *createWindow(Window *toplevel) = 0;
/**
* @brief Creates the Scene backend of an EffectFrame.
......@@ -223,12 +215,7 @@ public:
Q_SIGNALS:
void frameRendered();
public Q_SLOTS:
// a window has been closed
void windowClosed(Window *c, Deleted *deleted);
protected:
virtual SceneWindow *createWindow(Window *toplevel) = 0;
void createStackingOrder();
void clearStackingOrder();
// shared implementation, starts painting the screen
......@@ -280,7 +267,6 @@ protected:
private:
std::chrono::milliseconds m_expectedPresentTimestamp = std::chrono::milliseconds::zero();
QList<SceneDelegate *> m_delegates;
QHash<Window *, SceneWindow *> m_windows;
QRect m_geometry;
QMatrix4x4 m_renderTargetProjectionMatrix;
QRect m_renderTargetRect;
......@@ -332,7 +318,7 @@ public:
// is the window visible at all
bool isVisible() const;
QRegion decorationShape() const;
void updateToplevel(Deleted *deleted);
void setToplevel(Window *window);
void referencePreviousPixmap();
void unreferencePreviousPixmap();
WindowItem *windowItem() const;
......
......@@ -421,15 +421,14 @@ void WindowThumbnailItem::updateOffscreenTexture()
projectionMatrix.ortho(geometry.x(), geometry.x() + geometry.width(),
geometry.y(), geometry.y() + geometry.height(), -1, 1);
EffectWindowImpl *effectWindow = m_client->effectWindow();
WindowPaintData data(effectWindow);
WindowPaintData data(m_client->effectWindow());
data.setProjectionMatrix(projectionMatrix);
// The thumbnail must be rendered using kwin's opengl context as VAOs are not
// shared across contexts. Unfortunately, this also introduces a latency of 1
// frame, which is not ideal, but it is acceptable for things such as thumbnails.
const int mask = Scene::PAINT_WINDOW_TRANSFORMED;
effectWindow->sceneWindow()->performPaint(mask, infiniteRegion(), data);
m_client->sceneWindow()->performPaint(mask, infiniteRegion(), data);
GLFramebuffer::popFramebuffer();
// The fence is needed to avoid the case where qtquick renderer starts using
......
......@@ -185,6 +185,10 @@ void Window::copyToDeleted(Window *c)
if (m_effectWindow != nullptr) {
m_effectWindow->setWindow(this);
}
m_sceneWindow = c->m_sceneWindow;
if (m_sceneWindow != nullptr) {
m_sceneWindow->setToplevel(this);
}
m_shadow = c->m_shadow;
if (m_shadow) {
m_shadow->setToplevel(this);
......@@ -344,7 +348,9 @@ bool Window::setupCompositing()
m_effectWindow = new EffectWindowImpl(this);
updateShadow();
Compositor::self()->scene()->addToplevel(this);
m_sceneWindow = Compositor::self()->scene()->createWindow(this);
m_effectWindow->setSceneWindow(m_sceneWindow);
connect(windowItem(), &WindowItem::positionChanged, this, &Window::visibleGeometryChanged);
connect(windowItem(), &WindowItem::boundingRectChanged, this, &Window::visibleGeometryChanged);
......@@ -366,6 +372,9 @@ void Window::finishCompositing(ReleaseReason releaseReason)
if (m_effectWindow && m_effectWindow->window() == this) { // otherwise it's already passed to Deleted, don't free data
deleteEffectWindow();
}
if (m_sceneWindow && m_sceneWindow->window() == this) { // otherwise it's already passed to Deleted, don't free data
deleteSceneWindow();
}
}
void Window::addRepaint(const QRect &rect)
......@@ -447,6 +456,12 @@ void Window::deleteEffectWindow()
m_effectWindow = nullptr;
}
void Window::deleteSceneWindow()
{
delete m_sceneWindow;
m_sceneWindow = nullptr;
}
int Window::screen() const
{
return kwinApp()->platform()->enabledOutputs().indexOf(m_output);
......@@ -500,16 +515,16 @@ void Window::updateShadow()
SurfaceItem *Window::surfaceItem() const
{
if (effectWindow() && effectWindow()->sceneWindow()) {
return effectWindow()->sceneWindow()->surfaceItem();
if (m_sceneWindow) {
return m_sceneWindow->surfaceItem();
}
return nullptr;
}
WindowItem *Window::windowItem() const
{
if (effectWindow() && effectWindow()->sceneWindow()) {
return effectWindow()->sceneWindow()->windowItem();
if (m_sceneWindow) {
return m_sceneWindow->windowItem();
}
return nullptr;
}
......
......@@ -50,6 +50,7 @@ class Output;
class ClientMachine;
class Deleted;
class EffectWindowImpl;
class SceneWindow;
class Shadow;
class SurfaceItem;
class VirtualDesktop;
......@@ -732,6 +733,7 @@ public:
void addWorkspaceRepaint(const QRegion &region);
EffectWindowImpl *effectWindow();
const EffectWindowImpl *effectWindow() const;
SceneWindow *sceneWindow() const;
SurfaceItem *surfaceItem() const;
WindowItem *windowItem() const;
/**
......@@ -1547,6 +1549,7 @@ protected:
void disownDataPassedToDeleted();
void deleteShadow();
void deleteEffectWindow();
void deleteSceneWindow();
void setDepth(int depth);
Output *m_output = nullptr;
......@@ -1876,6 +1879,7 @@ private:
Xcb::Window m_client;
bool is_shape;
EffectWindowImpl *m_effectWindow;
SceneWindow *m_sceneWindow = nullptr;
Shadow *m_shadow = nullptr;
QByteArray resource_name;
QByteArray resource_class;
......@@ -2206,6 +2210,11 @@ inline const EffectWindowImpl *Window::effectWindow() const
return m_effectWindow;
}
inline SceneWindow *Window::sceneWindow() const
{
return m_sceneWindow;
}
inline bool Window::isOnAllDesktops() const
{
return desktops().isEmpty();
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment