From 9460038b3ff1aa3767924b6ef44be4e0da7f9b39 Mon Sep 17 00:00:00 2001
From: Vadim Zeitlin <vadim@wxwidgets.org>
Date: Sun, 4 Nov 2018 17:50:55 +0100
Subject: [PATCH] Return empty rectangle from GetItemRect() if item is not
 visible

This was already the case if the item was not visible because its parent
was not expanded, but now make it also true for the items which are not
visible due to the current scrollbar position.

Add unit tests checking for this and also verifying that GetItemRect()
returns the coordinates in the physical window coordinates.
---
 interface/wx/dataview.h             |  5 +++--
 src/generic/datavgen.cpp            | 15 +++++++++++++--
 tests/controls/dataviewctrltest.cpp | 27 +++++++++++++++++++++++++++
 3 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/interface/wx/dataview.h b/interface/wx/dataview.h
index 1a8ab039ae..85bc40acdb 100644
--- a/interface/wx/dataview.h
+++ b/interface/wx/dataview.h
@@ -1448,8 +1448,9 @@ public:
     /**
         Returns item rectangle.
 
-        If item is not currently visible because its parent is collapsed,
-        return an empty rectangle.
+        If item is not currently visible, either because its parent is
+        collapsed or it is outside of the visible part of the control due to
+        the current vertical scrollbar position, return an empty rectangle.
 
         Coordinates of the rectangle are specified in wxDataViewCtrl client
         area coordinates.
diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp
index d82c2f051f..41c3b7fbed 100644
--- a/src/generic/datavgen.cpp
+++ b/src/generic/datavgen.cpp
@@ -3839,6 +3839,14 @@ wxRect wxDataViewMainWindow::GetItemRect( const wxDataViewItem & item,
     GetOwner()->CalcScrolledPosition(  itemRect.x,  itemRect.y,
                                       &itemRect.x, &itemRect.y );
 
+    // Check if the rectangle is completely outside of the currently visible
+    // area and, if so, return an empty rectangle to indicate that the item is
+    // not visible.
+    if ( itemRect.GetBottom() < 0 || itemRect.GetTop() > GetClientSize().y )
+    {
+        return wxRect();
+    }
+
     return itemRect;
 }
 
@@ -5853,8 +5861,11 @@ wxRect wxDataViewCtrl::GetItemRect( const wxDataViewItem & item,
     // Convert position from the main window coordinates to the control coordinates.
     // (They can be different due to the presence of the header.).
     wxRect r = m_clientArea->GetItemRect(item, column);
-    const wxPoint ctrlPos = ScreenToClient(m_clientArea->ClientToScreen(r.GetPosition()));
-    r.SetPosition(ctrlPos);
+    if ( r.width || r.height )
+    {
+        const wxPoint ctrlPos = ScreenToClient(m_clientArea->ClientToScreen(r.GetPosition()));
+        r.SetPosition(ctrlPos);
+    }
     return r;
 }
 
diff --git a/tests/controls/dataviewctrltest.cpp b/tests/controls/dataviewctrltest.cpp
index 720fdce938..f7fcaca67e 100644
--- a/tests/controls/dataviewctrltest.cpp
+++ b/tests/controls/dataviewctrltest.cpp
@@ -208,6 +208,9 @@ TEST_CASE_METHOD(SingleSelectDataViewCtrlTestCase,
     const wxRect rect1 = m_dvc->GetItemRect(m_child1);
     const wxRect rect2 = m_dvc->GetItemRect(m_child2);
 
+    CHECK( rect1 != wxRect() );
+    CHECK( rect2 != wxRect() );
+
     CHECK( rect1.x == rect2.x );
     CHECK( rect1.width == rect2.width );
     CHECK( rect1.height == rect2.height );
@@ -215,6 +218,30 @@ TEST_CASE_METHOD(SingleSelectDataViewCtrlTestCase,
 
     const wxRect rectNotShown = m_dvc->GetItemRect(m_grandchild);
     CHECK( rectNotShown == wxRect() );
+
+    // Append enough items to make the window scrollable.
+    for ( int i = 3; i < 100; ++i )
+        m_dvc->AppendItem(m_root, wxString::Format("child%d", i));
+
+    const wxDataViewItem last = m_dvc->AppendItem(m_root, "last");
+
+    // This should scroll the window to bring this item into view.
+    m_dvc->EnsureVisible(last);
+
+    // Check that this was indeed the case.
+    const wxDataViewItem top = m_dvc->GetTopItem();
+    CHECK( top != m_root );
+
+    // Verify that the coordinates are returned in physical coordinates of the
+    // window and not the logical coordinates affected by scrolling.
+    const wxRect rectScrolled = m_dvc->GetItemRect(top);
+    CHECK( rectScrolled.GetBottom() > 0 );
+    CHECK( rectScrolled.GetTop() <= m_dvc->GetClientSize().y );
+
+    // Also check that the root item is not currently visible (because it's
+    // scrolled off).
+    const wxRect rectRoot = m_dvc->GetItemRect(m_root);
+    CHECK( rectRoot == wxRect() );
 #endif
 }