-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126961/#review91946
-----------------------------------------------------------




shell/shellcorona.cpp (line 629)
<https://git.reviewboard.kde.org/r/126961/#comment62733>

    m_screenConfiguration may still be a nullptr here, since it's set only 
after GetConfigOperation returns.
    
    That may well be the reason why the sync QScreen's count is used here, but 
this leads to problems down the road, as you note.
    
    We could make sure we have the number of screens returned here by making 
GetConfigOperation sync, so using its exec() and thus blocking in the setShell 
call.


- Sebastian Kügler


On Feb. 1, 2016, 11:08 p.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126961/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2016, 11:08 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> We were mixing KScreen and QScreen API badly.
> 
> Corona.cpp checks we are requesting a containment for a valid screen
> if (screen >= 0 && screen < numScreens()) {
> 
> This fails as numScreens() is Qt API based, whereas the signal we're
> adding the output for is ShellCorona::addOutput so we have an effective race 
> condition.
> 
> BUG: 351777
> 
> 
> Diffs
> -----
> 
>   shell/shellcorona.cpp 762e503bf59fe648fb0f5b76a36229aa43c563e5 
> 
> Diff: https://git.reviewboard.kde.org/r/126961/diff/
> 
> 
> Testing
> -------
> 
> Started Plasma on dual screen.
> 
> Ideally we need to do more testing before backporting, as that entire 
> codebase is a disgrace.
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to