Review Request: Fix layout problems in the pager applet

Anthony Bryant antjbryant at gmail.com
Sat Jul 25 23:40:41 CEST 2009


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1124/
-----------------------------------------------------------

(Updated 2009-07-25 21:40:41.819952)


Review request for Plasma.


Changes
-------

Tried implementing Aaron's suggestion to not call anything that could cause a resize if m_ignoreNextSizeConstraint.
Unfortunately, when changing a panel between vertical and horizontal it resulted in the applet taking up excessive widths/heights because of the swap between rows and columns that happens during the form factor constraints event.
Another solution that seems to work quite well is to always set the new preferred size if the height (or in vertical panels: width) has changed, so that has been implemented in this version of the patch.


Summary
-------

This patch fixes a few problems with the pager that are especially apparent in vertical panels. Specifically, the rows and columns are unnecessarily swapped in recalculateGeometry() which causes bug 200013.
Also, due to the way the rectangle sizes were calculated it was possible for certain areas to be clipped. This patch makes sure every item can fit into the current space.
This also fixes a bug I only noticed when reading the code, where extra unnecessary columns could be added if desktopCount % rows > 1

My main concern about this patch is that it removes a mechanism for ignoring some constraints events, I think these events are needed in order to avoid clipping some of the virtual desktops, but I'm not sure how to reproduce the bug that made this filtering necessary in the first place.


This addresses bug 200013.
    https://bugs.kde.org/show_bug.cgi?id=200013


Diffs (updated)
-----

  /trunk/KDE/kdebase/workspace/plasma/applets/pager/pager.h 1002081 
  /trunk/KDE/kdebase/workspace/plasma/applets/pager/pager.cpp 1002081 

Diff: http://reviewboard.kde.org/r/1124/diff


Testing
-------

Tested in vertical and horizontal panels and on the desktop.


Thanks,

Anthony



More information about the Plasma-devel mailing list