Common way to announce support for specific effects through X11

Instead of each effect, which needs to announce support, having custom
code to create a property and set it on the root window, there is now a
common API in EffectsHandler to take care of this.

The methods takes care of creating the atom if it has not already done
and set the property on the root window. Furthermore it allows multiple
effects to announce the same property without getting in conflict with
each other.

As a further convenience the property is automatically removed when the
effect is unloaded, so less things an effect author has to care about.

REVIEW: 107815

effects.cpp

@@ -625,6 +625,57 @@ void EffectsHandlerImpl::registerPropertyType(long atom, bool reg)
     }
 }
 
+xcb_atom_t EffectsHandlerImpl::announceSupportProperty(const QByteArray &propertyName, Effect *effect)
+{
+    PropertyEffectMap::iterator it = m_propertiesForEffects.find(propertyName);
+    if (it != m_propertiesForEffects.end()) {
+        // property has already been registered for an effect
+        // just append Effect and return the atom stored in m_managedProperties
+        if (!it.value().contains(effect)) {
+            it.value().append(effect);
+        }
+        return m_managedProperties.value(propertyName);
+    }
+    // get the atom for the propertyName
+    QScopedPointer<xcb_intern_atom_reply_t> atomReply(xcb_intern_atom_reply(connection(),
+        xcb_intern_atom_unchecked(connection(), false, propertyName.size(), propertyName.constData()),
+        NULL));
+    if (atomReply.isNull()) {
+        return XCB_ATOM_NONE;
+    }
+    // announce property on root window
+    unsigned char dummy = 0;
+    xcb_change_property(connection(), XCB_PROP_MODE_REPLACE, rootWindow(), atomReply->atom, atomReply->atom, 8, 1, &dummy);
+    // TODO: add to _NET_SUPPORTED
+    m_managedProperties.insert(propertyName, atomReply->atom);
+    m_propertiesForEffects.insert(propertyName, QList<Effect*>() << effect);
+    registerPropertyType(atomReply->atom, true);
+    return atomReply->atom;
+}
+
+void EffectsHandlerImpl::removeSupportProperty(const QByteArray &propertyName, Effect *effect)
+{
+    PropertyEffectMap::iterator it = m_propertiesForEffects.find(propertyName);
+    if (it == m_propertiesForEffects.end()) {
+        // property is not registered - nothing to do
+        return;
+    }
+    if (!it.value().contains(effect)) {
+        // property is not registered for given effect - nothing to do
+        return;
+    }
+    it.value().removeAll(effect);
+    if (!it.value().isEmpty()) {
+        // property still registered for another effect - nothing further to do
+        return;
+    }
+    // remove property from root window
+    const xcb_atom_t atom = m_managedProperties.take(propertyName);
+    deleteRootProperty(atom);
+    registerPropertyType(atom, false);
+    m_propertiesForEffects.remove(propertyName);
+}
+
 QByteArray EffectsHandlerImpl::readRootProperty(long atom, long type, int format) const
 {
     return readWindowProperty(rootWindow(), atom, type, format);
@@ -1358,6 +1409,11 @@ void EffectsHandlerImpl::unloadEffect(const QString& name)
             if (activeFullScreenEffect() == it.value().second) {
                 setActiveFullScreenEffect(0);
             }
+            // remove support properties for the effect
+            const QList<QByteArray> properties = m_propertiesForEffects.keys();
+            foreach (const QByteArray &property, properties) {
+                removeSupportProperty(property, it.value().second);
+            }
             delete it.value().second;
             effect_order.erase(it);
             effectsChanged();

effects.h

@@ -149,6 +149,8 @@ public:
     virtual void registerPropertyType(long atom, bool reg);
     virtual QByteArray readRootProperty(long atom, long type, int format) const;
     virtual void deleteRootProperty(long atom) const;
+    virtual xcb_atom_t announceSupportProperty(const QByteArray& propertyName, Effect* effect);
+    virtual void removeSupportProperty(const QByteArray& propertyName, Effect* effect);
 
     virtual bool hasDecorationShadows() const;
 
@@ -235,6 +237,9 @@ private:
     QList< Effect* >::iterator m_currentPaintEffectFrameIterator;
     QList< Effect* >::iterator m_currentPaintScreenIterator;
     QList< Effect* >::iterator m_currentBuildQuadsIterator;
+    typedef QHash< QByteArray, QList< Effect*> > PropertyEffectMap;
+    PropertyEffectMap m_propertiesForEffects;
+    QHash<QByteArray, qulonglong> m_managedProperties;
     Compositor *m_compositor;
     Scene *m_scene;
 };

