Bug 327709 - Remaining padding for scrollbar after fast window resize in icon-view mode.
Summary: Remaining padding for scrollbar after fast window resize in icon-view mode.
Status: RESOLVED FIXED
Alias: None
Product: dolphin
Classification: Applications
Component: view-engine: general (show other bugs)
Version: 4.11.90
Platform: Arch Linux Linux
: NOR normal
Target Milestone: ---
Assignee: Dolphin Bug Assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-16 23:11 UTC by Yichao Yu
Modified: 2013-11-20 23:50 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In: 4.11.4


Attachments
Screenshot when the problem appears. (235.85 KB, image/png)
2013-11-16 23:12 UTC, Yichao Yu
Details
Screenshot for compact mode (7.43 KB, image/png)
2013-11-18 14:36 UTC, Yichao Yu
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Yichao Yu 2013-11-16 23:11:45 UTC
More detail of the bug can be found in (the second screenshot of) this[1] qtcurve bug report. When a dolphin icon view window is resized so that the scrollbar disappears, the padding for the scrolbar doesn't disappear immediately (this can be seen either by making the window bigger pixel by pixel carefully, or by simply maximize a small window, or by turning off splitting) leaving an ugly padding at where the scrollbar is.

This effect can be reproduced in both QtCurve style and Oxygen still.

[1] https://github.com/QtCurve/qtcurve-qt4/issues/9


Reproducible: Always

Steps to Reproduce:
1. Open a dolphin window in iconview mode, make sure all the icons can fit in a maximized window.
2. Make the window smaller so that the scrollbar on the right appears.
3. Maximize the window and observe the space left for the scrollbar on the right.

Actual Results:  
Ugly padding at the where the scrollbar used to be.


Expected Results:  
No padding.
Comment 1 Yichao Yu 2013-11-16 23:12:35 UTC
Created attachment 83603 [details]
Screenshot when the problem appears.
Comment 2 Yichao Yu 2013-11-17 16:19:38 UTC
Update, this also happens to compact view (although in compact view the scrollbar is at the bottom instead of on the side, making it slightly harder to notice sometimes). The only layout that is not affected is the detail view, which doesn't need dynamic content resizing, I'm wonderring if this can help with figuring out what's the problem.
Comment 3 Yichao Yu 2013-11-18 14:36:59 UTC
Created attachment 83621 [details]
Screenshot for compact mode
Comment 4 Yichao Yu 2013-11-18 14:37:50 UTC
It only happens in compact mode for a folder in some condition (size change cannot be too dramatic etc..).
When it happens, it looks like the second screenshot. It seems that the content is draw to the right size but the view port is wrong.

The widget that has the wrong size is KItemListContainer > QGraphicsView > QWidget, if that helps.
Comment 5 Frank Reininghaus 2013-11-18 22:59:48 UTC
Thanks for the bug report. I can confirm the problem when using QtCurve. But I have no idea what the cause of the problem is. Any help would be appreciated - I'm quite unfamiliar with anything related to drawing and styles.
Comment 6 Yichao Yu 2013-11-19 15:19:31 UTC
From what I have found, it seems that the problem is triggered in `KItemListContainer::updateGeometries`.

When resizing the window and when `KItemListContainer::updateGeometries` is called before the scrollbar visibility is updated, a relayout is triggered in `m_controller->view()->setGeometry` (bt attached) which updates the scrollbar visibility and calls back to `KItemListContainer::updateGeometries` again. Since the first call, which has the wrong geometry (due to the incorrect scrollbar states), updates the geometries of the scene and viewport after the second call (which has the right geometry!!) returns, the final result is a size corresponds to old scrollbar state.

So somehow we need to avoid updating with the old parameters. The quickest hack I am using right now to workaround the problem is to move `m_controller->view()->setGeometry` to the bottom but I'm not sure what side effects this reorderring may have. Another way which I have just tested is to use `geometry()` instead of `rect` (or update `rect` with `geometry()`) after `m_controller->view()->setGeometry` returns (this one sounds like a better solution for me).

Maybe also better check how this is done in other part of Qt?

Thought?

:-)

