View Issue Details

IDProjectCategoryView StatusLast Update
0028212LazarusWidgetsetpublic2015-06-19 17:33
ReporterDavid Jenkins Assigned ToZeljan Rikalo  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
PlatformQtOSLinux 
Product Version1.4 
Summary0028212: TreeView item width set to -1 and do not show
DescriptionFixes for Mantis' 27043, 27696, and 26770 set the sizeHint for QTreeWidgetItems. The height is always set but the width is only being set if ImageIndex = -1. If ImageIndex > -1 then width is left at default -1.
Steps To ReproduceUse attached example project.
Additional InformationQt code assumes that if one sizeHint value (QSize.cy or QSize.cx) is set then both are valid and it will use those values and not do any calculations for height or width. This means that if one value is calculated and set then both need to be calculated and set. This problem first arose with the fix to 27043 that first changed sizeHint. Without that change then 27696, 26770, and this problem do not arise.

So I am suggesting that all the changes starting with 27043 be pulled out and a different approach put in (see attach diff files).

The change is to not set sizeHint but do the same thing through the ItemDelegateSizeHint. This allows the height to be modified to match imagelist.height without affecting the width that the qt layers calculates. I pull the desired value from option.decorationSize.

For this to work I had to add two lines of code in TQtWSCustomListView.setViewStyle that sets the size to ImageList.size for vsList and vsReport (similar to vsIcon and vsSmallIcon). I believe this is consistent with intent i.e. if an ImageList is assigned then that should be the desired height not QStylePM_ListViewIconsize.

If that is not true then the same thing could be done ItemDelegatesizeHint by querying the LCLObject for an appropriate ImageList and grabbing the height from there.

I also noticed that when QVariant_create() was called, if a invalid value was desired that it was created with QVariant_create(QVariantInvalid). QVariantInvalid is uint zero ('0') which means that a QVariant of type Uint is created with value '0' and if the QVariant.isValid() returns TRUE. I belive the proper method is 'bob = QVariant_create()'. I've included these changes in the attached diff.
TagsNo tags attached.
Fixed in Revision49362
LazTarget-
WidgetsetQT
Attached Files

Relationships

related to 0027043 closedZeljan Rikalo TListView does not allow setting item height with GTK2 and QT 
related to 0027696 resolvedZeljan Rikalo Tlistview view on QT only will not show text if icon is assigned from imagelist to tlistitem. 
related to 0026770 closedZeljan Rikalo TListView: icons are too smaal on QT Linux 

Activities

David Jenkins

2015-05-29 21:15

reporter  

example.zip (132,317 bytes)

David Jenkins

2015-05-29 21:15

reporter  

QtFix.diff (4,670 bytes)   
Index: Lazarus/laz-src/lcl/interfaces/qt/qtwscomctrls.pp
===================================================================
--- Lazarus/laz-src/lcl/interfaces/qt/qtwscomctrls.pp	(revision 20283)
+++ Lazarus/laz-src/lcl/interfaces/qt/qtwscomctrls.pp	(revision 20284)
@@ -1467,27 +1467,6 @@
     else
       AIconWidth := 0;
 
