I don’t know this code so I can’t really review the logic. There are some small problems you’ll have to fix anyway:

-the coding style (http://qt.gitorious.org/qt/pages/QtCodingStyle).
-you should squash the two commits.
-a unit test is needed given the nature of the bug (it is not a trivial bug you are fixing).
-finally, note that we generally don’t use foreach() in Qt itself.

Quick comments before I go to sleep:
you should not put space after and before parenthesis in these cases:
“static QList findLayoutsContaining( const QWidget widget, QLayout parent = 0 )”
“parent->itemAt( i );” –> “parent->itemAt(i);”

Do not hesitate to squash the commits.

Tell us when it’s ready :)

All done, including squashing the commits. Had to find out how that works, first ;–)

Jan-Arve or Paul are the best ones to review this. I prefer to let them check this patch.

Hi, thanks for spotting this!

However, it looks like a potential expensive operation, especially because of the QLists that are returned on the stack.
I suggest to use this approach:
Normally, (but not always) the widget has a d->widgetItem that you can use which will be the widgets corresponding QWidgetItem. If you have that widget item you can simply traverse all ancestors until you reach the top level and invalidate them. That should then have virtually no performance penalty.

For the case where the widget’s d->widgetItem is null you'd have to fallback to your solution. However, I think I would rather just search all layouts recursively and return the actual layout holding a reference to the widget. That will eliminate all the construction and deconstruction of qlists on the stack, and you can then reuse the same codepath (that invalidates the ancestors) from the “optimized case”.

Also, the test is too big, and it is not pinpointing the actual reason for the bugfix/failure. (the actual reason is that gridLayout_2’s sizeHint is not correct at the point show() is called because it was not invalidated)
Autotest code should be understandable so that people know where they should look if a test fails. It can also serve as a source to understanding the fix itself.

(If you are not sure how to clean up the autotest you can just submit the fix for qwidget.cpp in your next proposal and I can write the autotest)

Made it directectly invalidate all the layouts and not moving around a QList.

Note: I see no way how to get the ancestors of a QLayoutItem.

I couldn’t figure out how to make the test case smaller :–(

Any news on this?

Sorry for not answering. But we realized that the suggested approach can have a unfortunate performance regression.

Suppose I have a layout with several QLabels and I update the content of all those labels. If there is n labels, the complexity of invalidating one layout will be O(n^2), while it is simply O(n) today.
It should be possible to fix this, but it might not be trivial (and yeah you’re right – you cannot get the ancestors of a QLayoutItem efficiently). Thus, I need to put some more thoughts into this as soon as I have some spare cycles.

There are also similar problems in the layout system today (for instance sizeHint() is not always reliable due to that we post layout requests) that I would also like to see if we can fix.

(Of course, if you want to contribute you can still do that, but it requires good knowledge of how the invalidation and activation works).

I change the task status due to the previous comment.
It set “rejected” for this merge request but as Jan Arve said, we are open to a better solution if one exist.