This fixes a bug that made the color profile not to be applied on

startup and also left the device-id empty, due to a call that
only got cached values from X which on session startup didn't had
the EDID data for the outputs set.
It also fixes a bug that failed to emit output changed when
an external monitor was plugged in.
BUG: 309252

5 files changed, 36 insertions, 40 deletions

diff --git a/colord-kded/ColorD.cpp b/colord-kded/ColorD.cpp
index 9b440c4..4dd677b 100644
--- a/colord-kded/ColorD.cpp
+++ b/colord-kded/ColorD.cpp
@@ -44,7 +44,8 @@ typedef QList<QDBusObjectPath> ObjectPathList;
 
 ColorD::ColorD(QObject *parent, const QVariantList &args) :
     KDEDModule(parent),
-    m_x11EventHandler(0)
+    m_x11EventHandler(0),
+    m_profilesWatcher(0)
 {
     // There's not much use for args in a KDED
     Q_UNUSED(args)
@@ -59,7 +60,10 @@ ColorD::ColorD(QObject *parent, const QVariantList &args) :
     connectToColorD();
 
     // Connect to the display
-    connectToDisplay();
+    if ((m_resources = connectToDisplay()) == 0) {
+        kWarning() << "Failed to connect to DISPLAY and get the needed resources";
+        return;
+    }
 
     // Make sure we know is colord is running
     QDBusServiceWatcher *watcher;
@@ -434,14 +438,18 @@ void ColorD::removeOutput(const Output &output)
     m_connectedOutputs.removeOne(output);
 }
 
-void ColorD::connectToDisplay()
+XRRScreenResources *ColorD::connectToDisplay()
 {
     m_dpy = QX11Info::display();
 
     // Check extension
     int eventBase;
-    if (XRRQueryExtension(m_dpy, &eventBase, &m_errorBase) == false) {
-        kWarning() << "Failed to retrieve RandR extension";
+    int major_version, minor_version;
+    if (!XRRQueryExtension(m_dpy, &eventBase, &m_errorBase) ||
+            !XRRQueryVersion(m_dpy, &major_version, &minor_version))
+    {
+        kWarning() << "RandR extension missing";
+        return 0;
     }
 
     // Install our X event handler
@@ -450,12 +458,6 @@ void ColorD::connectToDisplay()
             this, SLOT(checkOutputs()));
     kapp->installX11EventFilter(m_x11EventHandler);
 
-    int major_version, minor_version;
-    XRRQueryVersion(m_dpy, &major_version, &minor_version);
-
-    m_version = i18n("X Resize and Rotate extension version %1.%2",
-                     major_version,minor_version);
-
     // check if we have the new version of the XRandR extension
     bool has_1_2;
     has_1_2 = (major_version > 1 || (major_version == 1 && minor_version >= 2));
@@ -469,19 +471,18 @@ void ColorD::connectToDisplay()
         kDebug() << "Using legacy XRANDR extension (1.1 or earlier).";
     }
 
-    kDebug() << "XRANDR error base: " << m_errorBase;
-
-    /* XRRGetScreenResourcesCurrent is less expensive than
-     * XRRGetScreenResources, however it is available only
-     * in RandR 1.3 or higher
-     */
     m_root = RootWindow(m_dpy, 0);
 
-    if (m_has_1_3) {
-        m_resources = XRRGetScreenResourcesCurrent(m_dpy, m_root);
-    } else {
-        m_resources = XRRGetScreenResources(m_dpy, m_root);
-    }
+    // This call is CRITICAL:
+    // RR 1.3 has a new method called XRRGetScreenResourcesCurrent,
+    // that method is less expensive since it only gets the current
+    // cached values from X. On the other hand in the case of
+    // a session startup the EDID of the output is not cached, leading
+    // to our code failing to get a valid EDID. This call ensures the
+    // X server will probe all outputs again and cache the EDID.
+    // Note: This code only runs once so it's nothing that would
+    // actually slow things down.
+    return XRRGetScreenResources(m_dpy, m_root);
 }
 
 void ColorD::checkOutputs()
