Smarter desktop applet placement

[Ambroz Bizjak]

Aaron J. Seigo wrote:
> * 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)
Good, changed it.

> * 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)
I've added some comments, but it may not be enough to understand when
first reading. It would definitely help if you try to draw the process of
looking for obstacles on a piece of paper.
Also, the second function (vertical positioning) is not commented so
thoroughly, as it's the same algorithm with just some things swapped.

> * positionHorizontally and positionVertically can also be const?
Changed it.

> * 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?
Yes, xused actually meant "out of X place at this Y".
I changed the names. Also I checked when it runs out of the other coordinate
so it won't place applets outside of the screen if there is no place.

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

> 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.
Since I don't have a very good understanding of the architecture, I added
the code specifically to the desktop containment and created a
reimplementable function in Containment to position the applet. I think
that would be fine, as the positioning is specific to the desktop.

>> 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.
So the defaults are hardcoded for now.

> I almost
> wonder if a snap-to grid would be good enough though...
Really, considering the varying size of applets?

> 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)
Yes good idea, but that will come a little later.

> one can query for the availableGeometry() from QDesktopWidget for the
> screen
> associated with the containment, if any...
Good enough for now, but what do I call this method on? It doesn't seem to
be a part of Containment.

>> - There is no spacing between applets
> shouldn't be hard to fix =)
Already done :) default 20px on all sides of an applet

New version of my work is attached.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: plasma-placement2.patch
Type: application/octet-stream
Size: 16049 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/plasma-devel/attachments/20080802/cb8b343b/attachment-0001.obj