/usr/lib/libdolphinprivate.so.4(KItemListContainer::updateGeometries()+0x19)[0x7f654bf7b9f9]
/usr/lib/libQtGui.so.4(QWidget::event(QEvent*)+0x8ab)[0x7f65493ed5db]
/usr/lib/libQtGui.so.4(QFrame::event(QEvent*)+0x1e)[0x7f65497d646e]
/usr/lib/libQtCore.so.4(QCoreApplicationPrivate::sendThroughObjectEventFilters(QObject*, QEvent*)+0xc7)[0x7f6548986027]
/usr/lib/libQtGui.so.4(QApplicationPrivate::notify_helper(QObject*, QEvent*)+0x81)[0x7f6549395281]
/usr/lib/libQtGui.so.4(QApplication::notify(QObject*, QEvent*)+0x27d)[0x7f654939c14d]
/usr/lib/libkdeui.so.5(KApplication::notify(QObject*, QEvent*)+0x2a)[0x7f654a17abea]
/usr/lib/libQtCore.so.4(QCoreApplication::notifyInternal(QObject*, QEvent*)+0x6d)[0x7f6548985e4d]
/usr/lib/libQtGui.so.4(QWidgetPrivate::setGeometry_sys(int, int, int, int, bool)+0x31e)[0x7f6549435a3e]
/usr/lib/libQtGui.so.4(QWidget::setGeometry(QRect const&)+0x115)[0x7f65493e4c55]
/usr/lib/libQtGui.so.4(QAbstractScrollAreaPrivate::layoutChildren()+0x311)[0x7f654985c511]
/usr/lib/libQtGui.so.4(QAbstractScrollArea::event(QEvent*)+0x30d)[0x7f654985da9d]
/usr/lib/libQtGui.so.4(QApplicationPrivate::notify_helper(QObject*, QEvent*)+0x91)[0x7f6549395291]
/usr/lib/libQtGui.so.4(QApplication::notify(QObject*, QEvent*)+0x27d)[0x7f654939c14d]
/usr/lib/libkdeui.so.5(KApplication::notify(QObject*, QEvent*)+0x2a)[0x7f654a17abea]
/usr/lib/libQtCore.so.4(QCoreApplication::notifyInternal(QObject*, QEvent*)+0x6d)[0x7f6548985e4d]
/usr/lib/libQtGui.so.4(QWidget::setContentsMargins(int, int, int, int)+0x112)[0x7f65493e8932]
/usr/lib/libQtGui.so.4(QAbstractScrollAreaPrivate::layoutChildren()+0x47a)[0x7f654985c67a]
/usr/lib/libQtGui.so.4(QAbstractScrollArea::setVerticalScrollBarPolicy(Qt::ScrollBarPolicy)+0x58)[0x7f654985d0e8]
/usr/lib/libdolphinprivate.so.4(KItemListContainer::updateScrollOffsetScrollBarPolicy()+0xf5)[0x7f654bf7b3a5]
/usr/lib/libQtCore.so.4(QMetaObject::activate(QObject*, QMetaObject const*, int, void**)+0x2d8)[0x7f654899b5b8]
/usr/lib/libdolphinprivate.so.4(KItemListView::maximumScrollOffsetChanged(double, double)+0x3d)[0x7f654bf863ad]
/usr/lib/libdolphinprivate.so.4(KItemListView::emitOffsetChanges()+0x10a)[0x7f654bf8656a]
/usr/lib/libdolphinprivate.so.4(KItemListView::doLayout(KItemListView::LayoutAnimationHint, int, int)+0x843)[0x7f654bf89523]
/usr/lib/libdolphinprivate.so.4(KItemListView::setGeometry(QRectF const&)+0x153)[0x7f654bf8c503]
/usr/lib/libdolphinprivate.so.4(KItemListContainer::updateGeometries()+0xa63)[0x7f654bf7c443]
/usr/lib/libQtGui.so.4(QWidget::event(QEvent*)+0x8ab)[0x7f65493ed5db]
/usr/lib/libQtGui.so.4(QFrame::event(QEvent*)+0x1e)[0x7f65497d646e]
/usr/lib/libQtCore.so.4(QCoreApplicationPrivate::sendThroughObjectEventFilters(QObject*, QEvent*)+0xc7)[0x7f6548986027]
/usr/lib/libQtGui.so.4(QApplicationPrivate::notify_helper(QObject*, QEvent*)+0x81)[0x7f6549395281]
/usr/lib/libQtGui.so.4(QApplication::notify(QObject*, QEvent*)+0x27d)[0x7f654939c14d]
/usr/lib/libkdeui.so.5(KApplication::notify(QObject*, QEvent*)+0x2a)[0x7f654a17abea]
/usr/lib/libQtCore.so.4(QCoreApplication::notifyInternal(QObject*, QEvent*)+0x6d)[0x7f6548985e4d]
/usr/lib/libQtGui.so.4(QWidgetPrivate::setGeometry_sys(int, int, int, int, bool)+0x31e)[0x7f6549435a3e]
/usr/lib/libQtGui.so.4(QWidget::setGeometry(QRect const&)+0x115)[0x7f65493e4c55]
/usr/lib/libQtGui.so.4(QAbstractScrollAreaPrivate::layoutChildren()+0x311)[0x7f654985c511]
/usr/lib/libQtGui.so.4(QAbstractScrollArea::event(QEvent*)+0x30d)[0x7f654985da9d]
/usr/lib/libQtGui.so.4(QApplicationPrivate::notify_helper(QObject*, QEvent*)+0x91)[0x7f6549395291]
/usr/lib/libQtGui.so.4(QApplication::notify(QObject*, QEvent*)+0x27d)[0x7f654939c14d]
/usr/lib/libkdeui.so.5(KApplication::notify(QObject*, QEvent*)+0x2a)[0x7f654a17abea]
/usr/lib/libQtCore.so.4(QCoreApplication::notifyInternal(QObject*, QEvent*)+0x6d)[0x7f6548985e4d]
/usr/lib/libQtGui.so.4(QWidgetPrivate::setGeometry_sys(int, int, int, int, bool)+0x31e)[0x7f6549435a3e]
/usr/lib/libQtGui.so.4(QWidget::setGeometry(QRect const&)+0x115)[0x7f65493e4c55]
/usr/lib/libQtGui.so.4(QWidgetItem::setGeometry(QRect const&)+0x125)[0x7f65493c5a55]
/usr/lib/libQtGui.so.4(QBoxLayout::setGeometry(QRect const&)+0x7ee)[0x7f65493a525e]
/usr/lib/libQtGui.so.4(QLayoutPrivate::doResize(QSize const&)+0x7e)[0x7f65493c19fe]
/usr/lib/libQtGui.so.4(QApplicationPrivate::notify_helper(QObject*, QEvent*)+0x73)[0x7f6549395273]
/usr/lib/libQtGui.so.4(QApplication::notify(QObject*, QEvent*)+0x27d)[0x7f654939c14d]
/usr/lib/libkdeui.so.5(KApplication::notify(QObject*, QEvent*)+0x2a)[0x7f654a17abea]
/usr/lib/libQtCore.so.4(QCoreApplication::notifyInternal(QObject*, QEvent*)+0x6d)[0x7f6548985e4d]
/usr/lib/libQtGui.so.4(QWidgetPrivate::setGeometry_sys(int, int, int, int, bool)+0x31e)[0x7f6549435a3e]
/usr/lib/libQtGui.so.4(QWidget::setGeometry(QRect const&)+0x115)[0x7f65493e4c55]
/usr/lib/libQtGui.so.4(QWidgetItem::setGeometry(QRect const&)+0x125)[0x7f65493c5a55]
/usr/lib/libQtGui.so.4(QBoxLayout::setGeometry(QRect const&)+0x7ee)[0x7f65493a525e]
/usr/lib/libQtGui.so.4(QLayoutPrivate::doResize(QSize const&)+0x7e)[0x7f65493c19fe]
/usr/lib/libQtGui.so.4(QApplicationPrivate::notify_helper(QObject*, QEvent*)+0x73)[0x7f6549395273]
/usr/lib/libQtGui.so.4(QApplication::notify(QObject*, QEvent*)+0x27d)[0x7f654939c14d]
/usr/lib/libkdeui.so.5(KApplication::notify(QObject*, QEvent*)+0x2a)[0x7f654a17abea]
/usr/lib/libQtCore.so.4(QCoreApplication::notifyInternal(QObject*, QEvent*)+0x6d)[0x7f6548985e4d]
/usr/lib/libQtGui.so.4(QWidgetPrivate::setGeometry_sys(int, int, int, int, bool)+0x31e)[0x7f6549435a3e]
/usr/lib/libQtGui.so.4(QWidget::setGeometry(QRect const&)+0x115)[0x7f65493e4c55]
/usr/lib/libQtGui.so.4(+0x6650c8)[0x7f65498300c8]
/usr/lib/libQtGui.so.4(+0x667ac8)[0x7f6549832ac8]
/usr/lib/libQtGui.so.4(QWidget::event(QEvent*)+0x8ab)[0x7f65493ed5db]
/usr/lib/libQtGui.so.4(QApplicationPrivate::notify_helper(QObject*, QEvent*)+0x91)[0x7f6549395291]
/usr/lib/libQtGui.so.4(QApplication::notify(QObject*, QEvent*)+0x27d)[0x7f654939c14d]
/usr/lib/libkdeui.so.5(KApplication::notify(QObject*, QEvent*)+0x2a)[0x7f654a17abea]
/usr/lib/libQtCore.so.4(QCoreApplication::notifyInternal(QObject*, QEvent*)+0x6d)[0x7f6548985e4d]
/usr/lib/libQtGui.so.4(QWidgetPrivate::setGeometry_sys(int, int, int, int, bool)+0x31e)[0x7f6549435a3e]
/usr/lib/libQtGui.so.4(QWidget::setGeometry(QRect const&)+0x115)[0x7f65493e4c55]
/usr/lib/libQtGui.so.4(QWidgetItem::setGeometry(QRect const&)+0x125)[0x7f65493c5a55]
/usr/lib/libQtGui.so.4(QBoxLayout::setGeometry(QRect const&)+0x7ee)[0x7f65493a525e]
/usr/lib/libQtGui.so.4(QLayoutPrivate::doResize(QSize const&)+0x7e)[0x7f65493c19fe]
/usr/lib/libQtGui.so.4(QApplicationPrivate::notify_helper(QObject*, QEvent*)+0x73)[0x7f6549395273]
/usr/lib/libQtGui.so.4(QApplication::notify(QObject*, QEvent*)+0x27d)[0x7f654939c14d]
/usr/lib/libkdeui.so.5(KApplication::notify(QObject*, QEvent*)+0x2a)[0x7f654a17abea]
/usr/lib/libQtCore.so.4(QCoreApplication::notifyInternal(QObject*, QEvent*)+0x6d)[0x7f6548985e4d]
/usr/lib/libQtGui.so.4(QWidgetPrivate::setGeometry_sys(int, int, int, int, bool)+0x31e)[0x7f6549435a3e]
/usr/lib/libQtGui.so.4(QWidget::setGeometry(QRect const&)+0x115)[0x7f65493e4c55]
/usr/lib/libQtGui.so.4(QWidget::qt_metacall(QMetaObject::Call, int, void**)+0x32d)[0x7f65493ef84d]
/usr/lib/libQtCore.so.4(+0x63e82)[0x7f6548861e82]
/usr/lib/libQtCore.so.4(+0x6160a)[0x7f654885f60a]
/usr/lib/libQtCore.so.4(+0x618e4)[0x7f654885f8e4]
/usr/lib/libQtCore.so.4(QPropertyAnimation::updateState(QAbstractAnimation::State, QAbstractAnimation::State)+0x2e3)[0x7f65488624f3]
/usr/lib/libQtCore.so.4(+0x5ee12)[0x7f654885ce12]
/usr/lib/libQtGui.so.4(+0x6a578f)[0x7f654987078f]
/usr/lib/libQtGui.so.4(+0x6000e8)[0x7f65497cb0e8]
/usr/lib/libQtGui.so.4(+0x629ed1)[0x7f65497f4ed1]
/usr/lib/libQtGui.so.4(+0x62e829)[0x7f65497f9829]
/usr/lib/libQtGui.so.4(QLayoutPrivate::doResize(QSize const&)+0x7e)[0x7f65493c19fe]
/usr/lib/libQtGui.so.4(QApplicationPrivate::notify_helper(QObject*, QEvent*)+0x73)[0x7f6549395273]
/usr/lib/libQtGui.so.4(QApplication::notify(QObject*, QEvent*)+0x27d)[0x7f654939c14d]
/usr/lib/libkdeui.so.5(KApplication::notify(QObject*, QEvent*)+0x2a)[0x7f654a17abea]
/usr/lib/libQtCore.so.4(QCoreApplication::notifyInternal(QObject*, QEvent*)+0x6d)[0x7f6548985e4d]
/usr/lib/libQtGui.so.4(+0x24b7a7)[0x7f65494167a7]
/usr/lib/libQtGui.so.4(QApplication::x11ProcessEvent(_XEvent*)+0xc59)[0x7f65494177a9]
/usr/lib/libQtGui.so.4(+0x274192)[0x7f654943f192]
/usr/lib/libglib-2.0.so.0(g_main_context_dispatch+0x148)[0x7f654d2cb278]
/usr/lib/libglib-2.0.so.0(+0x5061f)[0x7f654d2cb61f]
/usr/lib/libglib-2.0.so.0(g_main_context_iteration+0x5e)[0x7f654d2cb69e]
/usr/lib/libQtCore.so.4(QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>)+0x65)[0x7f65489b5a25]
/usr/lib/libQtGui.so.4(+0x274286)[0x7f654943f286]
/usr/lib/libQtCore.so.4(QEventLoop::processEvents(QFlags<QEventLoop::ProcessEventsFlag>)+0x2f)[0x7f654898426f]
/usr/lib/libQtCore.so.4(QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>)+0x115)[0x7f6548984505]
/usr/lib/libQtCore.so.4(QCoreApplication::exec()+0x89)[0x7f654898a3d9]
/usr/lib/libkdeinit4_dolphin.so(kdemain+0xc87)[0x7f654cf73c47]
/usr/lib/libc.so.6(__libc_start_main+0xf5)[0x7f654cb98bc5]
dolphin[0x400691]
Comment 7 Yichao Yu 2013-11-19 15:39:35 UTC
https://git.reviewboard.kde.org/r/113939/
Comment 8 Tsu Jan 2013-11-19 15:56:47 UTC
@ Yichao Yu
I tried your first way (moving m_controller->view()->setGeometry to the bottom) and it worked. I don't see how it could have any side effect.

