Smarter desktop applet placement

[Aaron J. Seigo]

On Friday 01 August 2008, Ambroz Bizjak wrote:
> I've coded a little algorithm to make Plasma place desktop applet where
> there's actually space. It has the ability to stack applets into any of
> the four corners of the desktop, and can stack them both vertically and
> horizontally.

cool, and much needed... comments on the substance of the patch itself first:

* algorithimically, it looks ok, as in reading through it it seems like it 
should work fine; i haven't compiled and tested it yet as i think there are 
some bigger issues to deal with first (particularly the "should this be a 
layout" issue; see below)

* at first i wondered if the code shouldn't be called from 
Containment::addApplet(Applet *applet, const QPointF &pos, bool delayInit) 
when pos == (-1, -1); if the code were to remain where it is, i think that 
would be better in fact (it covers more use cases); but i think this code 
doesn't belong in Containment at all (see below) making this a moot point

* instead of using magic numbers for setting the float members to "big number"  
(e.g. 9999999) use umeric_limits<float>::max()

* instead of float, use qreal (same thing on the platforms we use, but more 
portable in general and what qt uses for, e.g. qgraphicsview stuff)

* the alignment parameters Yalign and Xalign should be of type 
ContainmentPrivate::VertAlign and ContainmentPrivate::HorizAlign, not int 
(compile time checking is good)

* this is not code that is simple enough to just look at and understand 
immediately, but there are no comments anywhere. that needs to be fixed =) 
(this is a maintainability issue)

* positionHorizontally and positionVertically can also be const?

* are the xused/yused variables reversed in meaning? it seems to me that xused 
should be true if X > 0 when Xalign == Right? the logic is right in the code, 
but it read a bit "backwards" to me at first until i realized that that "xused" 
really meant "x isn't valid" which is a bit contradictory to the name of the 
var? =)

* code style issues (the most minor part of all this): every if/else needs 
{}s, even if they are one liners; X and Y should be x and y (first letter of 
vars not capitalized) while xused and yused should probably be xUsed and 
yUsed.

> Currently I've just thrown the code into containment.c,
> Containment::Private class; do you think there's a better place for it?

this code doesn't make any sense for Containments with managed layouts (e.g. 
the DefaultPanel plugin); i can also imagine that containments with their own 
additional contents (such as the folderview and it's icons) may want to 
infuence the "free space" concept.

really, this is for Containments that have no layout management. 

so what *might* work better is turn this into a QGraphicsLayout subclass that 
is included with libplasma and let, e.g., DesktopContainment create a top 
level layout. then DesktopContainment can, just as PanelContainment does, 
connect to the appletAdded signal and add the applet to the layout.

this would make it completely opt-in/out for containmets.

> Also, any ideas in how that could be integrated into the desktop? I mean
> the ability to choose the type of stacking. I was thinking about the
> "Desktop Settings" from Plasma's main context menu.

half of me cringes at this. let's try going with a sensible default to start 
with and see how that goes.

> And some option to
> reposition all applets on the desktop.

this is a requested feature on bugs.kde.org in the form of "allow alignment to 
grid". there are, however, a general set of re-alignment requests .. i almost 
wonder if a snap-to grid would be good enough though...

> Otherwise, there are currently some problems:
> - Some applets don't report their size immediately, causing them to be
> placed where there is no space for them (calendar, character selector)

proxy widgets ... these probably aren't really positionable until they are on 
the canvas. i wonder if it would work out ok visually to place the item on the 
canvas first and then position it (e.g. would we get a repaint immediately or 
only later, allowing the applet to be moved first without visual artifacts)

> - Applets are placed over the whole screen, ignoring panels - how can I
> get only the available screen? I use containment->geometry().

one can query for the availableGeometry() from QDesktopWidget for the screen 
associated with the containment, if any... but this is really not enough as it 
is a simple rect (meaning it doesn't take panel-is-on-a-different-screen or 
panel-only-takes-a-port-of-the-screen into consideration).

we probably really need to eventually provide some sort of "collidesWithPanel" 
functionality inside of plasma itself as that is the only way to provide high-
fidelity results given what X gives us otherwise.

> - There is no spacing between applets

shouldn't be hard to fix =)

anyways, this is a rather nice start to getting a solution to this issue...

-- 
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/plasma-devel/attachments/20080801/e817b5ba/attachment-0001.pgp