Bug 213068

Created attachment 38079 [details]
screenshot taken

Judging from the screenshot this is not the get hot new stuff dialog but the script management dialog.

*** Bug 222295 has been marked as a duplicate of this bug. ***

Bug 222295 indicates that this may depend on the used style. Can you confirm that changing style to Plastique helps?

Since there is additional information provided in Bug 222295 I copy it here:
this does not happen when using plastique style
this does happen when using oxygen, or QtCurve.

I just compiled Amarok from git, and the bug also happens with Skulpture, which is not KStyle based. It looks like a problem in KPluginSelector not updating its size somehow when the plugins are added after it has been shown. I looked quickly at Amarok sources, and the plugins are added later.

The bug may be invisible on KDE 4.3, because KDE 4.4 has a new implementation of the KCategorizedView, so that may be the reason Amarok developers never hit that.

I will look into it later, but if my assumption makes sense, please reassign to kdelibs/kdeui.

eheh. Glad to see I'm off the hook.

Since this only happens with KDE 4.4, we suspect a KDE bug, reassigning

Myriam, would it be possible to test with KDE 4.3.4, but running on Qt 4.6.0? Not every distribution supports this combination, but would be nice if I got feedback on this.

What I did: I removed the proxy model from KPluginSelector, i.e. the list directly shows the model via d->listView->setModel(d->pluginModel); Now everything works fine (except the filter and sorting does not work, of course).

Looking at the proxy model, it has not changed for KDE 4.4, neither in kutils, nor in kdelibs/kdeui. What has changed is QSortFilterProxyModel. So this might be a Qt 4.6 problem, but my experience with Model-View classes is limited.

Stephen, any idea? KPluginSelector is in kdelibs/kutils, the other model classes are in kdelibs/kdeui/itemviews.

*** Bug 217326 has been marked as a duplicate of this bug. ***

Bug 217326 has a video. Issue is with KTorrent KPluginSelector.

I tried but couldn't build either KTorrent or Amarok straight away. I don't have time to spend on it, but I'll be at CampKDE if anyone there can show me the issue.

It would be odd for the proxy model to cause that. Has anyone tried using a different view to see if the issue remains?

On 01/12/2010 03:45 PM, Christoph Feck wrote:
> https://bugs.kde.org/show_bug.cgi?id=213068
>
>
> Christoph Feck<christoph@maxiom.de>  changed:
>
>             What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                   CC|                            |steveire@gmail.com
>
>
>
>
> --- Comment #9 from Christoph Feck<christoph maxiom de>   2010-01-12 23:45:12 ---
> Myriam, would it be possible to test with KDE 4.3.4, but running on Qt 4.6.0?
> Not every distribution supports this combination, but would be nice if I got
> feedback on this.
>
> What I did: I removed the proxy model from KPluginSelector, i.e. the list
> directly shows the model via d->listView->setModel(d->pluginModel); Now
> everything works fine (except the filter and sorting does not work, of course).
>
>    
Interestingly, just commenting out

d->proxyModel->setCategorizedModel(true);
in kdelibs/kutils/kpluginselector.cpp
(line 267) fixes it for me ...



> Looking at the proxy model, it has not changed for KDE 4.4, neither in kutils,
> nor in kdelibs/kdeui. What has changed is QSortFilterProxyModel. So this might
> be a Qt 4.6 problem, but my experience with Model-View classes is limited.
>
> Stephen, any idea? KPluginSelector is in kdelibs/kutils, the other model
> classes are in kdelibs/kdeui/itemviews.
>
>

More info after digging the code.
Some how there is an semi-infinite loop generated (possibly due to the style) between 
KCategorizedView::rowsAboutToBeRemoved
and KCategorizedView::visualRect

(which gets disabled when you don't set the list as cathegorized)

I'm still trying to investigate what causes this.

sorry read
KCategorizedView::updateGeometries 
KCategorizedView::visualRect

ok. I think what happens is that the range and the position of the vertical is trying to be updated in updateGeometries(), while this scrollbar is hidden (and not necessary). This makes the scrollbar visible (maybe this is new to Qt 4.6, which triggers updateGeometries again. And from there the recursive loop).

Anyway if you force the scrollbar to be visible, or if you disable the update if the scrollbar is hidden, things go well.
See: 

--- kdeui/itemviews/kcategorizedview.cpp        (revision 1074886)
+++ kdeui/itemviews/kcategorizedview.cpp        (working copy)
@@ -1214,8 +1214,11 @@
         bottomRange = lastItemRect.bottomRight().y() + verticalOffset() - viewport()->height();
     }

-    verticalScrollBar()->setRange(0, bottomRange);
-    verticalScrollBar()->setValue(verticalOff);
+    if( verticalScrollBar()->isVisible() )
+    {
+        verticalScrollBar()->setRange(0, bottomRange);
+        verticalScrollBar()->setValue(verticalOff);
+    }

     //TODO: also consider working with the horizontal scroll bar. since at this level I am not 

