From 6d6f7cc1ec70de954c7ae065740e6c7a418b8f24 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 12 Oct 2019 16:03:31 +0200 Subject: [PATCH 01/23] Allow running all wxGrid tests using wxUIActionSimulator in wxGTK These tests are still disabled by default during run-time, but at least allow explicitly enabling them (by setting WX_UI_TESTS=1) even when using wxGTK where they're known to fail. --- tests/controls/gridtest.cpp | 34 ++++++++++------------------------ 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/tests/controls/gridtest.cpp b/tests/controls/gridtest.cpp index 8d7f9fe671..1cbbcb5c91 100644 --- a/tests/controls/gridtest.cpp +++ b/tests/controls/gridtest.cpp @@ -23,20 +23,6 @@ #include "asserthelper.h" #include "wx/uiaction.h" -// FIXME: A lot of mouse-related tests sporadically fail in wxGTK. This happens -// almost all the time but sometimes the tests do pass and the failure -// doesn't happen when debugging so this looks like some kind of event -// dispatching/simulating problem rather than a real problem in wxGrid. -// -// Just disable these tests for now but it would be really great to -// really fix the problem. -#ifdef __WXGTK__ - #define NONGTK_TEST(test) -#else - #define NONGTK_TEST(test) WXUISIM_TEST(test) -#endif - - class GridTestCase : public CppUnit::TestCase { public: @@ -48,13 +34,13 @@ public: private: CPPUNIT_TEST_SUITE( GridTestCase ); WXUISIM_TEST( CellEdit ); - NONGTK_TEST( CellClick ); - NONGTK_TEST( ReorderedColumnsCellClick ); - NONGTK_TEST( CellSelect ); - NONGTK_TEST( LabelClick ); - NONGTK_TEST( SortClick ); + WXUISIM_TEST( CellClick ); + WXUISIM_TEST( ReorderedColumnsCellClick ); + WXUISIM_TEST( CellSelect ); + WXUISIM_TEST( LabelClick ); + WXUISIM_TEST( SortClick ); WXUISIM_TEST( Size ); - NONGTK_TEST( RangeSelect ); + WXUISIM_TEST( RangeSelect ); CPPUNIT_TEST( Cursor ); CPPUNIT_TEST( Selection ); CPPUNIT_TEST( AddRowCol ); @@ -71,15 +57,15 @@ private: WXUISIM_TEST( ResizeScrolledHeader ); WXUISIM_TEST( ColumnMinWidth ); CPPUNIT_TEST( PseudoTest_NativeHeader ); - NONGTK_TEST( LabelClick ); - NONGTK_TEST( SortClick ); + WXUISIM_TEST( LabelClick ); + WXUISIM_TEST( SortClick ); CPPUNIT_TEST( ColumnOrder ); WXUISIM_TEST( ResizeScrolledHeader ); WXUISIM_TEST( ColumnMinWidth ); CPPUNIT_TEST( DeleteAndAddRowCol ); CPPUNIT_TEST( PseudoTest_NativeLabels ); - NONGTK_TEST( LabelClick ); - NONGTK_TEST( SortClick ); + WXUISIM_TEST( LabelClick ); + WXUISIM_TEST( SortClick ); CPPUNIT_TEST( ColumnOrder ); WXUISIM_TEST( WindowAsEditorControl ); CPPUNIT_TEST_SUITE_END(); From 6530886d3137cb8072be8f8b4557a277ff1c6183 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 12 Oct 2019 17:27:05 +0200 Subject: [PATCH 02/23] Call XFlush() after simulating key press in wxGTK This makes simulating keys much more reliable, previously they were just completely lost (i.e. never resulted in key-press-event signal being generated by GTK) sometimes. --- src/unix/uiactionx11.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/unix/uiactionx11.cpp b/src/unix/uiactionx11.cpp index 96fe8c0629..4722aff96b 100644 --- a/src/unix/uiactionx11.cpp +++ b/src/unix/uiactionx11.cpp @@ -341,7 +341,12 @@ bool wxUIActionSimulatorX11Impl::DoKey(int keycode, int modifiers, bool isDown) if ( xkeycode == NoSymbol ) return false; - return DoX11Key(xkeycode, modifiers, isDown); + if ( !DoX11Key(xkeycode, modifiers, isDown) ) + return false; + + XFlush(m_display); + + return true; } wxUIActionSimulator::wxUIActionSimulator() From 0656823e2af1610055351a8d8c2d532a86b6cee3 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 12 Oct 2019 17:34:35 +0200 Subject: [PATCH 03/23] Document that wxGrid::ShowCellEditControl() does not start editing This was sufficiently misleading that event our own wxGrid unit tests used this function in an attempt to start editing a grid cell -- even though it actually doesn't do it at all. Unfortunately documenting the surprising semantics of this functions looks like the best thing we can do because it appears to have always behaved like this and changing it now to actually show the cell editor control, i.e. starting to edit the cell, is almost certain to break some existing code. --- interface/wx/grid.h | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/interface/wx/grid.h b/interface/wx/grid.h index fafb571bb8..e11dfb6167 100644 --- a/interface/wx/grid.h +++ b/interface/wx/grid.h @@ -2866,8 +2866,14 @@ public: /** Enables or disables in-place editing of grid cell data. - The grid will issue either a @c wxEVT_GRID_EDITOR_SHOWN or - @c wxEVT_GRID_EDITOR_HIDDEN event. + Enabling in-place editing generates @c wxEVT_GRID_EDITOR_SHOWN and, if + it isn't vetoed by the application, shows the in-place editor which + allows the user to change the cell value. + + Disabling in-place editing does nothing if the in-place editor isn't + currently show, otherwise the @c wxEVT_GRID_EDITOR_HIDDEN event is + generated but, unlike the "shown" event, it can't be vetoed and the + in-place editor is dismissed unconditionally. */ void EnableCellEditControl(bool enable = true); @@ -3249,7 +3255,11 @@ public: void SetReadOnly(int row, int col, bool isReadOnly = true); /** - Displays the in-place cell edit control for the current cell. + Displays the active in-place cell edit control for the current cell + after it was hidden. + + Note that this method does @em not start editing the cell, this is only + done by EnableCellEditControl(). */ void ShowCellEditControl(); From 18c9d7375b041cb1b30e59a0a63582c25bfb8caa Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 12 Oct 2019 17:36:38 +0200 Subject: [PATCH 04/23] Remove useless calls to ShowCellEditControl() from wxGrid tests This function simply does nothing if the current cells is not always being edited, so it's completely unnecessary to call it. --- tests/controls/gridtest.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/controls/gridtest.cpp b/tests/controls/gridtest.cpp index 1cbbcb5c91..d46ab0ae98 100644 --- a/tests/controls/gridtest.cpp +++ b/tests/controls/gridtest.cpp @@ -162,7 +162,6 @@ void GridTestCase::CellEdit() m_grid->SetFocus(); m_grid->SetGridCursor(1, 1); - m_grid->ShowCellEditControl(); sim.Text("abab"); sim.Char(WXK_RETURN); @@ -756,7 +755,6 @@ void GridTestCase::Editable() m_grid->SetFocus(); m_grid->SetGridCursor(1, 1); - m_grid->ShowCellEditControl(); sim.Text("abab"); wxYield(); @@ -787,8 +785,6 @@ void GridTestCase::ReadOnly() CPPUNIT_ASSERT(m_grid->IsCurrentCellReadOnly()); - m_grid->ShowCellEditControl(); - sim.Text("abab"); wxYield(); From b66a4b994864b384bcb4825ceead778390c04287 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 12 Oct 2019 17:46:18 +0200 Subject: [PATCH 05/23] Add a menu item to start editing the cell to grid sample Show that this is done using EnableCellEditControl() and not ShowCellEditControl(), as might have been expected. --- samples/grid/griddemo.cpp | 7 +++++++ samples/grid/griddemo.h | 2 ++ 2 files changed, 9 insertions(+) diff --git a/samples/grid/griddemo.cpp b/samples/grid/griddemo.cpp index 4f5ea81c00..30617b9ee7 100644 --- a/samples/grid/griddemo.cpp +++ b/samples/grid/griddemo.cpp @@ -183,6 +183,7 @@ wxBEGIN_EVENT_TABLE( GridFrame, wxFrame ) EVT_MENU( ID_DELETEROW, GridFrame::DeleteSelectedRows ) EVT_MENU( ID_DELETECOL, GridFrame::DeleteSelectedCols ) EVT_MENU( ID_CLEARGRID, GridFrame::ClearGrid ) + EVT_MENU( ID_EDITCELL, GridFrame::EditCell ) EVT_MENU( ID_SETCORNERLABEL, GridFrame::SetCornerLabelValue ) EVT_MENU( ID_SHOWSEL, GridFrame::ShowSelection ) EVT_MENU( ID_SELCELLS, GridFrame::SelectCells ) @@ -372,6 +373,7 @@ GridFrame::GridFrame() editMenu->Append( ID_DELETEROW, "Delete selected ro&ws" ); editMenu->Append( ID_DELETECOL, "Delete selected co&ls" ); editMenu->Append( ID_CLEARGRID, "Cl&ear grid cell contents" ); + editMenu->Append( ID_EDITCELL, "&Edit current cell" ); editMenu->Append( ID_SETCORNERLABEL, "&Set corner label..." ); editMenu->AppendCheckItem( ID_FREEZE_OR_THAW, "Freeze up to cursor\tCtrl-F" ); @@ -1161,6 +1163,11 @@ void GridFrame::ClearGrid( wxCommandEvent& WXUNUSED(ev) ) grid->ClearGrid(); } +void GridFrame::EditCell( wxCommandEvent& WXUNUSED(ev) ) +{ + grid->EnableCellEditControl(); +} + void GridFrame::SetCornerLabelValue( wxCommandEvent& WXUNUSED(ev) ) { wxTextEntryDialog dialog(this, diff --git a/samples/grid/griddemo.h b/samples/grid/griddemo.h index 086136b7ed..9c7ab3e8f3 100644 --- a/samples/grid/griddemo.h +++ b/samples/grid/griddemo.h @@ -69,6 +69,7 @@ class GridFrame : public wxFrame void DeleteSelectedRows( wxCommandEvent& ); void DeleteSelectedCols( wxCommandEvent& ); void ClearGrid( wxCommandEvent& ); + void EditCell( wxCommandEvent& ); void SetCornerLabelValue( wxCommandEvent& ); void ShowSelection( wxCommandEvent& ); void SelectCells( wxCommandEvent& ); @@ -179,6 +180,7 @@ public: ID_DELETEROW, ID_DELETECOL, ID_CLEARGRID, + ID_EDITCELL, ID_SETCORNERLABEL, ID_SHOWSEL, ID_CHANGESEL, From 879d6e40b20da6d8cabd4380ccb30d1223132551 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 12 Oct 2019 16:07:11 +0200 Subject: [PATCH 06/23] Fix wxGrid cell editing unit test in wxGTK Wait until the in-place editor is actually shown and then wait again until is hidden. --- tests/controls/gridtest.cpp | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/controls/gridtest.cpp b/tests/controls/gridtest.cpp index d46ab0ae98..7c82e85b83 100644 --- a/tests/controls/gridtest.cpp +++ b/tests/controls/gridtest.cpp @@ -23,6 +23,10 @@ #include "asserthelper.h" #include "wx/uiaction.h" +#ifdef __WXGTK__ + #include "wx/stopwatch.h" +#endif // __WXGTK__ + class GridTestCase : public CppUnit::TestCase { public: @@ -163,11 +167,42 @@ void GridTestCase::CellEdit() m_grid->SetFocus(); m_grid->SetGridCursor(1, 1); + wxYield(); + sim.Text("abab"); + + // We need to wait until the editor is really shown under GTK, consider + // that it happens once it gets focus. +#ifdef __WXGTK__ + for ( wxStopWatch sw; wxWindow::FindFocus() == m_grid; ) + { + if ( sw.Time() > 250 ) + { + WARN("Editor control not shown until timeout expiration"); + break; + } + + wxYield(); + } +#endif // __WXGTK__ + sim.Char(WXK_RETURN); wxYield(); +#ifdef __WXGTK__ + for ( wxStopWatch sw; wxWindow::FindFocus() != m_grid; ) + { + if ( sw.Time() > 250 ) + { + WARN("Editor control not hidden until timeout expiration"); + break; + } + + wxYield(); + } +#endif // __WXGTK__ + CPPUNIT_ASSERT_EQUAL(1, created.GetCount()); CPPUNIT_ASSERT_EQUAL(1, changing.GetCount()); CPPUNIT_ASSERT_EQUAL(1, changed.GetCount()); From 97b3e6e50b773cfdbac9afdac22849586d2df3a8 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 12 Oct 2019 17:44:53 +0200 Subject: [PATCH 07/23] Check that the grid cell has the expected value in the test In addition to checking that we get the expected events, also verify that editing the cell actually worked. --- tests/controls/gridtest.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/controls/gridtest.cpp b/tests/controls/gridtest.cpp index 7c82e85b83..6e6a0da108 100644 --- a/tests/controls/gridtest.cpp +++ b/tests/controls/gridtest.cpp @@ -203,6 +203,8 @@ void GridTestCase::CellEdit() } #endif // __WXGTK__ + CHECK(m_grid->GetCellValue(1, 1) == "abab"); + CPPUNIT_ASSERT_EQUAL(1, created.GetCount()); CPPUNIT_ASSERT_EQUAL(1, changing.GetCount()); CPPUNIT_ASSERT_EQUAL(1, changed.GetCount()); From ddd7ef45d08e3a8b06c0b8c1a3064a8c4cfc24de Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 12 Oct 2019 17:45:41 +0200 Subject: [PATCH 08/23] Use CHECK() instead of CPPUNIT_ASSERT_EQUAL(), i.e. REQUIRE() Perform all the checks, even if one (or more) of them fails. --- tests/controls/gridtest.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/controls/gridtest.cpp b/tests/controls/gridtest.cpp index 6e6a0da108..bae013388d 100644 --- a/tests/controls/gridtest.cpp +++ b/tests/controls/gridtest.cpp @@ -205,9 +205,9 @@ void GridTestCase::CellEdit() CHECK(m_grid->GetCellValue(1, 1) == "abab"); - CPPUNIT_ASSERT_EQUAL(1, created.GetCount()); - CPPUNIT_ASSERT_EQUAL(1, changing.GetCount()); - CPPUNIT_ASSERT_EQUAL(1, changed.GetCount()); + CHECK(created.GetCount() == 1); + CHECK(changing.GetCount() == 1); + CHECK(changed.GetCount() == 1); #endif } From 2228caa8d4f5735dcbae0bc4a4642f6885a9e1d4 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 14 Oct 2019 23:40:08 +0200 Subject: [PATCH 09/23] Don't disable logging in the tests if WXTRACE is set Disabling logging makes WXTRACE useless, so avoid doing it in this case to facilitate debugging the code exercised by the tests. --- tests/test.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test.cpp b/tests/test.cpp index f5135f87fb..1d42529be5 100644 --- a/tests/test.cpp +++ b/tests/test.cpp @@ -577,11 +577,11 @@ bool TestApp::ProcessEvent(wxEvent& event) int TestApp::RunTests() { #if wxUSE_LOG - // Switch off logging unless --verbose - bool verbose = wxLog::GetVerbose(); - wxLog::EnableLogging(verbose); -#else - bool verbose = false; + // Switch off logging to avoid interfering with the tests output unless + // WXTRACE is set, as otherwise setting it would have no effect while + // running the tests. + if ( !wxGetEnv("WXTRACE", NULL) ) + wxLog::EnableLogging(false); #endif // Cast is needed under MSW where Catch also provides an overload taking From 1df0b0ac99f811b7c8f1fbf9972cc75bf363f021 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 14 Oct 2019 23:58:18 +0200 Subject: [PATCH 10/23] Use "CurrentTime" symbolic constant in XTestFakeXXXEvent() No real changes, just replace "0" with a more clear constant. --- src/unix/uiactionx11.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/unix/uiactionx11.cpp b/src/unix/uiactionx11.cpp index 4722aff96b..6ea7f5ce63 100644 --- a/src/unix/uiactionx11.cpp +++ b/src/unix/uiactionx11.cpp @@ -243,7 +243,7 @@ private: bool wxUIActionSimulatorXTestImpl::DoX11Button(int xbutton, bool isDown) { - return XTestFakeButtonEvent(m_display, xbutton, isDown, 0) != 0; + return XTestFakeButtonEvent(m_display, xbutton, isDown, CurrentTime) != 0; } bool wxUIActionSimulatorXTestImpl::DoX11MouseMove(long x, long y) @@ -266,7 +266,7 @@ bool wxUIActionSimulatorXTestImpl::DoX11MouseMove(long x, long y) #endif // GTK+ 3.10+ #endif // __WXGTK3__ - return XTestFakeMotionEvent(m_display, -1, x, y, 0) != 0; + return XTestFakeMotionEvent(m_display, -1, x, y, CurrentTime) != 0; } bool @@ -274,7 +274,7 @@ wxUIActionSimulatorXTestImpl::DoX11Key(KeyCode xkeycode, int WXUNUSED(modifiers), bool isDown) { - return XTestFakeKeyEvent(m_display, xkeycode, isDown, 0) != 0; + return XTestFakeKeyEvent(m_display, xkeycode, isDown, CurrentTime) != 0; } #endif // wxUSE_XTEST From f65133a41f2d494f6535d20b26cbc1105ec0fd47 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 15 Oct 2019 00:07:38 +0200 Subject: [PATCH 11/23] Call XFlush() after simulating mouse button events too For some reason, mouse press events were still received without flushing, but mouse release ones were not received by GTK itself (and a fortiori by wxGTK) without it. --- src/unix/uiactionx11.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/unix/uiactionx11.cpp b/src/unix/uiactionx11.cpp index 6ea7f5ce63..8d72835868 100644 --- a/src/unix/uiactionx11.cpp +++ b/src/unix/uiactionx11.cpp @@ -108,7 +108,12 @@ bool wxUIActionSimulatorX11Impl::SendButtonEvent(int button, bool isDown) // all pending events, notably mouse moves. XSync(m_display, False /* don't discard */); - return DoX11Button(xbutton, isDown); + if ( !DoX11Button(xbutton, isDown) ) + return false; + + XFlush(m_display); + + return true; } #if wxUSE_PLAINX_IMPL From ed783b95f87743c6e97620a265eb1921dcd138e1 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 15 Oct 2019 03:18:28 +0200 Subject: [PATCH 12/23] Add a hack to make wxGrid UI unit test pass This is an ugly workaround for a mysterious problem occurring with the simulated "Enter" presses under GTK, but it's worth it, as it allows all grid tests, including the ones using wxUIActionSimulator, to pass now under wxGTK2 (a couple of tests still fail under wxGTK3). --- tests/controls/gridtest.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/controls/gridtest.cpp b/tests/controls/gridtest.cpp index bae013388d..d2c42b4441 100644 --- a/tests/controls/gridtest.cpp +++ b/tests/controls/gridtest.cpp @@ -818,6 +818,16 @@ void GridTestCase::ReadOnly() CPPUNIT_ASSERT(m_grid->IsReadOnly(1, 1)); m_grid->SetFocus(); + +#ifdef __WXGTK__ + // This is a mystery, but we somehow get WXK_RETURN generated by the + // previous test (Editable) in this one. In spite of wxYield() in that + // test, the key doesn't get dispatched there and we have to consume it + // here before setting the current grid cell, as getting WXK_RETURN later + // would move the selection down, to a non read-only cell. + wxYield(); +#endif // __WXGTK__ + m_grid->SetGridCursor(1, 1); CPPUNIT_ASSERT(m_grid->IsCurrentCellReadOnly()); From 7d6d514984e3fe4f03f301cc2156747f9f36d8f6 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 17 Oct 2019 17:07:00 +0200 Subject: [PATCH 13/23] Improve WaitForPaint helper used in window unit tests Unbind the event handler referencing a local variable, as leaving it bound could result in a crash later if another paint event was generated for the window for whatever reason. Doing it like this requires using 2 different objects, but the complexity can be still hidden inside WaitForPaint class, with the 2nd object being just a member of it, and, in fact, makes the code using it simpler as it doesn't need to use a boolean variable with it. --- tests/window/setsize.cpp | 51 +++++++++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/tests/window/setsize.cpp b/tests/window/setsize.cpp index 81b3eeb8d5..8728413336 100644 --- a/tests/window/setsize.cpp +++ b/tests/window/setsize.cpp @@ -51,23 +51,48 @@ protected: class WaitForPaint { public: - // Note that we have to use a pointer here, i.e. we can't just store the - // flag inside the class itself because it's going to be cloned inside wx - // and querying the flag of the original copy is not going to work. - explicit WaitForPaint(bool* painted) - : m_painted(*painted) + explicit WaitForPaint(wxWindow* win) + : m_win(*win), + m_painted(false), + m_handler(&m_painted) { - m_painted = false; + m_win.Bind(wxEVT_PAINT, m_handler); } - void operator()(wxPaintEvent& event) + bool GotPaintEvent() const { - event.Skip(); - m_painted = true; + return m_painted; + } + + ~WaitForPaint() + { + m_win.Unbind(wxEVT_PAINT, m_handler); } private: - bool& m_painted; + wxWindow& m_win; + bool m_painted; + + class PaintHandler + { + public: + // Note that we have to use a pointer here, i.e. we can't just store + // the flag inside the class itself because it's going to be cloned + // inside wx and querying the flag of the original copy wouldtn' work. + explicit PaintHandler(bool* painted) + : m_painted(*painted) + { + } + + void operator()(wxPaintEvent& event) + { + event.Skip(); + m_painted = true; + } + + private: + bool& m_painted; + } m_handler; }; // This function should be used to show the window and wait until we can get @@ -79,14 +104,12 @@ void ShowAndWaitForPaint(wxWindow* w) // geometry. And it's not clear how long should we wait, so we do it until // we get the first paint event -- by then the window really should have // its final size. - bool painted; - WaitForPaint waitForPaint(&painted); - w->Bind(wxEVT_PAINT, waitForPaint); + WaitForPaint waitForPaint(w); w->Show(); wxStopWatch sw; - while ( !painted ) + while ( !waitForPaint.GotPaintEvent() ) { wxYield(); From c810bfad4716db5062b74ee4c66a0410a3ddcbad Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 17 Oct 2019 21:59:06 +0200 Subject: [PATCH 14/23] Move WaitForPaint helper into a header No real changes, just make this class, used in wxWindow::SetSize() tests only so far, reusable in the other tests. --- tests/waitforpaint.h | 75 ++++++++++++++++++++++++++++++++++++++++ tests/window/setsize.cpp | 61 ++------------------------------ 2 files changed, 78 insertions(+), 58 deletions(-) create mode 100644 tests/waitforpaint.h diff --git a/tests/waitforpaint.h b/tests/waitforpaint.h new file mode 100644 index 0000000000..2d749bcf79 --- /dev/null +++ b/tests/waitforpaint.h @@ -0,0 +1,75 @@ +/////////////////////////////////////////////////////////////////////////////// +// Name: tests/waitforpaint.h +// Purpose: Helper WaitForPaint class +// Author: Vadim Zeitlin +// Created: 2019-10-17 (extracted from tests/window/setsize.cpp) +// Copyright: (c) 2019 Vadim Zeitlin +/////////////////////////////////////////////////////////////////////////////// + +#ifndef _WX_TESTS_WAITFORPAINT_H_ +#define _WX_TESTS_WAITFORPAINT_H_ + +#include "wx/stopwatch.h" +#include "wx/window.h" + +// Class used to check if we received the (first) paint event. +class WaitForPaint +{ +public: + explicit WaitForPaint(wxWindow* win) + : m_win(*win), + m_painted(false), + m_handler(&m_painted) + { + m_win.Bind(wxEVT_PAINT, m_handler); + } + + // This function waits up to the given number of milliseconds for the paint + // event to come and returns true if we did get it or false otherwise. + bool YieldUntilPainted(int timeoutInMS = 250) const + { + wxStopWatch sw; + for ( ;; ) + { + wxYield(); + + if ( m_painted ) + return true; + + if ( sw.Time() > timeoutInMS ) + return false; + } + } + + ~WaitForPaint() + { + m_win.Unbind(wxEVT_PAINT, m_handler); + } + +private: + wxWindow& m_win; + bool m_painted; + + class PaintHandler + { + public: + // Note that we have to use a pointer here, i.e. we can't just store + // the flag inside the class itself because it's going to be cloned + // inside wx and querying the flag of the original copy wouldtn' work. + explicit PaintHandler(bool* painted) + : m_painted(*painted) + { + } + + void operator()(wxPaintEvent& event) + { + event.Skip(); + m_painted = true; + } + + private: + bool& m_painted; + } m_handler; +}; + +#endif // _WX_TESTS_WAITFORPAINT_H_ diff --git a/tests/window/setsize.cpp b/tests/window/setsize.cpp index 8728413336..e908da2257 100644 --- a/tests/window/setsize.cpp +++ b/tests/window/setsize.cpp @@ -23,9 +23,9 @@ #endif // WX_PRECOMP #include "wx/scopedptr.h" -#include "wx/stopwatch.h" #include "asserthelper.h" +#include "waitforpaint.h" // ---------------------------------------------------------------------------- // tests helpers @@ -47,54 +47,6 @@ protected: virtual wxSize DoGetBestSize() const wxOVERRIDE { return wxSize(50, 250); } }; -// Class used to check if we received the (first) paint event. -class WaitForPaint -{ -public: - explicit WaitForPaint(wxWindow* win) - : m_win(*win), - m_painted(false), - m_handler(&m_painted) - { - m_win.Bind(wxEVT_PAINT, m_handler); - } - - bool GotPaintEvent() const - { - return m_painted; - } - - ~WaitForPaint() - { - m_win.Unbind(wxEVT_PAINT, m_handler); - } - -private: - wxWindow& m_win; - bool m_painted; - - class PaintHandler - { - public: - // Note that we have to use a pointer here, i.e. we can't just store - // the flag inside the class itself because it's going to be cloned - // inside wx and querying the flag of the original copy wouldtn' work. - explicit PaintHandler(bool* painted) - : m_painted(*painted) - { - } - - void operator()(wxPaintEvent& event) - { - event.Skip(); - m_painted = true; - } - - private: - bool& m_painted; - } m_handler; -}; - // This function should be used to show the window and wait until we can get // its real geometry. void ShowAndWaitForPaint(wxWindow* w) @@ -108,16 +60,9 @@ void ShowAndWaitForPaint(wxWindow* w) w->Show(); - wxStopWatch sw; - while ( !waitForPaint.GotPaintEvent() ) + if ( !waitForPaint.YieldUntilPainted() ) { - wxYield(); - - if ( sw.Time() > 250 ) - { - WARN("Didn't get a paint event until timeout expiration"); - break; - } + WARN("Didn't get a paint event until timeout expiration"); } } From 8534ec4699f234149c6ac838aaceccafe9537ecd Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 17 Oct 2019 22:01:36 +0200 Subject: [PATCH 15/23] Get rid of an unnecessary function in wxWindow unit test No real changes, just inline a short function only used once. --- tests/window/setsize.cpp | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/tests/window/setsize.cpp b/tests/window/setsize.cpp index e908da2257..e99407faac 100644 --- a/tests/window/setsize.cpp +++ b/tests/window/setsize.cpp @@ -47,25 +47,6 @@ protected: virtual wxSize DoGetBestSize() const wxOVERRIDE { return wxSize(50, 250); } }; -// This function should be used to show the window and wait until we can get -// its real geometry. -void ShowAndWaitForPaint(wxWindow* w) -{ - // Unfortunately showing the window is asynchronous, at least when using - // X11, so we have to wait for some time before retrieving its true - // geometry. And it's not clear how long should we wait, so we do it until - // we get the first paint event -- by then the window really should have - // its final size. - WaitForPaint waitForPaint(w); - - w->Show(); - - if ( !waitForPaint.YieldUntilPainted() ) - { - WARN("Didn't get a paint event until timeout expiration"); - } -} - } // anonymous namespace // ---------------------------------------------------------------------------- @@ -111,7 +92,19 @@ TEST_CASE("wxWindow::MovePreservesSize", "[window][size][move]") wxScopedPtr w(new wxFrame(wxTheApp->GetTopWindow(), wxID_ANY, "Test child frame")); - ShowAndWaitForPaint(w.get()); + // Unfortunately showing the window is asynchronous, at least when using + // X11, so we have to wait for some time before retrieving its true + // geometry. And it's not clear how long should we wait, so we do it until + // we get the first paint event -- by then the window really should have + // its final size. + WaitForPaint waitForPaint(w.get()); + + w->Show(); + + if ( !waitForPaint.YieldUntilPainted() ) + { + WARN("Didn't get a paint event until timeout expiration"); + } const wxRect rectOrig = w->GetRect(); From 94bdb7402d232022fd0b6be251c94a6dc21348fe Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 17 Oct 2019 22:03:44 +0200 Subject: [PATCH 16/23] Don't do anything in WaitForPaint class under non-GTK platforms Under MSW the tests pass even without it, so don't make them take longer unnecessarily. --- tests/waitforpaint.h | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/waitforpaint.h b/tests/waitforpaint.h index 2d749bcf79..481fcc8b9a 100644 --- a/tests/waitforpaint.h +++ b/tests/waitforpaint.h @@ -12,7 +12,11 @@ #include "wx/stopwatch.h" #include "wx/window.h" -// Class used to check if we received the (first) paint event. +// Class used to check if we received the (first) paint event: this is +// currently used under GTK only, as MSW doesn't seem to need to wait for the +// things to work, while under Mac nothing works anyhow. +#ifdef __WXGTK__ + class WaitForPaint { public: @@ -72,4 +76,21 @@ private: } m_handler; }; +#else // !__WXGTK__ + +class WaitForPaint +{ +public: + explicit WaitForPaint(wxWindow* WXUNUSED(win)) + { + } + + bool YieldUntilPainted(int WXUNUSED(timeoutInMS) = 250) const + { + return true; + } +}; + +#endif // __WXGTK__/!__WXGTK__ + #endif // _WX_TESTS_WAITFORPAINT_H_ From 57d2be63fc525905b914383d808770d483ac1582 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 17 Oct 2019 22:04:20 +0200 Subject: [PATCH 17/23] Wait until grid is fully shown in the unit test Without this, the mouse clicks sometimes are received by the parent frame and not the grid itself. --- tests/controls/gridtest.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/controls/gridtest.cpp b/tests/controls/gridtest.cpp index d2c42b4441..62771e7400 100644 --- a/tests/controls/gridtest.cpp +++ b/tests/controls/gridtest.cpp @@ -27,6 +27,8 @@ #include "wx/stopwatch.h" #endif // __WXGTK__ +#include "waitforpaint.h" + class GridTestCase : public CppUnit::TestCase { public: @@ -132,8 +134,15 @@ void GridTestCase::setUp() if( ms_nativelabels ) m_grid->SetUseNativeColLabels(); + WaitForPaint waitForPaint(m_grid->GetGridWindow()); + m_grid->Refresh(); m_grid->Update(); + + if ( !waitForPaint.YieldUntilPainted() ) + { + WARN("Grid not repainted until timeout expiration"); + } } void GridTestCase::tearDown() From 8ed116bdc6461523dd757158c95086621978797b Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 17 Oct 2019 22:05:17 +0200 Subject: [PATCH 18/23] Use a hack to work around mouse click emulation bug with GTK 3 Somehow, mouse release events generated immediately after the mouse press ones are sometimes (not always, but often enough to reliably prevent the test suite from passing) simply lost, i.e. not received by the GTK event loop. The only workaround seems to be to introduce an artificial delay, which does avoid the problem, at the price of making the tests run longer and, worse, not really solving the underlying problem, whatever it is. But it's still better than nothing. --- src/unix/uiactionx11.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/unix/uiactionx11.cpp b/src/unix/uiactionx11.cpp index 8d72835868..5233e6d64f 100644 --- a/src/unix/uiactionx11.cpp +++ b/src/unix/uiactionx11.cpp @@ -28,6 +28,8 @@ #include "wx/unix/utilsx11.h" #ifdef __WXGTK3__ +#include "wx/utils.h" + #include "wx/gtk/private/wrapgtk.h" GtkWidget* wxGetTopLevelGTK(); #endif @@ -333,6 +335,13 @@ bool wxUIActionSimulatorX11Impl::MouseMove(long x, long y) bool wxUIActionSimulatorX11Impl::MouseUp(int button) { +#ifdef __WXGTK3__ + // This is a horrible hack, but some mouse click events are just lost + // without any apparent reason when using GTK 3 without this, i.e. they + // simply never reach GTK in some runs of the tests. + wxMilliSleep(10); +#endif + return SendButtonEvent(button, false); } From 9a424602e466832f1497fc1b5791181ff286df5f Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 18 Oct 2019 01:37:11 +0200 Subject: [PATCH 19/23] Stop deriving wxGrid from wxPanel This is unnecessary as TAB navigation is not supposed to work between wxGrid children and actually harmful as this resulted in SetFocus() doing nothing, instead of setting focus to wxGridWindow, if the focus was on wxGrid itself for some reason (this happened at least in the grid unit tests and resulted in failures because the in-place editor didn't appear as expected). --- include/wx/generic/grid.h | 4 ++-- src/generic/grid.cpp | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/include/wx/generic/grid.h b/include/wx/generic/grid.h index 2bc396d140..f0f2ef0938 100644 --- a/include/wx/generic/grid.h +++ b/include/wx/generic/grid.h @@ -946,7 +946,7 @@ struct WXDLLIMPEXP_CORE wxGridSizesInfo // wxGrid // ---------------------------------------------------------------------------- -class WXDLLIMPEXP_CORE wxGrid : public wxScrolledWindow +class WXDLLIMPEXP_CORE wxGrid : public wxScrolledCanvas { public: // possible selection modes @@ -2287,7 +2287,7 @@ protected: private: - // implement wxScrolledWindow method to return m_gridWin size + // implement wxScrolledCanvas method to return m_gridWin size virtual wxSize GetSizeAvailableForScrollTarget(const wxSize& size) wxOVERRIDE; // depending on the values of m_numFrozenRows and m_numFrozenCols, it will diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index 7ae8040aef..1841bf34bb 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -2257,7 +2257,7 @@ void wxGridWindow::OnFocus(wxFocusEvent& event) ///////////////////////////////////////////////////////////////////// -wxBEGIN_EVENT_TABLE( wxGrid, wxScrolledWindow ) +wxBEGIN_EVENT_TABLE( wxGrid, wxScrolledCanvas ) EVT_PAINT( wxGrid::OnPaint ) EVT_SIZE( wxGrid::OnSize ) EVT_KEY_DOWN( wxGrid::OnKeyDown ) @@ -2271,7 +2271,7 @@ bool wxGrid::Create(wxWindow *parent, wxWindowID id, const wxPoint& pos, const wxSize& size, long style, const wxString& name) { - if (!wxScrolledWindow::Create(parent, id, pos, size, + if (!wxScrolledCanvas::Create(parent, id, pos, size, style | wxWANTS_CHARS, name)) return false; @@ -3390,7 +3390,7 @@ wxGridCellCoordsArray wxGrid::CalcCellsExposed( const wxRegion& reg, void wxGrid::PrepareDCFor(wxDC &dc, wxGridWindow *gridWindow) { - wxScrolledWindow::PrepareDC( dc ); + wxScrolledCanvas::PrepareDC( dc ); wxPoint dcOrigin = dc.GetDeviceOrigin() - GetGridWindowOffset(gridWindow); @@ -5186,7 +5186,7 @@ void wxGrid::Refresh(bool eraseb, const wxRect* rect) if ( m_created && !GetBatchCount() ) { // Refresh to get correct scrolled position: - wxScrolledWindow::Refresh(eraseb, rect); + wxScrolledCanvas::Refresh(eraseb, rect); if (rect) { @@ -6824,7 +6824,7 @@ void wxGrid::ForceRefresh() void wxGrid::DoEnable(bool enable) { - wxScrolledWindow::DoEnable(enable); + wxScrolledCanvas::DoEnable(enable); Refresh(false /* don't erase background */); } @@ -7908,7 +7908,7 @@ void wxGrid::SetRowLabelSize( int width ) m_rowLabelWidth = width; InvalidateBestSize(); CalcWindowSizes(); - wxScrolledWindow::Refresh( true ); + wxScrolledCanvas::Refresh( true ); } } @@ -7938,7 +7938,7 @@ void wxGrid::SetColLabelSize( int height ) m_colLabelHeight = height; InvalidateBestSize(); CalcWindowSizes(); - wxScrolledWindow::Refresh( true ); + wxScrolledCanvas::Refresh( true ); } } From 38247f596c1e1d1214091e301f773a0eb425ba37 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 18 Oct 2019 03:45:31 +0200 Subject: [PATCH 20/23] Remove wrong wxGrid::GetMainWindowOfCompositeControl() override This method is supposed to be overridden in the sub-windows of a composite control (and is indeed correctly implemented in wxGridSubwindow), but it doesn't make any sense to implement it in the parent window itself. This method was probably never executed (which is how the problem went unnoticed for 10+ years since 760be3f7cb386924420a87045af761bbc964f041), but it's still wrong to define it here, so remove it. --- include/wx/generic/grid.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/wx/generic/grid.h b/include/wx/generic/grid.h index f0f2ef0938..6ab7d5299e 100644 --- a/include/wx/generic/grid.h +++ b/include/wx/generic/grid.h @@ -1947,8 +1947,6 @@ public: // override some base class functions - virtual wxWindow *GetMainWindowOfCompositeControl() wxOVERRIDE - { return (wxWindow*)m_gridWin; } virtual void Fit() wxOVERRIDE; // implementation only From b4dee76b4c74a2db13677e9a7447e09349e956b0 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 18 Oct 2019 03:57:22 +0200 Subject: [PATCH 21/23] Set focus to the main grid window in wxGrid::SetFocus() After changing wxGrid to not inherit from wxPanel, this needs to be done explicitly now. --- include/wx/generic/grid.h | 1 + src/generic/grid.cpp | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/include/wx/generic/grid.h b/include/wx/generic/grid.h index 6ab7d5299e..90c4869f7d 100644 --- a/include/wx/generic/grid.h +++ b/include/wx/generic/grid.h @@ -1948,6 +1948,7 @@ public: // override some base class functions virtual void Fit() wxOVERRIDE; + virtual void SetFocus() wxOVERRIDE; // implementation only void CancelMouseCapture(); diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index 1841bf34bb..9643698bb8 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -9659,6 +9659,11 @@ void wxGrid::Fit() AutoSize(); } +void wxGrid::SetFocus() +{ + m_gridWin->SetFocus(); +} + #if WXWIN_COMPATIBILITY_2_8 wxPen& wxGrid::GetDividerPen() const { From f29b6564b1a9b39eb1e03b097f284c71efc2c3da Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 19 Oct 2019 18:28:03 +0200 Subject: [PATCH 22/23] Really fix recent regression in grid content autosizing Fix autosizing broken in 3c72396a3670326e6824d1605954920b351a76d2 and not fully fixed by f7e335c031e83874bf0ee64568ae72d5d0df73cf. Simplify the code to make it more obviously correct, by separating the computation of the extent suitable for the label and determining the size to use taking into account the extents of both the column data and the its column. Also add the unit test checking that auto-sizing works correctly in all the different cases. Closes https://github.com/wxWidgets/wxWidgets/pull/1600 Co-Authored-By: Ilya Sinitsyn --- src/generic/grid.cpp | 59 +++++++++++-------- tests/controls/gridtest.cpp | 110 ++++++++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 22 deletions(-) diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index 9643698bb8..da5fbe00fa 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -9397,52 +9397,67 @@ wxGrid::AutoSizeColOrRow(int colOrRow, bool setAsMin, wxGridDirection direction) } // now also compare with the column label extent - wxCoord w, h; + wxCoord extentLabel; dc.SetFont( GetLabelFont() ); - bool addMargin = true; + // We add some margin around text for better readability. + const int margin = column ? 10 : 6; if ( column ) { if ( m_useNativeHeader ) { - w = GetGridColHeader()->GetColumnTitleWidth(colOrRow); + extentLabel = GetGridColHeader()->GetColumnTitleWidth(colOrRow); - // GetColumnTitleWidth already adds margins internally. - addMargin = false; - h = 0; + // Note that GetColumnTitleWidth already adds margins internally, + // so we don't need to add them here. } else { - dc.GetMultiLineTextExtent( GetColLabelValue(colOrRow), &w, &h ); - if ( GetColLabelTextOrientation() == wxVERTICAL ) - w = h; + const wxSize + size = dc.GetMultiLineTextExtent(GetColLabelValue(colOrRow)); + extentLabel = GetColLabelTextOrientation() == wxVERTICAL + ? size.y + : size.x; + + // Add some margins around text for better readability. + extentLabel += margin; } } else { - dc.GetMultiLineTextExtent( GetRowLabelValue(colOrRow), &w, &h ); + extentLabel = dc.GetMultiLineTextExtent(GetRowLabelValue(colOrRow)).y; + + // As above, add some margins for readability, although a smaller one + // in vertical direction. + extentLabel += margin; } - extent = column ? w : h; - if ( extent > extentMax ) - extentMax = extent; + // Finally determine the suitable extent fitting both the cells contents + // and the label. if ( !extentMax ) { - // empty column - give default extent (notice that if extentMax is less - // than default extent but != 0, it's OK) - extentMax = column ? m_defaultColWidth : m_defaultRowHeight; + // Special case: all the cells are empty, use the label extent. + extentMax = extentLabel; + if ( !extentMax ) + { + // But if the label is empty too, use the default width/height. + extentMax = column ? m_defaultColWidth : m_defaultRowHeight; + } } - else if ( addMargin ) + else // We have some data in the column cells. { - // leave some space around text - if ( column ) - extentMax += 10; - else - extentMax += 6; + // Ensure we have the same margin around the cells text as we use + // around the label. + extentMax += margin; + + // And increase it to fit the label if necessary. + if ( extentLabel > extentMax ) + extentMax = extentLabel; } + if ( column ) { // Ensure automatic width is not less than minimal width. See the diff --git a/tests/controls/gridtest.cpp b/tests/controls/gridtest.cpp index 62771e7400..e8d83e7474 100644 --- a/tests/controls/gridtest.cpp +++ b/tests/controls/gridtest.cpp @@ -16,9 +16,11 @@ #ifndef WX_PRECOMP #include "wx/app.h" + #include "wx/dcclient.h" #endif // WX_PRECOMP #include "wx/grid.h" +#include "wx/headerctrl.h" #include "testableframe.h" #include "asserthelper.h" #include "wx/uiaction.h" @@ -62,18 +64,21 @@ private: WXUISIM_TEST( ReadOnly ); WXUISIM_TEST( ResizeScrolledHeader ); WXUISIM_TEST( ColumnMinWidth ); + WXUISIM_TEST( AutoSizeColumn ); CPPUNIT_TEST( PseudoTest_NativeHeader ); WXUISIM_TEST( LabelClick ); WXUISIM_TEST( SortClick ); CPPUNIT_TEST( ColumnOrder ); WXUISIM_TEST( ResizeScrolledHeader ); WXUISIM_TEST( ColumnMinWidth ); + WXUISIM_TEST( AutoSizeColumn ); CPPUNIT_TEST( DeleteAndAddRowCol ); CPPUNIT_TEST( PseudoTest_NativeLabels ); WXUISIM_TEST( LabelClick ); WXUISIM_TEST( SortClick ); CPPUNIT_TEST( ColumnOrder ); WXUISIM_TEST( WindowAsEditorControl ); + WXUISIM_TEST( AutoSizeColumn ); CPPUNIT_TEST_SUITE_END(); void CellEdit(); @@ -100,10 +105,25 @@ private: void WindowAsEditorControl(); void ResizeScrolledHeader(); void ColumnMinWidth(); + void AutoSizeColumn(); void PseudoTest_NativeHeader() { ms_nativeheader = true; } void PseudoTest_NativeLabels() { ms_nativeheader = false; ms_nativelabels = true; } + // The helper function to determine the width of the column label depending + // on whether the native column is used. + int GetColumnLabelWidth(wxClientDC& dc, int col, int margin) const + { + if (ms_nativeheader) + return m_grid->GetGridColHeader()->GetColumnTitleWidth(col); + + int w, h; + dc.GetMultiLineTextExtent(m_grid->GetColLabelValue(col), &w, &h); + return w + margin; + } + + void CheckFirstColAutoSize(int expected); + static bool ms_nativeheader; static bool ms_nativelabels; @@ -988,4 +1008,94 @@ void GridTestCase::ColumnMinWidth() #endif } +void GridTestCase::CheckFirstColAutoSize(int expected) +{ + m_grid->AutoSizeColumn(0); + + wxYield(); + CHECK(m_grid->GetColSize(0) == expected); +} + +void GridTestCase::AutoSizeColumn() +{ + // Hardcoded extra margin for the columns used in grid.cpp. + const int margin = 10; + + wxGridCellAttr *attr = m_grid->GetOrCreateCellAttr(0, 0); + wxGridCellRenderer *renderer = attr->GetRenderer(m_grid, 0, 0); + REQUIRE(renderer != NULL); + + wxClientDC dcCell(m_grid->GetGridWindow()); + + wxClientDC dcLabel(m_grid->GetGridWindow()); + dcLabel.SetFont(m_grid->GetLabelFont()); + + const wxString shortStr = "W"; + const wxString mediumStr = "WWWW"; + const wxString longStr = "WWWWWWWW"; + const wxString multilineStr = mediumStr + "\n" + longStr; + + SECTION("Empty column and label") + { + m_grid->SetColLabelValue(0, wxString()); + CheckFirstColAutoSize( m_grid->GetDefaultColSize() ); + } + + SECTION("Empty column with label") + { + m_grid->SetColLabelValue(0, mediumStr); + CheckFirstColAutoSize( GetColumnLabelWidth(dcLabel, 0, margin) ); + } + + SECTION("Column with empty label") + { + m_grid->SetColLabelValue(0, wxString()); + m_grid->SetCellValue(0, 0, mediumStr); + m_grid->SetCellValue(1, 0, shortStr); + m_grid->SetCellValue(3, 0, longStr); + + CheckFirstColAutoSize( + renderer->GetBestWidth(*m_grid, *attr, dcCell, 3, 0, + m_grid->GetRowHeight(3)) + margin ); + } + + SECTION("Column with label longer than contents") + { + m_grid->SetColLabelValue(0, multilineStr); + m_grid->SetCellValue(0, 0, mediumStr); + m_grid->SetCellValue(1, 0, shortStr); + CheckFirstColAutoSize( GetColumnLabelWidth(dcLabel, 0, margin) ); + } + + SECTION("Column with contents longer than label") + { + m_grid->SetColLabelValue(0, mediumStr); + m_grid->SetCellValue(0, 0, mediumStr); + m_grid->SetCellValue(1, 0, shortStr); + m_grid->SetCellValue(3, 0, multilineStr); + CheckFirstColAutoSize( + renderer->GetBestWidth(*m_grid, *attr, dcCell, 3, 0, + m_grid->GetRowHeight(3)) + margin ); + } + + SECTION("Column with equally sized contents and label") + { + m_grid->SetColLabelValue(0, mediumStr); + m_grid->SetCellValue(0, 0, mediumStr); + m_grid->SetCellValue(1, 0, mediumStr); + m_grid->SetCellValue(3, 0, mediumStr); + + const int labelWidth = GetColumnLabelWidth(dcLabel, 0, margin); + + const int cellWidth = + renderer->GetBestWidth(*m_grid, *attr, dcCell, 3, 0, + m_grid->GetRowHeight(3)) + + margin; + + // We can't be sure which size will be greater because of different fonts + // so just calculate the maximum width. + CheckFirstColAutoSize( wxMax(labelWidth, cellWidth) ); + } +} + #endif //wxUSE_GRID From 16619c5e77b31327dd1c8f795fac8b1b3351fab0 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 19 Oct 2019 18:31:50 +0200 Subject: [PATCH 23/23] Use DIPs for the margins around text in wxGrid columns Improve the appearance on high DPI displays where the margins were previously too small under MSW. --- src/generic/grid.cpp | 2 +- tests/controls/gridtest.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index da5fbe00fa..a9b5d36648 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -9401,7 +9401,7 @@ wxGrid::AutoSizeColOrRow(int colOrRow, bool setAsMin, wxGridDirection direction) dc.SetFont( GetLabelFont() ); // We add some margin around text for better readability. - const int margin = column ? 10 : 6; + const int margin = FromDIP(column ? 10 : 6); if ( column ) { diff --git a/tests/controls/gridtest.cpp b/tests/controls/gridtest.cpp index e8d83e7474..cadc110392 100644 --- a/tests/controls/gridtest.cpp +++ b/tests/controls/gridtest.cpp @@ -1019,7 +1019,7 @@ void GridTestCase::CheckFirstColAutoSize(int expected) void GridTestCase::AutoSizeColumn() { // Hardcoded extra margin for the columns used in grid.cpp. - const int margin = 10; + const int margin = m_grid->FromDIP(10); wxGridCellAttr *attr = m_grid->GetOrCreateCellAttr(0, 0); wxGridCellRenderer *renderer = attr->GetRenderer(m_grid, 0, 0);