Review Request 118454: Separate width and height info when calculating the required item size

[Frank Reininghaus]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118454/
-----------------------------------------------------------

(Updated June 4, 2014, 7:45 p.m.)


Review request for Dolphin.


Repository: kde-baseapps


Description
-------

The view layout can handle items with different sizes. Simple example: files with very long names might need multiple text lines below the icon in Icons View, and the view will take this into account and reserve sufficient space for each item.

The size that each item requires is determined via the class KItemListSizeHintResolver, which caches the size for each item and uses

KItemListView::calculateItemSizeHints(QVector<QSizeF>& sizeHints)

and

KStandardItemListWidgetInformant::calculateItemSizeHints(QVector<QSizeF>& sizeHints, const KItemListView* view)

In many cases, this function is now the thing that takes the most time (at least inside Dolphin) when opening a directory, because calculating the text layout for each file name is expensive, and most other parts of the code base have been optimized quite considerably already.

I think that one could save time here by only calculating the required size for the visible items, using guessed values for the others, and, when the user scrolls the view, calculating the size for newly visible items and updating the layout accordingly.

I'll try to look into that, but before that, I thought that simplifying some unnecessarily complex things might make sense, in order to make future changes easier.

For example, we calculate and store the required width and height for every item, even though the width or height is the same for all items in Icons/Details and Compact View, respectively.

By separating the width and height info, we can save some unnecessary overhead in terms of memory and CPU cycles, and make the calculation of the height of a row (or the width of a column in Compact View) a bit simpler. To achieve this, this patch extends the concept of "logical rows" (which are actually columns in Compact View) to "logical width" and "logical height" (which is the actual height and width, respectively, in Compact View). The distinction between rows/columns and "logical" rows/columns may be a bit confusing, but the confusion is already in the current code, and I hope that it will be mitigated a bit by prefixing the corresponding variables with "logical".


Diffs (updated)
-----

  dolphin/src/kitemviews/kitemlistview.h 8a522a6 
  dolphin/src/kitemviews/kitemlistview.cpp 222a29c 
  dolphin/src/kitemviews/kitemlistwidget.h cfb9155 
  dolphin/src/kitemviews/kstandarditemlistwidget.h ba426d0 
  dolphin/src/kitemviews/kstandarditemlistwidget.cpp 0372269 
  dolphin/src/kitemviews/private/kitemlistsizehintresolver.h 86580bf 
  dolphin/src/kitemviews/private/kitemlistsizehintresolver.cpp 029bedd 
  dolphin/src/kitemviews/private/kitemlistviewlayouter.cpp 9da5384 

Diff: https://git.reviewboard.kde.org/r/118454/diff/


Testing
-------

The view layout still seems to work OK for me.


Thanks,

Frank Reininghaus

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