Can someone test this ? Confirm whether this is the right thing to do or not ?

I used the wrong commit keyword (CCMAIL instead of CCBUG). Repasting manually:


    Fix infinite recursion issues when scrolling KCategorizedView

    I don't know this code, but it seems the view is not ready to handle
    QAbstractItemView::ScrollPerItem. See approx line 1222:

        if (viewMode() == ListMode && verticalScrollMode() == ScrollPerItem) {
            //TODO: think more about this case
        } else {

    The default is not specified in the QAbstractItemView docs, so it may have been
    changed in Qt 4.6. I have no idea.

    Todays hack is to use QAbstractItemView::ScrollPerPixel by default instead.

    Please re-test the bug and I will backport if it fixes Amarok and KTorrent.
    I've so far only reproduced in a test app.

    CCMAIL: 213068

    git-svn-id: svn+ssh://svn.kde.org/home/kde/trunk/KDE/kdelibs@1075987 283d02a7-25f6-0310-bc7c-ecb5cbfe19da

diff --git a/kdeui/itemviews/kcategorizedview.cpp b/kdeui/itemviews/kcategorizedview.cpp
index 63d5dfa..4d5beb1 100644
--- a/kdeui/itemviews/kcategorizedview.cpp
+++ b/kdeui/itemviews/kcategorizedview.cpp
@@ -474,6 +474,7 @@ KCategorizedView::KCategorizedView(QWidget *parent)
     : QListView(parent)
     , d(new Private(this))
 {
+  setVerticalScrollMode(ScrollPerPixel);
 }

 KCategorizedView::~KCategorizedView()

mmm. Unless I did something wrong, Comment #17 does not work for me ...

Yeah, I just realized the thing which made my example not infinite loop was commenting QListView::resizeEvent(event); in the reimplementation. Probably not an acceptable workaround to commit.

Created attachment 40486 [details]
Fix layout recursion

Hugo, if you do not change the values for invisible scrollbars, then they are never shown?

Anyway, I prepared a patch that only changes the scrollbars when the layout of the model changes, not on every updateGeometries().

It fixes the Amarok issue, but I have no clue if it causes any regressions.

@Raphael, tagging of 4.4.0 is in some 20+ hours, maybe we have a chance to fix that... :)

Comment on attachment 40486 [details]
Fix layout recursion

Scratch that... it does not respect the changed size of the items after updateGeometries() creates the "Goya" widgets...

*** Bug 230239 has been marked as a duplicate of this bug. ***

*** Bug 231027 has been marked as a duplicate of this bug. ***

I can't reproduce this issue. It is possible that this got fixed with some changes I have done to this class (KCategorizedView).

Please if you can still reproduce this on trunk, tell me how.

Still reproducable, on kdelibs trunk from today with Qt 4.7 branch from last week.

Start Amarok, select Settings/Configure, resize the config window as small as possible, then click on "Internet Services". The effect can be seen best when you now try to maximize the config window.

From what I understand, the problem is in Qt, getting into a loop where showing a scroll bar causes a relayout, causing the second scroll bar to be shown, causing a relayout, causing the second scroll bar to be hidden, causing a relayout, ...

Interesting. I could now reproduce. Thanks. Will have a look.

SVN commit 1104321 by ereslibre:

Fix infinite loop. However scrollbars still need to be polished to improve their look (the step 
size probably). This is more important than the look, since it is blocking some applications 
(such as Amarok) under certain circumstances. I could reproduce the issue and I believe now is
fixed. Please if you could recheck.

BUG: 213068


 M  +1 -2      kcategorizedview.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1104321

SVN commit 1104322 by ereslibre:

Backport revision 1104321 to 4.4. Fix infinite recursion under certain circumstances.

CCBUG: 213068


 M  +1 -2      kcategorizedview.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1104322

SVN commit 1118520 by cfeck:

Never show horizontal scroll bar

Revert commit r1104321, and try a different workaround.

Something inside Qt causes an infinite layout recursion (see bug 213068),
but not updating scroll bar geometry causes a major usability regression
(see bug 233163).

The only workaround I can see for now is to force hiding the horizontal
scroll bar; KCategorizedView does not yet support it anyway.

BUG: 233163
CCBUG: 213068


 M  +4 -1      kcategorizedview.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1118520

SVN commit 1118521 by cfeck:

Never show horizontal scroll bar (backport r1118520)

Revert commit r1104322, and try a different workaround.

CCBUG: 233163
CCBUG: 213068


 M  +4 -1      kcategorizedview.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1118521

It looks like all the fixes we tried are mutually exclusive ... Rafael, any other idea?