-    {issue #27696 for autosized columns we must provide sizehint}
-    if (ALV.ColumnCount > 0) and (ALV.Column[0].AutoSize) then
-      QtTreeWidget.SetItemSizeHint(TWI, 0, Str, AIconWidth);
-
-    // issue #27043
-    if (ALV.Items[AIndex].ImageIndex = -1) then
-    begin
-      AImages := TCustomListViewHack(ALV).LargeImages;
-      if not Assigned(AImages) then
-        AImages := TCustomListViewHack(ALV).SmallImages;
-      if Assigned(AImages) then
-      begin
-        AMetric := QStyle_pixelMetric(QApplication_style(), QStylePM_FocusFrameVMargin, nil, nil) * 2;
-        QTreeWidgetItem_sizeHint(TWI, @ASizeHint, 0);
-        ASizeHint.cy := AImages.Height + AMetric;
-        QTreeWidgetItem_setSizeHint(TWI, 0, @ASizeHint);
-        for i := 0 to AItem.SubItems.Count - 1 do
-          QTreeWidgetItem_setSizeHint(TWI, i + 1, @ASizeHint);
-      end;
-    end;
-
     for i := 0 to AItem.SubItems.Count - 1 do
     begin
       AAlignment := QtAlignLeft or QtAlignVCenter;
@@ -1498,9 +1477,6 @@
         Str := GetUtf8String(AItem.Subitems.Strings[i]);
         QtTreeWidget.setItemText(TWI, i + 1, Str, AAlignment);
         QtTreeWidget.setItemData(TWI, i + 1, AItem);
-        {issue #27696 for autosized columns we must provide sizehint}
-        if (i + 1 < ALV.ColumnCount) and (ALV.Column[i + 1].AutoSize) then
-          QtTreeWidget.SetItemSizeHint(TWI, i + 1, Str, AIconWidth);
       end;
     end;
 
@@ -1546,17 +1522,6 @@
       if (TCustomListViewHack(ALV).Columns.Count > 0) and (ASubIndex < TCustomListViewHack(ALV).Columns.Count)  then
         AAlignment := AlignmentToQtAlignmentMap[ALV.Column[ASubIndex].Alignment]  or QtAlignVCenter;
       QtTreeWidget.setItemText(TWI, ASubIndex, Str, AAlignment);
-      {issue #27696 for autosized columns we must provide sizehint}
-      if (TCustomListViewHack(ALV).Columns.Count > 0) and (ASubIndex >= 0) and
-        (ASubIndex < TCustomListViewHack(ALV).Columns.Count) and
-        ALV.Column[ASubIndex].AutoSize then
-      begin
-        if Assigned(TCustomListViewHack(ALV).SmallImages) then
-          AIconWidth := TCustomListViewHack(ALV).SmallImages.Width
-        else
-          AIconWidth := 0;
-        QtTreeWidget.SetItemSizeHint(TWI, ASubIndex, Str, AIconWidth);
-      end;
     end;
   end;
 end;
@@ -2305,6 +2270,11 @@
         x := GetPixelMetric(QStylePM_ListViewIconSize, nil, ItemViewWidget);
         Size.cx := x;
         Size.cy := x;
+        if Assigned(TCustomListViewHack(ALV).SmallImages) then
+        begin
+          Size.cy := TCustomListViewHack(ALV).SmallImages.Height;
+          Size.cx := TCustomListViewHack(ALV).SmallImages.Width;
+        end;
         TQtAbstractItemView(ALV.Handle).OwnerDrawn :=
           TCustomListViewHack(ALV).IsCustomDrawn(dtControl, cdPrePaint) or
           (TCustomListViewHack(ALV).OwnerDraw and
Index: Lazarus/laz-src/lcl/interfaces/qt/qtwidgets.pas
===================================================================
--- Lazarus/laz-src/lcl/interfaces/qt/qtwidgets.pas	(revision 20283)
+++ Lazarus/laz-src/lcl/interfaces/qt/qtwidgets.pas	(revision 20284)
@@ -1911,6 +1911,7 @@
 
 uses
   Buttons,
+  math,
   qtCaret,
   qtproc,
   qtprivate,
@@ -5166,7 +5167,7 @@
 var
   AVariant: QVariantH;
 begin
-  AVariant := QVariant_create(QVariantInvalid);
+  AVariant := QVariant_create;
   QObject_setProperty(AObject, APropName, AVariant);
   QVariant_destroy(AVariant);
 end;
@@ -14342,7 +14343,7 @@
   v: QVariantH;
 begin
   if Data = nil then
-    v := QVariant_create(QVariantInvalid)
+    v := QVariant_create
   else
     v := QVariant_create(Int64({%H-}PtrUInt(Data)));
   QTreeWidgetItem_setData(AItem, AColumn, ARole, v);
@@ -17781,10 +17782,14 @@
 var
   Msg: TLMMeasureItem;
   MeasureItemStruct: TMeasureItemStruct;
+  decorationSize: TSize;
+  Metric: Integer;
 begin
+  QStyleOptionViewItem_decorationSize(option, @decorationSize);
+  Metric := QStyle_pixelMetric(QApplication_style(), QStylePM_FocusFrameVMargin, nil, nil) * 2;
   MeasureItemStruct.itemID := UINT(QModelIndex_row(index));
   MeasureItemStruct.itemWidth := UINT(Size^.cx);
-  MeasureItemStruct.itemHeight := UINT(Size^.cy);
+  MeasureItemStruct.itemHeight := UINT(Max(Size^.cy, (decorationSize.cy + Metric)));
   Msg.Msg := LM_MEASUREITEM;
   Msg.MeasureItemStruct := @MeasureItemStruct;
   DeliverMessage(Msg);
QtFix.diff (4,670 bytes)   

David Jenkins

2015-05-29 21:50

reporter   ~0084112

Note: I tested the attached fix against the examples from 27043, 27696, and 26770 and the attached project. It works for all those cases.

David Jenkins

2015-06-01 19:13

reporter  

QtTreeWidget.diff (6,109 bytes)   
Index: latest/lcl/interfaces/qt/qtwidgets.pas
===================================================================
--- latest/lcl/interfaces/qt/qtwidgets.pas	(revision 20285)
+++ latest/lcl/interfaces/qt/qtwidgets.pas	(working copy)
@@ -1461,8 +1461,6 @@
     function currentItem: QTreeWidgetItemH;
     procedure setCurrentItem(AItem: QTreeWidgetItemH);
 
-    function SetItemSizeHint(AItem: QTreeWidgetItemH;
-      AColumn: integer; AText: widestring; AIconSize: integer): boolean;
     function getRow(AItem: QTreeWidgetItemH): integer;
     function headerItem: QTreeWidgetItemH;
     function itemAt(APoint: TPoint): QTreeWidgetItemH; overload;
@@ -1911,6 +1909,7 @@
 
 uses
   Buttons,
+  math,
   qtCaret,
   qtproc,
   qtprivate,
@@ -5164,7 +5163,7 @@
 var
   AVariant: QVariantH;
 begin
-  AVariant := QVariant_create(QVariantInvalid);
+  AVariant := QVariant_create;
   QObject_setProperty(AObject, APropName, AVariant);
   QVariant_destroy(AVariant);
 end;
@@ -14180,33 +14179,6 @@
   QTreeWidget_setCurrentItem(QTreeWidgetH(Widget), AItem);
 end;
 
-function TQtTreeWidget.SetItemSizeHint(AItem: QTreeWidgetItemH;
-  AColumn: integer; AText: widestring; AIconSize: integer): boolean;
-var
-  R: TRect;
-  ATextWidth: Integer;
-  AMargin: Integer;
-  ASizeHint: TSize;
-begin
-  Result := False;
-  if AIconSize = 0 then
-    exit;
-  R := measureText(AText, 0);
-  ATextWidth := R.Right - R.Left;
-  if AIconSize > 0 then
-  begin
-    AMargin := QStyle_pixelMetric(QApplication_style(), QStylePM_ButtonMargin, nil, Widget);
-    if AColumn = 0 then
-      ATextWidth += AIconSize + (AMargin * 2)
-    else
-      ATextWidth += AMargin;
-  end;
-  QTreeWidgetItem_sizeHint(AItem, @ASizeHint, AColumn);
-  ASizeHint.cx := ATextWidth;
-  QTreeWidgetItem_setSizeHint(AItem, AColumn, @ASizeHint);
-  Result := True;
-end;
-
 function TQtTreeWidget.getRow(AItem: QTreeWidgetItemH): integer;
 begin
   Result := QTreeWidget_indexOfTopLevelItem(QTreeWidgetH(Widget), AItem);
@@ -14340,7 +14312,7 @@
   v: QVariantH;
 begin
   if Data = nil then
-    v := QVariant_create(QVariantInvalid)
+    v := QVariant_create
   else
     v := QVariant_create(Int64({%H-}PtrUInt(Data)));
   QTreeWidgetItem_setData(AItem, AColumn, ARole, v);
@@ -17769,10 +17741,14 @@
 var
   Msg: TLMMeasureItem;
   MeasureItemStruct: TMeasureItemStruct;
+  decorationSize: TSize;
+  Metric: Integer;
 begin
+  QStyleOptionViewItem_decorationSize(option, @decorationSize);
+  Metric := QStyle_pixelMetric(QApplication_style(), QStylePM_FocusFrameVMargin, nil, nil) * 2;
   MeasureItemStruct.itemID := UINT(QModelIndex_row(index));
   MeasureItemStruct.itemWidth := UINT(Size^.cx);
-  MeasureItemStruct.itemHeight := UINT(Size^.cy);
+  MeasureItemStruct.itemHeight := UINT(Max(Size^.cy, (decorationSize.cy + Metric)));
   Msg.Msg := LM_MEASUREITEM;
   Msg.MeasureItemStruct := @MeasureItemStruct;
   DeliverMessage(Msg);
Index: latest/lcl/interfaces/qt/qtwscomctrls.pp
===================================================================
--- latest/lcl/interfaces/qt/qtwscomctrls.pp	(revision 20285)
+++ latest/lcl/interfaces/qt/qtwscomctrls.pp	(working copy)
@@ -1467,27 +1467,6 @@
     else
       AIconWidth := 0;
 
-    {issue #27696 for autosized columns we must provide sizehint}
-    if (ALV.ColumnCount > 0) and (ALV.Column[0].AutoSize) then
-      QtTreeWidget.SetItemSizeHint(TWI, 0, Str, AIconWidth);
-
-    // issue #27043
-    if (ALV.Items[AIndex].ImageIndex = -1) then
-    begin
-      AImages := TCustomListViewHack(ALV).LargeImages;
-      if not Assigned(AImages) then
-        AImages := TCustomListViewHack(ALV).SmallImages;
-      if Assigned(AImages) then
-      begin
-        AMetric := QStyle_pixelMetric(QApplication_style(), QStylePM_FocusFrameVMargin, nil, nil) * 2;
-        QTreeWidgetItem_sizeHint(TWI, @ASizeHint, 0);
-        ASizeHint.cy := AImages.Height + AMetric;
-        QTreeWidgetItem_setSizeHint(TWI, 0, @ASizeHint);
-        for i := 0 to AItem.SubItems.Count - 1 do
-          QTreeWidgetItem_setSizeHint(TWI, i + 1, @ASizeHint);
-      end;
-    end;
-
     for i := 0 to AItem.SubItems.Count - 1 do
     begin
       AAlignment := QtAlignLeft or QtAlignVCenter;
@@ -1498,9 +1477,6 @@
         Str := GetUtf8String(AItem.Subitems.Strings[i]);
         QtTreeWidget.setItemText(TWI, i + 1, Str, AAlignment);
         QtTreeWidget.setItemData(TWI, i + 1, AItem);
-        {issue #27696 for autosized columns we must provide sizehint}
-        if (i + 1 < ALV.ColumnCount) and (ALV.Column[i + 1].AutoSize) then
-          QtTreeWidget.SetItemSizeHint(TWI, i + 1, Str, AIconWidth);
       end;
     end;
 
@@ -1546,17 +1522,6 @@
       if (TCustomListViewHack(ALV).Columns.Count > 0) and (ASubIndex < TCustomListViewHack(ALV).Columns.Count)  then
         AAlignment := AlignmentToQtAlignmentMap[ALV.Column[ASubIndex].Alignment]  or QtAlignVCenter;
       QtTreeWidget.setItemText(TWI, ASubIndex, Str, AAlignment);
-      {issue #27696 for autosized columns we must provide sizehint}
-      if (TCustomListViewHack(ALV).Columns.Count > 0) and (ASubIndex >= 0) and
-        (ASubIndex < TCustomListViewHack(ALV).Columns.Count) and
-        ALV.Column[ASubIndex].AutoSize then
-      begin
-        if Assigned(TCustomListViewHack(ALV).SmallImages) then
-          AIconWidth := TCustomListViewHack(ALV).SmallImages.Width
-        else
-          AIconWidth := 0;
-        QtTreeWidget.SetItemSizeHint(TWI, ASubIndex, Str, AIconWidth);
-      end;
     end;
   end;
 end;
@@ -2302,9 +2267,14 @@
       end;
     vsList, vsReport:
       begin
-        x := GetPixelMetric(QStylePM_ListViewIconSize, nil, ItemViewWidget);
+        x := 0;
         Size.cx := x;
         Size.cy := x;
+        if Assigned(TCustomListViewHack(ALV).SmallImages) then
+        begin
+          Size.cy := TCustomListViewHack(ALV).SmallImages.Height;
+          Size.cx := TCustomListViewHack(ALV).SmallImages.Width;
+        end;
         TQtAbstractItemView(ALV.Handle).OwnerDrawn :=
           TCustomListViewHack(ALV).IsCustomDrawn(dtControl, cdPrePaint) or
           (TCustomListViewHack(ALV).OwnerDraw and
QtTreeWidget.diff (6,109 bytes)   

David Jenkins

2015-06-01 19:20

reporter   ~0084158

I redid the patch. The original patch was against our local versions of qtwidgets.pas and qtwscomctrls.pp. The new patch (QtTreeWidget.diff) is against the 1.4 release version. The QtFix.diff patch can be removed.

Also I changed the default value of IconSize for vsList and vsReport in TQtWSCustomListView.setViewStyle from QStylePM-ListViewIconSize to 0. In the case that there are no icons in the ListView then we want the height to be set by the text height not the stylized icon height. I think there is no reason for LCL to use QStylePM_ListViewIconSize at all. I think we want the height to either be textheight (no ImageList) or ImageList.Height if an imagelist is assigned.

Zeljan Rikalo

2015-06-18 13:20

developer   ~0084541

Please test and close if ok. I've modified a bit your patch....added text alignment to center when viewStyle=vsIcon, also removed unused vars. Thanks for the patch.

Issue History

Date Modified Username Field Change
2015-05-29 21:15 David Jenkins New Issue
2015-05-29 21:15 David Jenkins File Added: example.zip
2015-05-29 21:15 David Jenkins File Added: QtFix.diff
2015-05-29 21:50 David Jenkins Note Added: 0084112
2015-05-30 00:51 Juha Manninen Relationship added related to 0027043
2015-05-30 00:51 Juha Manninen Relationship added related to 0027696
2015-05-30 00:52 Juha Manninen Relationship added related to 0026770
2015-05-30 08:51 Zeljan Rikalo Assigned To => Zeljan Rikalo
2015-05-30 08:51 Zeljan Rikalo Status new => assigned
2015-06-01 19:13 David Jenkins File Added: QtTreeWidget.diff
2015-06-01 19:20 David Jenkins Note Added: 0084158
2015-06-18 13:20 Zeljan Rikalo Fixed in Revision => 49362
2015-06-18 13:20 Zeljan Rikalo LazTarget => -
2015-06-18 13:20 Zeljan Rikalo Note Added: 0084541
2015-06-18 13:20 Zeljan Rikalo Status assigned => resolved
2015-06-18 13:20 Zeljan Rikalo Resolution open => fixed
2015-06-19 17:33 David Jenkins Status resolved => closed