Re: Review Request 123836: Ensure KNotification can be used from a non-GUI thread
On May 18, 2015, 4:15 p.m., Aleix Pol Gonzalez wrote: This fixes my boot. I wasn't able to boot for the whole day because it was showing a QWidget from the wrong thread. On a related note, let's port away from QDesktopWidget, it has these things... Ok, QScreen then? - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123836/#review80575 --- On May 18, 2015, 2:13 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123836/ --- (Updated May 18, 2015, 2:13 p.m.) Review request for KDE Frameworks and Plasma. Repository: knotifications Description --- The NotifyByPopup plugin is accessing QApplication::desktop() in constructor which Qt does not like when that happens on non-GUI thread and asserts. So this moves that code only when actually needed plus it checks first if the code is run in non-GUI thread and does nothing if it's not. This is only for the popup fallback notifications, normal notifications work just fine. Diffs - src/notifybypopup.cpp 2f84ab0 Diff: https://git.reviewboard.kde.org/r/123836/diff/ Testing --- Tested with both multi-threaded and single-threaded codes. Thanks, Martin Klapetek ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 123836: Ensure KNotification can be used from a non-GUI thread
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123836/ --- (Updated May 18, 2015, 3:34 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Plasma. Changes --- Submitted with commit bbf19c13ee61fe0f09263d2fdd40207a71dca6fd by Martin Klapetek to branch master. Repository: knotifications Description --- The NotifyByPopup plugin is accessing QApplication::desktop() in constructor which Qt does not like when that happens on non-GUI thread and asserts. So this moves that code only when actually needed plus it checks first if the code is run in non-GUI thread and does nothing if it's not. This is only for the popup fallback notifications, normal notifications work just fine. Diffs - src/notifybypopup.cpp 2f84ab0 Diff: https://git.reviewboard.kde.org/r/123836/diff/ Testing --- Tested with both multi-threaded and single-threaded codes. Thanks, Martin Klapetek ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 123836: Ensure KNotification can be used from a non-GUI thread
On May 18, 2015, 4:15 p.m., Aleix Pol Gonzalez wrote: This fixes my boot. I wasn't able to boot for the whole day because it was showing a QWidget from the wrong thread. On a related note, let's port away from QDesktopWidget, it has these things... Martin Klapetek wrote: Ok, QScreen then? That would work. You can do notification-widget()-screen()-geometry()-top(). - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123836/#review80575 --- On May 18, 2015, 2:13 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123836/ --- (Updated May 18, 2015, 2:13 p.m.) Review request for KDE Frameworks and Plasma. Repository: knotifications Description --- The NotifyByPopup plugin is accessing QApplication::desktop() in constructor which Qt does not like when that happens on non-GUI thread and asserts. So this moves that code only when actually needed plus it checks first if the code is run in non-GUI thread and does nothing if it's not. This is only for the popup fallback notifications, normal notifications work just fine. Diffs - src/notifybypopup.cpp 2f84ab0 Diff: https://git.reviewboard.kde.org/r/123836/diff/ Testing --- Tested with both multi-threaded and single-threaded codes. Thanks, Martin Klapetek ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 123836: Ensure KNotification can be used from a non-GUI thread
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123836/#review80575 --- Ship it! This fixes my boot. I wasn't able to boot for the whole day because it was showing a QWidget from the wrong thread. On a related note, let's port away from QDesktopWidget, it has these things... - Aleix Pol Gonzalez On May 18, 2015, 2:13 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123836/ --- (Updated May 18, 2015, 2:13 p.m.) Review request for KDE Frameworks and Plasma. Repository: knotifications Description --- The NotifyByPopup plugin is accessing QApplication::desktop() in constructor which Qt does not like when that happens on non-GUI thread and asserts. So this moves that code only when actually needed plus it checks first if the code is run in non-GUI thread and does nothing if it's not. This is only for the popup fallback notifications, normal notifications work just fine. Diffs - src/notifybypopup.cpp 2f84ab0 Diff: https://git.reviewboard.kde.org/r/123836/diff/ Testing --- Tested with both multi-threaded and single-threaded codes. Thanks, Martin Klapetek ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request 123836: Ensure KNotification can be used from a non-GUI thread
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123836/ --- Review request for KDE Frameworks and Plasma. Repository: knotifications Description --- The NotifyByPopup plugin is accessing QApplication::desktop() in constructor which Qt does not like when that happens on non-GUI thread and asserts. So this moves that code only when actually needed plus it checks first if the code is run in non-GUI thread and does nothing if it's not. This is only for the popup fallback notifications, normal notifications work just fine. Diffs - src/notifybypopup.cpp 2f84ab0 Diff: https://git.reviewboard.kde.org/r/123836/diff/ Testing --- Tested with both multi-threaded and single-threaded codes. Thanks, Martin Klapetek ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 123836: Ensure KNotification can be used from a non-GUI thread
On May 18, 2015, 4:15 p.m., Aleix Pol Gonzalez wrote: This fixes my boot. I wasn't able to boot for the whole day because it was showing a QWidget from the wrong thread. On a related note, let's port away from QDesktopWidget, it has these things... Martin Klapetek wrote: Ok, QScreen then? Aleix Pol Gonzalez wrote: That would work. You can do notification-widget()-screen()-geometry()-top(). That wouldn't work because the widget() does not need to be set. So I'll use QScreen directly. - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123836/#review80575 --- On May 18, 2015, 5:34 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123836/ --- (Updated May 18, 2015, 5:34 p.m.) Review request for KDE Frameworks and Plasma. Repository: knotifications Description --- The NotifyByPopup plugin is accessing QApplication::desktop() in constructor which Qt does not like when that happens on non-GUI thread and asserts. So this moves that code only when actually needed plus it checks first if the code is run in non-GUI thread and does nothing if it's not. This is only for the popup fallback notifications, normal notifications work just fine. Diffs - src/notifybypopup.cpp 2f84ab0 Diff: https://git.reviewboard.kde.org/r/123836/diff/ Testing --- Tested with both multi-threaded and single-threaded codes. Thanks, Martin Klapetek ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
