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

davidedmundson (David Edmundson) noreply at phabricator.kde.org
Wed Jul 20 16:26:40 UTC 2016


davidedmundson added a comment.


  When adding a new design can you describe the design in words. i.e what roles a class has
  It'd make things a lot easier.
  
  ScreenPool: 
  It seems to have two jobs:
  
  - you've got the converting screen "names" to IDs.
  - "Tracking" DesktopViews
  
  Why do we have m_idsForConnector and m_idsForDesktopView
  
  m_idsForDesktopView  is bound to be the same as 
  m_idsForConnector(view->screenToFollow()->name())   right?
  
  at which point we can kill the second one, and that simplifies a lot of the code and avoid something that can go out of sync.
  
  Panels are now a bit of a mix:
  
  In createWaitingPanels we base the panel screen based on where the DesktopView is
  in primaryChanged - we don't and use screens API directly (BUG: and setting screenToFollow)
  in removeView we're using the View as the indicator of what screen is which.
  screenForContainment is also going via View.
  
  Is it meant to be going via a View or not?
  
  Personally I think it's a bit weird as we can generate the index directly from the screen, but whatever it should be it should be consistent
  
  bugs:
  I think you also have a potential crash if you switch primary on a panel and then disconnect the first screen
  
  This code will have a bug if you drag a panelview to another screen - then resize the first screen.
  Same for if you have a panel on a primary screen then switch primary then alter the screen.
  
  You have a crash if:
  
  - you have previously inserted a containment on screen A
  - you reboot with screen B, C
  - number of screens ==2, firstAvailableIndex=1
  - the first containment will restore
  - the second one will be a new containment  with an ID of 3
  - createContainment checks number of ScreensAvailable and will return a null pointer.

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/20160720/55b0e3be/attachment.html>


More information about the Plasma-devel mailing list