I'll try the second way and tell you the result.
Comment 9 Yichao Yu 2013-11-19 16:02:36 UTC
@Tsu Jan
The review board is the second approach, which I came up with when writing the comment... = = ...
Comment 10 Tsu Jan 2013-11-19 16:37:15 UTC
@Yichao Yu
The second method works all right. Well done!

BTW, your were right to be suspicious of the first approach: it DID have side effects (for details view, at least).
Comment 11 Yichao Yu 2013-11-19 16:45:04 UTC
@Tsu Jan
Good that you have noticed this! Actually my fix is purely coming from the backtrace and I still have no idea why this only happens to icon-view and compact-view but not detail-view... Maybe some subtle difference in relayout handling.... I think the second one shouldn't have any side effects.... It only changes the logic in the bug condition, in a way that should be a no-op.....
Comment 12 Frank Reininghaus 2013-11-19 17:12:09 UTC
Many thanks for your investigations, Yichao and Tsu! This is really very nice debugging work :-)

The patch looks very reasonable. I haven't been able to test it yet, but I will try to do it during the next days and then push it to KDE/4.11.

I'm not 100% sure, but the reason why the behavior in Details View is different from Icons and Compact might be that the scroll bar in Icons/Compact reduces the space that is available for layouting the items: if the scroll bar is added, less width/height is available, and fewer columns/rows can be used for the items, which means that more rows/columns will be needed to show all items. If the view is then enlarged by a few pixels, the presence of the scroll bar may prevent that another column/row can be used for layouting - the space required by the scroll bar is then the reason why the scroll bar is needed!

