Review Request 111322: Prevent some unnecessary relayoutings when the view size is changed

[Frank Reininghaus]

> On July 1, 2013, 11:42 p.m., Christoph Feck wrote:
> > > This seems to be due to a glitch in KUrlNavigator which changes the view height by 1 pixel every time the window becomes visible or invisible after a desktop switch.
> > 
> > This is related to bug 304551, but I have yet to check, why it also changes height when not changing mode. Do you run kdelibs from source, and could check the behavior, if I create a patch for kdelibs?

Thanks for your comments, Emmanuel and Christoph!

> This is related to bug 304551

Yes, you are right! If I hide the Places Panel, then it does not happen.

> I have yet to check, why it also changes height when not changing mode.

It seems that DolphinMainWindow::slotPlacesPanelVisibilityChanged(bool visible) is called also when switching desktops (which makes sense, because the panel's visibility does change then).

> Do you run kdelibs from source, and could check the behavior, if I create a patch for kdelibs?

It would be great if you could provide a patch for kdelibs! I build it from source on my home machine, so I can test patches there.

Even if you fix the KUrlNavigator issue in kdelibs, I still think that it makes sense to push this patch to Dolphin, because it also prevents unnecessary layoutings when the user changes the view height (or view width when using Compact View) manually.


- Frank


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111322/#review35410
-----------------------------------------------------------


On June 30, 2013, 1:07 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111322/
> -----------------------------------------------------------
> 
> (Updated June 30, 2013, 1:07 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Description
> -------
> 
> Currently, we always trigger a relayouting of the view if the view geometry changes. However, in Icons/Details View the view height doesn't matter at all for the layout, and neither does the view width in Compact View.
> 
> I propose to not force a realayouting in these cases, but only a recalculation of the visible indexes.
> 
> Enabling the debug output in KItemListViewLayouter showed me that we actually re-layout the view far too often: for example, every time the desktop is switched. This seems to be due to a glitch in KUrlNavigator which changes the view height by 1 pixel every time the window becomes visible or invisible after a desktop switch. This might be worth investigating and fixing in kdelibs, but I think that this should be fixed on the Dolphin side as well.
> 
> I have a few more ideas how we can speed up the layouting, which can become a major bottleneck in Icons View in folders with many items (at least the first time, when no cached item sizes are available in KItemListSizeHintResolver yet): in many cases, file names are so short that we can tell that only one line is needed for showing the name without letting QTextLayout do a very expensive full calculation in
> 
> QSizeF KStandardItemListWidgetInformant::itemSizeHint(int index, const KItemListView* view).
> 
> Moreover, everything in that function seems to be reentrant, so we can actually speed up the itemSizeHint calculation by making use of multiple CPU cores. But these things require changes that might be too intrusive to push to master at this point of the release cycle. I'll investigate this for KDE 4.12.
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/private/kitemlistviewlayouter.cpp c15b44e 
> 
> Diff: http://git.reviewboard.kde.org/r/111322/diff/
> 
> 
> Testing
> -------
> 
> No unnecessary layoutings are done any more. Everything else still seems to work nicely.
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20130702/8d4e5a32/attachment.htm>