*** Bug 239925 has been marked as a duplicate of this bug. ***

*** Bug 240853 has been marked as a duplicate of this bug. ***

*** Bug 225557 has been marked as a duplicate of this bug. ***

*** Bug 241275 has been marked as a duplicate of this bug. ***

*** Bug 241369 has been marked as a duplicate of this bug. ***

SVN commit 1137013 by cfeck:

Next attempt at KPluginSelector lockup

Force a scrollbar on the plugin selector. For users with few
plugins, it might look odd why it still shows the scroll bar,
but still better than an application freeze.

If I only could prove it is a Qt bug ...

BUG: 213068


 M  +0 -6      kdeui/itemviews/kcategorizedview.cpp  
 M  +1 -0      kutils/kpluginselector.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1137013

SVN commit 1138391 by cfeck:

Next attempt at KPluginSelector lockup (backport r1137013)

CCBUG: 213068


 M  +0 -6      kdeui/itemviews/kcategorizedview.cpp  
 M  +1 -0      kutils/kpluginselector.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1138391

*** Bug 231398 has been marked as a duplicate of this bug. ***

*** Bug 241946 has been marked as a duplicate of this bug. ***

*** Bug 242977 has been marked as a duplicate of this bug. ***

*** Bug 250593 has been marked as a duplicate of this bug. ***

Bug 250593 is from KDE SC 4.5.1, reopening :/

*** Bug 258316 has been marked as a duplicate of this bug. ***

Git commit 70c6350fac3ab2b74d61811606eb250da7c5a061 by Thomas Lübking.
Committed on 01/12/2011 at 19:46.
Pushed by luebking into branch 'KDE/4.7'.

Fix regression induced (but likely not caused) by commit e91e5fed6b1aad365e12e919f430c3e8147552d3

Commiting on behalf of Jaime Torres.

Ratio:
On k-c-d I suggested this would be another ouotcome of bug #213068 and Jaime could confirm this and the
scrollbar policy workaround to work.
He suggested to fix it by "setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff);" instead (since presumingly the
pluginselector is always top-down oriented)
I tested that variant as well and could not reproduce the issue.

Since Jaime has no acces to his development environment for today he asked me to either revert his commit or
apply the hotfix working around bug #213068 as mentioned above.

Because i could not spot any bug in his commit i'll go with the hotfix to prevent a regression.
Resolving the actual recursion bug is an outstanding issue.

CCBUG: 213068
CCBUG: 287847

M  +1    -0    kutils/kpluginselector.cpp

http://commits.kde.org/kdelibs/70c6350fac3ab2b74d61811606eb250da7c5a061

Git commit ee00a9f572a383b98c4fd01f0b0477ff9148c5d7 by Aaron Seigo, on behalf of Thomas Lübking.
Committed on 01/12/2011 at 19:46.
Pushed by aseigo into branch 'frameworks'.

Fix regression induced (but likely not caused) by commit e91e5fed6b1aad365e12e919f430c3e8147552d3

Commiting on behalf of Jaime Torres.

Ratio:
On k-c-d I suggested this would be another ouotcome of bug #213068 and Jaime could confirm this and the
scrollbar policy workaround to work.
He suggested to fix it by "setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff);" instead (since presumingly the
pluginselector is always top-down oriented)
I tested that variant as well and could not reproduce the issue.

Since Jaime has no acces to his development environment for today he asked me to either revert his commit or
apply the hotfix working around bug #213068 as mentioned above.

Because i could not spot any bug in his commit i'll go with the hotfix to prevent a regression.
Resolving the actual recursion bug is an outstanding issue.

CCBUG: 213068
CCBUG: 287847

M  +1    -0    kutils/kpluginselector.cpp

http://commits.kde.org/kdelibs/ee00a9f572a383b98c4fd01f0b0477ff9148c5d7

Git commit 08325ba32b72326030004cc28430431193d82fc2 by Thomas Lübking.
Committed on 05/12/2011 at 14:56.
Pushed by luebking into branch 'KDE/4.7'.

fix KCategorizedView race

BUG: 213068
BUG: 287847
REVIEW: 103335

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

M  +23   -1    kdeui/itemviews/kcategorizedview.cpp
M  +0    -2    kutils/kpluginselector.cpp

http://commits.kde.org/kdelibs/08325ba32b72326030004cc28430431193d82fc2

Git commit 8095c11d181e967cb161de3452f45c1df81bbc66 by Thomas Lübking.
Committed on 08/12/2011 at 18:58.
Pushed by luebking into branch 'KDE/4.7'.

Fix KCategorizedView race - better :p

BUG: 213068
BUG: 287847
REVIEW: 103335

M  +24   -9    kdeui/itemviews/kcategorizedview.cpp

http://commits.kde.org/kdelibs/8095c11d181e967cb161de3452f45c1df81bbc66