Review Request 113871: Make the handling of the "maximum text lines" setting in Icons View more robust

[Frank Reininghaus]

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

(Updated Feb. 24, 2014, 8:10 p.m.)


Status
------

This change has been marked as submitted.


Review request for Dolphin and Christoph Feck.


Bugs: 308022 and 323841
    http://bugs.kde.org/show_bug.cgi?id=308022
    http://bugs.kde.org/show_bug.cgi?id=323841


Repository: kde-baseapps


Description
-------

If the user sets a maximum number of text lines in the settings, this number is translated into a maximum height in pixels using QFontMetrics::lineSpacing().
 
In In KStandardItemListWidgetInformant::itemSizeHint(int, const KItemListView*), this maximum height limits the size that is reserved for the item.

However, in KStandardItemListWidget::updateIconsLayoutTextCache(), the maximum height is translated back into a maximum number of lines, which limits the number of lines that are created using the QTextLayout.

This approach can lead to problems if the real height of the layouted text is 1 pixel more or less than QFontMetrics::lineSpacing() times "number of lines". According to Christoph's analysis, this is the reason for the blurry icons in bug 323841.

I propose to change this by not storing a "maximum height" inside the "maximum size" explicitly, but store a maximum number of lines and a maximum with (for Compact View) separately, and then use the number of lines also to calculate the required size in KStandardItemListWidgetInformant::itemSizeHint(int, const KItemListView*). This should make sure that the correct height is reserved for each item. It also makes the code a bit simpler IMHO.


Diffs
-----

  dolphin/src/kitemviews/kitemliststyleoption.h 1a304fc 
  dolphin/src/kitemviews/kitemliststyleoption.cpp ac25879 
  dolphin/src/kitemviews/kitemlistview.cpp 7f49721 
  dolphin/src/kitemviews/kstandarditemlistwidget.cpp 302150f 
  dolphin/src/views/dolphinitemlistview.cpp 4799d76 

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


Testing
-------

I could not see any regressions. I was not able to reproduce the bug, so I cannot really verify if this patch really fixes it.

@Christoph: your comment in bug 323841 sounds like you could reproduce the problem? Could you have a look at this patch? Thanks in advance!


Thanks,

Frank Reininghaus

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