Add addToplevel and removeToplevel to Scene

Summary:
Code that destroys all scene windows when finishing compositing is not
readable. E.g. can you tell what this piece of code is doing without
looking into the source code of Scene::windowClosed?

    foreach (Client * c, Workspace::self()->clientList())
        m_scene->windowClosed(c, NULL);

This change intoduces removeToplevel(as well its counterpart) method to
the Scene class. The name of the new method much better describes what
we're doing.

Reviewers: #kwin, davidedmundson

Reviewed By: #kwin, davidedmundson

Subscribers: kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D18210

composite.cpp

@@ -323,7 +323,7 @@ void Compositor::startupWithWorkspace()
     m_timeSinceLastVBlank = fpsInterval - (options->vBlankTime() + 1); // means "start now" - we don't have even a slight idea when the first vsync will occur
     scheduleRepaint();
     kwinApp()->platform()->createEffectsHandler(this, m_scene);   // sets also the 'effects' pointer
-    connect(Workspace::self(), &Workspace::deletedRemoved, m_scene, &Scene::windowDeleted);
+    connect(Workspace::self(), &Workspace::deletedRemoved, m_scene, &Scene::removeToplevel);
     connect(effects, SIGNAL(screenGeometryChanged(QSize)), SLOT(addRepaintFull()));
     addRepaintFull();
     foreach (Client * c, Workspace::self()->clientList()) {
@@ -381,13 +381,13 @@ void Compositor::finish()
 
     if (Workspace::self()) {
         foreach (Client * c, Workspace::self()->clientList())
-            m_scene->windowClosed(c, NULL);
+            m_scene->removeToplevel(c);
         foreach (Client * c, Workspace::self()->desktopList())
-            m_scene->windowClosed(c, NULL);
+            m_scene->removeToplevel(c);
         foreach (Unmanaged * c, Workspace::self()->unmanagedList())
-            m_scene->windowClosed(c, NULL);
+            m_scene->removeToplevel(c);
         foreach (Deleted * c, Workspace::self()->deletedList())
-            m_scene->windowDeleted(c);
+            m_scene->removeToplevel(c);
         foreach (Client * c, Workspace::self()->clientList())
         c->finishCompositing();
         foreach (Client * c, Workspace::self()->desktopList())
@@ -402,10 +402,10 @@ void Compositor::finish()
     }
     if (waylandServer()) {
         foreach (ShellClient *c, waylandServer()->clients()) {
-            m_scene->windowClosed(c, nullptr);
+            m_scene->removeToplevel(c);
         }
         foreach (ShellClient *c, waylandServer()->internalClients()) {
-            m_scene->windowClosed(c, nullptr);
+            m_scene->removeToplevel(c);
         }
         foreach (ShellClient *c, waylandServer()->clients()) {
             c->finishCompositing();
@@ -955,7 +955,7 @@ bool Toplevel::setupCompositing()
     damage_region = QRegion(0, 0, width(), height());
     effect_window = new EffectWindowImpl(this);
 
-    Compositor::self()->scene()->windowAdded(this);
+    Compositor::self()->scene()->addToplevel(this);
 
     // With unmanaged windows there is a race condition between the client painting the window
     // and us setting up damage tracking.  If the client wins we won't get a damage event even

scene.cpp

@@ -390,7 +390,7 @@ void Scene::paintSimpleScreen(int orig_mask, QRegion region)
     }
 }
 
-void Scene::windowAdded(Toplevel *c)
+void Scene::addToplevel(Toplevel *c)
 {
     assert(!m_windows.contains(c));
     Scene::Window *w = createWindow(c);
@@ -412,28 +412,27 @@ void Scene::windowAdded(Toplevel *c)
     );
 }
 
-void Scene::windowClosed(Toplevel *c, Deleted *deleted)
+void Scene::removeToplevel(Toplevel *toplevel)
 {
-    assert(m_windows.contains(c));
-    if (deleted != NULL) {
-        // replace c with deleted
-        Window* w = m_windows.take(c);
-        w->updateToplevel(deleted);
-        if (w->shadow()) {
-            w->shadow()->setToplevel(deleted);
-        }
-        m_windows[ deleted ] = w;
-    } else {
-        delete m_windows.take(c);
-        c->effectWindow()->setSceneWindow(NULL);
-    }
+    Q_ASSERT(m_windows.contains(toplevel));
+    delete m_windows.take(toplevel);
+    toplevel->effectWindow()->setSceneWindow(nullptr);
 }
 
-void Scene::windowDeleted(Deleted *c)
+void Scene::windowClosed(Toplevel *toplevel, Deleted *deleted)
 {
-    assert(m_windows.contains(c));
-    delete m_windows.take(c);
-    c->effectWindow()->setSceneWindow(NULL);
+    if (!deleted) {
+        removeToplevel(toplevel);
+        return;
+    }
+
+    Q_ASSERT(m_windows.contains(toplevel));
+    Window *window = m_windows.take(toplevel);
+    window->updateToplevel(deleted);
+    if (window->shadow()) {
+        window->shadow()->setToplevel(deleted);
+    }
+    m_windows[deleted] = window;
 }
 
 void Scene::windowGeometryShapeChanged(Toplevel *c)

scene.h

@@ -78,11 +78,25 @@ public:
     // ie. "what of this frame is lost to painting"
     virtual qint64 paint(QRegion damage, ToplevelList windows) = 0;
 
-    // Notification function - KWin core informs about changes.
-    // Used to mainly discard cached data.
+    /**
+     * Adds the Toplevel to the Scene.
+     *
+     * If the toplevel gets deleted, then the scene will try automatically
+     * to re-bind an underlying scene window to the corresponding Deleted.
+     *
+     * @param toplevel The window to be added.
+     * @note You can add a toplevel to scene only once.
+     **/
+    void addToplevel(Toplevel *toplevel);
+
+    /**
+     * Removes the Toplevel from the Scene.
+     *
+     * @param toplevel The window to be removed.
+     * @note You can remove a toplevel from the scene only once.
+     **/
+    void removeToplevel(Toplevel *toplevel);
 
-    // a new window has been created
-    void windowAdded(Toplevel*);
     /**
      * @brief Creates the Scene backend of an EffectFrame.
      *
@@ -189,8 +203,6 @@ Q_SIGNALS:
     void resetCompositing();
 
 public Q_SLOTS:
-    // a window has been destroyed
-    void windowDeleted(KWin::Deleted*);
     // shape/size of a window changed
     void windowGeometryShapeChanged(KWin::Toplevel* c);
     // a window has been closed