Re: Review Request: Fix KCategorizedView race
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/#review8815 --- It works for me. You have my ship it. - Jaime Torres Amate On Dec. 8, 2011, 6:04 p.m., Thomas Lübking wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/ --- (Updated Dec. 8, 2011, 6:04 p.m.) Review request for kdelibs, Rafael Fernández López and Jaime Torres Amate. Description --- Also see https://git.reviewboard.kde.org/r/103313/ QListView::updateGeometries() has it's own opinion on whether the scrollbars should be visible (valid range) or not and triggers a (sometimes additionally timered) resize through ::layoutChildren() http://qt.gitorious.org/qt/qt/blobs/4.7/src/gui/itemviews/qlistview.cpp#line1499 (the comment above the main block isn't all accurate, layoutChldren is called regardless of the policy) As a result QListView and KCategorizedView occasionally started a race on the scrollbar visibility, effectively blocking the UI So we prevent QListView from having an own opinion on the scrollbar visibility by fixing it before calling the baseclass QListView::updateGeometries() and restoring the policy afterwards This addresses bugs 213068 and 287847. http://bugs.kde.org/show_bug.cgi?id=213068 http://bugs.kde.org/show_bug.cgi?id=287847 Diffs - kdeui/itemviews/kcategorizedview.cpp 5e33861 Diff: http://git.reviewboard.kde.org/r/103335/diff/diff Testing --- Yes, resizing the kcmshell4 kwincompositing, all effects KPluginSelector with large scrollbar sizes (bespin/position indicator/32px; oxygen was often sufficient with the default size) after commit e91e5fed6b1aad365e12e919f430c3e8147552d3 (see https://git.reviewboard.kde.org/r/103165/ ) was a reliable way to trigger the issue for me. It also showed that resize/updateGeometries occured in pairs (ie 4calls forming a block), what's never happened again with the patch. Thanks, Thomas Lübking
Re: Review Request: Fix KCategorizedView race
On Dec. 9, 2011, 8:46 a.m., Jaime Torres Amate wrote: It works for me. You have my ship it. If nobody else has a comment and since it's confirmed by Jaime and myself to be better than the current state, i'll push this 21:00 CET - so shout HOLD right now ;-) - Thomas --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/#review8815 --- On Dec. 8, 2011, 6:04 p.m., Thomas Lübking wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/ --- (Updated Dec. 8, 2011, 6:04 p.m.) Review request for kdelibs, Rafael Fernández López and Jaime Torres Amate. Description --- Also see https://git.reviewboard.kde.org/r/103313/ QListView::updateGeometries() has it's own opinion on whether the scrollbars should be visible (valid range) or not and triggers a (sometimes additionally timered) resize through ::layoutChildren() http://qt.gitorious.org/qt/qt/blobs/4.7/src/gui/itemviews/qlistview.cpp#line1499 (the comment above the main block isn't all accurate, layoutChldren is called regardless of the policy) As a result QListView and KCategorizedView occasionally started a race on the scrollbar visibility, effectively blocking the UI So we prevent QListView from having an own opinion on the scrollbar visibility by fixing it before calling the baseclass QListView::updateGeometries() and restoring the policy afterwards This addresses bugs 213068 and 287847. http://bugs.kde.org/show_bug.cgi?id=213068 http://bugs.kde.org/show_bug.cgi?id=287847 Diffs - kdeui/itemviews/kcategorizedview.cpp 5e33861 Diff: http://git.reviewboard.kde.org/r/103335/diff/diff Testing --- Yes, resizing the kcmshell4 kwincompositing, all effects KPluginSelector with large scrollbar sizes (bespin/position indicator/32px; oxygen was often sufficient with the default size) after commit e91e5fed6b1aad365e12e919f430c3e8147552d3 (see https://git.reviewboard.kde.org/r/103165/ ) was a reliable way to trigger the issue for me. It also showed that resize/updateGeometries occured in pairs (ie 4calls forming a block), what's never happened again with the patch. Thanks, Thomas Lübking
Re: Review Request: Fix KCategorizedView race
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/#review8834 --- This review has been submitted with commit 8095c11d181e967cb161de3452f45c1df81bbc66 by Thomas Lübking to branch KDE/4.7. - Commit Hook On Dec. 8, 2011, 6:04 p.m., Thomas Lübking wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/ --- (Updated Dec. 8, 2011, 6:04 p.m.) Review request for kdelibs, Rafael Fernández López and Jaime Torres Amate. Description --- Also see https://git.reviewboard.kde.org/r/103313/ QListView::updateGeometries() has it's own opinion on whether the scrollbars should be visible (valid range) or not and triggers a (sometimes additionally timered) resize through ::layoutChildren() http://qt.gitorious.org/qt/qt/blobs/4.7/src/gui/itemviews/qlistview.cpp#line1499 (the comment above the main block isn't all accurate, layoutChldren is called regardless of the policy) As a result QListView and KCategorizedView occasionally started a race on the scrollbar visibility, effectively blocking the UI So we prevent QListView from having an own opinion on the scrollbar visibility by fixing it before calling the baseclass QListView::updateGeometries() and restoring the policy afterwards This addresses bugs 213068 and 287847. http://bugs.kde.org/show_bug.cgi?id=213068 http://bugs.kde.org/show_bug.cgi?id=287847 Diffs - kdeui/itemviews/kcategorizedview.cpp 5e33861 Diff: http://git.reviewboard.kde.org/r/103335/diff/diff Testing --- Yes, resizing the kcmshell4 kwincompositing, all effects KPluginSelector with large scrollbar sizes (bespin/position indicator/32px; oxygen was often sufficient with the default size) after commit e91e5fed6b1aad365e12e919f430c3e8147552d3 (see https://git.reviewboard.kde.org/r/103165/ ) was a reliable way to trigger the issue for me. It also showed that resize/updateGeometries occured in pairs (ie 4calls forming a block), what's never happened again with the patch. Thanks, Thomas Lübking
Re: Review Request: Fix KCategorizedView race
On Dec. 6, 2011, 6:39 p.m., Commit Hook wrote: This review has been submitted with commit 08325ba32b72326030004cc28430431193d82fc2 by Thomas Lübking to branch KDE/4.7. This commit causes systemsettings to go into endless recursion, see http://privatepaste.com/0e43ad214c zemo on #plasma confirmed that reverting it fixes the regression. - Christoph --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/#review8765 --- On Dec. 5, 2011, 2:21 p.m., Thomas Lübking wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/ --- (Updated Dec. 5, 2011, 2:21 p.m.) Review request for kdelibs, Rafael Fernández López and Jaime Torres Amate. Description --- Also see https://git.reviewboard.kde.org/r/103313/ QListView::updateGeometries() has it's own opinion on whether the scrollbars should be visible (valid range) or not and triggers a (sometimes additionally timered) resize through ::layoutChildren() http://qt.gitorious.org/qt/qt/blobs/4.7/src/gui/itemviews/qlistview.cpp#line1499 (the comment above the main block isn't all accurate, layoutChldren is called regardless of the policy) As a result QListView and KCategorizedView occasionally started a race on the scrollbar visibility, effectively blocking the UI So we prevent QListView from having an own opinion on the scrollbar visibility by fixing it before calling the baseclass QListView::updateGeometries() and restoring the policy afterwards This addresses bugs 213068 and 287847. http://bugs.kde.org/show_bug.cgi?id=213068 http://bugs.kde.org/show_bug.cgi?id=287847 Diffs - kdeui/itemviews/kcategorizedview.cpp 46a1cde kutils/kpluginselector.cpp ca0691d Diff: http://git.reviewboard.kde.org/r/103335/diff/diff Testing --- Yes, resizing the kcmshell4 kwincompositing, all effects KPluginSelector with large scrollbar sizes (bespin/position indicator/32px; oxygen was often sufficient with the default size) after commit e91e5fed6b1aad365e12e919f430c3e8147552d3 (see https://git.reviewboard.kde.org/r/103165/ ) was a reliable way to trigger the issue for me. It also showed that resize/updateGeometries occured in pairs (ie 4calls forming a block), what's never happened again with the patch. Thanks, Thomas Lübking
Re: Review Request: Fix KCategorizedView race
The new endless loop (causing a stack overflow here when I resize systemsettings view) this time is in: QListView::resizeEvent(QResizeEvent *e) that calls QAbstractItemView::resizeEvent(e); QAbstractItemView::resizeEvent(QResizeEvent *event) that calls KCategorizedView::updateGeometries(); QFrame::setFrameRect(QRect const) that calls QWidget::setContextsMargins(cr.left(), cr.top(), rect().right() - cr.right(), rect().bottom() - cr.bottom()); QWidget::setContestsMargin(int,int,int,int) that calls QCoreApplication::notifyInternal(receiver, event) QAbstractScrollArea::setHorizontalScrollBarPolicy(Qt::ScrollBarPolicy policy) that calls QAbstractScrollAreaPrivate::layoutChildren() This time the infinite recursion is even harder (two squares with 8 squares inside each one). In my machine, the infinite recursions seen until now (krunner and systemsettings) are gone with the attached patch. (tested resizing them all I can). Best Regards. 2011/12/8 Christoph Feck christ...@maxiom.de This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/ On December 6th, 2011, 6:39 p.m., *Commit Hook* wrote: This review has been submitted with commit 08325ba32b72326030004cc28430431193d82fc2 by Thomas Lübking to branch KDE/4.7. This commit causes systemsettings to go into endless recursion, see http://privatepaste.com/0e43ad214c zemo on #plasma confirmed that reverting it fixes the regression. - Christoph On December 5th, 2011, 2:21 p.m., Thomas Lübking wrote: Review request for kdelibs, Rafael Fernández López and Jaime Torres Amate. By Thomas Lübking. *Updated Dec. 5, 2011, 2:21 p.m.* Description Also see https://git.reviewboard.kde.org/r/103313/ QListView::updateGeometries() has it's own opinion on whether the scrollbars should be visible (valid range) or not and triggers a (sometimes additionally timered) resize through ::layoutChildren()http://qt.gitorious.org/qt/qt/blobs/4.7/src/gui/itemviews/qlistview.cpp#line1499 (the comment above the main block isn't all accurate, layoutChldren is called regardless of the policy) As a result QListView and KCategorizedView occasionally started a race on the scrollbar visibility, effectively blocking the UI So we prevent QListView from having an own opinion on the scrollbar visibility by fixing it before calling the baseclass QListView::updateGeometries() and restoring the policy afterwards Testing Yes, resizing the kcmshell4 kwincompositing, all effects KPluginSelector with large scrollbar sizes (bespin/position indicator/32px; oxygen was often sufficient with the default size) after commit e91e5fed6b1aad365e12e919f430c3e8147552d3 (see https://git.reviewboard.kde.org/r/103165/ ) was a reliable way to trigger the issue for me. It also showed that resize/updateGeometries occured in pairs (ie 4calls forming a block), what's never happened again with the patch. *Bugs: * 213068 http://bugs.kde.org/show_bug.cgi?id=213068, 287847http://bugs.kde.org/show_bug.cgi?id=287847 Diffs - kdeui/itemviews/kcategorizedview.cpp (46a1cde) - kutils/kpluginselector.cpp (ca0691d) View Diff http://git.reviewboard.kde.org/r/103335/diff/ infinite.diff Description: Binary data
Re: Review Request: Fix KCategorizedView race
On Dec. 6, 2011, 6:39 p.m., Commit Hook wrote: This review has been submitted with commit 08325ba32b72326030004cc28430431193d82fc2 by Thomas Lübking to branch KDE/4.7. Christoph Feck wrote: This commit causes systemsettings to go into endless recursion, see http://privatepaste.com/0e43ad214c zemo on #plasma confirmed that reverting it fixes the regression. *g* - gonna check tonight, have solution tomorrow. thomas angry. | - Thomas --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/#review8765 --- On Dec. 5, 2011, 2:21 p.m., Thomas Lübking wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/ --- (Updated Dec. 5, 2011, 2:21 p.m.) Review request for kdelibs, Rafael Fernández López and Jaime Torres Amate. Description --- Also see https://git.reviewboard.kde.org/r/103313/ QListView::updateGeometries() has it's own opinion on whether the scrollbars should be visible (valid range) or not and triggers a (sometimes additionally timered) resize through ::layoutChildren() http://qt.gitorious.org/qt/qt/blobs/4.7/src/gui/itemviews/qlistview.cpp#line1499 (the comment above the main block isn't all accurate, layoutChldren is called regardless of the policy) As a result QListView and KCategorizedView occasionally started a race on the scrollbar visibility, effectively blocking the UI So we prevent QListView from having an own opinion on the scrollbar visibility by fixing it before calling the baseclass QListView::updateGeometries() and restoring the policy afterwards This addresses bugs 213068 and 287847. http://bugs.kde.org/show_bug.cgi?id=213068 http://bugs.kde.org/show_bug.cgi?id=287847 Diffs - kdeui/itemviews/kcategorizedview.cpp 46a1cde kutils/kpluginselector.cpp ca0691d Diff: http://git.reviewboard.kde.org/r/103335/diff/diff Testing --- Yes, resizing the kcmshell4 kwincompositing, all effects KPluginSelector with large scrollbar sizes (bespin/position indicator/32px; oxygen was often sufficient with the default size) after commit e91e5fed6b1aad365e12e919f430c3e8147552d3 (see https://git.reviewboard.kde.org/r/103165/ ) was a reliable way to trigger the issue for me. It also showed that resize/updateGeometries occured in pairs (ie 4calls forming a block), what's never happened again with the patch. Thanks, Thomas Lübking
Re: Review Request: Fix KCategorizedView race
On Thu, Dec 8, 2011 at 2:33 AM, Thomas Lübking thomas.luebk...@web.dewrote: This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/ On December 6th, 2011, 6:39 p.m., *Commit Hook* wrote: This review has been submitted with commit 08325ba32b72326030004cc28430431193d82fc2 by Thomas Lübking to branch KDE/4.7. On December 8th, 2011, 2:40 a.m., *Christoph Feck* wrote: This commit causes systemsettings to go into endless recursion, see http://privatepaste.com/0e43ad214c zemo on #plasma confirmed that reverting it fixes the regression. *g* - gonna check tonight, have solution tomorrow. thomas angry. | What about jaime patch? - Thomas On December 5th, 2011, 2:21 p.m., Thomas Lübking wrote: Review request for kdelibs, Rafael Fernández López and Jaime Torres Amate. By Thomas Lübking. *Updated Dec. 5, 2011, 2:21 p.m.* Description Also see https://git.reviewboard.kde.org/r/103313/ QListView::updateGeometries() has it's own opinion on whether the scrollbars should be visible (valid range) or not and triggers a (sometimes additionally timered) resize through ::layoutChildren()http://qt.gitorious.org/qt/qt/blobs/4.7/src/gui/itemviews/qlistview.cpp#line1499 (the comment above the main block isn't all accurate, layoutChldren is called regardless of the policy) As a result QListView and KCategorizedView occasionally started a race on the scrollbar visibility, effectively blocking the UI So we prevent QListView from having an own opinion on the scrollbar visibility by fixing it before calling the baseclass QListView::updateGeometries() and restoring the policy afterwards Testing Yes, resizing the kcmshell4 kwincompositing, all effects KPluginSelector with large scrollbar sizes (bespin/position indicator/32px; oxygen was often sufficient with the default size) after commit e91e5fed6b1aad365e12e919f430c3e8147552d3 (see https://git.reviewboard.kde.org/r/103165/ ) was a reliable way to trigger the issue for me. It also showed that resize/updateGeometries occured in pairs (ie 4calls forming a block), what's never happened again with the patch. *Bugs: * 213068 http://bugs.kde.org/show_bug.cgi?id=213068, 287847http://bugs.kde.org/show_bug.cgi?id=287847 Diffs - kdeui/itemviews/kcategorizedview.cpp (46a1cde) - kutils/kpluginselector.cpp (ca0691d) View Diff http://git.reviewboard.kde.org/r/103335/diff/
Re: Review Request: Fix KCategorizedView race
Am Thu, 8 Dec 2011 07:59:43 -0800 schrieb Tomaz Canabrava tcanabr...@kde.org: What about jaime patch? yeah, static policies work - we know that ;-) This one is quite different (recursion, not race - though maybe both), from the first rough look there seem two recursions, one direct (easy to catch) and one through the viewport event (i recall that tricking them wasn't all easy) I *will* have a non static policy fix for this by tomorrow morning (and if that means to rewrite the entire class tonight, i'll just do that ;-) With a little luck of course i'll post a fix asap. Cheers, Thomas
Re: Review Request: Fix KCategorizedView race
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/#review8802 --- - Thomas Lübking On Dec. 8, 2011, 6:04 p.m., Thomas Lübking wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/ --- (Updated Dec. 8, 2011, 6:04 p.m.) Review request for kdelibs, Rafael Fernández López and Jaime Torres Amate. Description --- Also see https://git.reviewboard.kde.org/r/103313/ QListView::updateGeometries() has it's own opinion on whether the scrollbars should be visible (valid range) or not and triggers a (sometimes additionally timered) resize through ::layoutChildren() http://qt.gitorious.org/qt/qt/blobs/4.7/src/gui/itemviews/qlistview.cpp#line1499 (the comment above the main block isn't all accurate, layoutChldren is called regardless of the policy) As a result QListView and KCategorizedView occasionally started a race on the scrollbar visibility, effectively blocking the UI So we prevent QListView from having an own opinion on the scrollbar visibility by fixing it before calling the baseclass QListView::updateGeometries() and restoring the policy afterwards This addresses bugs 213068 and 287847. http://bugs.kde.org/show_bug.cgi?id=213068 http://bugs.kde.org/show_bug.cgi?id=287847 Diffs - kdeui/itemviews/kcategorizedview.cpp 5e33861 Diff: http://git.reviewboard.kde.org/r/103335/diff/diff Testing --- Yes, resizing the kcmshell4 kwincompositing, all effects KPluginSelector with large scrollbar sizes (bespin/position indicator/32px; oxygen was often sufficient with the default size) after commit e91e5fed6b1aad365e12e919f430c3e8147552d3 (see https://git.reviewboard.kde.org/r/103165/ ) was a reliable way to trigger the issue for me. It also showed that resize/updateGeometries occured in pairs (ie 4calls forming a block), what's never happened again with the patch. Thanks, Thomas Lübking
Re: Review Request: Fix KCategorizedView race
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/#review8803 --- kdeui/itemviews/kcategorizedview.cpp http://git.reviewboard.kde.org/r/103335/#comment7375 bug in reviewboard? the first comment meant to say: relict, gone in commit - Thomas Lübking On Dec. 8, 2011, 6:04 p.m., Thomas Lübking wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/ --- (Updated Dec. 8, 2011, 6:04 p.m.) Review request for kdelibs, Rafael Fernández López and Jaime Torres Amate. Description --- Also see https://git.reviewboard.kde.org/r/103313/ QListView::updateGeometries() has it's own opinion on whether the scrollbars should be visible (valid range) or not and triggers a (sometimes additionally timered) resize through ::layoutChildren() http://qt.gitorious.org/qt/qt/blobs/4.7/src/gui/itemviews/qlistview.cpp#line1499 (the comment above the main block isn't all accurate, layoutChldren is called regardless of the policy) As a result QListView and KCategorizedView occasionally started a race on the scrollbar visibility, effectively blocking the UI So we prevent QListView from having an own opinion on the scrollbar visibility by fixing it before calling the baseclass QListView::updateGeometries() and restoring the policy afterwards This addresses bugs 213068 and 287847. http://bugs.kde.org/show_bug.cgi?id=213068 http://bugs.kde.org/show_bug.cgi?id=287847 Diffs - kdeui/itemviews/kcategorizedview.cpp 5e33861 Diff: http://git.reviewboard.kde.org/r/103335/diff/diff Testing --- Yes, resizing the kcmshell4 kwincompositing, all effects KPluginSelector with large scrollbar sizes (bespin/position indicator/32px; oxygen was often sufficient with the default size) after commit e91e5fed6b1aad365e12e919f430c3e8147552d3 (see https://git.reviewboard.kde.org/r/103165/ ) was a reliable way to trigger the issue for me. It also showed that resize/updateGeometries occured in pairs (ie 4calls forming a block), what's never happened again with the patch. Thanks, Thomas Lübking
Re: Review Request: Fix KCategorizedView race
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/ --- (Updated Dec. 8, 2011, 6:04 p.m.) Review request for kdelibs, Rafael Fernández López and Jaime Torres Amate. Changes --- Suck this, you little nasty bug ) It's required to keep the policy down all the time and change the visibility according to it's original at the end. I was not able to reproduce issues with either systemsettings or plugins. Tests, anyone? Description --- Also see https://git.reviewboard.kde.org/r/103313/ QListView::updateGeometries() has it's own opinion on whether the scrollbars should be visible (valid range) or not and triggers a (sometimes additionally timered) resize through ::layoutChildren() http://qt.gitorious.org/qt/qt/blobs/4.7/src/gui/itemviews/qlistview.cpp#line1499 (the comment above the main block isn't all accurate, layoutChldren is called regardless of the policy) As a result QListView and KCategorizedView occasionally started a race on the scrollbar visibility, effectively blocking the UI So we prevent QListView from having an own opinion on the scrollbar visibility by fixing it before calling the baseclass QListView::updateGeometries() and restoring the policy afterwards This addresses bugs 213068 and 287847. http://bugs.kde.org/show_bug.cgi?id=213068 http://bugs.kde.org/show_bug.cgi?id=287847 Diffs (updated) - kdeui/itemviews/kcategorizedview.cpp 5e33861 Diff: http://git.reviewboard.kde.org/r/103335/diff/diff Testing --- Yes, resizing the kcmshell4 kwincompositing, all effects KPluginSelector with large scrollbar sizes (bespin/position indicator/32px; oxygen was often sufficient with the default size) after commit e91e5fed6b1aad365e12e919f430c3e8147552d3 (see https://git.reviewboard.kde.org/r/103165/ ) was a reliable way to trigger the issue for me. It also showed that resize/updateGeometries occured in pairs (ie 4calls forming a block), what's never happened again with the patch. Thanks, Thomas Lübking
Re: Review Request: Fix KCategorizedView race
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/#review8750 --- Ship it! Ship It! - Aaron J. Seigo On Dec. 5, 2011, 2:21 p.m., Thomas Lübking wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/ --- (Updated Dec. 5, 2011, 2:21 p.m.) Review request for kdelibs, Rafael Fernández López and Jaime Torres Amate. Description --- Also see https://git.reviewboard.kde.org/r/103313/ QListView::updateGeometries() has it's own opinion on whether the scrollbars should be visible (valid range) or not and triggers a (sometimes additionally timered) resize through ::layoutChildren() http://qt.gitorious.org/qt/qt/blobs/4.7/src/gui/itemviews/qlistview.cpp#line1499 (the comment above the main block isn't all accurate, layoutChldren is called regardless of the policy) As a result QListView and KCategorizedView occasionally started a race on the scrollbar visibility, effectively blocking the UI So we prevent QListView from having an own opinion on the scrollbar visibility by fixing it before calling the baseclass QListView::updateGeometries() and restoring the policy afterwards This addresses bugs 213068 and 287847. http://bugs.kde.org/show_bug.cgi?id=213068 http://bugs.kde.org/show_bug.cgi?id=287847 Diffs - kdeui/itemviews/kcategorizedview.cpp 46a1cde kutils/kpluginselector.cpp ca0691d Diff: http://git.reviewboard.kde.org/r/103335/diff/diff Testing --- Yes, resizing the kcmshell4 kwincompositing, all effects KPluginSelector with large scrollbar sizes (bespin/position indicator/32px; oxygen was often sufficient with the default size) after commit e91e5fed6b1aad365e12e919f430c3e8147552d3 (see https://git.reviewboard.kde.org/r/103165/ ) was a reliable way to trigger the issue for me. It also showed that resize/updateGeometries occured in pairs (ie 4calls forming a block), what's never happened again with the patch. Thanks, Thomas Lübking
Re: Review Request: Fix KCategorizedView race
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/#review8765 --- This review has been submitted with commit 08325ba32b72326030004cc28430431193d82fc2 by Thomas Lübking to branch KDE/4.7. - Commit Hook On Dec. 5, 2011, 2:21 p.m., Thomas Lübking wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/ --- (Updated Dec. 5, 2011, 2:21 p.m.) Review request for kdelibs, Rafael Fernández López and Jaime Torres Amate. Description --- Also see https://git.reviewboard.kde.org/r/103313/ QListView::updateGeometries() has it's own opinion on whether the scrollbars should be visible (valid range) or not and triggers a (sometimes additionally timered) resize through ::layoutChildren() http://qt.gitorious.org/qt/qt/blobs/4.7/src/gui/itemviews/qlistview.cpp#line1499 (the comment above the main block isn't all accurate, layoutChldren is called regardless of the policy) As a result QListView and KCategorizedView occasionally started a race on the scrollbar visibility, effectively blocking the UI So we prevent QListView from having an own opinion on the scrollbar visibility by fixing it before calling the baseclass QListView::updateGeometries() and restoring the policy afterwards This addresses bugs 213068 and 287847. http://bugs.kde.org/show_bug.cgi?id=213068 http://bugs.kde.org/show_bug.cgi?id=287847 Diffs - kdeui/itemviews/kcategorizedview.cpp 46a1cde kutils/kpluginselector.cpp ca0691d Diff: http://git.reviewboard.kde.org/r/103335/diff/diff Testing --- Yes, resizing the kcmshell4 kwincompositing, all effects KPluginSelector with large scrollbar sizes (bespin/position indicator/32px; oxygen was often sufficient with the default size) after commit e91e5fed6b1aad365e12e919f430c3e8147552d3 (see https://git.reviewboard.kde.org/r/103165/ ) was a reliable way to trigger the issue for me. It also showed that resize/updateGeometries occured in pairs (ie 4calls forming a block), what's never happened again with the patch. Thanks, Thomas Lübking
Re: Review Request: Fix KCategorizedView race
On Dec. 6, 2011, 2:07 p.m., Aaron J. Seigo wrote: Ship It! What's the policy for merging things into the frameworks branch? - Merge? - Cherry-pick? - Have someone else do it anyway? - Thomas --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/#review8750 --- On Dec. 5, 2011, 2:21 p.m., Thomas Lübking wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/ --- (Updated Dec. 5, 2011, 2:21 p.m.) Review request for kdelibs, Rafael Fernández López and Jaime Torres Amate. Description --- Also see https://git.reviewboard.kde.org/r/103313/ QListView::updateGeometries() has it's own opinion on whether the scrollbars should be visible (valid range) or not and triggers a (sometimes additionally timered) resize through ::layoutChildren() http://qt.gitorious.org/qt/qt/blobs/4.7/src/gui/itemviews/qlistview.cpp#line1499 (the comment above the main block isn't all accurate, layoutChldren is called regardless of the policy) As a result QListView and KCategorizedView occasionally started a race on the scrollbar visibility, effectively blocking the UI So we prevent QListView from having an own opinion on the scrollbar visibility by fixing it before calling the baseclass QListView::updateGeometries() and restoring the policy afterwards This addresses bugs 213068 and 287847. http://bugs.kde.org/show_bug.cgi?id=213068 http://bugs.kde.org/show_bug.cgi?id=287847 Diffs - kdeui/itemviews/kcategorizedview.cpp 46a1cde kutils/kpluginselector.cpp ca0691d Diff: http://git.reviewboard.kde.org/r/103335/diff/diff Testing --- Yes, resizing the kcmshell4 kwincompositing, all effects KPluginSelector with large scrollbar sizes (bespin/position indicator/32px; oxygen was often sufficient with the default size) after commit e91e5fed6b1aad365e12e919f430c3e8147552d3 (see https://git.reviewboard.kde.org/r/103165/ ) was a reliable way to trigger the issue for me. It also showed that resize/updateGeometries occured in pairs (ie 4calls forming a block), what's never happened again with the patch. Thanks, Thomas Lübking
Re: Review Request: Fix KCategorizedView race
On Dec. 6, 2011, 2:07 p.m., Aaron J. Seigo wrote: Ship It! Thomas Lübking wrote: What's the policy for merging things into the frameworks branch? - Merge? - Cherry-pick? - Have someone else do it anyway? we merge KDE/4.7 into frameworks every so often. i did it just the other day; sometimes dfaure does it. so don't worry, this will happen for you :) - Aaron J. --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/#review8750 --- On Dec. 5, 2011, 2:21 p.m., Thomas Lübking wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103335/ --- (Updated Dec. 5, 2011, 2:21 p.m.) Review request for kdelibs, Rafael Fernández López and Jaime Torres Amate. Description --- Also see https://git.reviewboard.kde.org/r/103313/ QListView::updateGeometries() has it's own opinion on whether the scrollbars should be visible (valid range) or not and triggers a (sometimes additionally timered) resize through ::layoutChildren() http://qt.gitorious.org/qt/qt/blobs/4.7/src/gui/itemviews/qlistview.cpp#line1499 (the comment above the main block isn't all accurate, layoutChldren is called regardless of the policy) As a result QListView and KCategorizedView occasionally started a race on the scrollbar visibility, effectively blocking the UI So we prevent QListView from having an own opinion on the scrollbar visibility by fixing it before calling the baseclass QListView::updateGeometries() and restoring the policy afterwards This addresses bugs 213068 and 287847. http://bugs.kde.org/show_bug.cgi?id=213068 http://bugs.kde.org/show_bug.cgi?id=287847 Diffs - kdeui/itemviews/kcategorizedview.cpp 46a1cde kutils/kpluginselector.cpp ca0691d Diff: http://git.reviewboard.kde.org/r/103335/diff/diff Testing --- Yes, resizing the kcmshell4 kwincompositing, all effects KPluginSelector with large scrollbar sizes (bespin/position indicator/32px; oxygen was often sufficient with the default size) after commit e91e5fed6b1aad365e12e919f430c3e8147552d3 (see https://git.reviewboard.kde.org/r/103165/ ) was a reliable way to trigger the issue for me. It also showed that resize/updateGeometries occured in pairs (ie 4calls forming a block), what's never happened again with the patch. Thanks, Thomas Lübking
