> On Nov. 25, 2014, 11:56 a.m., Aleix Pol Gonzalez wrote: > > shell/shellcorona.cpp, line 328 > > <https://git.reviewboard.kde.org/r/121240/diff/1/?file=329712#file329712line328> > > > > Are you sure this is correct? > > > > This was done because at some point Configuration::primaryOutput and > > Output::isPrimary were not consistent and this was a way to make it > > consistent. > > Daniel Vrátil wrote: > The behaviour has changed a little with the new API (mostly because the > way Outputs are updated have changed). Now Configuration::primaryOutput is > changed after all Outputs have been updated. That's why this patch also > switches from listening to Output::isPrimaryChanged to > Configuration::primaryOutputChanged. > > The problem with reacting to Output::isPrimaryChanged is, that you will > get the signal always twice: once for the output that is set to be primary, > and once for the output where primary flag is unset. If the order is first > unset, then set, then everything is OK, but if the order happens to be that > first you get signal from the output that was set a primary and then from the > one that was unset from primary, stuff gets broken and you start hitting > various asserts in the codepath. > > By reacting to Config::primaryOutputChanged, you are sure that all the > changes have already been applied (including primary), and that calling > Config::primaryOutput gives you what you expect.
Ok, merge the patch, I'll keep an eye for regressions. - Aleix ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121240/#review70911 ----------------------------------------------------------- On Nov. 25, 2014, 9:18 a.m., Daniel Vrátil wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/121240/ > ----------------------------------------------------------- > > (Updated Nov. 25, 2014, 9:18 a.m.) > > > Review request for Plasma. > > > Repository: plasma-workspace > > > Description > ------- > > This patch ports ShellCorona and PanelView to new KScreen API. The new API is > completely asynchronous and is using shared pointers. The internals have also > undergone some major changes, but they don't directly affect Plasma. > > Additionally to the port, this patch also changes the way ShellCorona reacts > to primary screen changes: instead of listening to Output::isPrimaryChanged > on each output, it listens now to Config::primaryOutputChanged. The reason is > that when some output is set as primary, the signal is emitted right away. > This can happen before the old primary is unset though, which then causes > crashes in screenInvariants() in some situations/configurations. Listening to > Config::primaryOutputChanges ensures that Plasma reacts only once, and only > when the Config is consistent. > > The new KScreen API is available in dev/redesign branches in libkscreen.git. > I'll merge the branch to "frameworks" branch once this review is approved in > order not to break build. > > > Diffs > ----- > > shell/panelview.cpp 0dc5740 > shell/shellcorona.h 5e97e02 > shell/shellcorona.cpp 0da789f > > Diff: https://git.reviewboard.kde.org/r/121240/diff/ > > > Testing > ------- > > Been using this patch and the new KScreen for couple weeks now, works better > than the old one. > > > Thanks, > > Daniel Vrátil > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
