[Differential] [Commented On] D2218: New logic for screen numbers in plasmashell

[davidedmundson (David Edmundson)]

davidedmundson added a comment.


  > i may be able to kill idsfordesktopview tough, will update the rr
  
  Yep, that's exactly what I meant. ++
  Also means panels can use the API directly. (see comment #3)
  
  > perhaps would be more clear if the logic for that is completely moved in screenpool?
  
  There should definitely be some consistency. Either screenpool is in charge of managing views or shellcorona is.
  
  Either woudl be valid  but currently this patch current inserts and removals of DesktopView to the right screen are all handled by ShellCorona setting the screen, but primary changes of DesktopView are done by ScreenPool; and Panels are mixed up too.
  
  Personally, I think the part that's out of place is ScreenPool touching the desktop views; if you move m_desktopViewforId to shellCorona the design all fits:
  insert/remove in ScreenPool become reflective
  and all DesktopView stuff is within one class, screen to ID mapping is in one class.

INLINE COMMENTS

> panelview.cpp:647
>  {
> +    //it's important the panel is positioned immediately in order to not have the
> +    //qscreen changed under his feet

well, this is now completely not true as we're following screenToFollow

> shellcorona.cpp:1421
>      if (view) {
>          QScreen *screen = view->screen();
> +        foreach (int id, m_screenPool->ids()) {

surely this should be screenToFollow()?

> shellcorona.cpp:1422
>          QScreen *screen = view->screen();
> -        for (int i = 0; i < m_views.count(); i++) {
> -            if (m_views[i]->screen() == screen) {
> -                return i;
> +        foreach (int id, m_screenPool->ids()) {
> +            if (m_screenPool->view(id)->screen() == screen) {

this is silly.

We're looping through the desktops to find one with the same screen as us - then looking up the name of that.

We have the screen, we can use m_screenPool->id(screen-name());

REPOSITORY
  rPLASMAWORKSPACE Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D2218

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: mart, #plasma
Cc: davidedmundson, rwooninck, graesslin, plasma-devel, jensreuterberg, abetts, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160722/56112183/attachment.html>