Re: Review Request 121113: Use Qt's method of blocking for component completion rather than our own
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121113/ --- (Updated Nov. 19, 2014, 10:39 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Plasma. Repository: kdeclarative Description --- We had a while loop which processed all application events whilst we were in the middle of creating objects. This leads to weird bugs, and workarounds in ShellCorona. Qt's methods forceCompletion does not seem to have the same problems and works just as well. Diffs - src/kdeclarative/qmlobject.cpp 029edaf Diff: https://git.reviewboard.kde.org/r/121113/diff/ Testing --- Ran plasmashell with many panels filled with applets Added debug on void ShellCorona::createWaitingPanels() to make sure it was never called with m_loading true. Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 121113: Use Qt's method of blocking for component completion rather than our own
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121113/#review70598 --- Ship it! Ship It! - Marco Martin On Nov. 13, 2014, 6:24 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121113/ --- (Updated Nov. 13, 2014, 6:24 p.m.) Review request for KDE Frameworks and Plasma. Repository: kdeclarative Description --- We had a while loop which processed all application events whilst we were in the middle of creating objects. This leads to weird bugs, and workarounds in ShellCorona. Qt's methods forceCompletion does not seem to have the same problems and works just as well. Diffs - src/kdeclarative/qmlobject.cpp 029edaf Diff: https://git.reviewboard.kde.org/r/121113/diff/ Testing --- Ran plasmashell with many panels filled with applets Added debug on void ShellCorona::createWaitingPanels() to make sure it was never called with m_loading true. Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 121113: Use Qt's method of blocking for component completion rather than our own
On Nov. 14, 2014, 12:16 p.m., Marco Martin wrote: hm, i don't really like it. is it working around a problem in particular? if i try the patch, the difference during startup (or just duringopening a popup on the first time) is pretty noticeable like, the wallpaper appearing several *seconds* later. This way, i think it's not using the incubator at all, and i don't think it's really acceptable. In QML itself, QQuickView does internally use an incubator as well, even tough in a slightly different manner it seems. Aleix Pol Gonzalez wrote: Well, since I started developing in Qt, I've been told that using nested event loops is bad parallelization. A good way to fix the WallpaperInterface issue (I understand it's an example, but still applies) is that instead of calling completeInitialization() (wallpaperinterface.cpp:147) we should be connect to a signal that notifies us about the background readiness (i.e. statusChanged) and then react to the initialization by connecting the object into the graphical elements, but forcing the end of the initialization is, of course, not parallelizable. eh, they shouldn't put it in the documentation as an example how to di ti ;) so ok, let's try to go for it - Marco --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121113/#review70364 --- On Nov. 13, 2014, 6:24 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121113/ --- (Updated Nov. 13, 2014, 6:24 p.m.) Review request for KDE Frameworks and Plasma. Repository: kdeclarative Description --- We had a while loop which processed all application events whilst we were in the middle of creating objects. This leads to weird bugs, and workarounds in ShellCorona. Qt's methods forceCompletion does not seem to have the same problems and works just as well. Diffs - src/kdeclarative/qmlobject.cpp 029edaf Diff: https://git.reviewboard.kde.org/r/121113/diff/ Testing --- Ran plasmashell with many panels filled with applets Added debug on void ShellCorona::createWaitingPanels() to make sure it was never called with m_loading true. Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 121113: Use Qt's method of blocking for component completion rather than our own
On Nov. 14, 2014, 12:16 p.m., Marco Martin wrote: hm, i don't really like it. is it working around a problem in particular? if i try the patch, the difference during startup (or just duringopening a popup on the first time) is pretty noticeable like, the wallpaper appearing several *seconds* later. This way, i think it's not using the incubator at all, and i don't think it's really acceptable. In QML itself, QQuickView does internally use an incubator as well, even tough in a slightly different manner it seems. Well, since I started developing in Qt, I've been told that using nested event loops is bad parallelization. A good way to fix the WallpaperInterface issue (I understand it's an example, but still applies) is that instead of calling completeInitialization() (wallpaperinterface.cpp:147) we should be connect to a signal that notifies us about the background readiness (i.e. statusChanged) and then react to the initialization by connecting the object into the graphical elements, but forcing the end of the initialization is, of course, not parallelizable. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121113/#review70364 --- On Nov. 13, 2014, 6:24 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121113/ --- (Updated Nov. 13, 2014, 6:24 p.m.) Review request for KDE Frameworks and Plasma. Repository: kdeclarative Description --- We had a while loop which processed all application events whilst we were in the middle of creating objects. This leads to weird bugs, and workarounds in ShellCorona. Qt's methods forceCompletion does not seem to have the same problems and works just as well. Diffs - src/kdeclarative/qmlobject.cpp 029edaf Diff: https://git.reviewboard.kde.org/r/121113/diff/ Testing --- Ran plasmashell with many panels filled with applets Added debug on void ShellCorona::createWaitingPanels() to make sure it was never called with m_loading true. Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 121113: Use Qt's method of blocking for component completion rather than our own
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121113/#review70364 --- hm, i don't really like it. is it working around a problem in particular? if i try the patch, the difference during startup (or just duringopening a popup on the first time) is pretty noticeable like, the wallpaper appearing several *seconds* later. This way, i think it's not using the incubator at all, and i don't think it's really acceptable. In QML itself, QQuickView does internally use an incubator as well, even tough in a slightly different manner it seems. - Marco Martin On Nov. 13, 2014, 6:24 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121113/ --- (Updated Nov. 13, 2014, 6:24 p.m.) Review request for KDE Frameworks and Plasma. Repository: kdeclarative Description --- We had a while loop which processed all application events whilst we were in the middle of creating objects. This leads to weird bugs, and workarounds in ShellCorona. Qt's methods forceCompletion does not seem to have the same problems and works just as well. Diffs - src/kdeclarative/qmlobject.cpp 029edaf Diff: https://git.reviewboard.kde.org/r/121113/diff/ Testing --- Ran plasmashell with many panels filled with applets Added debug on void ShellCorona::createWaitingPanels() to make sure it was never called with m_loading true. Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 121113: Use Qt's method of blocking for component completion rather than our own
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121113/#review70338 --- +1, having this behaviour was a pain in the ass. Also it won't be slower because we were not really parallelizing much, given that we were avoiding to create different containments at the same time. One thing to look into after this is whether we can somehow batch-incubate the plasmoids within a same containment. - Aleix Pol Gonzalez On Nov. 13, 2014, 6:24 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121113/ --- (Updated Nov. 13, 2014, 6:24 p.m.) Review request for KDE Frameworks and Plasma. Repository: kdeclarative Description --- We had a while loop which processed all application events whilst we were in the middle of creating objects. This leads to weird bugs, and workarounds in ShellCorona. Qt's methods forceCompletion does not seem to have the same problems and works just as well. Diffs - src/kdeclarative/qmlobject.cpp 029edaf Diff: https://git.reviewboard.kde.org/r/121113/diff/ Testing --- Ran plasmashell with many panels filled with applets Added debug on void ShellCorona::createWaitingPanels() to make sure it was never called with m_loading true. Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 121113: Use Qt's method of blocking for component completion rather than our own
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121113/#review70354 --- +1 If you have to be blocking, use the proper blocking methods. Obviosuly it'd be better if no blocking call is used but since i know nothing about the code i'll just shut up now :D - Albert Astals Cid On nov. 13, 2014, 6:24 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121113/ --- (Updated nov. 13, 2014, 6:24 p.m.) Review request for KDE Frameworks and Plasma. Repository: kdeclarative Description --- We had a while loop which processed all application events whilst we were in the middle of creating objects. This leads to weird bugs, and workarounds in ShellCorona. Qt's methods forceCompletion does not seem to have the same problems and works just as well. Diffs - src/kdeclarative/qmlobject.cpp 029edaf Diff: https://git.reviewboard.kde.org/r/121113/diff/ Testing --- Ran plasmashell with many panels filled with applets Added debug on void ShellCorona::createWaitingPanels() to make sure it was never called with m_loading true. Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 121113: Use Qt's method of blocking for component completion rather than our own
On Nov. 13, 2014, 11:44 p.m., Albert Astals Cid wrote: +1 If you have to be blocking, use the proper blocking methods. Obviosuly it'd be better if no blocking call is used but since i know nothing about the code i'll just shut up now :D We've been going through the code with david before, and there's some things we could do to make it more asynchronous. We agreed on having this as a first step. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121113/#review70354 --- On Nov. 13, 2014, 6:24 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121113/ --- (Updated Nov. 13, 2014, 6:24 p.m.) Review request for KDE Frameworks and Plasma. Repository: kdeclarative Description --- We had a while loop which processed all application events whilst we were in the middle of creating objects. This leads to weird bugs, and workarounds in ShellCorona. Qt's methods forceCompletion does not seem to have the same problems and works just as well. Diffs - src/kdeclarative/qmlobject.cpp 029edaf Diff: https://git.reviewboard.kde.org/r/121113/diff/ Testing --- Ran plasmashell with many panels filled with applets Added debug on void ShellCorona::createWaitingPanels() to make sure it was never called with m_loading true. Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