This kind of complicated interaction between the geometry and the "scroll bar/no scroll bar" question (which does not occur in Details View, where the scroll bar does never affect the number of rows, and thus the required height) may have something to do with the problem.
Comment 13 Yichao Yu 2013-11-19 17:25:45 UTC
Hmmm, interesting.

I traced the calls once more and it seems that in detail-view, `updateGeometries` is also called twice. However the difference is that, in icon and compact mode, this (usually) happens in `m_controller->view()->setGeometry(newGeometry);`, which is triggered by relayout and was set back to wrong geometry by calls later in `updateGeometries` but in detail-mode, this happens in `updateScrollOffsetScrollBar` and there is no functions afterward to reset the geometries.

I would guess my first hack may work if I just put `m_controller->view()->setGeometry(newGeometry);` right above `updateScrollOffsetScrollBar`. Anyway, the second (reviewboard) approach looks better....

:-)
Comment 14 Frank Reininghaus 2013-11-20 23:50:54 UTC
Git commit b3322111c1dd58723b10d2308bf790e61fc9523b by Frank Reininghaus, on behalf of Yichao Yu.
Committed on 20/11/2013 at 23:47.
Pushed by freininghaus into branch 'KDE/4.11'.

Fix incorrect geometry updates in KItemListContainer

When resizing the window and when KItemListContainer::updateGeometries
is called before the scrollbar visibility is updated, a relayout is
triggered in `m_controller->view()->setGeometry` which updates the
scrollbar visibility and calls back to
`KItemListContainer::updateGeometries` again. Since the first call,
which has the wrong geometry (due to the incorrect scrollbar states),
updates the geometries of the scene and viewport after the second call
(which has the right geometry!!) returns, the final result is a size
that corresponded to the old scrollbar state before this commit.

This patch uses the new geometry of the view after updating it (since
it might not be the size we put in) and therefore makes the sizes
consistent.
FIXED-IN: 4.11.4
REVIEW: 113939

M  +6    -5    dolphin/src/kitemviews/kitemlistcontainer.cpp

http://commits.kde.org/kde-baseapps/b3322111c1dd58723b10d2308bf790e61fc9523b