[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    Re: [PATCH] fix plasma and Qt monitor-hotplug issues
From:       Aike J Sommer <dev () aikesommer ! name>
Date:       2008-03-31 12:31:02
Message-ID: 200803311431.02466.dev () aikesommer ! name
[Download RAW message or body]

Am Freitag 28 März 2008 17:29:41 schrieb Aaron J. Seigo:
> On Friday 28 March 2008, Aike J Sommer wrote:
> > The patch to qt-copy fixes:
> > - screenResized not beeing emitted on some changes
> > - screenResized always reporting screen 0 to have changed
>
> this needs to go to Trolltech, and may already be out of date with the
> merging of patch 0172... i'll track down bhughes about this.
>
> > This is a little hacky... Qt uses Xinerama to query for screens, so
> > listening to XRandR 1.2 events doesnt really work...
>
> it uses xrandr in the latest upstream code afaik.

At least in the newest snapshots its still just Xinerama... Patch 0172 only 
changed the Xinerama code to better work with XRandR's Xinerama emulation.


>
> > The patch to kdebase basically only calls view->show() after creating
> > it...
>
> yes, this part looks good. pls commit.

Hmm... Dont have a svn-account... Should i apply for one that "early"??


> iterating over the screens isn't a very expensive set of operations
> (looping with an int, looping through our collection of containments) that
> is only done at startup and when the screen counts change. it ensures that
> it works no matter what happens elsewhere and makes the method more self
> contained (e.g. it's obvious on reading it that it works and what it does
> without having to read the rest of the code in the class), so the cost is
> well worth it imho.

Changed that part... Hope its okay now?!?! :-)

> > One thing left: Should views be destroyed if a monitor is disconnected?
> > And
>
> it's not required, but it would be cool if they did, yes.
>
> > if so i guess using screenOwnerChanged or a (new) signal
> > containmentRemoved wouldnt be right, since containments should stay
> > associated with the screen...
>
> this could probably be done easily as the else statement to:
>
> +    if (screen < QApplication::desktop()->numScreens())
>
> in DesktopCorona::screenResized, which can then emit a signal..
> screenRemoved() perhaps? i don't really like how it breaks the symmetry of
> responding to containmentAdded and screenOwnerChanged, but ... *shrug* it
> should worlk in any case.

I did it in RootWidget::adjustSize(screen), it just felt more at home 
there! ;-)

I also changed View::setContainment(containment) to allow unsetting of 
containments, probably isnt really needed, but i wanted to be sure...

:-)

["kdebase-patch-05.diff" (text/x-diff)]

Index: workspace/libs/plasma/view.cpp
===================================================================
--- workspace/libs/plasma/view.cpp	(Revision 792135)
+++ workspace/libs/plasma/view.cpp	(Arbeitskopie)
@@ -124,7 +124,7 @@
 
 void View::setContainment(Containment *containment)
 {
-    if (!containment || containment == d->containment) {
+    if (containment == d->containment) {
         return;
     }
 
@@ -136,6 +136,10 @@
     }
 
     d->containment = containment;
+    if (! containment) {
+        return;
+    }
+    
     if (screen > -1) {
         containment->setScreen(screen);
     }
Index: workspace/plasma/plasma/desktopcorona.cpp
===================================================================
--- workspace/plasma/plasma/desktopcorona.cpp	(Revision 792135)
+++ workspace/plasma/plasma/desktopcorona.cpp	(Arbeitskopie)
@@ -63,21 +63,19 @@
             c->setScreen(i);
             c->setFormFactor(Plasma::Planar);
             c->flushUpdatedConstraints();
+        } else if (i >= m_numScreens) {
+            // now ensure that if our screen count changed we actually get views
+            // for them, even if the Containment already existed for that screen
+            // so we "lie" and emit a containmentAdded signal for every new screen
+            // regardless of whether it actually already existed, or just got added
+            // and therefore had this signal emitted. plasma can handle such
+            // multiple emissions of the signal, and this is simply the most
+            // straightforward way of accomplishing this
+            kDebug() << "Notifying of new screen: " << i;
+            emit containmentAdded(containmentForScreen(i));
         }
     }
 
-    // now ensure that if our screen count changed we actually get views
-    // for them, even if the Containment already existed for that screen
-    // so we "lie" and emit a containmentAdded signal for every new screen
-    // regardless of whether it actually already existed, or just got added
-    // and therefore had this signal emitted. plasma can handle such
-    // multiple emissions of the signal, and this is simply the most
-    // straightforward way of accomplishing this
-    for (int i = m_numScreens; i < numScreens; ++i) {
-        kDebug() << "Notifying of new screen: " << i;
-        emit containmentAdded(containmentForScreen(i));
-    }
-
     m_numScreens = numScreens;
 }
 
@@ -131,18 +129,19 @@
 
 void DesktopCorona::screenResized(int screen)
 {
-    bool desktopFound = false;
-    foreach (Plasma::Containment *c, containments()) {
-        if (c->screen() == screen) {
-            // trigger a relayout
-            c->setScreen(screen);
-            desktopFound = desktopFound ||
-                           c->containmentType() == Plasma::Containment::DesktopContainment ||
-                           c->containmentType() == Plasma::Containment::CustomContainment;
+    int numScreens = QApplication::desktop()->numScreens();
+    if (screen < numScreens) {
+        foreach (Plasma::Containment *c, containments()) {
+            if (c->screen() == screen) {
+                // trigger a relayout
+                c->setScreen(screen);
+            }
         }
+    
+        checkScreens(); // ensure we have containments for every screen
+    } else {
+        m_numScreens = numScreens;
     }
-
-    checkScreens(); // ensure we have containments for every screen
 }
 
 #include "desktopcorona.moc"
Index: workspace/plasma/plasma/rootwidget.cpp
===================================================================
--- workspace/plasma/plasma/rootwidget.cpp	(Revision 792135)
+++ workspace/plasma/plasma/rootwidget.cpp	(Arbeitskopie)
@@ -107,9 +107,18 @@
     QDesktopWidget *desktop = QApplication::desktop();
     setGeometry(desktop->geometry());
     DesktopView *view = viewForScreen(screen);
-
+    
     if (view) {
-        view->adjustSize();
+        if (screen < desktop->numScreens()) {
+            view->adjustSize();
+        } else {
+            // the screen was removed, so we'll destroy the
+            // corresponding view
+            kDebug() << "removing the view for screen" << screen;
+            view->setContainment(0);
+            m_desktops.removeAll(view);
+            delete view;
+        }
     }
 }
 
@@ -138,6 +147,7 @@
     DesktopView *view = new DesktopView(containment, this);
     view->setGeometry(QApplication::desktop()->screenGeometry(containment->screen()));
     m_desktops.append(view);
+    view->show();
 }
 
 void RootWidget::screenOwnerChanged(int wasScreen, int isScreen, Plasma::Containment* containment)


_______________________________________________
Panel-devel mailing list
Panel-devel@kde.org
https://mail.kde.org/mailman/listinfo/panel-devel


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic