Merge branch 'listctrl-itemrect'

Improve wxListCtrl::GetSubItemRect() and add a unit test for it.

See https://github.com/wxWidgets/wxWidgets/pull/1511

include/wx/generic/private/listctrl.h

@@ -649,7 +649,8 @@ public:
     {
         return GetSubItemRect(item, wxLIST_GETSUBITEMRECT_WHOLEITEM, rect);
     }
-    bool GetSubItemRect( long item, long subItem, wxRect& rect ) const;
+    bool GetSubItemRect( long item, long subItem, wxRect& rect,
+                         int code = wxLIST_RECT_BOUNDS ) const;
     wxRect GetViewRect() const;
     bool GetItemPosition( long item, wxPoint& pos ) const;
     int GetSelectedItemCount() const;

interface/wx/listctrl.h

@@ -756,6 +756,11 @@ public:
         @a code can be one of @c wxLIST_RECT_BOUNDS, @c wxLIST_RECT_ICON or
         @c wxLIST_RECT_LABEL.
 
+        Note that using @c wxLIST_RECT_ICON with any sub-item but the first one
+        isn't very useful as only the first sub-item can have an icon in
+        wxListCtrl. In this case, i.e. for @c subItem > 0, this function simply
+        returns an empty rectangle in @a rect.
+
         @since 2.7.0
     */
     bool GetSubItemRect(long item, long subItem, wxRect& rect,

src/generic/listctrl.cpp

@@ -83,6 +83,15 @@ static const int EXTRA_HEIGHT = 4;
 static const int EXTRA_BORDER_X = 2;
 static const int EXTRA_BORDER_Y = 2;
 
+#ifdef __WXGTK__
+    // This probably needs to be done
+    // on all platforms as the icons
+    // otherwise nearly touch the border
+    static const int ICON_OFFSET_X = 2;
+#else
+    static const int ICON_OFFSET_X = 0;
+#endif
+
 // offset for the header window
 static const int HEADER_OFFSET_X = 0;
 static const int HEADER_OFFSET_Y = 0;
@@ -800,16 +809,12 @@ void wxListLineData::DrawInReportMode( wxDC *dc,
     //       different columns - to do it, just add "col" argument to
     //       GetAttr() and move these lines into the loop below
 
+    // Note: GetSubItemRect() needs to be modified if the layout here changes.
+
     ApplyAttributes(dc, rectHL, highlighted, current);
 
-    wxCoord x = rect.x + HEADER_OFFSET_X,
+    wxCoord x = rect.x + HEADER_OFFSET_X + ICON_OFFSET_X,
             yMid = rect.y + rect.height/2;
-#ifdef __WXGTK__
-    // This probably needs to be done
-    // on all platforms as the icons
-    // otherwise nearly touch the border
-    x += 2;
-#endif
 
     if ( m_owner->HasCheckBoxes() )
     {
@@ -3675,7 +3680,8 @@ wxRect wxListMainWindow::GetViewRect() const
 }
 
 bool
-wxListMainWindow::GetSubItemRect(long item, long subItem, wxRect& rect) const
+wxListMainWindow::GetSubItemRect(long item, long subItem, wxRect& rect,
+                                 int code) const
 {
     wxCHECK_MSG( subItem == wxLIST_GETSUBITEMRECT_WHOLEITEM || InReportView(),
                  false,
@@ -3703,6 +3709,51 @@ wxListMainWindow::GetSubItemRect(long item, long subItem, wxRect& rect) const
             rect.x += GetColumnWidth(i);
         }
         rect.width = GetColumnWidth(subItem);
+
+        switch ( code )
+        {
+            case wxLIST_RECT_BOUNDS:
+                // Nothing to do.
+                break;
+
+            case wxLIST_RECT_ICON:
+            case wxLIST_RECT_LABEL:
+                // Note: this needs to be kept in sync with DrawInReportMode().
+                {
+                    rect.x += ICON_OFFSET_X;
+                    rect.width -= ICON_OFFSET_X;
+
+                    wxListLineData* const line = GetLine(item);
+                    if ( subItem == 0 && line->HasImage() )
+                    {
+                        int ix, iy;
+                        GetImageSize(line->GetImage(), ix, iy);
+
+                        const int iconWidth = ix + IMAGE_MARGIN_IN_REPORT_MODE;
+
+                        if ( code == wxLIST_RECT_ICON )
+                        {
+                            rect.width = iconWidth;
+                        }
+                        else // wxLIST_RECT_LABEL
+                        {
+                            rect.x += iconWidth;
+                            rect.width -= iconWidth;
+                        }
+                    }
+                    else // No icon
+                    {
+                        if ( code == wxLIST_RECT_ICON )
+                            rect = wxRect();
+                        //else: label rect is the same as the full one
+                    }
+                }
+                break;
+
+            default:
+                wxFAIL_MSG(wxS("Unknown rectangle requested"));
+                return false;
+        }
     }
 
     GetListCtrl()->CalcScrolledPosition(rect.x, rect.y, &rect.x, &rect.y);
@@ -4463,7 +4514,7 @@ int wxListMainWindow::GetItemWidthWithImage(wxListItem * item)
     {
         int ix, iy;
         GetImageSize( item->GetImage(), ix, iy );
-        width += ix + 5;
+        width += ix + IMAGE_MARGIN_IN_REPORT_MODE;
     }
 
     if (!item->GetText().empty())
@@ -5030,9 +5081,9 @@ bool wxGenericListCtrl::GetItemRect(long item, wxRect& rect, int code) const
 bool wxGenericListCtrl::GetSubItemRect(long item,
                                        long subItem,
                                        wxRect& rect,
-                                       int WXUNUSED(code)) const
+                                       int code) const
 {
-    if ( !m_mainWin->GetSubItemRect( item, subItem, rect ) )
+    if ( !m_mainWin->GetSubItemRect( item, subItem, rect, code ) )
         return false;
 
     if ( m_mainWin->HasHeader() )

src/msw/listctrl.cpp

@@ -1223,16 +1223,33 @@ bool wxListCtrl::GetSubItemRect(long item, long subItem, wxRect& rect, int code)
                  wxT("invalid item in GetSubItemRect") );
 
     int codeWin;
-    if ( code == wxLIST_RECT_BOUNDS )
+    switch ( code )
-        codeWin = LVIR_BOUNDS;
-    else if ( code == wxLIST_RECT_ICON )
-        codeWin = LVIR_ICON;
-    else if ( code == wxLIST_RECT_LABEL )
-        codeWin = LVIR_LABEL;
-    else
     {
-        wxFAIL_MSG( wxT("incorrect code in GetItemRect() / GetSubItemRect()") );
+        case wxLIST_RECT_BOUNDS:
             codeWin = LVIR_BOUNDS;
+            break;
+
+        case wxLIST_RECT_ICON:
+            // Only the first subitem can have an icon, so it doesn't make
+            // sense to query the native control for the other ones --
+            // especially because it returns a nonsensical non-empty icon
+            // rectangle for them.
+            if ( subItem > 0 )
+            {
+                rect = wxRect();
+                return true;
+            }
+
+            codeWin = LVIR_ICON;
+            break;
+
+        case wxLIST_RECT_LABEL:
+            codeWin = LVIR_LABEL;
+            break;
+
+        default:
+            wxFAIL_MSG( wxT("incorrect code in GetItemRect() / GetSubItemRect()") );
+            return false;
     }
 
     RECT rectWin;
@@ -1248,19 +1265,6 @@ bool wxListCtrl::GetSubItemRect(long item, long subItem, wxRect& rect, int code)
         return false;
     }
 
-    // Although LVIR_LABEL exists, it returns the same results as LVIR_BOUNDS
-    // and not just the label rectangle as would be expected, so account for
-    // the icon ourselves in this case.
-    if ( code == wxLIST_RECT_LABEL )
-    {
-        RECT rectIcon;
-        if ( !wxGetListCtrlSubItemRect(GetHwnd(), item, subItem, LVIR_ICON,
-                                       rectIcon) )
-            return false;
-
-        rectWin.left = rectIcon.right;
-    }
-
     wxCopyRECTToRect(rectWin, rect);
 
     // there is no way to retrieve the first sub item bounding rectangle using

tests/controls/listctrltest.cpp

@@ -24,6 +24,8 @@
 #endif // WX_PRECOMP
 
 #include "wx/listctrl.h"
+#include "wx/artprov.h"
+#include "wx/imaglist.h"
 #include "listbasetest.h"
 #include "testableframe.h"
 #include "wx/uiaction.h"
@@ -48,9 +50,11 @@ private:
         CPPUNIT_TEST( EditLabel );
         WXUISIM_TEST( ColumnClick );
         WXUISIM_TEST( ColumnDrag );
+        CPPUNIT_TEST( SubitemRect );
     CPPUNIT_TEST_SUITE_END();
 
     void EditLabel();