diff --git a/colord-kded/ColorD.h b/colord-kded/ColorD.h
index ae52082..e7f6b3d 100644
--- a/colord-kded/ColorD.h
+++ b/colord-kded/ColorD.h
@@ -59,7 +59,7 @@ private slots:
 
 private:
     quint8* readEdidData(RROutput output, size_t &len);
-    void connectToDisplay();
+    XRRScreenResources *connectToDisplay();
     void connectToColorD();
     void addOutput(Output &output);
     void outputChanged(Output &output);
@@ -76,7 +76,6 @@ private:
     Window m_root;
 
     QString m_errorCode;
-    QString m_version;
 
     bool m_has_1_3;
     int m_errorBase;
diff --git a/colord-kded/Output.cpp b/colord-kded/Output.cpp
index 7dbe8e0..13893e9 100644
--- a/colord-kded/Output.cpp
+++ b/colord-kded/Output.cpp
@@ -168,8 +168,8 @@ Edid Output::readEdidData()
     size_t size;
     const quint8 *data;
     data = readEdidData(size);
-    if (data == 0) {
-        kWarning() << "unable to get EDID for output";
+    if (data == NULL) {
+        kWarning() << "Unable to get EDID for output" << name();
         Edid ret;
         m_id = ret.deviceId(name());
         return ret;
@@ -225,10 +225,6 @@ quint8* Output::readEdidData(size_t &len)
 
     edid_atom = XInternAtom(QX11Info::display(), RR_PROPERTY_RANDR_EDID, false);
     result = getProperty(QX11Info::display(), m_output, edid_atom, len);
-    if (result == NULL) {
-        edid_atom = XInternAtom(QX11Info::display(), "EDID_DATA", false);
-        result = getProperty(QX11Info::display(), m_output, edid_atom, len);
-    }
 
     if (result) {
         if (len % 128 == 0) {
@@ -239,7 +235,7 @@ quint8* Output::readEdidData(size_t &len)
         }
     }
 
-    return 0;
+    return NULL;
 }
 
 bool Output::operator==(const Output &output) const
diff --git a/colord-kded/XEventHandler.cpp b/colord-kded/XEventHandler.cpp
index 08cd091..51e564b 100644
--- a/colord-kded/XEventHandler.cpp
+++ b/colord-kded/XEventHandler.cpp
@@ -23,18 +23,18 @@
 
 #include <X11/extensions/Xrandr.h>
 
-XEventHandler::XEventHandler(int randr_base) :
-    m_randr_notify(randr_base + RRNotify)
+XEventHandler::XEventHandler(int randrBase) :
+    m_randrBase(randrBase)
 {
 }
 
 bool XEventHandler::x11Event(XEvent *event)
 {
-    if (event->xany.type == m_randr_notify) {
-        XRRNotifyEvent *notifyEvent = reinterpret_cast<XRRNotifyEvent*>(event);
-        if(notifyEvent->subtype == RRNotify_OutputChange) {
-            emit outputChanged();
-        }
+    int eventCode = event->xany.type - m_randrBase;
+    // We care about any kind of screen change
+    if (eventCode == RRScreenChangeNotify) {
+        emit outputChanged();
     }
+
     return QWidget::x11Event(event);
 }
diff --git a/colord-kded/XEventHandler.h b/colord-kded/XEventHandler.h
index c8989a6..c33afa7 100644
--- a/colord-kded/XEventHandler.h
+++ b/colord-kded/XEventHandler.h
@@ -26,7 +26,7 @@ class XEventHandler : public QWidget
 {
     Q_OBJECT
 public:
-    XEventHandler(int randr_base);
+    XEventHandler(int randrBase);
 
 signals:
     void outputChanged();
@@ -35,7 +35,7 @@ protected:
     virtual bool x11Event(XEvent *event);
 
 private:
-    int m_randr_notify;
+    int m_randrBase;
 };
 
 #endif // XEVENTHANDLER_H