From 6d2f903a48a209d3389377ce566d65bfea33fe6f Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 26 Oct 2017 23:51:29 +0200 Subject: [PATCH 01/48] Remove an apparently unnecessary line from MSW wxProgressDialog It doesn't seem necessary to disable the use of common buttons, so don't do it. --- src/msw/progdlg.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index 462903ec35..e3472e3b6c 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -889,7 +889,6 @@ void* wxProgressDialogTaskRunner::Entry() // Undo some of the effects of MSWCommonTaskDialogInit(). tdc.dwFlags &= ~TDF_EXPAND_FOOTER_AREA; // Expand in content area. - tdc.dwCommonButtons = 0; // Don't use common buttons. if ( m_sharedData.m_style & wxPD_CAN_SKIP ) wxTdc.AddTaskDialogButton( tdc, Id_SkipBtn, 0, _("Skip") ); From 0736bdfb2859d07329160c18b19e2b4aa11041d0 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 27 Oct 2017 00:01:18 +0200 Subject: [PATCH 02/48] Prevent constant size changes in native MSW wxProgressDialog MSW implementation of wxProgressDialog adjusted the dialog size to the size of the message shown in it on each update, resulting in visually unpleasant constant jumping around (this is the same problem that we used to have in wxGenericProgressDialog long time ago, see #10624). Minimize this by using TDM_UPDATE_ELEMENT_TEXT instead of TDM_SET_ELEMENT_TEXT for changing the element text. This still increases the dialog size if the new element text is longer than the old value, but at least doesn't shrink it back if it is shorter, which is already quite an improvement. Notice that this change requires using TDF_EXPAND_FOOTER_AREA style, as otherwise the expanded information can't be updated without a re-layout. But this doesn't seem to be a big loss and it's not really clear why did we explicitly clear this flag before anyhow. Update the dialogs sample to make it easy to test for this behaviour and the documentation to mention MSW version peculiarities. --- interface/wx/progdlg.h | 4 +++- samples/dialogs/dialogs.cpp | 15 ++++++++++++- src/msw/progdlg.cpp | 43 ++++++++++++++++++++++++++++++------- 3 files changed, 52 insertions(+), 10 deletions(-) diff --git a/interface/wx/progdlg.h b/interface/wx/progdlg.h index 0c8f60eb1f..001bf37afa 100644 --- a/interface/wx/progdlg.h +++ b/interface/wx/progdlg.h @@ -194,7 +194,9 @@ public: Notice that you may want to call Fit() to change the dialog size to conform to the length of the new message if desired. The dialog does - not do this automatically. + not do this automatically, except for the native MSW implementation + which does increase the dialog size if necessary (but still doesn't + shrink it back even if the text becomes shorter). @param value The new value of the progress meter. It should be less than or equal to diff --git a/samples/dialogs/dialogs.cpp b/samples/dialogs/dialogs.cpp index 7ddc6701a5..58e69fbbde 100644 --- a/samples/dialogs/dialogs.cpp +++ b/samples/dialogs/dialogs.cpp @@ -349,7 +349,20 @@ bool MyApp::OnInit() ); for ( int i = 0; i <= PROGRESS_COUNT; i++ ) { - if ( !dlg.Update(i) ) + wxString msg; + switch ( i ) + { + case 15: + msg = "And the same dialog but with a very, very, very long" + " message, just to test how it appears in this case."; + break; + + case 30: + msg = "Back to brevity"; + break; + } + + if ( !dlg.Update(i, msg) ) break; wxMilliSleep(50); diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index e3472e3b6c..1c7903a241 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -81,6 +81,7 @@ public: m_value = 0; m_progressBarMarquee = false; m_skipped = false; + m_msgChangeElementText = TDM_SET_ELEMENT_TEXT; m_notifications = 0; m_parent = NULL; } @@ -105,6 +106,14 @@ public: bool m_progressBarMarquee; bool m_skipped; + // The task dialog message to use for changing the text of its elements: + // it's TDM_SET_ELEMENT_TEXT initially as this message should be used to + // let the dialog adjust itself to the size of its elements, but + // TDM_UPDATE_ELEMENT_TEXT later to prevent the dialog from performing a + // layout on each update, which is annoying as it can result in its size + // constantly changing. + int m_msgChangeElementText; + // Bit field that indicates fields that have been modified by the // main thread so the task dialog runner knows what to update. int m_notifications; @@ -260,14 +269,27 @@ void PerformNotificationUpdates(HWND hwnd, } ::SendMessage( hwnd, - TDM_SET_ELEMENT_TEXT, + sharedData->m_msgChangeElementText, TDE_MAIN_INSTRUCTION, wxMSW_CONV_LPARAM(title) ); ::SendMessage( hwnd, - TDM_SET_ELEMENT_TEXT, + sharedData->m_msgChangeElementText, TDE_CONTENT, wxMSW_CONV_LPARAM(body) ); + + // After using TDM_SET_ELEMENT_TEXT once, we don't want to use it for + // the subsequent updates as it could result in dialog size changing + // unexpectedly, so reset it (which does nothing if we had already done + // it, of course, but it's not a problem). + // + // Notice that, contrary to its documentation, even using this message + // still increases the dialog size if the new text is longer (at least + // under Windows 7), but it doesn't shrink back if the text becomes + // shorter later and stays at the bigger size which is still a big gain + // as it prevents jumping back and forth between the smaller and larger + // sizes. + sharedData->m_msgChangeElementText = TDM_UPDATE_ELEMENT_TEXT; } if ( sharedData->m_notifications & wxSPDD_EXPINFO_CHANGED ) @@ -276,8 +298,15 @@ void PerformNotificationUpdates(HWND hwnd, sharedData->m_expandedInformation; if ( !expandedInformation.empty() ) { + // Here we never need to use TDM_SET_ELEMENT_TEXT as the size of + // the expanded information doesn't change drastically. + // + // Notice that TDM_UPDATE_ELEMENT_TEXT for this element only works + // when using TDF_EXPAND_FOOTER_AREA, as we do. Without this flag, + // only TDM_SET_ELEMENT_TEXT could be used as otherwise the dialog + // layout becomes completely mangled (at least under Windows 7). ::SendMessage( hwnd, - TDM_SET_ELEMENT_TEXT, + TDM_UPDATE_ELEMENT_TEXT, TDE_EXPANDED_INFORMATION, wxMSW_CONV_LPARAM(expandedInformation) ); } @@ -777,8 +806,9 @@ bool wxProgressDialog::Show(bool show) wxPD_ESTIMATED_TIME | wxPD_REMAINING_TIME) ) { - // Use a non-empty string just to have the collapsible pane shown. - m_sharedData->m_expandedInformation = " "; + // Set the expanded information field from the beginning to avoid + // having to re-layout the dialog later when it changes. + UpdateExpandedInformation(0); } // Do launch the thread. @@ -887,9 +917,6 @@ void* wxProgressDialogTaskRunner::Entry() tdc.pfCallback = TaskDialogCallbackProc; tdc.lpCallbackData = (LONG_PTR) &m_sharedData; - // Undo some of the effects of MSWCommonTaskDialogInit(). - tdc.dwFlags &= ~TDF_EXPAND_FOOTER_AREA; // Expand in content area. - if ( m_sharedData.m_style & wxPD_CAN_SKIP ) wxTdc.AddTaskDialogButton( tdc, Id_SkipBtn, 0, _("Skip") ); From 0473d14ef192226cc78a20aa46f3c55b908cf7f6 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 27 Oct 2017 01:34:10 +0200 Subject: [PATCH 03/48] Make wxProgressDialog::Fit() work in native MSW version This method is supposed to adjust the dialog size to its contents and while the dialog increases automatically when using native implementation under MSW, it doesn't shrink back on its own and so it's still useful to allow Fit() to do it. Update the sample to test Fit() too. --- include/wx/msw/progdlg.h | 1 + interface/wx/progdlg.h | 3 ++- samples/dialogs/dialogs.cpp | 5 +++++ src/msw/progdlg.cpp | 18 ++++++++++++++++++ 4 files changed, 26 insertions(+), 1 deletion(-) diff --git a/include/wx/msw/progdlg.h b/include/wx/msw/progdlg.h index 4c9dd94d54..9e3e8422d6 100644 --- a/include/wx/msw/progdlg.h +++ b/include/wx/msw/progdlg.h @@ -44,6 +44,7 @@ public: virtual void SetIcons(const wxIconBundle& icons) wxOVERRIDE; virtual void DoMoveWindow(int x, int y, int width, int height) wxOVERRIDE; virtual void DoGetPosition(int *x, int *y) const wxOVERRIDE; + virtual void Fit() wxOVERRIDE; virtual bool Show( bool show = true ) wxOVERRIDE; diff --git a/interface/wx/progdlg.h b/interface/wx/progdlg.h index 001bf37afa..1f69759dfe 100644 --- a/interface/wx/progdlg.h +++ b/interface/wx/progdlg.h @@ -196,7 +196,8 @@ public: conform to the length of the new message if desired. The dialog does not do this automatically, except for the native MSW implementation which does increase the dialog size if necessary (but still doesn't - shrink it back even if the text becomes shorter). + shrink it back even if the text becomes shorter and you need to call + Fit() explicitly if you want this to happen). @param value The new value of the progress meter. It should be less than or equal to diff --git a/samples/dialogs/dialogs.cpp b/samples/dialogs/dialogs.cpp index 58e69fbbde..63be7bef0f 100644 --- a/samples/dialogs/dialogs.cpp +++ b/samples/dialogs/dialogs.cpp @@ -360,6 +360,11 @@ bool MyApp::OnInit() case 30: msg = "Back to brevity"; break; + + case 80: + msg = "Back and adjusted"; + dlg.Fit(); + break; } if ( !dlg.Update(i, msg) ) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index 1c7903a241..202a09b360 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -767,6 +767,24 @@ void wxProgressDialog::DoGetPosition(int *x, int *y) const wxGenericProgressDialog::DoGetPosition(x, y); } +void wxProgressDialog::Fit() +{ +#ifdef wxHAS_MSW_TASKDIALOG + if ( HasNativeTaskDialog() ) + { + wxCriticalSectionLocker locker(m_sharedData->m_cs); + + // Force the task dialog to use this message to adjust it layout. + m_sharedData->m_msgChangeElementText = TDM_SET_ELEMENT_TEXT; + + // Don't change the message, but pretend that it did change. + m_sharedData->m_notifications |= wxSPDD_MESSAGE_CHANGED; + } +#endif // wxHAS_MSW_TASKDIALOG + + wxGenericProgressDialog::Fit(); +} + bool wxProgressDialog::Show(bool show) { #ifdef wxHAS_MSW_TASKDIALOG From 4101849b4d0a4e1291e45cd990611eb9408c68a1 Mon Sep 17 00:00:00 2001 From: Andrew Radke Date: Fri, 27 Oct 2017 01:43:10 +0200 Subject: [PATCH 04/48] Grow wxGenericProgressDialog automatically if needed If the new message is longer than the previously shown one, increase the dialog size to fit it, instead of truncating the message. Still don't do anything if the new message is shorter to avoid unwanted constant changes in the dialog size if the message keeps changing. This is the original patch proposed in #10624, which it really makes sense to apply now because it makes the generic version match the behaviour of the native one under MSW (and the behaviour of the native version cannot be changed). See #10624. --- src/generic/progdlgg.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/generic/progdlgg.cpp b/src/generic/progdlgg.cpp index e61586b2c9..f1a6fcc365 100644 --- a/src/generic/progdlgg.cpp +++ b/src/generic/progdlgg.cpp @@ -765,8 +765,17 @@ void wxGenericProgressDialog::UpdateMessage(const wxString &newmsg) { if ( !newmsg.empty() && newmsg != m_msg->GetLabel() ) { + const wxSize sizeOld = m_msg->GetSize(); + m_msg->SetLabel(newmsg); + if ( m_msg->GetSize().x > sizeOld.x ) + { + // Resize the dialog to fit its new, longer contents instead of + // just truncating it. + Fit(); + } + // allow the window to repaint: // NOTE: since we yield only for UI events with this call, there // should be no side-effects From ec82ae13020fe1a05c980356aed063fc0d4e55c7 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 27 Oct 2017 01:45:50 +0200 Subject: [PATCH 05/48] Document new wxProgressDialog auto-resizing behaviour The dialog now grows automatically as needed and Fit() can be called to shrink it back if desired and this behaviour is consistent under all platforms. --- interface/wx/progdlg.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/interface/wx/progdlg.h b/interface/wx/progdlg.h index 1f69759dfe..78118681c4 100644 --- a/interface/wx/progdlg.h +++ b/interface/wx/progdlg.h @@ -192,12 +192,12 @@ public: for the user to dismiss it, meaning that this function does not return until this happens. - Notice that you may want to call Fit() to change the dialog size to - conform to the length of the new message if desired. The dialog does - not do this automatically, except for the native MSW implementation - which does increase the dialog size if necessary (but still doesn't - shrink it back even if the text becomes shorter and you need to call - Fit() explicitly if you want this to happen). + Notice that if @a newmsg is longer than the currently shown message, + the dialog size will be automatically increased to account for it. + However if the new message is shorter than the previous one, the dialog + doesn't shrink back to avoid constant resizes if the message is changed + often. To shrink back the dialog to fit its current contents you may + call Fit() explicitly. @param value The new value of the progress meter. It should be less than or equal to From 27110bfa72f11f32010c6555a0fcbb5dbc643579 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 27 Oct 2017 02:07:01 +0200 Subject: [PATCH 06/48] Gracefully handle parent disappearance in wxGenericProgressDialog Use a weak reference to the parent instead of a pointer to avoid crashing if the parent TLW is destroyed before wxGenericProgressDialog itself is. This is unlikely to happen in C++ code, where objects of this class are typically created on the stack, but can happen in e.g. wxPython where this is not the case. Closes #16378. --- include/wx/generic/progdlgg.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/include/wx/generic/progdlgg.h b/include/wx/generic/progdlgg.h index 77b90f76f4..e5f4bb0eb3 100644 --- a/include/wx/generic/progdlgg.h +++ b/include/wx/generic/progdlgg.h @@ -12,6 +12,7 @@ #define __PROGDLGH_G__ #include "wx/dialog.h" +#include "wx/weakref.h" class WXDLLIMPEXP_FWD_CORE wxButton; class WXDLLIMPEXP_FWD_CORE wxEventLoop; @@ -183,8 +184,9 @@ private: *m_estimated, *m_remaining; - // parent top level window (may be NULL) - wxWindow *m_parentTop; + // Reference to the parent top level window, automatically becomes NULL if + // it it is destroyed and could be always NULL if it's not given at all. + wxWindowRef m_parentTop; // Progress dialog styles: this is not the same as m_windowStyle because // wxPD_XXX constants clash with the existing TLW styles so to be sure we From 8c5dae949111609238f9f4373e1bef26aee5b6e7 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 28 Oct 2017 02:01:24 +0200 Subject: [PATCH 07/48] Make wxGenericProgressDialog::Resume() virtual It needs to be overridden in the native MSW wxProgressDialog implementation, but was only hidden by the function with the same name in the derived class, making it impossible to call the right function via the base class pointer, for example. --- include/wx/generic/progdlgg.h | 2 +- include/wx/msw/progdlg.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/wx/generic/progdlgg.h b/include/wx/generic/progdlgg.h index e5f4bb0eb3..ae74bd122d 100644 --- a/include/wx/generic/progdlgg.h +++ b/include/wx/generic/progdlgg.h @@ -44,7 +44,7 @@ public: virtual bool Update(int value, const wxString& newmsg = wxEmptyString, bool *skip = NULL); virtual bool Pulse(const wxString& newmsg = wxEmptyString, bool *skip = NULL); - void Resume(); + virtual void Resume(); int GetValue() const; int GetRange() const; diff --git a/include/wx/msw/progdlg.h b/include/wx/msw/progdlg.h index 9e3e8422d6..f073c66060 100644 --- a/include/wx/msw/progdlg.h +++ b/include/wx/msw/progdlg.h @@ -26,7 +26,7 @@ public: virtual bool Update(int value, const wxString& newmsg = wxEmptyString, bool *skip = NULL) wxOVERRIDE; virtual bool Pulse(const wxString& newmsg = wxEmptyString, bool *skip = NULL) wxOVERRIDE; - void Resume(); + virtual void Resume() wxOVERRIDE; int GetValue() const; wxString GetMessage() const; From 3b4a71c4dc95d637d439055b2e9539c54f79feeb Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 27 Oct 2017 02:26:00 +0200 Subject: [PATCH 08/48] Allow testing wxGenericProgressDialog in the dialogs sample too Add the possibility to test the generic implementation of the class when we use the native one by default, this is useful to allow comparing the behaviour of the two classes. --- samples/dialogs/dialogs.cpp | 35 +++++++++++++++++++++++++++++++++-- samples/dialogs/dialogs.h | 5 +++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/samples/dialogs/dialogs.cpp b/samples/dialogs/dialogs.cpp index 63be7bef0f..22748f5f85 100644 --- a/samples/dialogs/dialogs.cpp +++ b/samples/dialogs/dialogs.cpp @@ -233,6 +233,9 @@ wxBEGIN_EVENT_TABLE(MyFrame, wxFrame) #if wxUSE_PROGRESSDLG EVT_MENU(DIALOGS_PROGRESS, MyFrame::ShowProgress) +#ifdef wxHAS_NATIVE_PROGRESSDIALOG + EVT_MENU(DIALOGS_PROGRESS_GENERIC, MyFrame::ShowProgressGeneric) +#endif // wxHAS_NATIVE_PROGRESSDIALOG #endif // wxUSE_PROGRESSDLG EVT_MENU(DIALOGS_APP_PROGRESS, MyFrame::ShowAppProgress) @@ -508,6 +511,10 @@ bool MyApp::OnInit() #if wxUSE_PROGRESSDLG info_menu->Append(DIALOGS_PROGRESS, wxT("Pro&gress dialog\tCtrl-G")); + #ifdef wxHAS_NATIVE_PROGRESSDIALOG + info_menu->Append(DIALOGS_PROGRESS_GENERIC, + wxT("Generic progress dialog\tCtrl-Alt-G")); + #endif // wxHAS_NATIVE_PROGRESSDIALOG #endif // wxUSE_PROGRESSDLG info_menu->Append(DIALOGS_APP_PROGRESS, wxT("&App progress\tShift-Ctrl-G")); @@ -2670,10 +2677,10 @@ void MyFrame::OnExit(wxCommandEvent& WXUNUSED(event) ) #if wxUSE_PROGRESSDLG +static const int max = 100; + void MyFrame::ShowProgress( wxCommandEvent& WXUNUSED(event) ) { - static const int max = 100; - wxProgressDialog dialog("Progress dialog example", // "Reserve" enough space for the multiline // messages below, we'll change it anyhow @@ -2691,6 +2698,30 @@ void MyFrame::ShowProgress( wxCommandEvent& WXUNUSED(event) ) wxPD_SMOOTH // - makes indeterminate mode bar on WinXP very small ); + DoShowProgress(dialog); +} + +#ifdef wxHAS_NATIVE_PROGRESSDIALOG +void MyFrame::ShowProgressGeneric( wxCommandEvent& WXUNUSED(event) ) +{ + wxGenericProgressDialog dialog("Generic progress dialog example", + wxString(' ', 100) + "\n\n\n\n", + max, + this, + wxPD_CAN_ABORT | + wxPD_CAN_SKIP | + wxPD_APP_MODAL | + wxPD_ELAPSED_TIME | + wxPD_ESTIMATED_TIME | + wxPD_REMAINING_TIME | + wxPD_SMOOTH); + + DoShowProgress(dialog); +} +#endif // wxHAS_NATIVE_PROGRESSDIALOG + +void MyFrame::DoShowProgress(wxGenericProgressDialog& dialog) +{ bool cont = true; for ( int i = 0; i <= max; i++ ) { diff --git a/samples/dialogs/dialogs.h b/samples/dialogs/dialogs.h index e611a355db..482d636e3a 100644 --- a/samples/dialogs/dialogs.h +++ b/samples/dialogs/dialogs.h @@ -452,6 +452,10 @@ public: #if wxUSE_PROGRESSDLG void ShowProgress(wxCommandEvent& event); +#ifdef wxHAS_NATIVE_PROGRESSDIALOG + void ShowProgressGeneric(wxCommandEvent& event); +#endif // wxHAS_NATIVE_PROGRESSDIALOG + void DoShowProgress(wxGenericProgressDialog& dialog); #endif // wxUSE_PROGRESSDLG void ShowAppProgress(wxCommandEvent& event); @@ -596,6 +600,7 @@ enum DIALOGS_ONTOP, DIALOGS_MODELESS_BTN, DIALOGS_PROGRESS, + DIALOGS_PROGRESS_GENERIC, DIALOGS_APP_PROGRESS, DIALOGS_ABOUTDLG_SIMPLE, DIALOGS_ABOUTDLG_FANCY, From 25d9faca17d89e06cf6fe34ef2425486c59a0e44 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 28 Oct 2017 01:26:42 +0200 Subject: [PATCH 09/48] Remove state assigning from wxProgressDialog::GetHandle() This was added in 01bd848eb9699a60e0bdd5105b9a9c591c9e1b20 without any explanation and probably was a copy-and-paste typo as it just doesn't make any sense to change the state of the dialog in this accessor function (and if the state doesn't change, then this assignment is just completely useless). Remove the apparently unnecessary assignment and also an unnecessary temporary variable. --- src/msw/progdlg.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index 202a09b360..f6df346553 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -576,13 +576,8 @@ WXWidget wxProgressDialog::GetHandle() const #ifdef wxHAS_MSW_TASKDIALOG if ( HasNativeTaskDialog() ) { - HWND hwnd; - { - wxCriticalSectionLocker locker(m_sharedData->m_cs); - m_sharedData->m_state = m_state; - hwnd = m_sharedData->m_hwnd; - } - return hwnd; + wxCriticalSectionLocker locker(m_sharedData->m_cs); + return m_sharedData->m_hwnd; } #endif return wxGenericProgressDialog::GetHandle(); From 3f6e557f1807b1a6a02eb75c5627932fed01677c Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 28 Oct 2017 01:28:39 +0200 Subject: [PATCH 10/48] Tiny style fix in wxProgressDialog::GetHandle() No real changes, just remove the trailing spaces and add post-#endif comment for consistency. --- src/msw/progdlg.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index f6df346553..43b897ff32 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -571,15 +571,16 @@ void wxProgressDialog::Resume() #endif // wxHAS_MSW_TASKDIALOG } -WXWidget wxProgressDialog::GetHandle() const -{ +WXWidget wxProgressDialog::GetHandle() const +{ #ifdef wxHAS_MSW_TASKDIALOG if ( HasNativeTaskDialog() ) { wxCriticalSectionLocker locker(m_sharedData->m_cs); return m_sharedData->m_hwnd; } -#endif +#endif // wxHAS_MSW_TASKDIALOG + return wxGenericProgressDialog::GetHandle(); } From 759c0461e122b8daf418caacc158743b951c241c Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 28 Oct 2017 01:26:42 +0200 Subject: [PATCH 11/48] Remove state assigning from wxProgressDialog::DoGetPosition() too This was accidentally copy-and-pasted from another function in 1ef1f8fda6d48594809e11c18194184ba9372b0f, just remove it as it was done for the original check in the last commit but one. --- src/msw/progdlg.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index 43b897ff32..3f2b36328b 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -748,7 +748,6 @@ void wxProgressDialog::DoGetPosition(int *x, int *y) const wxPoint pos; { wxCriticalSectionLocker locker(m_sharedData->m_cs); - m_sharedData->m_state = m_state; pos = m_sharedData->m_winPosition; } if (x) From ed88275a88c3e219c9d4252477a28589575dab44 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 28 Oct 2017 22:59:58 +0200 Subject: [PATCH 12/48] Ensure we don't crash in wxProgressDialog::DoGetPosition() Check that we do have the shared data before dereferencing the pointer to it. While this normally will always be the case, it could be null if some error happened, so add a check for it, just as we already do it elsewhere. --- src/msw/progdlg.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index 3f2b36328b..ab7d57e8c3 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -746,6 +746,7 @@ void wxProgressDialog::DoGetPosition(int *x, int *y) const if ( HasNativeTaskDialog() ) { wxPoint pos; + if ( m_sharedData ) { wxCriticalSectionLocker locker(m_sharedData->m_cs); pos = m_sharedData->m_winPosition; From 1206b7e0bda3ff813cb0bfe3eb109a58543b3d94 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 28 Oct 2017 01:37:29 +0200 Subject: [PATCH 13/48] Return HRESULT, not BOOL, from TaskDialogCallbackProc Although this doesn't change anything because, by a happy concourse of circumstances, FALSE is the same as S_OK and TRUE is the same as S_FALSE numerically, it is still better and more clear, because consistent with the documentation, to return these constants from the task dialog callback function rather than boolean ones. --- src/msw/progdlg.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index ab7d57e8c3..5c261f57c4 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -1028,7 +1028,7 @@ wxProgressDialogTaskRunner::TaskDialogCallbackProc case Id_SkipBtn: ::SendMessage(hwnd, TDM_ENABLE_BUTTON, Id_SkipBtn, FALSE); sharedData->m_skipped = true; - return TRUE; + return S_FALSE; case IDCANCEL: if ( sharedData->m_state == wxProgressDialog::Finished ) @@ -1038,7 +1038,7 @@ wxProgressDialogTaskRunner::TaskDialogCallbackProc sharedData->m_state = wxProgressDialog::Dismissed; // Let Windows close the dialog. - return FALSE; + return S_OK; } // Close button on the window triggers an IDCANCEL press, @@ -1060,7 +1060,7 @@ wxProgressDialogTaskRunner::TaskDialogCallbackProc sharedData->m_state = wxProgressDialog::Canceled; } - return TRUE; + return S_FALSE; } break; @@ -1091,11 +1091,11 @@ wxProgressDialogTaskRunner::TaskDialogCallbackProc sharedData->m_winPosition = wxPoint(r.left, r.top); } - return TRUE; + return S_FALSE; } // Return anything. - return 0; + return S_OK; } #endif // wxHAS_MSW_TASKDIALOG From aac673391ccee2469765c76341cd60105e144248 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 28 Oct 2017 01:39:19 +0200 Subject: [PATCH 14/48] Make state change in wxProgressDialog::Resume() more explicit Reset the state to "Continue" instead of assigning it the dialogs own m_state which was also set to "Continue" from the base class Resume() called just before, but this wasn't necessarily obvious from just reading the code. No real changes. --- src/msw/progdlg.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index 5c261f57c4..cfb9338bd0 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -543,7 +543,7 @@ void wxProgressDialog::Resume() { wxCriticalSectionLocker locker(m_sharedData->m_cs); - m_sharedData->m_state = m_state; + m_sharedData->m_state = Continue; // "Skip" was disabled when "Cancel" had been clicked, so re-enable // it now. From 046d3be2156e28e332019e532b74d9d84b8db2f7 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 28 Oct 2017 19:31:30 +0200 Subject: [PATCH 15/48] Don't require calling DoNativeBeforeUpdate() with locked CS Acquire the lock in wxProgressDialog::DoNativeBeforeUpdate() itself instead of relying on the caller to do it. This is just a refactoring in preparation for further changes. --- include/wx/msw/progdlg.h | 5 +++-- src/msw/progdlg.cpp | 28 +++++++++++++++++----------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/include/wx/msw/progdlg.h b/include/wx/msw/progdlg.h index f073c66060..46650bd9c3 100644 --- a/include/wx/msw/progdlg.h +++ b/include/wx/msw/progdlg.h @@ -54,8 +54,9 @@ public: virtual WXWidget GetHandle() const wxOVERRIDE; private: - // Performs common routines to Update() and Pulse(). Requires the - // shared object to have been entered. + // Common part of Update() and Pulse(). + // + // Returns false if the user requested cancelling the dialog. bool DoNativeBeforeUpdate(bool *skip); // Updates the various timing informations for both determinate diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index cfb9338bd0..dd497efd93 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -404,17 +404,19 @@ bool wxProgressDialog::Update(int value, const wxString& newmsg, bool *skip) #ifdef wxHAS_MSW_TASKDIALOG if ( HasNativeTaskDialog() ) { + if ( !DoNativeBeforeUpdate(skip) ) + { + // Dialog was cancelled. + return false; + } + + value /= m_factor; + + wxASSERT_MSG( value <= m_maximum, wxT("invalid progress value") ); + { wxCriticalSectionLocker locker(m_sharedData->m_cs); - // Do nothing in canceled state. - if ( !DoNativeBeforeUpdate(skip) ) - return false; - - value /= m_factor; - - wxASSERT_MSG( value <= m_maximum, wxT("invalid progress value") ); - m_sharedData->m_value = value; m_sharedData->m_notifications |= wxSPDD_VALUE_CHANGED; @@ -474,11 +476,13 @@ bool wxProgressDialog::Pulse(const wxString& newmsg, bool *skip) #ifdef wxHAS_MSW_TASKDIALOG if ( HasNativeTaskDialog() ) { - wxCriticalSectionLocker locker(m_sharedData->m_cs); - - // Do nothing in canceled state. if ( !DoNativeBeforeUpdate(skip) ) + { + // Dialog was cancelled. return false; + } + + wxCriticalSectionLocker locker(m_sharedData->m_cs); if ( !m_sharedData->m_progressBarMarquee ) { @@ -509,6 +513,8 @@ bool wxProgressDialog::DoNativeBeforeUpdate(bool *skip) #ifdef wxHAS_MSW_TASKDIALOG if ( HasNativeTaskDialog() ) { + wxCriticalSectionLocker locker(m_sharedData->m_cs); + if ( m_sharedData->m_skipped ) { if ( skip && !*skip ) From 93c9ec2f01187be03340da1029c0deb7bc8ba03d Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 28 Oct 2017 19:39:29 +0200 Subject: [PATCH 16/48] Remove unnecessary check from DoNativeBeforeUpdate() Don't test if we're using the native task dialog in this function because it is only called if we are, so simplify the code by omitting this check. --- src/msw/progdlg.cpp | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index dd497efd93..bae1c0c723 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -511,31 +511,28 @@ bool wxProgressDialog::Pulse(const wxString& newmsg, bool *skip) bool wxProgressDialog::DoNativeBeforeUpdate(bool *skip) { #ifdef wxHAS_MSW_TASKDIALOG - if ( HasNativeTaskDialog() ) + wxCriticalSectionLocker locker(m_sharedData->m_cs); + + if ( m_sharedData->m_skipped ) { - wxCriticalSectionLocker locker(m_sharedData->m_cs); - - if ( m_sharedData->m_skipped ) + if ( skip && !*skip ) { - if ( skip && !*skip ) - { - *skip = true; - m_sharedData->m_skipped = false; - m_sharedData->m_notifications |= wxSPDD_ENABLE_SKIP; - } + *skip = true; + m_sharedData->m_skipped = false; + m_sharedData->m_notifications |= wxSPDD_ENABLE_SKIP; } - - if ( m_sharedData->m_state == Canceled ) - m_timeStop = m_sharedData->m_timeStop; - - return m_sharedData->m_state != Canceled; } -#endif // wxHAS_MSW_TASKDIALOG + if ( m_sharedData->m_state == Canceled ) + m_timeStop = m_sharedData->m_timeStop; + + return m_sharedData->m_state != Canceled; +#else // !wxHAS_MSW_TASKDIALOG wxUnusedVar(skip); wxFAIL_MSG( "unreachable" ); return false; +#endif // wxHAS_MSW_TASKDIALOG/!wxHAS_MSW_TASKDIALOG } void wxProgressDialog::Resume() From 0b96d3b905eeee72c3fc24ff81f366fce1a300f2 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 28 Oct 2017 19:46:36 +0200 Subject: [PATCH 17/48] Dispatch events from the native MSW wxProgressDialog too Do this for compatibility with wxGenericProgressDialog, which always did this and can't really do otherwise as it needs to react to the clicks on its buttons, and also because not doing it results in the other application windows being marked as "not responding" by MSW, which looks bad. Closes #17768. --- include/wx/msw/progdlg.h | 4 ++++ src/msw/progdlg.cpp | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/include/wx/msw/progdlg.h b/include/wx/msw/progdlg.h index 46650bd9c3..c54d96c689 100644 --- a/include/wx/msw/progdlg.h +++ b/include/wx/msw/progdlg.h @@ -59,6 +59,10 @@ private: // Returns false if the user requested cancelling the dialog. bool DoNativeBeforeUpdate(bool *skip); + // Dispatch the pending events to let the windows to update, just as the + // generic version does. This is done as part of DoNativeBeforeUpdate(). + void DispatchEvents(); + // Updates the various timing informations for both determinate // and indeterminate modes. Requires the shared object to have // been entered. diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index bae1c0c723..f3e812ca82 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -508,9 +508,29 @@ bool wxProgressDialog::Pulse(const wxString& newmsg, bool *skip) return wxGenericProgressDialog::Pulse( newmsg, skip ); } +void wxProgressDialog::DispatchEvents() +{ +#ifdef wxHAS_MSW_TASKDIALOG + // No need for HasNativeTaskDialog() check, we're only called when this is + // the case. + wxEventLoopBase* const loop = wxEventLoop::GetActive(); + if ( loop ) + { + // We don't need to dispatch the user input events as the task dialog + // handles its own ones in its thread and we shouldn't react to any + // other user actions while the dialog is shown. + loop->YieldFor(wxEVT_CATEGORY_ALL & ~wxEVT_CATEGORY_USER_INPUT); + } +#else // !wxHAS_MSW_TASKDIALOG + wxFAIL_MSG( "unreachable" ); +#endif // wxHAS_MSW_TASKDIALOG/!wxHAS_MSW_TASKDIALOG +} + bool wxProgressDialog::DoNativeBeforeUpdate(bool *skip) { #ifdef wxHAS_MSW_TASKDIALOG + DispatchEvents(); + wxCriticalSectionLocker locker(m_sharedData->m_cs); if ( m_sharedData->m_skipped ) From 814a67453199ad818d7757f57ca4ffce38955823 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 28 Oct 2017 22:47:32 +0200 Subject: [PATCH 18/48] Speed up the sample progress dialogs in the sample Just make them finish faster, it's too boring to wait for them to do it when testing when they take so long. --- samples/dialogs/dialogs.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/samples/dialogs/dialogs.cpp b/samples/dialogs/dialogs.cpp index 22748f5f85..b073c30a18 100644 --- a/samples/dialogs/dialogs.cpp +++ b/samples/dialogs/dialogs.cpp @@ -2785,7 +2785,7 @@ void MyFrame::DoShowProgress(wxGenericProgressDialog& dialog) dialog.Resume(); } - wxMilliSleep(200); + wxMilliSleep(100); } if ( !cont ) @@ -2817,7 +2817,7 @@ void MyFrame::ShowAppProgress( wxCommandEvent& WXUNUSED(event) ) { progress.SetValue(i); - wxMilliSleep(500); + wxMilliSleep(200); } wxLogStatus("Progress finished"); From 6818d0909e3a148d00d7c96ba030dd22b8b44c2b Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 28 Oct 2017 22:44:22 +0200 Subject: [PATCH 19/48] Factor out wxGenericProgressDialog::EnsureActiveEventLoopExists() Make this function reusable in order to allow it to be called from the native MSW wxProgressDialog implementation in the upcoming commit. --- include/wx/generic/progdlgg.h | 3 +++ src/generic/progdlgg.cpp | 15 ++++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/include/wx/generic/progdlgg.h b/include/wx/generic/progdlgg.h index ae74bd122d..8ff7e97aa5 100644 --- a/include/wx/generic/progdlgg.h +++ b/include/wx/generic/progdlgg.h @@ -105,6 +105,9 @@ protected: // Converts seconds to HH:mm:ss format. static wxString GetFormattedTime(unsigned long timeInSec); + // Create a new event loop if there is no currently running one. + void EnsureActiveEventLoopExists(); + // callback for optional abort button void OnCancel(wxCommandEvent&); diff --git a/src/generic/progdlgg.cpp b/src/generic/progdlgg.cpp index f1a6fcc365..b6ec082df9 100644 --- a/src/generic/progdlgg.cpp +++ b/src/generic/progdlgg.cpp @@ -160,11 +160,7 @@ bool wxGenericProgressDialog::Create( const wxString& title, // even if this means we have to start it ourselves (this happens most // commonly during the program initialization, e.g. for the progress // dialogs shown from overridden wxApp::OnInit()). - if ( !wxEventLoopBase::GetActive() ) - { - m_tempEventLoop = new wxEventLoop; - wxEventLoop::SetActive(m_tempEventLoop); - } + EnsureActiveEventLoopExists(); #if defined(__WXMSW__) && !defined(__WXUNIVERSAL__) // we have to remove the "Close" button from the title bar then as it is @@ -363,6 +359,15 @@ wxString wxGenericProgressDialog::GetFormattedTime(unsigned long timeInSec) return timeAsHMS; } +void wxGenericProgressDialog::EnsureActiveEventLoopExists() +{ + if ( !wxEventLoopBase::GetActive() ) + { + m_tempEventLoop = new wxEventLoop; + wxEventLoop::SetActive(m_tempEventLoop); + } +} + wxStaticText * wxGenericProgressDialog::CreateLabel(const wxString& text, wxSizer *sizer) { From 0f1590a90ba15eb0f7094b2bbac4a52884c450ee Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 28 Oct 2017 19:20:28 +0200 Subject: [PATCH 20/48] Create MSW task dialog progress window with non-null parent This ensures the correct relationship between wxProgressDialog and its parent window, fixing different problems with Z-order and the progress dialog icon, but requires attaching the progress dialog thread input to the main thread, which creates its own problems: in particular, the task dialog is now only updated when the messages are dispatched in the main thread, i.e. more sluggishly than before. It also requires taking care to avoid the deadlocks as the task dialog thread now waits for the main thread to dispatch its messages too, as the child dialog sends messages to the parent window. In particular, we can't simply wait for the task dialog thread to terminate, but must do it while dispatching the events as well. Closes #13185. --- src/msw/progdlg.cpp | 102 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 90 insertions(+), 12 deletions(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index f3e812ca82..f660421f8a 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -366,6 +366,7 @@ wxProgressDialog::wxProgressDialog( const wxString& title, SetPDStyle(style); SetMaximum(maximum); + EnsureActiveEventLoopExists(); Show(); DisableOtherWindows(); @@ -388,14 +389,49 @@ wxProgressDialog::~wxProgressDialog() m_sharedData->m_notifications |= wxSPDD_DESTROYED; } - m_taskDialogRunner->Wait(); + // We can't use simple wxThread::Wait() here as we would deadlock because + // the task dialog thread expects this thread to process some messages + // (presumably those the task dialog sends to its parent during its + // destruction). + const WXHANDLE hThread = m_taskDialogRunner->MSWGetHandle(); + for ( bool cont = true; cont; ) + { + DWORD rc = ::MsgWaitForMultipleObjects + ( + 1, // number of objects to wait for + (HANDLE *)&hThread, // the objects + false, // wait for any objects, not all + INFINITE, // no timeout + QS_ALLINPUT | // return as soon as there are any events + QS_ALLPOSTMESSAGE + ); - delete m_taskDialogRunner; + switch ( rc ) + { + case 0xFFFFFFFF: + // This is unexpected, but we can't do anything about it and + // probably shouldn't continue waiting as we risk doing it + // forever. + wxLogLastError("MsgWaitForMultipleObjectsEx"); + cont = false; + break; + case WAIT_OBJECT_0: + // Thread has terminated. + cont = false; + break; + + default: + // An event has arrive, so dispatch it. + wxEventLoop::GetActive()->Dispatch(); + } + } + + // Enable the windows before deleting the task dialog to ensure that we + // can regain the activation. ReenableOtherWindows(); - if ( GetTopParent() ) - GetTopParent()->Raise(); + delete m_taskDialogRunner; #endif // wxHAS_MSW_TASKDIALOG } @@ -513,14 +549,12 @@ void wxProgressDialog::DispatchEvents() #ifdef wxHAS_MSW_TASKDIALOG // No need for HasNativeTaskDialog() check, we're only called when this is // the case. - wxEventLoopBase* const loop = wxEventLoop::GetActive(); - if ( loop ) - { - // We don't need to dispatch the user input events as the task dialog - // handles its own ones in its thread and we shouldn't react to any - // other user actions while the dialog is shown. - loop->YieldFor(wxEVT_CATEGORY_ALL & ~wxEVT_CATEGORY_USER_INPUT); - } + + // We don't need to dispatch the user input events as the task dialog + // handles its own ones in its thread and we shouldn't react to any + // other user actions while the dialog is shown. + wxEventLoop::GetActive()-> + YieldFor(wxEVT_CATEGORY_ALL & ~wxEVT_CATEGORY_USER_INPUT); #else // !wxHAS_MSW_TASKDIALOG wxFAIL_MSG( "unreachable" ); #endif // wxHAS_MSW_TASKDIALOG/!wxHAS_MSW_TASKDIALOG @@ -861,6 +895,16 @@ bool wxProgressDialog::Show(bool show) return false; } + // Wait until the dialog is shown as the program may need some time + // before it calls Update() and we want to show something to the user + // in the meanwhile. + while ( wxEventLoop::GetActive()->Dispatch() ) + { + wxCriticalSectionLocker locker(m_sharedData->m_cs); + if ( m_sharedData->m_hwnd ) + break; + } + // Do not show the underlying dialog. return false; } @@ -936,12 +980,42 @@ void wxProgressDialog::UpdateExpandedInformation(int value) void* wxProgressDialogTaskRunner::Entry() { + // If we have a parent, we must use it to have correct Z-order and icon, + // but this can only be done if we attach this thread input to the thread + // that created the parent window, i.e. the main thread. + wxWindow* parent = NULL; + { + wxCriticalSectionLocker locker(m_sharedData.m_cs); + parent = m_sharedData.m_parent; + } + + if ( parent ) + { + // Force the creation of the message queue for this thread. + MSG msg; + ::PeekMessage(&msg, NULL, WM_USER, WM_USER, PM_NOREMOVE); + + // Attach its message queue to the main thread in order to allow using + // the parent window created by the main thread as task dialog parent. + if ( !::AttachThreadInput(::GetCurrentThreadId(), + wxThread::GetMainId(), + TRUE) ) + { + wxLogLastError(wxT("AttachThreadInput")); + + // Don't even try using the parent window from another thread, this + // won't work. + parent = NULL; + } + } + WinStruct tdc; wxMSWTaskDialogConfig wxTdc; { wxCriticalSectionLocker locker(m_sharedData.m_cs); + wxTdc.parent = parent; wxTdc.caption = m_sharedData.m_title.wx_str(); wxTdc.message = m_sharedData.m_message.wx_str(); @@ -1004,6 +1078,10 @@ wxProgressDialogTaskRunner::TaskDialogCallbackProc // Store the HWND for the main thread use. sharedData->m_hwnd = hwnd; + // The main thread is sitting in an event dispatching loop waiting + // for this dialog to be shown, so make sure it does get an event. + wxWakeUpIdle(); + // Set the maximum value and disable Close button. ::SendMessage( hwnd, TDM_SET_PROGRESS_BAR_RANGE, From 79e2adf9162bdfcc35d2a822a4309d75d35a2d04 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 28 Oct 2017 23:22:15 +0200 Subject: [PATCH 21/48] Remove unused wxSPDD_DISABLE_ABORT notification This notification and the code handling it were never used, so just remove it. --- src/msw/progdlg.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index f660421f8a..e7063df445 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -57,7 +57,6 @@ const int wxSPDD_EXPINFO_CHANGED = 0x0020; const int wxSPDD_ENABLE_SKIP = 0x0040; const int wxSPDD_ENABLE_ABORT = 0x0080; const int wxSPDD_DISABLE_SKIP = 0x0100; -const int wxSPDD_DISABLE_ABORT = 0x0200; const int wxSPDD_FINISHED = 0x0400; const int wxSPDD_DESTROYED = 0x0800; const int wxSPDD_ICON_CHANGED = 0x1000; @@ -321,9 +320,6 @@ void PerformNotificationUpdates(HWND hwnd, if ( sharedData->m_notifications & wxSPDD_DISABLE_SKIP ) ::SendMessage( hwnd, TDM_ENABLE_BUTTON, Id_SkipBtn, FALSE ); - if ( sharedData->m_notifications & wxSPDD_DISABLE_ABORT ) - ::SendMessage( hwnd, TDM_ENABLE_BUTTON, IDCANCEL, FALSE ); - // Is the progress finished? if ( sharedData->m_notifications & wxSPDD_FINISHED ) { From fdfd8efa835e8f7347200315d82784cca9c258ee Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 28 Oct 2017 23:24:51 +0200 Subject: [PATCH 22/48] Factor out wxTopLevelWindow::MSWEnableCloseButton() Make it possible to reuse this code for other, non-wx, windows. No real changes, this is just a pure refactoring. --- include/wx/msw/toplevel.h | 3 +++ src/msw/toplevel.cpp | 12 +++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/include/wx/msw/toplevel.h b/include/wx/msw/toplevel.h index 42c667b9a2..c79754f722 100644 --- a/include/wx/msw/toplevel.h +++ b/include/wx/msw/toplevel.h @@ -90,6 +90,9 @@ public: // NULL if getting the system menu failed. wxMenu *MSWGetSystemMenu() const; + // Enable or disable the close button of the specified window. + static bool MSWEnableCloseButton(WXHWND hwnd, bool enable = true); + // implementation from now on // -------------------------- diff --git a/src/msw/toplevel.cpp b/src/msw/toplevel.cpp index 2b815c74f6..4d7e00cc54 100644 --- a/src/msw/toplevel.cpp +++ b/src/msw/toplevel.cpp @@ -986,10 +986,11 @@ void wxTopLevelWindowMSW::SetIcons(const wxIconBundle& icons) DoSelectAndSetIcon(icons, SM_CXICON, SM_CYICON, ICON_BIG); } -bool wxTopLevelWindowMSW::EnableCloseButton(bool enable) +// static +bool wxTopLevelWindowMSW::MSWEnableCloseButton(WXHWND hwnd, bool enable) { // get system (a.k.a. window) menu - HMENU hmenu = GetSystemMenu(GetHwnd(), FALSE /* get it */); + HMENU hmenu = GetSystemMenu(hwnd, FALSE /* get it */); if ( !hmenu ) { // no system menu at all -- ok if we want to remove the close button @@ -1008,7 +1009,7 @@ bool wxTopLevelWindowMSW::EnableCloseButton(bool enable) return false; } // update appearance immediately - if ( !::DrawMenuBar(GetHwnd()) ) + if ( !::DrawMenuBar(hwnd) ) { wxLogLastError(wxT("DrawMenuBar")); } @@ -1016,6 +1017,11 @@ bool wxTopLevelWindowMSW::EnableCloseButton(bool enable) return true; } +bool wxTopLevelWindowMSW::EnableCloseButton(bool enable) +{ + return MSWEnableCloseButton(GetHwnd(), enable); +} + // Window must have wxCAPTION and either wxCLOSE_BOX or wxSYSTEM_MENU for the // button to be visible. Also check for wxMAXIMIZE_BOX because we don't want // to enable a button that is excluded from the current style. From 12efb20ad23815d74fd7bfb14149ee408720fb97 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 28 Oct 2017 23:27:12 +0200 Subject: [PATCH 23/48] Disable close title bar button in wxProgressDialog too When the "Cancel" button inside the dialog is disabled, disable the close title bar button as well as it serves the same purpose. In particular, this avoids asserts when clicking the close title bar button while showing the confirmation message box asking the user whether the dialog should be cancelled in the dialogs sample. --- src/msw/progdlg.cpp | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index e7063df445..f732dd5bf4 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -190,6 +190,16 @@ BOOL CALLBACK DisplayCloseButton(HWND hwnd, LPARAM lParam) return TRUE; } +// This function enables or disables both the cancel button in the task dialog +// and the close button in its title bar, as they perform the same function and +// so should be kept in the same state. +void EnableCloseButtons(HWND hwnd, bool enable) +{ + ::SendMessage(hwnd, TDM_ENABLE_BUTTON, IDCANCEL, enable ? TRUE : FALSE); + + wxTopLevelWindow::MSWEnableCloseButton(hwnd, enable); +} + void PerformNotificationUpdates(HWND hwnd, wxProgressDialogSharedData *sharedData) { @@ -315,7 +325,7 @@ void PerformNotificationUpdates(HWND hwnd, ::SendMessage( hwnd, TDM_ENABLE_BUTTON, Id_SkipBtn, TRUE ); if ( sharedData->m_notifications & wxSPDD_ENABLE_ABORT ) - ::SendMessage( hwnd, TDM_ENABLE_BUTTON, IDCANCEL, TRUE ); + EnableCloseButtons(hwnd, true); if ( sharedData->m_notifications & wxSPDD_DISABLE_SKIP ) ::SendMessage( hwnd, TDM_ENABLE_BUTTON, Id_SkipBtn, FALSE ); @@ -329,7 +339,7 @@ void PerformNotificationUpdates(HWND hwnd, { // Change Cancel into Close and activate the button. ::SendMessage( hwnd, TDM_ENABLE_BUTTON, Id_SkipBtn, FALSE ); - ::SendMessage( hwnd, TDM_ENABLE_BUTTON, IDCANCEL, TRUE ); + EnableCloseButtons(hwnd, true); ::EnumChildWindows( hwnd, DisplayCloseButton, (LPARAM) sharedData ); } @@ -1116,7 +1126,7 @@ wxProgressDialogTaskRunner::TaskDialogCallbackProc // If we can't be aborted, the "Close" button will only be enabled // when the progress ends (and not even then with wxPD_AUTO_HIDE). if ( !(sharedData->m_style & wxPD_CAN_ABORT) ) - ::SendMessage( hwnd, TDM_ENABLE_BUTTON, IDCANCEL, FALSE ); + EnableCloseButtons(hwnd, false); break; case TDN_BUTTON_CLICKED: @@ -1151,7 +1161,7 @@ wxProgressDialogTaskRunner::TaskDialogCallbackProc ); ::SendMessage(hwnd, TDM_ENABLE_BUTTON, Id_SkipBtn, FALSE); - ::SendMessage(hwnd, TDM_ENABLE_BUTTON, IDCANCEL, FALSE); + EnableCloseButtons(hwnd, false); sharedData->m_timeStop = wxGetCurrentTime(); sharedData->m_state = wxProgressDialog::Canceled; From 58f90d36a01090bb0cd66ec59a96f73ae13d10a6 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 29 Oct 2017 18:59:34 +0100 Subject: [PATCH 24/48] Remove redundant wxGenericProgressDialog::m_parentTop assignment The line setting m_parentTop directly was mistakenly left over even after 7e083675342b17762735ef2223fb9d264d6fc937 added the call to SetTopParent() just above it. Remove it now (better late than never), it's redundant at best and even wrong if the top level parent can't be used as the dialog parent because SetTopParent() checks for this but this line did not. --- src/generic/progdlgg.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/generic/progdlgg.cpp b/src/generic/progdlgg.cpp index b6ec082df9..712d07519c 100644 --- a/src/generic/progdlgg.cpp +++ b/src/generic/progdlgg.cpp @@ -144,7 +144,6 @@ bool wxGenericProgressDialog::Create( const wxString& title, { SetTopParent(parent); - m_parentTop = wxGetTopLevelParent(parent); m_pdStyle = style; wxWindow* const From d240c1f20f902991fea5b0ec59da23d70e85922c Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 29 Oct 2017 19:05:31 +0100 Subject: [PATCH 25/48] Just use wxGenericProgressDialog::m_parentTop directly Remove another redundant line of code from wxGenericProgressDialog:: Create(), there is no need to call GetParentForModalDialog() again when we just did it to initialize m_parentTop, just use the latter directly. --- src/generic/progdlgg.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/generic/progdlgg.cpp b/src/generic/progdlgg.cpp index 712d07519c..ff6ce27983 100644 --- a/src/generic/progdlgg.cpp +++ b/src/generic/progdlgg.cpp @@ -146,10 +146,7 @@ bool wxGenericProgressDialog::Create( const wxString& title, m_pdStyle = style; - wxWindow* const - realParent = GetParentForModalDialog(parent, GetWindowStyle()); - - if (!wxDialog::Create(realParent, wxID_ANY, title, wxDefaultPosition, wxDefaultSize, GetWindowStyle())) + if (!wxDialog::Create(m_parentTop, wxID_ANY, title, wxDefaultPosition, wxDefaultSize, GetWindowStyle())) return false; SetMaximum(maximum); From df2a0eb67ea58c90b02cb74a1470134651dca119 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 29 Oct 2017 19:43:46 +0100 Subject: [PATCH 26/48] Stop using m_winPosition to pass position to the main thread We can just ask for the window rectangle directly from the main thread, there is no need to update the position in wxProgressDialogSharedData all the time. --- src/msw/progdlg.cpp | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index f732dd5bf4..03fa9d7262 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -808,16 +808,16 @@ void wxProgressDialog::DoGetPosition(int *x, int *y) const #ifdef wxHAS_MSW_TASKDIALOG if ( HasNativeTaskDialog() ) { - wxPoint pos; + wxRect r; if ( m_sharedData ) { wxCriticalSectionLocker locker(m_sharedData->m_cs); - pos = m_sharedData->m_winPosition; + r = wxRectFromRECT(wxGetWindowRect(m_sharedData->m_hwnd)); } if (x) - *x = pos.x; + *x = r.x; if (y) - *y = pos.y; + *y = r.y; return; } @@ -1115,14 +1115,6 @@ wxProgressDialogTaskRunner::TaskDialogCallbackProc SWP_NOZORDER); } - // Store current position for the main thread use - // if no position update is pending. - if ( !(sharedData->m_notifications & wxSPDD_WINDOW_MOVED) ) - { - RECT r = wxGetWindowRect(hwnd); - sharedData->m_winPosition = wxPoint(r.left, r.top); - } - // If we can't be aborted, the "Close" button will only be enabled // when the progress ends (and not even then with wxPD_AUTO_HIDE). if ( !(sharedData->m_style & wxPD_CAN_ABORT) ) @@ -1192,12 +1184,6 @@ wxProgressDialogTaskRunner::TaskDialogCallbackProc } sharedData->m_notifications = 0; - { - // Update current position for the main thread use. - RECT r = wxGetWindowRect(hwnd); - sharedData->m_winPosition = wxPoint(r.left, r.top); - } - return S_FALSE; } From ffe84cfb994a5ff04977998cf30254a2bcbad91f Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 29 Oct 2017 19:45:35 +0100 Subject: [PATCH 27/48] Factor out wxProgressDialog::GetTaskDialogRect() No real changes, just prepare for implementing DoGetSize() in this class by extracting the common part between the existing DoGetPosition() and it in a new function. --- include/wx/msw/progdlg.h | 4 ++++ src/msw/progdlg.cpp | 24 ++++++++++++++++++------ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/include/wx/msw/progdlg.h b/include/wx/msw/progdlg.h index c54d96c689..08495c8e46 100644 --- a/include/wx/msw/progdlg.h +++ b/include/wx/msw/progdlg.h @@ -68,6 +68,10 @@ private: // been entered. void UpdateExpandedInformation(int value); + // Get the task dialog geometry when using the native dialog. + wxRect GetTaskDialogRect() const; + + wxProgressDialogTaskRunner *m_taskDialogRunner; wxProgressDialogSharedData *m_sharedData; diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index 03fa9d7262..feba07ea9b 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -803,17 +803,29 @@ void wxProgressDialog::DoMoveWindow(int x, int y, int width, int height) wxGenericProgressDialog::DoMoveWindow(x, y, width, height); } +wxRect wxProgressDialog::GetTaskDialogRect() const +{ + wxRect r; + +#ifdef wxHAS_MSW_TASKDIALOG + if ( m_sharedData ) + { + wxCriticalSectionLocker locker(m_sharedData->m_cs); + r = wxRectFromRECT(wxGetWindowRect(m_sharedData->m_hwnd)); + } +#else // !wxHAS_MSW_TASKDIALOG + wxFAIL_MSG( "unreachable" ); +#endif // wxHAS_MSW_TASKDIALOG/!wxHAS_MSW_TASKDIALOG + + return r; +} + void wxProgressDialog::DoGetPosition(int *x, int *y) const { #ifdef wxHAS_MSW_TASKDIALOG if ( HasNativeTaskDialog() ) { - wxRect r; - if ( m_sharedData ) - { - wxCriticalSectionLocker locker(m_sharedData->m_cs); - r = wxRectFromRECT(wxGetWindowRect(m_sharedData->m_hwnd)); - } + const wxRect r = GetTaskDialogRect(); if (x) *x = r.x; if (y) From a8eccd21c7902c223392c472f7a6ca61bde97863 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 29 Oct 2017 19:46:52 +0100 Subject: [PATCH 28/48] Implement wxProgressDialog::DoGetSize() for the native dialog too DoGetPosition() was done in 1ef1f8fda6d48594809e11c18194184ba9372b0f, implement DoGetSize() too now in order to give a chance Centre() (which needs both position and size to work). See #17768. --- include/wx/msw/progdlg.h | 1 + src/msw/progdlg.cpp | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/include/wx/msw/progdlg.h b/include/wx/msw/progdlg.h index 08495c8e46..6db08be01d 100644 --- a/include/wx/msw/progdlg.h +++ b/include/wx/msw/progdlg.h @@ -44,6 +44,7 @@ public: virtual void SetIcons(const wxIconBundle& icons) wxOVERRIDE; virtual void DoMoveWindow(int x, int y, int width, int height) wxOVERRIDE; virtual void DoGetPosition(int *x, int *y) const wxOVERRIDE; + virtual void DoGetSize(int *width, int *height) const wxOVERRIDE; virtual void Fit() wxOVERRIDE; virtual bool Show( bool show = true ) wxOVERRIDE; diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index feba07ea9b..2cf0fac97b 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -838,6 +838,24 @@ void wxProgressDialog::DoGetPosition(int *x, int *y) const wxGenericProgressDialog::DoGetPosition(x, y); } +void wxProgressDialog::DoGetSize(int *width, int *height) const +{ +#ifdef wxHAS_MSW_TASKDIALOG + if ( HasNativeTaskDialog() ) + { + const wxRect r = GetTaskDialogRect(); + if ( width ) + *width = r.width; + if ( height ) + *height = r.height; + + return; + } +#endif // wxHAS_MSW_TASKDIALOG + + wxGenericProgressDialog::DoGetSize(width, height); +} + void wxProgressDialog::Fit() { #ifdef wxHAS_MSW_TASKDIALOG From dc5802746d13ea4f7012fd095a98340661e80e8f Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 29 Oct 2017 19:49:40 +0100 Subject: [PATCH 29/48] Set wxProgressDialog::m_parent too This, in addition to the previous commit, allows Centre() to work correctly as it needs to retrieve the parent window rectangle. See #17768. --- include/wx/generic/progdlgg.h | 4 ++-- src/generic/progdlgg.cpp | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/include/wx/generic/progdlgg.h b/include/wx/generic/progdlgg.h index 8ff7e97aa5..310481f397 100644 --- a/include/wx/generic/progdlgg.h +++ b/include/wx/generic/progdlgg.h @@ -124,8 +124,8 @@ protected: // the dialog was shown void ReenableOtherWindows(); - // Set the top level parent we store from the parent window provided when - // creating the dialog. + // Store the parent window as wxWindow::m_parent and also set the top level + // parent reference we store in this class itself. void SetTopParent(wxWindow* parent); // return the top level parent window of this dialog (may be NULL) diff --git a/src/generic/progdlgg.cpp b/src/generic/progdlgg.cpp index ff6ce27983..8a3c9ede12 100644 --- a/src/generic/progdlgg.cpp +++ b/src/generic/progdlgg.cpp @@ -133,6 +133,7 @@ wxGenericProgressDialog::wxGenericProgressDialog(const wxString& title, void wxGenericProgressDialog::SetTopParent(wxWindow* parent) { + m_parent = parent; m_parentTop = GetParentForModalDialog(parent, GetWindowStyle()); } From d7ec02636a1f5c5afa371b8643a06b48674dba2a Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 29 Oct 2017 20:20:30 +0100 Subject: [PATCH 30/48] Avoid spurious assert when cancelling wxProgressDialog If Cancel/close button was pressed twice in a row, assert checking that the dialog state was "Continue" could be triggered, which was worse than annoying as it resulted in a deadlock due to trying to show the assert dialog box while holding the critical section that prevented the main thread from continuing. Allow the state to be either "Continue" or already be set to "Canceled" now to account for this case. Still assert for the other invalid states, but they really aren't supposed to be possible here. --- src/msw/progdlg.cpp | 40 ++++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index 2cf0fac97b..ce08c960c6 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -1175,18 +1175,38 @@ wxProgressDialogTaskRunner::TaskDialogCallbackProc // a finished dialog. if ( sharedData->m_style & wxPD_CAN_ABORT ) { - wxCHECK_MSG - ( - sharedData->m_state == wxProgressDialog::Continue, - TRUE, - "Dialog not in a cancelable state!" - ); + switch ( sharedData->m_state ) + { + case wxProgressDialog::Canceled: + // It can happen that we receive a second + // cancel request before we had time to process + // the first one, in which case simply do + // nothing for the subsequent one. + break; - ::SendMessage(hwnd, TDM_ENABLE_BUTTON, Id_SkipBtn, FALSE); - EnableCloseButtons(hwnd, false); + case wxProgressDialog::Continue: + ::SendMessage(hwnd, TDM_ENABLE_BUTTON, Id_SkipBtn, FALSE); + EnableCloseButtons(hwnd, false); - sharedData->m_timeStop = wxGetCurrentTime(); - sharedData->m_state = wxProgressDialog::Canceled; + sharedData->m_timeStop = wxGetCurrentTime(); + sharedData->m_state = wxProgressDialog::Canceled; + break; + + // States which shouldn't be possible here: + + // We shouldn't have an (enabled) cancel button at + // all then. + case wxProgressDialog::Uncancelable: + + // This one was already dealt with above. + case wxProgressDialog::Finished: + + // Normally it shouldn't be possible to get any + // notifications after switching to this state. + case wxProgressDialog::Dismissed: + wxFAIL_MSG( "unreachable" ); + break; + } } return S_FALSE; From 2b8e84ca493db01ce86334deb1cf63a326cb5058 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 29 Oct 2017 20:31:33 +0100 Subject: [PATCH 31/48] Avoid deadlock when closing the progress dialog Don't call EndDialog() while inside the critical section, this could (and did, sometimes) result in a deadlock if the main thread was trying to enter it in order to send us wxSPDD_DESTROYED notification as EndDialog() needs it to process some messages. --- src/msw/progdlg.cpp | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index ce08c960c6..69ec144ebd 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -1103,6 +1103,10 @@ wxProgressDialogTaskRunner::TaskDialogCallbackProc LONG_PTR dwRefData ) { + bool endDialog = false; + + // Block for shared data critical section. + { wxProgressDialogSharedData * const sharedData = (wxProgressDialogSharedData *) dwRefData; @@ -1230,13 +1234,28 @@ wxProgressDialogTaskRunner::TaskDialogCallbackProc (sharedData->m_state == wxProgressDialog::Finished && sharedData->m_style & wxPD_AUTO_HIDE) ) { - ::EndDialog( hwnd, IDCLOSE ); + // Don't call EndDialog() from here as it could deadlock + // because we are inside the shared data critical section, do + // it below after leaving it instead. + endDialog = true; } sharedData->m_notifications = 0; + + if ( endDialog ) + break; + return S_FALSE; } + } // Leave shared data critical section. + + if ( endDialog ) + { + ::EndDialog( hwnd, IDCLOSE ); + return S_FALSE; + } + // Return anything. return S_OK; } From ca7e1d8bd133e0d21012e9c7416ebde9d01d2246 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 29 Oct 2017 22:44:10 +0100 Subject: [PATCH 32/48] Remove manual wxProgressDialog centering code This is not necessary any longer now that we use the correct parent for the dialog window, the task dialog is centered automatically. And unlike our own code, comctl32.dll code always puts the dialog fully on one monitor instead of making it span two of them if the parent window does. --- src/msw/progdlg.cpp | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index 69ec144ebd..3545ba0700 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -1128,27 +1128,6 @@ wxProgressDialogTaskRunner::TaskDialogCallbackProc 0, MAKELPARAM(0, sharedData->m_range) ); - // We always create this task dialog with NULL parent because our - // parent in wx sense is a window created from a different thread - // and so can't be used as our real parent. However we still center - // this window on the parent one as the task dialogs do with their - // real parent usually. - if ( sharedData->m_parent ) - { - wxRect rect(wxRectFromRECT(wxGetWindowRect(hwnd))); - rect = rect.CentreIn(sharedData->m_parent->GetRect()); - ::SetWindowPos(hwnd, - NULL, - rect.x, - rect.y, - -1, - -1, - SWP_NOACTIVATE | - SWP_NOOWNERZORDER | - SWP_NOSIZE | - SWP_NOZORDER); - } - // If we can't be aborted, the "Close" button will only be enabled // when the progress ends (and not even then with wxPD_AUTO_HIDE). if ( !(sharedData->m_style & wxPD_CAN_ABORT) ) From 317470a39a381ef5a4834f1c98f47c525da698dd Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 29 Oct 2017 22:58:57 +0100 Subject: [PATCH 33/48] Use "max" instead of 100 in the progress dialog sample code No real changes, just don't hardcode 100 as we have a symbolic constant for it here already. --- samples/dialogs/dialogs.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/samples/dialogs/dialogs.cpp b/samples/dialogs/dialogs.cpp index b073c30a18..6935218d87 100644 --- a/samples/dialogs/dialogs.cpp +++ b/samples/dialogs/dialogs.cpp @@ -2770,8 +2770,8 @@ void MyFrame::DoShowProgress(wxGenericProgressDialog& dialog) { i += max/4; - if ( i >= 100 ) - i = 99; + if ( i >= max ) + i = max - 1; } if ( !cont ) From 1c62ebdcd76341eaa43a2a27fa96c1fe518ce890 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 29 Oct 2017 23:09:56 +0100 Subject: [PATCH 34/48] Minor optimizations in wxProgressDialog Don't bother performing the updates if nothing was requested. And ensure that nothing is requested even more often than it already was by not requesting an update if the new value is the same as the old one. --- src/msw/progdlg.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index 3545ba0700..2ef1d5c9d4 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -459,10 +459,13 @@ bool wxProgressDialog::Update(int value, const wxString& newmsg, bool *skip) { wxCriticalSectionLocker locker(m_sharedData->m_cs); - m_sharedData->m_value = value; - m_sharedData->m_notifications |= wxSPDD_VALUE_CHANGED; + if ( value != m_sharedData->m_value ) + { + m_sharedData->m_value = value; + m_sharedData->m_notifications |= wxSPDD_VALUE_CHANGED; + } - if ( !newmsg.empty() ) + if ( !newmsg.empty() && newmsg != m_message ) { m_message = newmsg; m_sharedData->m_message = newmsg; @@ -532,7 +535,7 @@ bool wxProgressDialog::Pulse(const wxString& newmsg, bool *skip) m_sharedData->m_notifications |= wxSPDD_PBMARQUEE_CHANGED; } - if ( !newmsg.empty() ) + if ( !newmsg.empty() && newmsg != m_message ) { m_message = newmsg; m_sharedData->m_message = newmsg; @@ -1197,7 +1200,9 @@ wxProgressDialogTaskRunner::TaskDialogCallbackProc break; case TDN_TIMER: - PerformNotificationUpdates(hwnd, sharedData); + // Don't perform updates if nothing needs to be done. + if ( sharedData->m_notifications ) + PerformNotificationUpdates(hwnd, sharedData); /* Decide whether we should end the dialog. This is done if either From cac3d39fa193723c21c11e8f3e7df4d5b6704488 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 30 Oct 2017 19:12:16 +0100 Subject: [PATCH 35/48] Don't explicitly attach progress dialog thread input This is actually completely unnecessary because this is done implicitly by Windows itself anyhow when we create the dialog with the owner window belonging to a different thread. Notice that this also explains why the thread input can't be detached later. --- src/msw/progdlg.cpp | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index 2ef1d5c9d4..6c6e6e0bfe 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -1028,26 +1028,6 @@ void* wxProgressDialogTaskRunner::Entry() parent = m_sharedData.m_parent; } - if ( parent ) - { - // Force the creation of the message queue for this thread. - MSG msg; - ::PeekMessage(&msg, NULL, WM_USER, WM_USER, PM_NOREMOVE); - - // Attach its message queue to the main thread in order to allow using - // the parent window created by the main thread as task dialog parent. - if ( !::AttachThreadInput(::GetCurrentThreadId(), - wxThread::GetMainId(), - TRUE) ) - { - wxLogLastError(wxT("AttachThreadInput")); - - // Don't even try using the parent window from another thread, this - // won't work. - parent = NULL; - } - } - WinStruct tdc; wxMSWTaskDialogConfig wxTdc; From 7481c743657bb3d85a31d45ca8c499f8f7a60b7c Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 31 Oct 2017 16:47:38 +0100 Subject: [PATCH 36/48] Make other wxGenericProgressDialog methods virtual too Solve the same problem as was recently with hiding, instead of overriding, Resume() in the native MSW wxProgressDialog class for all the other methods which were also affected by it. --- include/wx/generic/progdlgg.h | 12 ++++++------ include/wx/msw/progdlg.h | 10 +++++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/include/wx/generic/progdlgg.h b/include/wx/generic/progdlgg.h index 310481f397..ba8fb0bcc5 100644 --- a/include/wx/generic/progdlgg.h +++ b/include/wx/generic/progdlgg.h @@ -46,16 +46,16 @@ public: virtual void Resume(); - int GetValue() const; - int GetRange() const; - wxString GetMessage() const; + virtual int GetValue() const; + virtual int GetRange() const; + virtual wxString GetMessage() const; - void SetRange(int maximum); + virtual void SetRange(int maximum); // Return whether "Cancel" or "Skip" button was pressed, always return // false if the corresponding button is not shown. - bool WasCancelled() const; - bool WasSkipped() const; + virtual bool WasCancelled() const; + virtual bool WasSkipped() const; // Must provide overload to avoid hiding it (and warnings about it) virtual void Update() wxOVERRIDE { wxDialog::Update(); } diff --git a/include/wx/msw/progdlg.h b/include/wx/msw/progdlg.h index 6db08be01d..c22a79db91 100644 --- a/include/wx/msw/progdlg.h +++ b/include/wx/msw/progdlg.h @@ -28,15 +28,15 @@ public: virtual void Resume() wxOVERRIDE; - int GetValue() const; - wxString GetMessage() const; + virtual int GetValue() const wxOVERRIDE; + virtual wxString GetMessage() const wxOVERRIDE; - void SetRange(int maximum); + virtual void SetRange(int maximum) wxOVERRIDE; // Return whether "Cancel" or "Skip" button was pressed, always return // false if the corresponding button is not shown. - bool WasSkipped() const; - bool WasCancelled() const; + virtual bool WasSkipped() const wxOVERRIDE; + virtual bool WasCancelled() const wxOVERRIDE; virtual void SetTitle(const wxString& title) wxOVERRIDE; virtual wxString GetTitle() const wxOVERRIDE; From e49cde166f187714e954d41cf0ed79414d0fca23 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 31 Oct 2017 17:10:41 +0100 Subject: [PATCH 37/48] Improve progress bar updating in native wxMSW wxProgressDialog Since the switch to tying the task dialog thread message queue with the main thread, animating the progress bar didn't work well unless the dialog was updated very frequently from the main thread and could lag behind significantly, and confusingly for the user, otherwise. Work around this by avoiding the progress bar animation and setting it immediately to its real value. This works much better in practice. --- src/msw/progdlg.cpp | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index 6c6e6e0bfe..9d222beec1 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -214,6 +214,33 @@ void PerformNotificationUpdates(HWND hwnd, if ( sharedData->m_notifications & wxSPDD_VALUE_CHANGED ) { + // Use a hack to avoid progress bar animation: we can't afford to use + // it because animating the gauge movement smoothly requires a + // constantly running message loop and while it does run in this (task + // dialog) thread, it is often blocked from proceeding by some lock + // held by the main thread which is busy doing something and may not + // dispatch events frequently enough. So, in practice, the animation + // can lag far behind the real value and results in showing a wrong + // value in the progress bar. + // + // To prevent this from happening, set the progress bar value to + // something greater than its maximal value and then move it back: in + // the current implementations of the progress bar control, moving its + // position backwards does it directly, without using the animation, + // which is exactly what we want here. + // + // Finally notice that this hack doesn't really work for the last + // value, but while we could use a nested hack and temporarily increase + // the progress bar range when the value is equal to it, it isn't + // actually necessary in practice because when we reach the end of the + // bar the dialog is either hidden immediately anyhow or the main + // thread enters a modal event loop which does dispatch events and so + // it's not a problem to have an animated transition in this particular + // case. + ::SendMessage( hwnd, + TDM_SET_PROGRESS_BAR_POS, + sharedData->m_value + 1, + 0 ); ::SendMessage( hwnd, TDM_SET_PROGRESS_BAR_POS, sharedData->m_value, From ed086ea044385887e47acdef9d9983a89205e5c6 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 31 Oct 2017 17:21:28 +0100 Subject: [PATCH 38/48] Get rid of unnecessary variables in wxMSW wxProgressDialog There doesn't seem to be any need to have both "foo" and "realFoo", just reuse the existing variables instead. No real changes. --- src/msw/progdlg.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index 9d222beec1..44916657f9 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -987,14 +987,12 @@ void wxProgressDialog::UpdateExpandedInformation(int value) unsigned long remainingTime; UpdateTimeEstimates(value, elapsedTime, estimatedTime, remainingTime); - int realEstimatedTime = estimatedTime, - realRemainingTime = remainingTime; if ( m_sharedData->m_progressBarMarquee ) { // In indeterminate mode we don't have any estimation neither for the // remaining nor for estimated time. - realEstimatedTime = - realRemainingTime = -1; + estimatedTime = + remainingTime = static_cast(-1); } wxString expandedInformation; @@ -1014,7 +1012,7 @@ void wxProgressDialog::UpdateExpandedInformation(int value) expandedInformation << GetEstimatedLabel() << " " - << GetFormattedTime(realEstimatedTime); + << GetFormattedTime(estimatedTime); } if ( HasPDFlag(wxPD_REMAINING_TIME) ) @@ -1024,7 +1022,7 @@ void wxProgressDialog::UpdateExpandedInformation(int value) expandedInformation << GetRemainingLabel() << " " - << GetFormattedTime(realRemainingTime); + << GetFormattedTime(remainingTime); } // Update with new timing information. From 4ab676c967a48efcb4fbb1c1b84cbafd493692a2 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 31 Oct 2017 17:27:21 +0100 Subject: [PATCH 39/48] Show elapsed/estimated times in wxMSW wxProgressDialog by default This further improves the dialog usability when the main thread doesn't update it frequently enough, as these times can be seen immediately without having to expand the "details" part of the dialog which can be very sluggish in this case. It is also more consistent with the generic dialog and the behaviour of the native dialog before 6b91c5dfab876f0f1b17d54304bfb2fda27398ef which removed the code clearing TDF_EXPAND_FOOTER_AREA style. --- src/msw/progdlg.cpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index 44916657f9..49a33eb49e 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -987,10 +987,12 @@ void wxProgressDialog::UpdateExpandedInformation(int value) unsigned long remainingTime; UpdateTimeEstimates(value, elapsedTime, estimatedTime, remainingTime); - if ( m_sharedData->m_progressBarMarquee ) + // The value of 0 is special, we can't estimate anything before we have at + // least one update, so leave the times dependent on it indeterminate. + // + // Similarly, in indeterminate mode we don't have any estimations neither. + if ( !value || m_sharedData->m_progressBarMarquee ) { - // In indeterminate mode we don't have any estimation neither for the - // remaining nor for estimated time. estimatedTime = remainingTime = static_cast(-1); } @@ -1081,6 +1083,13 @@ void* wxProgressDialogTaskRunner::Entry() { tdc.pszExpandedInformation = m_sharedData.m_expandedInformation.t_str(); + + // If we have elapsed/estimated/... times to show, show them from + // the beginning for consistency with the generic version and also + // because showing them later may be very sluggish if the main + // thread doesn't update the dialog sufficiently frequently, while + // hiding them still works reasonably well. + tdc.dwFlags |= TDF_EXPANDED_BY_DEFAULT; } } From 5a3fd23a68128171b8701dbf48db343203c4962c Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 31 Oct 2017 17:32:27 +0100 Subject: [PATCH 40/48] Remove a now redundant test in UpdateExpandedInformation() Checking for m_progressBarMarquee is not necessary any longer, just testing the value is enough. Update the comments to explain why is it so. No real changes. --- src/msw/progdlg.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index 49a33eb49e..15b24e40bf 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -569,8 +569,8 @@ bool wxProgressDialog::Pulse(const wxString& newmsg, bool *skip) m_sharedData->m_notifications |= wxSPDD_MESSAGE_CHANGED; } - // The value passed here doesn't matter, only elapsed time makes sense - // in indeterminate mode anyhow. + // Value of 0 is special and is used when we can't estimate the + // remaining and total times, which is exactly what we need here. UpdateExpandedInformation(0); return m_sharedData->m_state != Canceled; @@ -990,8 +990,9 @@ void wxProgressDialog::UpdateExpandedInformation(int value) // The value of 0 is special, we can't estimate anything before we have at // least one update, so leave the times dependent on it indeterminate. // - // Similarly, in indeterminate mode we don't have any estimations neither. - if ( !value || m_sharedData->m_progressBarMarquee ) + // This value is also used by Pulse(), as in the indeterminate mode we can + // never estimate anything. + if ( !value ) { estimatedTime = remainingTime = static_cast(-1); From 3918c420b4d60fb463369a14ec6b832a8fa29310 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 31 Oct 2017 17:41:00 +0100 Subject: [PATCH 41/48] Check the active event loop in wxGenericProgressDialog dtor Verify that the active loop didn't change during this object lifetime as otherwise we could deactivate a different event loop from the one we installed. It's still a programming error to write code which doesn't destroy wxGenericProgressDialog early enough, but at least now "just" assert and leak memory in this case instead of resetting the active event loop and hanging the program. Closes #17983. --- src/generic/progdlgg.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/generic/progdlgg.cpp b/src/generic/progdlgg.cpp index 8a3c9ede12..03e1e7f933 100644 --- a/src/generic/progdlgg.cpp +++ b/src/generic/progdlgg.cpp @@ -697,6 +697,21 @@ wxGenericProgressDialog::~wxGenericProgressDialog() if ( m_tempEventLoop ) { + // If another event loop has been installed as active during the life + // time of this object, we shouldn't deactivate it, but we also can't + // delete our m_tempEventLoop in this case because it risks leaving the + // new event loop with a dangling pointer, which it will set back as + // the active loop when it exits, resulting in a crash. So we have no + // choice but to just leak this pointer then, which is, of course, bad + // and usually easily avoidable by just destroying the progress dialog + // sooner, so warn the programmer about it. + wxCHECK_RET + ( + wxEventLoopBase::GetActive() == m_tempEventLoop, + "current event loop must not be changed during " + "wxGenericProgressDialog lifetime" + ); + wxEventLoopBase::SetActive(NULL); delete m_tempEventLoop; } From 80ca6661d45da449c20a922fbdb3fe32edfe9544 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 31 Oct 2017 17:50:25 +0100 Subject: [PATCH 42/48] Explain that wxProgressDialog should be created on the stack Creating it on the heap is unnecessary and can be actually harmful, so document explicitly that it shouldn't be done. See #17983. --- interface/wx/progdlg.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/interface/wx/progdlg.h b/interface/wx/progdlg.h index 78118681c4..6f7fb1681d 100644 --- a/interface/wx/progdlg.h +++ b/interface/wx/progdlg.h @@ -33,6 +33,23 @@ wxProgressDialog in a multi-threaded application you should be sure to use wxThreadEvent for your inter-threads communications). + Although wxProgressDialog is not really modal, it should be created on the + stack, and not the heap, as other modal dialogs, e.g. use it like this: + @code + void MyFrame::SomeFunc() + { + wxProgressDialog dialog(...); + for ( int i = 0; i < 100; ++i ) { + if ( !dialog.Update(i)) { + // Cancelled by user. + break; + } + + ... do something time-consuming (but not too much) ... + } + } + @endcode + @beginStyleTable @style{wxPD_APP_MODAL} Make the progress dialog modal. If this flag is not given, it is From 23b7973c244c3d7e02e672fe7df097de7be5d0ff Mon Sep 17 00:00:00 2001 From: Arrigo Marchiori Date: Fri, 3 Nov 2017 17:45:53 +0100 Subject: [PATCH 43/48] Mention possible problem with wxProgressDialog created in OnInit() Such dialogs must be destroyed before the main event loop starts running to avoid problems with the temporary event loop created by the dialog internally. See #17983. --- interface/wx/progdlg.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/interface/wx/progdlg.h b/interface/wx/progdlg.h index 6f7fb1681d..e5b0ac3f36 100644 --- a/interface/wx/progdlg.h +++ b/interface/wx/progdlg.h @@ -49,6 +49,9 @@ } } @endcode + Note that this becomes even more important if the dialog is instantiated + during the program initialization, e.g. from wxApp::OnInit(): the dialog + must be destroyed before the main event loop is started in this case. @beginStyleTable @style{wxPD_APP_MODAL} From 3d98129b5142d581bcaacf111248d54817da6c7e Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 7 Nov 2017 22:22:40 +0100 Subject: [PATCH 44/48] Further improve wxProgressDialog resizing behaviour documentation Unfortunately it doesn't seem possible to prevent the native MSW dialog from changing its size in both upper and lower direction vertically, so at least mention this in the documentation and mention a possible workaround of manually adjusting the text to always have the same number of lines. --- interface/wx/progdlg.h | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/interface/wx/progdlg.h b/interface/wx/progdlg.h index e5b0ac3f36..8393273523 100644 --- a/interface/wx/progdlg.h +++ b/interface/wx/progdlg.h @@ -213,11 +213,14 @@ public: until this happens. Notice that if @a newmsg is longer than the currently shown message, - the dialog size will be automatically increased to account for it. - However if the new message is shorter than the previous one, the dialog - doesn't shrink back to avoid constant resizes if the message is changed - often. To shrink back the dialog to fit its current contents you may - call Fit() explicitly. + the dialog will be automatically made wider to account for it. However + if the new message is shorter than the previous one, the dialog doesn't + shrink back to avoid constant resizes if the message is changed often. + To do this and fit the dialog to its current contents you may call + Fit() explicitly. However the native MSW implementation of this class + does make the dialog shorter if the new text has fewer lines of text + than the old one, so it is recommended to keep the number of lines of + text constant in order to avoid jarring dialog size changes. @param value The new value of the progress meter. It should be less than or equal to From 83b9fa3119a8cba9ba4a5286a8fe38a7f53c9a27 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 8 Nov 2017 21:02:38 +0100 Subject: [PATCH 45/48] Get rid of unnecessary wxCriticalSectionLocker use No real changes, just don't lock a critical section for a short time only to lock it again almost immediately after unlocking -- just combine both blocks for which it is locked into one, there is no reason to release it for TASKDIALOGCONFIG and wxMSWTaskDialogConfig initialization which are both trivial operations not involving any callbacks. --- src/msw/progdlg.cpp | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index 15b24e40bf..a4a444c56f 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -1047,22 +1047,16 @@ void wxProgressDialog::UpdateExpandedInformation(int value) void* wxProgressDialogTaskRunner::Entry() { - // If we have a parent, we must use it to have correct Z-order and icon, - // but this can only be done if we attach this thread input to the thread - // that created the parent window, i.e. the main thread. - wxWindow* parent = NULL; - { - wxCriticalSectionLocker locker(m_sharedData.m_cs); - parent = m_sharedData.m_parent; - } - WinStruct tdc; wxMSWTaskDialogConfig wxTdc; { wxCriticalSectionLocker locker(m_sharedData.m_cs); - wxTdc.parent = parent; + // If we have a parent, we must use it to have correct Z-order and + // icon, even if this comes at the price of attaching this thread input + // to the thread that created the parent window, i.e. the main thread. + wxTdc.parent = m_sharedData.m_parent; wxTdc.caption = m_sharedData.m_title.wx_str(); wxTdc.message = m_sharedData.m_message.wx_str(); From 602e2e6abb06b6aa873827de7cd26f216cf95f82 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 8 Nov 2017 21:08:54 +0100 Subject: [PATCH 46/48] Avoid fitting wxProgressDialog to its contents on first update Use TDM_UPDATE_ELEMENT_TEXT even for the initial update as this allows to specify the message of roughly comparable (or greater) length than the messages that will be subsequently used while the dialog is shown and avoid size changes later, which is much more natural than having to do it for the first call to Update(). --- src/msw/progdlg.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index a4a444c56f..758cc1da02 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -80,7 +80,7 @@ public: m_value = 0; m_progressBarMarquee = false; m_skipped = false; - m_msgChangeElementText = TDM_SET_ELEMENT_TEXT; + m_msgChangeElementText = TDM_UPDATE_ELEMENT_TEXT; m_notifications = 0; m_parent = NULL; } @@ -106,11 +106,11 @@ public: bool m_skipped; // The task dialog message to use for changing the text of its elements: - // it's TDM_SET_ELEMENT_TEXT initially as this message should be used to - // let the dialog adjust itself to the size of its elements, but - // TDM_UPDATE_ELEMENT_TEXT later to prevent the dialog from performing a - // layout on each update, which is annoying as it can result in its size - // constantly changing. + // it is set to TDM_SET_ELEMENT_TEXT by Fit() to let the dialog adjust + // itself to the size of its elements during the next update, but otherwise + // TDM_UPDATE_ELEMENT_TEXT is used in order to prevent the dialog from + // performing a layout on each update, which is annoying as it can result + // in its size constantly changing. int m_msgChangeElementText; // Bit field that indicates fields that have been modified by the From d4d3222466180da9ac45dfc8173d05ac18e39183 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 16 Nov 2017 23:43:29 +0100 Subject: [PATCH 47/48] Split initial wxProgressDialog message consistently with updates Native wxMSW dialog split the provided message into the title and body in Update(), but didn't do it for the initial message specified when constructing the dialog, resulting in weird jumps, due to the font size change between the body and title dialog elements, if the updated message differed just slightly from the initial one. Fix this and refactor the code to reuse the same function for doing this splitting in both places. --- src/msw/progdlg.cpp | 66 ++++++++++++++++++++++++++++----------------- 1 file changed, 42 insertions(+), 24 deletions(-) diff --git a/src/msw/progdlg.cpp b/src/msw/progdlg.cpp index 758cc1da02..46416402c5 100644 --- a/src/msw/progdlg.cpp +++ b/src/msw/progdlg.cpp @@ -116,6 +116,37 @@ public: // Bit field that indicates fields that have been modified by the // main thread so the task dialog runner knows what to update. int m_notifications; + + + // Helper function to split a single message, passed via our public API, + // into the title and the main content body used by the native dialog. + // + // Note that it uses m_message and so must be called with m_cs locked. + void SplitMessageIntoTitleAndBody(wxString& title, wxString& body) const + { + title = m_message; + + const size_t posNL = title.find('\n'); + if ( posNL != wxString::npos ) + { + // There can an extra new line between the first and subsequent + // lines to separate them as it looks better with the generic + // version -- but in this one, they're already separated by the use + // of different dialog elements, so suppress the extra new line. + int numNLs = 1; + if ( posNL < title.length() - 1 && title[posNL + 1] == '\n' ) + numNLs++; + + body.assign(title, posNL + numNLs, wxString::npos); + title.erase(posNL); + } + else // A single line + { + // Don't use title without the body, this doesn't make sense. + body.clear(); + title.swap(body); + } + } }; // Runner thread that takes care of displaying and updating the @@ -280,29 +311,8 @@ void PerformNotificationUpdates(HWND hwnd, { // Split the message in the title string and the rest if it has // multiple lines. - wxString - title = sharedData->m_message, - body; - - const size_t posNL = title.find('\n'); - if ( posNL != wxString::npos ) - { - // There can an extra new line between the first and subsequent - // lines to separate them as it looks better with the generic - // version -- but in this one, they're already separated by the use - // of different dialog elements, so suppress the extra new line. - int numNLs = 1; - if ( posNL < title.length() - 1 && title[posNL + 1] == '\n' ) - numNLs++; - - body.assign(title, posNL + numNLs, wxString::npos); - title.erase(posNL); - } - else // A single line - { - // Don't use title without the body, this doesn't make sense. - title.swap(body); - } + wxString title, body; + sharedData->SplitMessageIntoTitleAndBody(title, body); ::SendMessage( hwnd, sharedData->m_msgChangeElementText, @@ -1058,7 +1068,15 @@ void* wxProgressDialogTaskRunner::Entry() // to the thread that created the parent window, i.e. the main thread. wxTdc.parent = m_sharedData.m_parent; wxTdc.caption = m_sharedData.m_title.wx_str(); - wxTdc.message = m_sharedData.m_message.wx_str(); + + // Split the message into the title and main body text in the same way + // as it's done later in PerformNotificationUpdates() when the message + // is changed by Update() or Pulse(). + m_sharedData.SplitMessageIntoTitleAndBody + ( + wxTdc.message, + wxTdc.extendedMessage + ); // MSWCommonTaskDialogInit() will add an IDCANCEL button but we need to // give it the correct label. From dc751d3f3310557379e86ada76816962e102488d Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 16 Nov 2017 23:50:22 +0100 Subject: [PATCH 48/48] Add a hint about making wxProgressDialog initially wide enough Using unbreakable spaces to achieve this looks like a hack, but seems to be the only thing working well in practice. --- interface/wx/progdlg.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/interface/wx/progdlg.h b/interface/wx/progdlg.h index 8393273523..589976076d 100644 --- a/interface/wx/progdlg.h +++ b/interface/wx/progdlg.h @@ -220,7 +220,11 @@ public: Fit() explicitly. However the native MSW implementation of this class does make the dialog shorter if the new text has fewer lines of text than the old one, so it is recommended to keep the number of lines of - text constant in order to avoid jarring dialog size changes. + text constant in order to avoid jarring dialog size changes. You may + also want to make the initial message, specified when creating the + dialog, wide enough to avoid having to resize the dialog later, e.g. by + appending a long string of unbreakable spaces (@c wxString(L'\u00a0', + 100)) to it. @param value The new value of the progress meter. It should be less than or equal to