Review Request: properly remove m_size var from panel code [now with geometryChanged signal]

[Chani Armitage]

> On 2008-02-29 09:42:07, Aaron Seigo wrote:
> > /trunk/KDE/kdebase/workspace/plasma/containments/panel/panel.cpp, lines 442-443
> > <http://mattr.info/r/219/diff/2/#file692line442>
> >
> >     that's a bit of an odd comment since the first thing setGeometry does is call setPos.
> >     
> >     in what way was it confusing the view?
> >     
> >     was the size() changing as well in this process?
> 
> Chani Armitage wrote:
>     by "confuses" I mean "the view doesn't notice anything has changed". no, size() was not changing.
>     /me looks at the setGeometry code...
>     oh, it must be that update() down at the bottom. maybe I just need to call that, really, instead of all this updatePos insanity. er, but I have a feeling I'm not really supposed to call update() from constraintsUpdated() - why do I have that feeling?
> 
> Chani Armitage wrote:
>     no, update() doesn't work after all. this means that something *else* is magically alerting the view. which makes no sense, because there should not be any other code triggered. I'm too tired to make sense of it tonight.
> 
> Chani Armitage wrote:
>     connect(panel, SIGNAL(geometryChanged()), this, SLOT(updatePanelGeometry()));
>     ^^ that's the culprit, there. the view is only reacting to geometry changes. I'm not sure which way I should deal with this.
>     
>     I could put back the setPos code that tries to position a panel containment on the corona based on where it should be on the screen, even though its position is ignored by the view. this could turn out to be useful for floating panels someday, and all those almost-pointless position changes for non-floating panels do change the geometry and trigger that signal.
>     on the other hand, I could take out the switch statement in setPos and just... cheat and emit geometryChanged() even though it hasn't really? this would get rid of the extra code for now, but if we introduce floating panels we might have to bring it back - and we'd also have to worry more about preventing multiple panels from overlapping on the corona.
>     
>     which solution sounds more sensible? I want to do #2 but I'm beginning to think #1 will be better long-term.

oh, hmm. I also have to consider the effect this would have on non-fullwidth panels and calculating whether to draw side borders... but... that code's already broken in other ways. :P


- Chani


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://mattr.info/r/219/#review216
-----------------------------------------------------------


On 2008-02-29 23:01:41, Chani Armitage wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://mattr.info/r/219/
> -----------------------------------------------------------
> 
> (Updated 2008-02-29 23:01:41)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> this completely removes m_size. I introduced a few functions to keep things clear. there is a small change in behaviour - the config dialog now shows the full size of the panel, including the borders.
> this might horribly break with multiple screens, I'm not sure. please test.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/workspace/plasma/containments/panel/panel.h
>   /trunk/KDE/kdebase/workspace/plasma/containments/panel/panel.cpp
> 
> Diff: http://mattr.info/r/219/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chani
> 
>