From eba26cf35ec88bc273806ce80ef876d9817456c9 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 27 Aug 2019 16:36:19 +0200 Subject: [PATCH 1/7] Remove unneeded workaround for wxLIST_RECT_LABEL in wxMSW The code added in https://github.com/wxWidgets/wxWidgets/pull/1461 is completely unnecessary, using LVIR_LABEL works just fine. --- src/msw/listctrl.cpp | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/msw/listctrl.cpp b/src/msw/listctrl.cpp index 653c375369..91d0985bf4 100644 --- a/src/msw/listctrl.cpp +++ b/src/msw/listctrl.cpp @@ -1248,19 +1248,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 From ba787af9328db1a9158f31d33bce5d65e357b270 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 27 Aug 2019 16:37:48 +0200 Subject: [PATCH 2/7] Replace a chain of "if"s with a "switch" in wxMSW wxListCtrl Also return false from GetSubItemRect() if an invalid code is passed in, instead of silently using wxLIST_RECT_BOUNDS instead of it. --- src/msw/listctrl.cpp | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/msw/listctrl.cpp b/src/msw/listctrl.cpp index 91d0985bf4..7e2512a7ad 100644 --- a/src/msw/listctrl.cpp +++ b/src/msw/listctrl.cpp @@ -1223,16 +1223,23 @@ bool wxListCtrl::GetSubItemRect(long item, long subItem, wxRect& rect, int code) wxT("invalid item in GetSubItemRect") ); int codeWin; - if ( code == wxLIST_RECT_BOUNDS ) - codeWin = LVIR_BOUNDS; - else if ( code == wxLIST_RECT_ICON ) - codeWin = LVIR_ICON; - else if ( code == wxLIST_RECT_LABEL ) - codeWin = LVIR_LABEL; - else + switch ( code ) { - wxFAIL_MSG( wxT("incorrect code in GetItemRect() / GetSubItemRect()") ); - codeWin = LVIR_BOUNDS; + case wxLIST_RECT_BOUNDS: + codeWin = LVIR_BOUNDS; + break; + + case wxLIST_RECT_ICON: + 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; From 0bf223be44628bba44ce359f4b3bc92685098fd7 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 27 Aug 2019 16:38:25 +0200 Subject: [PATCH 3/7] Return empty rect for wxLIST_RECT_ICON for subitems without icons It doesn't make sense to return anything else than an empty rectangle when querying the icon rectangle of the sub-items that can't show any icon. Also document this behaviour, just in case it's not obvious. --- interface/wx/listctrl.h | 5 +++++ src/msw/listctrl.cpp | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/interface/wx/listctrl.h b/interface/wx/listctrl.h index 6b9edb72d5..8ccaad95b1 100644 --- a/interface/wx/listctrl.h +++ b/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, diff --git a/src/msw/listctrl.cpp b/src/msw/listctrl.cpp index 7e2512a7ad..d1508e1f39 100644 --- a/src/msw/listctrl.cpp +++ b/src/msw/listctrl.cpp @@ -1230,6 +1230,16 @@ bool wxListCtrl::GetSubItemRect(long item, long subItem, wxRect& rect, int code) 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; From a87e596be79cdca74dd440207f3b102dca60f3e1 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 27 Aug 2019 17:09:58 +0200 Subject: [PATCH 4/7] Replace hardcoded number with a constant in generic wxListCtrl Use ICON_OFFSET_X instead of hardcoded 2. --- src/generic/listctrl.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/generic/listctrl.cpp b/src/generic/listctrl.cpp index fdf2cd43a1..aa3da04f48 100644 --- a/src/generic/listctrl.cpp +++ b/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; @@ -802,14 +811,8 @@ void wxListLineData::DrawInReportMode( wxDC *dc, 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() ) { From 9395d7f404cbf8d217ec9698dcb7b34eeba6781c Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 27 Aug 2019 17:11:08 +0200 Subject: [PATCH 5/7] Replace another hardcoded constant with a symbolic one Use the already existing IMAGE_MARGIN_IN_REPORT_MODE instead of "5". --- src/generic/listctrl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/generic/listctrl.cpp b/src/generic/listctrl.cpp index aa3da04f48..5b7c511bd2 100644 --- a/src/generic/listctrl.cpp +++ b/src/generic/listctrl.cpp @@ -4466,7 +4466,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()) From 32fe124899f341cb19ebacc64febc1ac4a61f7e8 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 27 Aug 2019 17:11:36 +0200 Subject: [PATCH 6/7] Add support for icon/label rectangles to generic wxListCtrl Honour the value of "code" parameter in GetSubItemRect(). --- include/wx/generic/private/listctrl.h | 3 +- src/generic/listctrl.cpp | 54 +++++++++++++++++++++++++-- 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/include/wx/generic/private/listctrl.h b/include/wx/generic/private/listctrl.h index ff6672fc99..c5741b6b13 100644 --- a/include/wx/generic/private/listctrl.h +++ b/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; diff --git a/src/generic/listctrl.cpp b/src/generic/listctrl.cpp index 5b7c511bd2..7e7e679158 100644 --- a/src/generic/listctrl.cpp +++ b/src/generic/listctrl.cpp @@ -809,6 +809,8 @@ 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 + ICON_OFFSET_X, @@ -3678,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, @@ -3706,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); @@ -5033,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() ) From 7e3bbedaa58dddbf491f2a6e6f342e885e33c430 Mon Sep 17 00:00:00 2001 From: oneeyeman1 Date: Sun, 11 Aug 2019 19:07:05 -0500 Subject: [PATCH 7/7] Add a test case for wxListCtrl::GetSubItemRect() Check that the results for the item icon/label rectangles are consistent. --- tests/controls/listctrltest.cpp | 49 +++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/controls/listctrltest.cpp b/tests/controls/listctrltest.cpp index 0c31dcb189..da55648f2a 100644 --- a/tests/controls/listctrltest.cpp +++ b/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() {