+    void SubitemRect();
 #if wxUSE_UIACTIONSIMULATOR
     // Column events are only supported in wxListCtrl currently so we test them
     // here rather than in ListBaseTest
@@ -93,6 +97,51 @@ void ListCtrlTestCase::EditLabel()
     m_list->EditLabel(0);
 }
 
+void ListCtrlTestCase::SubitemRect()
+{
+    wxBitmap bmp = wxArtProvider::GetBitmap(wxART_ERROR);
+
+    wxImageList* const iml = new wxImageList(bmp.GetWidth(), bmp.GetHeight());
+    iml->Add(bmp);
+    m_list->AssignImageList(iml, wxIMAGE_LIST_SMALL);
+
+    m_list->InsertColumn(0, "Column 0");
+    m_list->InsertColumn(1, "Column 1");
+    m_list->InsertColumn(2, "Column 2");
+    for ( int i = 0; i < 3; i++ )
+    {
+        long index = m_list->InsertItem(i, wxString::Format("This is item %d", i), 0);
+        m_list->SetItem(index, 1, wxString::Format("Column 1 item %d", i));
+        m_list->SetItem(index, 2, wxString::Format("Column 2 item %d", i));
+    }
+
+    wxRect rectLabel, rectIcon, rectItem;
+
+    // First check a subitem with an icon: it should have a valid icon
+    // rectangle and the label rectangle should be adjacent to it.
+    m_list->GetSubItemRect(1, 0, rectItem, wxLIST_RECT_BOUNDS);
+    m_list->GetSubItemRect(1, 0, rectIcon, wxLIST_RECT_ICON);
+    m_list->GetSubItemRect(1, 0, rectLabel, wxLIST_RECT_LABEL);
+
+    CHECK(!rectIcon.IsEmpty());
+    // Note that we can't use "==" here, in the native MSW version there is a
+    // gap between the item rectangle and the icon one.
+    CHECK(rectIcon.GetLeft() >= rectItem.GetLeft());
+    CHECK(rectLabel.GetLeft() == rectIcon.GetRight() + 1);
+    CHECK(rectLabel.GetRight() == rectItem.GetRight());
+
+    // For a subitem without an icon, label rectangle is the same one as the
+    // entire item one and the icon rectangle should be empty.
+    m_list->GetSubItemRect(1, 1, rectItem, wxLIST_RECT_BOUNDS);
+    m_list->GetSubItemRect(1, 1, rectIcon, wxLIST_RECT_ICON);
+    m_list->GetSubItemRect(1, 1, rectLabel, wxLIST_RECT_LABEL);
+
+    CHECK(rectIcon.IsEmpty());
+    // Here we can't check for exact equality neither as there can be a margin.
+    CHECK(rectLabel.GetLeft() >= rectItem.GetLeft());
+    CHECK(rectLabel.GetRight() == rectItem.GetRight());
+}
+
 #if wxUSE_UIACTIONSIMULATOR
 void ListCtrlTestCase::ColumnDrag()
 {