mart added a comment.
In https://phabricator.kde.org/D2218#41550, @davidedmundson wrote: > 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. I don't think I can drop idsForConnector, because (as connectorsForId) it contains associations also of screens that it remembers but may be disabled right now (it saves and loads to/from configuration file) i may be able to kill idsfordesktopview tough, will update the rr > 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) views gets switched in screens only in the case the primary screen changes, that's the one moment you want to see panels and desktops moving to a different screen do you think if panels would be completely managed in screenpool as well logic would be more readable? > 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 what we know there is that a desktopview must be removed due to a screen death, so the panels with that screen should be killed as well. perhaps would be more clear if the logic for that is completely moved in screenpool? > bugs: > I think you also have a potential crash if you switch primary on a panel and then disconnect the first screen hmm, i tried to brutally disconnect the primary screen, but doesn't seem to crash? > This code will have a bug if you drag a panelview to another screen - then resize the first screen. why? when dragging to another screen screentofollow of the panel would be changed as well. > 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. yes, there should be a new containment as the one of screen a should stay for when/if screen a is connected again (should be fine with latest libplasma) 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
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
