[PATCH] fix plasma and Qt monitor-hotplug issues

[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.

> The patch to kdebase basically only calls view->show() after creating it...

yes, this part looks good. pls commit.

> The rest just reduces some iterations over all screens, which are not
> needed anymore!

hm.. instead of calling checkScreen recursively, it would probably be a lot 
clearer to just wrap it in a "for (; screen >= m_numScreens; ++m_numScreens)"  
construct

things such as "should only be needed if new screens appear out of order." 
scare me as well ;) it's easy to make the code resilient against 
such "shouldn't happens", and thereby catch them when they do ;) and still 
behave properly

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.

btw, the coding style requires that even single-line conditional statements be 
enclosed in {}s.

> 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.

> Maybe in DesktopCorona::adjustSize()? 

there is no mapping between the corona size and the # of physical screens, 
really.

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Trolltech
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 194 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/panel-devel/attachments/20080328/44dca845/attachment.pgp