effects/blur/blur.cpp

@@ -48,18 +48,14 @@ BlurEffect::BlurEffect()
 
     target = new GLRenderTarget(tex);
 
-    net_wm_blur_region = XInternAtom(display(), "_KDE_NET_WM_BLUR_BEHIND_REGION", False);
-    effects->registerPropertyType(net_wm_blur_region, true);
-
     reconfigure(ReconfigureAll);
 
     // ### Hackish way to announce support.
     //     Should be included in _NET_SUPPORTED instead.
     if (shader && shader->isValid() && target->valid()) {
-        XChangeProperty(display(), rootWindow(), net_wm_blur_region, net_wm_blur_region,
-                        32, PropModeReplace, 0, 0);
+        net_wm_blur_region = effects->announceSupportProperty("_KDE_NET_WM_BLUR_BEHIND_REGION", this);
     } else {
-        XDeleteProperty(display(), rootWindow(), net_wm_blur_region);
+        net_wm_blur_region = 0;
     }
     connect(effects, SIGNAL(windowAdded(KWin::EffectWindow*)), this, SLOT(slotWindowAdded(KWin::EffectWindow*)));
     connect(effects, SIGNAL(windowDeleted(KWin::EffectWindow*)), this, SLOT(slotWindowDeleted(KWin::EffectWindow*)));
@@ -69,9 +65,6 @@ BlurEffect::BlurEffect()
 
 BlurEffect::~BlurEffect()
 {
-    effects->registerPropertyType(net_wm_blur_region, false);
-    XDeleteProperty(display(), rootWindow(), net_wm_blur_region);
-
     windows.clear();
 
     delete shader;

effects/dashboard/dashboard.cpp

@@ -35,8 +35,8 @@ DashboardEffect::DashboardEffect()
     , deactivateAnimation(false)
     , window(NULL)
 {
-    // propagate that the effect is loaded
-    propagate();
+    // TODO: better namespacing for atoms
+    atom = effects->announceSupportProperty("_WM_EFFECT_KDE_DASHBOARD", this);
 
     // read settings
     reconfigure(ReconfigureAll);
@@ -47,24 +47,6 @@ DashboardEffect::DashboardEffect()
 
 DashboardEffect::~DashboardEffect()
 {
-    unpropagate();
-}
-
-void DashboardEffect::propagate()
-{
-    // TODO: better namespacing for atoms
-    atom = XInternAtom(display(), "_WM_EFFECT_KDE_DASHBOARD", false);
-    effects->registerPropertyType(atom, true);
-
-    // TODO: maybe not the best way to propagate the loaded effect
-    unsigned char dummy = 0;
-    XChangeProperty(display(), rootWindow(), atom, atom, 8, PropModeReplace, &dummy, 1);
-}
-
-void DashboardEffect::unpropagate()
-{
-    effects->registerPropertyType(atom, false);
-    XDeleteProperty(display(), rootWindow(), atom);
 }
 
 void DashboardEffect::reconfigure(ReconfigureFlags)

effects/dashboard/dashboard.h

@@ -43,9 +43,7 @@ public:
     virtual void paintWindow(EffectWindow* w, int mask, QRegion region, WindowPaintData& data);
     virtual void prePaintScreen(ScreenPrePaintData& data, int time);
     virtual void postPaintScreen();
-    virtual void propagate();
     virtual void reconfigure(ReconfigureFlags);
-    virtual void unpropagate();
     virtual bool isActive() const;
 
     // for properties

effects/highlightwindow/highlightwindow.cpp

@@ -32,12 +32,7 @@ HighlightWindowEffect::HighlightWindowEffect()
     , m_fadeDuration(float(animationTime(150)))
     , m_monitorWindow(NULL)
 {
-    m_atom = XInternAtom(display(), "_KDE_WINDOW_HIGHLIGHT", False);
-    effects->registerPropertyType(m_atom, true);
-
-    // Announce support by creating a dummy version on the root window
-    unsigned char dummy = 0;
-    XChangeProperty(display(), rootWindow(), m_atom, m_atom, 8, PropModeReplace, &dummy, 1);
+    m_atom = effects->announceSupportProperty("_KDE_WINDOW_HIGHLIGHT", this);
     connect(effects, SIGNAL(windowAdded(KWin::EffectWindow*)), this, SLOT(slotWindowAdded(KWin::EffectWindow*)));
     connect(effects, SIGNAL(windowClosed(KWin::EffectWindow*)), this, SLOT(slotWindowClosed(KWin::EffectWindow*)));
     connect(effects, SIGNAL(windowDeleted(KWin::EffectWindow*)), this, SLOT(slotWindowDeleted(KWin::EffectWindow*)));
@@ -46,8 +41,6 @@ HighlightWindowEffect::HighlightWindowEffect()
 
 HighlightWindowEffect::~HighlightWindowEffect()
 {
-    XDeleteProperty(display(), rootWindow(), m_atom);
-    effects->registerPropertyType(m_atom, false);
 }
 
 static bool isInitiallyHidden(EffectWindow* w)

effects/presentwindows/presentwindows.cpp

@@ -66,15 +66,8 @@ PresentWindowsEffect::PresentWindowsEffect()
     , m_highlightedDropTarget(NULL)
     , m_dragToClose(false)
 {
-    m_atomDesktop = XInternAtom(display(), "_KDE_PRESENT_WINDOWS_DESKTOP", False);
-    m_atomWindows = XInternAtom(display(), "_KDE_PRESENT_WINDOWS_GROUP", False);
-    effects->registerPropertyType(m_atomDesktop, true);
-    effects->registerPropertyType(m_atomWindows, true);
-
-    // Announce support by creating a dummy version on the root window
-    unsigned char dummy = 0;
-    XChangeProperty(display(), rootWindow(), m_atomDesktop, m_atomDesktop, 8, PropModeReplace, &dummy, 1);
-    XChangeProperty(display(), rootWindow(), m_atomWindows, m_atomWindows, 8, PropModeReplace, &dummy, 1);
+    m_atomDesktop = effects->announceSupportProperty("_KDE_PRESENT_WINDOWS_DESKTOP", this);
+    m_atomWindows = effects->announceSupportProperty("_KDE_PRESENT_WINDOWS_GROUP", this);
 
     KActionCollection* actionCollection = new KActionCollection(this);
     KAction* a = (KAction*)actionCollection->addAction("Expose");
@@ -105,10 +98,6 @@ PresentWindowsEffect::PresentWindowsEffect()
 
 PresentWindowsEffect::~PresentWindowsEffect()
 {
-    XDeleteProperty(display(), rootWindow(), m_atomDesktop);
-    effects->registerPropertyType(m_atomDesktop, false);
-    XDeleteProperty(display(), rootWindow(), m_atomWindows);
-    effects->registerPropertyType(m_atomWindows, false);
     foreach (ElectricBorder border, m_borderActivate) {
         effects->unreserveElectricBorder(border);
     }

effects/slidingpopups/slidingpopups.cpp

@@ -31,11 +31,7 @@ KWIN_EFFECT(slidingpopups, SlidingPopupsEffect)
 
 SlidingPopupsEffect::SlidingPopupsEffect()
 {
-    mAtom = XInternAtom(display(), "_KDE_SLIDE", False);
-    effects->registerPropertyType(mAtom, true);
-    // TODO hackish way to announce support, make better after 4.0
-    unsigned char dummy = 0;
-    XChangeProperty(display(), rootWindow(), mAtom, mAtom, 8, PropModeReplace, &dummy, 1);
+    mAtom = effects->announceSupportProperty("_KDE_SLIDE", this);
     connect(effects, SIGNAL(windowAdded(KWin::EffectWindow*)), this, SLOT(slotWindowAdded(KWin::EffectWindow*)));
     connect(effects, SIGNAL(windowClosed(KWin::EffectWindow*)), this, SLOT(slotWindowClosed(KWin::EffectWindow*)));
     connect(effects, SIGNAL(windowDeleted(KWin::EffectWindow*)), this, SLOT(slotWindowDeleted(KWin::EffectWindow*)));
@@ -45,8 +41,6 @@ SlidingPopupsEffect::SlidingPopupsEffect()
 
 SlidingPopupsEffect::~SlidingPopupsEffect()
 {
-    XDeleteProperty(display(), rootWindow(), mAtom);
-    effects->registerPropertyType(mAtom, false);
 }
 
 void SlidingPopupsEffect::reconfigure(ReconfigureFlags flags)

effects/taskbarthumbnail/taskbarthumbnail.cpp

@@ -36,11 +36,7 @@ KWIN_EFFECT(taskbarthumbnail, TaskbarThumbnailEffect)
 
 TaskbarThumbnailEffect::TaskbarThumbnailEffect()
 {
-    atom = XInternAtom(display(), "_KDE_WINDOW_PREVIEW", False);
-    effects->registerPropertyType(atom, true);
-    // TODO hackish way to announce support, make better after 4.0
-    unsigned char dummy = 0;
-    XChangeProperty(display(), rootWindow(), atom, atom, 8, PropModeReplace, &dummy, 1);
+    atom = effects->announceSupportProperty("_KDE_WINDOW_PREVIEW", this);
     connect(effects, SIGNAL(windowAdded(KWin::EffectWindow*)), this, SLOT(slotWindowAdded(KWin::EffectWindow*)));
     connect(effects, SIGNAL(windowDeleted(KWin::EffectWindow*)), this, SLOT(slotWindowDeleted(KWin::EffectWindow*)));
     connect(effects, SIGNAL(windowDamaged(KWin::EffectWindow*,QRect)), this, SLOT(slotWindowDamaged(KWin::EffectWindow*,QRect)));
@@ -49,8 +45,6 @@ TaskbarThumbnailEffect::TaskbarThumbnailEffect()
 
 TaskbarThumbnailEffect::~TaskbarThumbnailEffect()
 {
-    XDeleteProperty(display(), rootWindow(), atom);
-    effects->registerPropertyType(atom, false);
 }
 
 void TaskbarThumbnailEffect::prePaintScreen(ScreenPrePaintData& data, int time)

libkwineffects/kwineffects.h

@@ -807,6 +807,37 @@ public:
     virtual void registerPropertyType(long atom, bool reg) = 0;
     virtual QByteArray readRootProperty(long atom, long type, int format) const = 0;
     virtual void deleteRootProperty(long atom) const = 0;
+    /**
+     * @brief Announces support for the feature with the given name. If no other Effect
+     * has announced support for this feature yet, an X11 property will be installed on
+     * the root window.
+     *
+     * The Effect will be notified for events through the signal propertyNotify().
+     *
+     * To remove the support again use @link removeSupportProperty. When an Effect is
+     * destroyed it is automatically taken care of removing the support. It is not
+     * required to call @link removeSupportProperty in the Effect's cleanup handling.
+     *
+     * @param propertyName The name of the property to announce support for
+     * @param effect The effect which announces support
+     * @return xcb_atom_t The created X11 atom
+     * @see removeSupportProperty
+     * @since 4.11
+     **/
+    virtual xcb_atom_t announceSupportProperty(const QByteArray &propertyName, Effect *effect) = 0;
+    /**
+     * @brief Removes support for the feature with the given name. If there is no other Effect left
+     * which has announced support for the given property, the property will be removed from the
+     * root window.
+     *
+     * In case the Effect had not registered support, calling this function does not change anything.
+     *
+     * @param propertyName The name of the property to remove support for
+     * @param effect The effect which had registered the property.
+     * @see announceSupportProperty
+     * @since 4.11
+     **/
+    virtual void removeSupportProperty(const QByteArray &propertyName, Effect *effect) = 0;
 
     /**
      * Returns @a true if the active window decoration has shadow API hooks.