wxQt MDI fixes (PR #25332)
AliKet
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/25332
Commit Summary
- fd9933d wxQt: Prevent Qt from deleting the underlying QMenuBar pointer
- c0487c9 wxQt: Fix warnings about invalid painter usage
- 9cb0343 wxQt: Don't derive wxMDIChildFrame from wxFrame
- 62d835f wxQt: Remove wxQtMDIParentFrame class
- f357acb wxQt: Fix memory leak in wxMDIParentFrame
- 0a0362e wxQt: Fix attaching/detaching the wxMDIChildFrame's menubar to the wxMDIParentFrame
- a9b8ae7 wxQt: Implement missing functionalities in MDI
- fd6c051 wxQt: Add support for showing the window menu in wxMDI{Child,Parent}Frame menubar
- b128656 wxQt: Fix a crash when deleting the wxMDIParentFrame
- fcdddc6 wxQt: Avoid assertion in docview sample when setting wxCURSOR_PENCIL
File Changes
(9 files)
- M include/wx/mdi.h (17)
- M include/wx/qt/mdi.h (23)
- M samples/docview/docview.cpp (6)
- M samples/docview/docview.h (4)
- M samples/docview/view.cpp (8)
- M samples/docview/view.h (2)
- M src/qt/frame.cpp (31)
- M src/qt/mdi.cpp (230)
- M src/qt/menu.cpp (2)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/25332.patch
- https://github.com/wxWidgets/wxWidgets/pull/25332.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 8 commits.
- 5a01376 wxQt: Don't derive wxMDIChildFrame from wxFrame
- 3dda23c wxQt: Remove wxQtMDIParentFrame class
- 5f36943 wxQt: Fix memory leak in wxMDIParentFrame
- cac2ab0 wxQt: Fix attaching/detaching the wxMDIChildFrame's menubar to the wxMDIParentFrame
- 87bafa8 wxQt: Implement missing functionalities in MDI
- 75eb75f wxQt: Add support for showing the window menu in wxMDI{Child,Parent}Frame menubar
- 69cc7ea wxQt: Fix a crash when deleting the wxMDIParentFrame
- 13ab926 wxQt: Avoid assertion in docview sample when setting wxCURSOR_PENCIL
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
AliKet left a comment (wxWidgets/wxWidgets#25332)@vadz Could you please restart the failing check? TIA.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
If the base class change of wxMDIChildFrameBase is unavoidable, could we use wxMDIChildFrame instead of wxFrameBase in the sample code? We try not to use wxFooBase in anything public, as they're supposed to be just an implementation detail.
In include/wx/mdi.h:
> +#ifdef __WXQT__ +using wxMDIChildFrameBaseBase = wxFrameBase; +#else +using wxMDIChildFrameBaseBase = wxFrame; +#endif
This is unfortunate because, as the changes to the sample show, almost all code using MDI will probably need to be changed to compile with wxQt. I wonder why does it need this when the other ports, even those using TDI, can live with inheriting from wxFrame here?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25332)@vadz Could you please restart the failing check? TIA.
Done (no idea what's going on today, but half of the CI builds fail with this error), but there seems to be a genuine build error in this job now.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
AliKet left a comment (wxWidgets/wxWidgets#25332)@vadz Could you please restart the failing check? TIA.
Done (no idea what's going on today, but half of the CI builds fail with this error), but there seems to be a genuine build error in this job now.
Thanks, the error is due to PCH not used when running under 18.04 and we need to include "wx/stockitem.h" in src/qt/mdi.cpp to be self-contained. I'll push the fix soone
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
AliKet left a comment (wxWidgets/wxWidgets#25332)If the base class change of
wxMDIChildFrameBaseis unavoidable, could we usewxMDIChildFrameinstead ofwxFrameBasein the sample code? We try not to usewxFooBasein anything public, as they're supposed to be just an implementation detail.
Well, we can partially revert this commit and continue deriving from wxFrame, but then we have to apply the following patch:
diff --git a/src/qt/frame.cpp b/src/qt/frame.cpp index e39ab82f52..97a94ec73a 100644 --- a/src/qt/frame.cpp +++ b/src/qt/frame.cpp @@ -49,13 +49,19 @@ public: wxFrame::~wxFrame() { - // central widget should be deleted by qt when the main window is destroyed - QtStoreWindowPointer( GetQMainWindow()->centralWidget(), nullptr ); + if ( GetQMainWindow() && GetQMainWindow()->centralWidget() ) + { + // central widget should be deleted by qt when the main window is destroyed + QtStoreWindowPointer(GetQMainWindow()->centralWidget(), nullptr); + } } bool wxFrame::Create( wxWindow *parent, wxWindowID id, const wxString& title, const wxPoint& pos, const wxSize& size, long style, const wxString& name ) { + // wxMDIChildFrame doesn't call this function at all, and has its own Create() + // function. + m_qtWindow = new wxQtMainWindow( parent, this ); // QMainWindow takes ownership of the central widget pointer. @@ -75,6 +81,9 @@ bool wxFrame::Create( wxWindow *parent, wxWindowID id, const wxString& title, void wxFrame::SetMenuBar( wxMenuBar *menuBar ) { + // wxMDIChildFrame overrides this function. So using GetQMainWindow() without + // a check is safe. + if ( menuBar ) { // To prevent QtMainWindow from deleting the old QMenuBar pointer (so that @@ -100,6 +109,12 @@ void wxFrame::SetMenuBar( wxMenuBar *menuBar ) #if wxUSE_STATUSBAR void wxFrame::SetStatusBar( wxStatusBar *statusBar ) { + if ( !GetQMainWindow() ) + { + // wxMDIChildFrame doesn't have a statusbar + return; + } + // The current status bar could be deleted by Qt when dereferencing it // TODO: add a mechanism like Detach in menus to avoid issues if ( statusBar != nullptr ) @@ -118,6 +133,12 @@ void wxFrame::SetStatusBar( wxStatusBar *statusBar ) #if wxUSE_TOOLBAR void wxFrame::SetToolBar(wxToolBar *toolbar) { + if ( !GetQMainWindow() ) + { + // wxMDIChildFrame doesn't have a toolbar + return; + } + if ( toolbar != nullptr ) { int area = 0; @@ -146,6 +167,12 @@ void wxFrame::SetWindowStyleFlag( long style ) { wxWindow::SetWindowStyleFlag( style ); + if ( !GetQMainWindow() ) + { + // This is a wxMDIChildFrame, so simply return + return; + } + Qt::WindowFlags qtFlags = Qt::CustomizeWindowHint; if ( HasFlag( wxFRAME_TOOL_WINDOW ) ) @@ -201,7 +228,10 @@ void wxFrame::SetWindowStyleFlag( long style ) QWidget* wxFrame::QtGetParentWidget() const { - return GetQMainWindow()->centralWidget(); + if ( GetQMainWindow() ) + return GetQMainWindow()->centralWidget(); + + return GetHandle(); } void wxFrame::AddChild( wxWindowBase *child ) @@ -222,10 +252,17 @@ void wxFrame::RemoveChild( wxWindowBase *child ) // excluding any menubar and toolbar if any. wxPoint wxFrame::GetClientAreaOrigin() const { - return wxQtConvertPoint( GetQMainWindow()->centralWidget()->pos() ); + if ( GetQMainWindow() ) + return wxQtConvertPoint( GetQMainWindow()->centralWidget()->pos() ); + + return wxPoint(); } QMainWindow *wxFrame::GetQMainWindow() const { - return static_cast<QMainWindow*>(m_qtWindow); + // Notice that wxMDIChildFrame which derives from this class for its + // interface only doesn't create a QMainWindow internally. So any call + // to this function must be checked before any usage. + + return qobject_cast<QMainWindow*>(m_qtWindow); } diff --git a/src/qt/window.cpp b/src/qt/window.cpp index 9fa13baf54..a86e178d90 100644 --- a/src/qt/window.cpp +++ b/src/qt/window.cpp @@ -731,7 +731,12 @@ QWidget *wxWindowQt::QtGetClientWidget() const { auto frame = wxDynamicCast(this, wxFrame); if ( frame ) - return frame->GetQMainWindow()->centralWidget(); + { + if ( auto qtMainWindow = frame->GetQMainWindow() ) + { + return qtMainWindow->centralWidget(); + } + } return wxQtGetDrawingWidget(m_qtContainer, GetHandle()); }
To avoid crashes when dereferencing the (INVALID) pointer returned by GetQMainWindow() to access the central widget, because wxMDIChildFrames are no longer creating these heavy QMainWindows, but simple widgets managed by QMdiSubWindow objects.
If you are ok with the (ugly!) patch above, then I have no objection in reverting the aforementioned commit and apply that patch. Please lt me know your opinion about this.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 3 commits.
- 91ad36c wxQt: Add support for showing the window menu in wxMDI{Child,Parent}Frame menubar
- 2477898 wxQt: Avoid assertion in docview sample when setting wxCURSOR_PENCIL
- be88d26 wxQt: Partially revert 5a01376685 commit
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 1 commit.
- fadd2f2 wxQt: Fix CI failure when trying to download Qt 6.8.3
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 6 commits.
- 762f349 wxQt: Fix attaching/detaching the wxMDIChildFrame's menubar to the wxMDIParentFrame
- 251782d wxQt: Implement missing functionalities in MDI
- fd37cd2 wxQt: Add support for showing the window menu in wxMDI{Child,Parent}Frame menubar
- b5390f5 wxQt: Avoid assertion in docview sample when setting wxCURSOR_PENCIL
- c7d211f wxQt: Partially revert 5a01376685 commit
- 7705d0d wxQt: Add helper function GetQtMdiArea() to wxMDIParentFrame
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 1 commit.
- e5ee718 wxQt: Add helper function GetQtMdiArea() to wxMDIParentFrame
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 6 commits.
- b0e3d85 wxQt: Add support for showing the window menu in wxMDI{Child,Parent}Frame menubar
- 4b66449 wxQt: Avoid assertion in docview sample when setting wxCURSOR_PENCIL
- 5d0f047 wxQt: Partially revert 5a01376685 commit
- 7879648 wxQt: Add helper function GetQtMdiArea() to wxMDIParentFrame
- 6e02c2e wxQt: Add wxMDIParentFrame::QtSetPreferredDI() member function
- 3f0a0de wxQt: Add code to show/hide and update window menu in MDI
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 1 commit.
- 49b7cc0 wxQt: Avoid Github Actions problems with installing Qt 6.8.3
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 1 commit.
- bec80c1 wxQt: Avoid Github Actions problems with installing Qt 6.8.3
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
AliKet left a comment (wxWidgets/wxWidgets#25332)@vadz the last commit is just shooting in the dark and I don't know what actually happens to the Qt installation process? Do you have any idea by chance?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
J.D. Purcell
jdpurcell left a comment (wxWidgets/wxWidgets#25332)Qt's main download site was down for almost a few days until early this morning. It seems to be working again, so the last commit isn't necessarily needed. That said, caching is currently broken in the official install-qt-action, so you could use either Kidev's or my fork if you want working caching so Qt doesn't need to be re-downloaded every time. However, both of our forks also changed the default value for aqtversion from ==3.1.* to ==3.2.* which doesn't work with Python 3.9 at the moment due to miurahr/aqtinstall#905 (fixed/merged but not released). So you could either go back to official install-qt-action and live without caching, or add aqtversion: '==3.1.*' under the with: section of the action.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 1 commit.
- 1269a32 wxQt: Fix installing Qt 6.8.* (latest) under GitHub Actions
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 15 commits.
- f042427 wxQt: Prevent Qt from deleting the underlying QMenuBar pointer
- 06f10a5 wxQt: Fix warnings about invalid painter usage
- 4365666 wxQt: Don't derive wxMDIChildFrame from wxFrame
- d8c7066 wxQt: Remove wxQtMDIParentFrame class
- fca2a5e wxQt: Fix memory leak in wxMDIParentFrame
- 6e199ac wxQt: Fix attaching/detaching the wxMDIChildFrame's menubar to the wxMDIParentFrame
- 3c5bdf9 wxQt: Implement missing functionalities in MDI
- 65466ad wxQt: Add support for showing the window menu in wxMDI{Child,Parent}Frame menubar
- a0e491f wxQt: Avoid assertion in docview sample when setting wxCURSOR_PENCIL
- 4b1310a wxQt: Partially revert 5a01376685 commit
- 8ecd85b wxQt: Add helper function GetQtMdiArea() to wxMDIParentFrame
- cba3f93 wxQt: Add wxMDIParentFrame::QtSetPreferredDI() member function
- 42e6a61 wxQt: Add code to show/hide and update window menu in MDI
- b971530 wxQt: Fix installing Qt 6.8.* (latest) under GitHub Actions
- 953767f wxQt: Generate wxEVT_ACTIVATE event when wxMDIChildFrame [de]activated
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 12 commits.
- 91c54f9 wxQt: Avoid assertion in docview sample when setting wxCURSOR_PENCIL
- 0804dff wxQt: Prevent Qt from deleting the underlying QMenuBar pointer
- 4836ec6 wxQt: Fix warnings about invalid painter usage
- 49410c9 wxQt: Fix memory leak in wxMDIParentFrame
- 5c706b9 wxQt: Get rid of the wxQtMDIParentFrame class
- adb070a wxQt: Add wxMDIParentFrame::GetQtMdiArea() helper function
- 69c7cbc wxQt: Fix wxMDIClientWindow being derived from wxFrame
- f9aa59a wxQt: Fix attaching/detaching the wxMDIChildFrame's menubar to the wxMDIParentFrame
- daba766 wxQt: Implement missing functionalities for MDI
- ef07cbf wxQt: Add code to show/hide/update window menu in MDI
- f8b9706 wxQt: Generate wxEVT_ACTIVATE event when wxMDIChildFrame [de]activated
- 956fde4 wxQt: Add wxMDIParentFrame::QtSetPreferredDI() member function
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
AliKet left a comment (wxWidgets/wxWidgets#25332)@vadz FYI this PR is updated for best history purposes and is ready for another review.
TIA.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 3 commits.
- fbf3b2c wxQt: Add code to show/hide/update window menu in MDI
- ad27e25 wxQt: Generate wxEVT_ACTIVATE event when wxMDIChildFrame [de]activated
- 51e26b4 wxQt: Add wxMDIParentFrame::QtSetPreferredDI() member function
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
Thanks for the updates! This looks mostly good and could be merged as is, but I think it might still be improved a bit as indicated by the remarks below — but please let me know if you disagree.
In src/qt/mdi.cpp:
>
bool wxMDIClientWindow::CreateClient(wxMDIParentFrame *parent, long WXUNUSED(style))
{
// create the MDI client area where the children window are displayed:
- m_qtWindow = new wxQtMdiArea( parent, this );
+ m_qtWindow = new wxQtMdiArea(parent, this);
+ m_qtContainer = dynamic_cast<QAbstractScrollArea*>(m_qtWindow);
I think it would be nice to add a comment indicating whether this cast is always supposed to succeed or if it can return nullptr in some situations (as I suspect it can?).
> private: + void OnMDICommand(wxCommandEvent& event); + + // add/remove window menu if we have it (i.e. m_windowMenu != nullptr) + void AddWindowMenu(); + void RemoveWindowMenu(); + + // update the window menu (if we have it) to enable or disable the commands + // which only make sense when we have more than one child + void UpdateWindowMenu(bool enable);
I think it would be more clear to rename this function to
- Make the meaning of
boolargument more understandable. - Use a different name from
wxMDIChildFramefunction with the same name doing something quite different.
I don't have any perfect suggestions but for (1) we really need to mention that the boolean indicates the presence of multiple children, so something like UpdateWindowMenuForMultipleChildren() (or ...IsSingleChild() with the inverted meaning of the value?) might do it.
> @@ -29,15 +34,54 @@ class WXDLLIMPEXP_CORE wxMDIParentFrame : public wxMDIParentFrameBase
long style = wxDEFAULT_FRAME_STYLE | wxVSCROLL | wxHSCROLL,
const wxString& name = wxASCII_STR(wxFrameNameStr));
+ QMdiArea* GetQtMdiArea() const;
+
+ void QtSetPreferredDI(bool isTDI = true);
Are we sure we're never going to have other variants? I'd use an enum here, e.g. something like
enum class Layout { Tabbed, MDI };
> @@ -207,7 +207,9 @@ MyCanvas::MyCanvas(wxView *view, wxWindow *parent)
m_currentSegment = nullptr;
m_lastMousePos = wxDefaultPosition;
+#ifndef __WXQT__
Could we avoid this, if necessary by just making SetCursor() do nothing in this case?
In src/qt/frame.cpp:
> @@ -49,8 +49,11 @@ class wxQtCentralWidget : public wxQtEventSignalHandler< QWidget, wxFrame >
wxFrame::~wxFrame()
{
- // central widget should be deleted by qt when the main window is destroyed
- QtStoreWindowPointer( GetQMainWindow()->centralWidget(), nullptr );
+ if ( GetQMainWindow() && GetQMainWindow()->centralWidget() )
I think it could be useful to mention that this test is false for MDI children here too, otherwise it's not very clear.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet commented on this pull request.
In src/qt/mdi.cpp:
>
bool wxMDIClientWindow::CreateClient(wxMDIParentFrame *parent, long WXUNUSED(style))
{
// create the MDI client area where the children window are displayed:
- m_qtWindow = new wxQtMdiArea( parent, this );
+ m_qtWindow = new wxQtMdiArea(parent, this);
+ m_qtContainer = dynamic_cast<QAbstractScrollArea*>(m_qtWindow);
Oops! The cast should always succeed and we can simply use static_cast<> instead of dynamic_cast<> because wxQtMdiArea derives from QMdiArea, which in turn derives from QAbstractScrollArea.
I'll update the code to:
// The cast is safe because we know wxQtMdiArea derives QAbstractScrollArea m_qtContainer = static_cast<QAbstractScrollArea*>(m_qtWindow);
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet commented on this pull request.
> @@ -29,15 +34,54 @@ class WXDLLIMPEXP_CORE wxMDIParentFrame : public wxMDIParentFrameBase
long style = wxDEFAULT_FRAME_STYLE | wxVSCROLL | wxHSCROLL,
const wxString& name = wxASCII_STR(wxFrameNameStr));
+ QMdiArea* GetQtMdiArea() const;
+
+ void QtSetPreferredDI(bool isTDI = true);
I made this change with MDI first to be compatible with QMdiArea::ViewMode enum. And change QtSetPreferredDI() to QtSetPreferredDILayout()
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
> private: + void OnMDICommand(wxCommandEvent& event); + + // add/remove window menu if we have it (i.e. m_windowMenu != nullptr) + void AddWindowMenu(); + void RemoveWindowMenu(); + + // update the window menu (if we have it) to enable or disable the commands + // which only make sense when we have more than one child + void UpdateWindowMenu(bool enable);
I changed wxMDIChildFrame::UpdateWindowMenu() to a more appropriate name AttachWindowMenuTo() and I left this one (wxMDIParentFrame's function) as is because this code simply is copied from wxMSW port. And I don't understand why we need to change it here (in wxQt) and leave it unchanged in wxMSW?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 12 commits.
- 9a357c1 wxQt: Avoid assertion in docview sample when setting wxCURSOR_PENCIL
- 1b78ac2 wxQt: Prevent Qt from deleting the underlying QMenuBar pointer
- 97ec4eb wxQt: Fix warnings about invalid painter usage
- ca13ef2 wxQt: Fix memory leak in wxMDIParentFrame
- dc3cb4f wxQt: Get rid of the wxQtMDIParentFrame class
- d6b23ad wxQt: Add wxMDIParentFrame::GetQtMdiArea() helper function
- c41c2e3 wxQt: Fix wxMDIClientWindow being derived from wxFrame
- a6b7009 wxQt: Fix attaching/detaching the wxMDIChildFrame's menubar to the wxMDIParentFrame
- 7c8a981 wxQt: Implement missing functionalities for MDI
- a0db458 wxQt: Add code to show/hide/update window menu in MDI
- 8bbe73d wxQt: Generate wxEVT_ACTIVATE event when wxMDIChildFrame [de]activated
- 649ee26 wxQt: Add wxMDIParentFrame::QtSetPreferredDILayout() member function
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet commented on this pull request.
> @@ -207,7 +207,9 @@ MyCanvas::MyCanvas(wxView *view, wxWindow *parent)
m_currentSegment = nullptr;
m_lastMousePos = wxDefaultPosition;
+#ifndef __WXQT__
Sorry I don't understand what you mean by this ?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 10 commits.
- 60f5bff wxQt: Fix warnings about invalid painter usage
- e0fef87 wxQt: Fix memory leak in wxMDIParentFrame
- 6073ed1 wxQt: Get rid of the wxQtMDIParentFrame class
- 1fab4f3 wxQt: Add wxMDIParentFrame::GetQtMdiArea() helper function
- 05221fa wxQt: Fix wxMDIClientWindow being derived from wxFrame
- 2f84c05 wxQt: Fix attaching/detaching the wxMDIChildFrame's menubar to the wxMDIParentFrame
- c59b9fe wxQt: Implement missing functionalities for MDI
- c1b2c4e wxQt: Add code to show/hide/update window menu in MDI
- f10b46c wxQt: Generate wxEVT_ACTIVATE event when wxMDIChildFrame [de]activated
- c9fda6e wxQt: Add wxMDIParentFrame::QtSetPreferredDILayout() member function
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
> @@ -207,7 +207,9 @@ MyCanvas::MyCanvas(wxView *view, wxWindow *parent)
m_currentSegment = nullptr;
m_lastMousePos = wxDefaultPosition;
+#ifndef __WXQT__
I'd like to avoid Qt-specific checks in the sample. I guess this is needed because SetCursor() doesn't work (in this case) currently?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
In .github/workflows/ci_cmake.yml:
> @@ -120,6 +120,7 @@ jobs:
arch: 'win64_msvc2022_64'
archives: 'qtbase qtsvg qtimageformats'
cache: 'true'
+ aqtversion: '==3.1.*'
Thanks for fixing the CI, I'll push this change separately at once.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25332)Just in case you missed it, there is now a failure in EventPropagationTestCase.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 1 commit.
- facf62b wxQt: Fix a failing test in EventPropagationTestCase::DocView() under linux
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 15 commits.
- 459c26d wxQt: Prevent Qt from deleting the underlying QMenuBar pointer
- d7e0f82 wxQt: Fix warnings about invalid painter usage
- 0f33d99 wxQt: Fix memory leak in wxMDIParentFrame
- 9178c7c wxQt: Get rid of the wxQtMDIParentFrame class
- 12274c4 wxQt: Add wxMDIParentFrame::GetQtMdiArea() helper function
- c07c636 wxQt: Fix wxMDIClientWindow being derived from wxFrame
- 3981868 wxQt: Fix attaching/detaching the wxMDIChildFrame's menubar to the wxMDIParentFrame
- c33c36f wxQt: Implement missing functionalities for MDI
- b5a818a wxQt: Add code to show/hide/update window menu in MDI
- 1a9925d wxQt: Generate wxEVT_ACTIVATE event when wxMDIChildFrame [de]activated
- db462c6 wxQt: Add wxMDIParentFrame::QtSetPreferredDILayout() member function
- e2c68fc wxQt: Fix GitHub Actions failure when downloading Qt 6.8.*
- d77f11e wQt: Enable some disabled tests in EventPropagationTestCase::DocView()
- 54d37f1 wxQt: Fix a test in EventPropagationTestCase::DocView() failing under linux
- 508fa6d wxQt: Avoid assertion in docview sample when setting wxCURSOR_PENCIL
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 3 commits.
- e2464cb wxQt: Enable some disabled tests in EventPropagationTestCase::DocView()
- 250c0d4 wxQt: Fix a test in EventPropagationTestCase::DocView() failing under linux
- 533cff8 wxQt: Avoid assertion in docview sample when setting wxCURSOR_PENCIL
You are receiving this because you are subscribed to this thread.
AliKet
AliKet left a comment (wxWidgets/wxWidgets#25332)FYI the last commit is just an experiment and I intend to change it
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 0 commits.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
AliKet left a comment (wxWidgets/wxWidgets#25332)Restarting the failing checks should fix Error: retrieving gpg key timed out.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 1 commit.
- b4d80a7 Use wxUIActionSimulator to test event propagation order DocView test
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
oneeyeman1 left a comment (wxWidgets/wxWidgets#25332)@AliKet ,
I just tried to test something else on my 3.2.7 code, but found something completely different.
Start docview sample.
Select Image
Close child frame
You will get segmentation fault.
Backtrace:
Thread 1 "docview" received signal SIGSEGV, Segmentation fault.
0x00007ffff6e42aa0 in ?? () from /usr/lib64/libQt5Widgets.so.5
(gdb) bt
#0 0x00007ffff6e42aa0 in ?? () from /usr/lib64/libQt5Widgets.so.5
#1 0x00007ffff78f467e in wxFrame::~wxFrame (this=0x555555a0b230, __in_chrg=<optimized out>) at ../src/qt/frame.cpp:47
#2 0x00007ffff790ffbe in wxMDIChildFrameBase::~wxMDIChildFrameBase (this=0x555555a0b230, __in_chrg=<optimized out>) at ../include/wx/mdi.h:146
#3 0x00007ffff7910b0a in wxMDIChildFrame::~wxMDIChildFrame (this=0x555555a0b230, __in_chrg=<optimized out>) at ../include/wx/qt/mdi.h:47
#4 0x00007ffff79c6fc4 in wxDocChildFrameAny<wxMDIChildFrame, wxMDIParentFrame>::~wxDocChildFrameAny (this=0x555555a0b230, __in_chrg=<optimized out>)
at ../include/wx/docview.h:676
#5 0x00007ffff79c701e in wxDocMDIChildFrame::~wxDocMDIChildFrame (this=0x555555a0b230, __in_chrg=<optimized out>) at ../include/wx/docmdi.h:62
#6 0x00007ffff79c703a in wxDocMDIChildFrame::~wxDocMDIChildFrame (this=0x555555a0b230, __in_chrg=<optimized out>) at ../include/wx/docmdi.h:62
#7 0x00007ffff731e7c9 in wxAppConsoleBase::DeletePendingObjects (this=0x5555556056b0) at ../src/common/appbase.cpp:655
#8 0x00007ffff731df9a in wxAppConsoleBase::ProcessIdle (this=0x5555556056b0) at ../src/common/appbase.cpp:458
#9 0x00007ffff796f641 in wxAppBase::ProcessIdle (this=0x5555556056b0) at ../src/common/appcmn.cpp:396
#10 0x00007ffff78e9b35 in wxQtIdleTimer::idle (this=0x555555b9a390) at ../src/qt/evtloop.cpp:77
#11 0x00007ffff78eb584 in QtPrivate::FunctorCall<QtPrivate::IndexesList<>, QtPrivate::List<>, void, void (wxQtIdleTimer::*)()>::call(void (wxQtIdleTimer::*)(), wxQtIdleTimer*, void**) (f=(void (wxQtIdleTimer::*)(wxQtIdleTimer * const)) 0x7ffff78e9aec <wxQtIdleTimer::idle()>, o=0x555555b9a390, arg=0x7fffffffd6a0)
at /usr/include/qt5/QtCore/qobjectdefs_impl.h:152
#12 0x00007ffff78eb23d in QtPrivate::FunctionPointer<void (wxQtIdleTimer::*)()>::call<QtPrivate::List<>, void>(void (wxQtIdleTimer::*)(), wxQtIdleTimer*, void**) (
f=(void (wxQtIdleTimer::*)(wxQtIdleTimer * const)) 0x7ffff78e9aec <wxQtIdleTimer::idle()>, o=0x555555b9a390, arg=0x7fffffffd6a0)
at /usr/include/qt5/QtCore/qobjectdefs_impl.h:185
#13 0x00007ffff78eb0b3 in QtPrivate::QSlotObject<void (wxQtIdleTimer::*)(), QtPrivate::List<>, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*)
(which=1, this_=0x555555b81700, r=0x555555b9a390, a=0x7fffffffd6a0, ret=0x0) at /usr/include/qt5/QtCore/qobjectdefs_impl.h:418
#14 0x00007ffff633a502 in ?? () from /usr/lib64/libQt5Core.so.5
#15 0x00007ffff633e13a in QTimer::timeout(QTimer::QPrivateSignal) () from /usr/lib64/libQt5Core.so.5
#16 0x00007ffff633301f in QObject::event(QEvent*) () from /usr/lib64/libQt5Core.so.5
#17 0x00007ffff6cf219f in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib64/libQt5Widgets.so.5
#18 0x00007ffff630c0c8 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /usr/lib64/libQt5Core.so.5
#19 0x00007ffff63561b3 in QTimerInfoList::activateTimers() () from /usr/lib64/libQt5Core.so.5
#20 0x00007ffff6356a31 in ?? () from /usr/lib64/libQt5Core.so.5
#21 0x00007ffff501eaec in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
#22 0x00007ffff501ed98 in ?? () from /usr/lib64/libglib-2.0.so.0
#23 0x00007ffff501ee4f in g_main_context_iteration () from /usr/lib64/libglib-2.0.so.0
#24 0x00007ffff6356e10 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib64/libQt5Core.so.5
#25 0x00007ffff630ab7b in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib64/libQt5Core.so.5
#26 0x00007ffff78e9e3e in wxQtEventLoopBase::DoRun (this=0x555555b94050) at ../src/qt/evtloop.cpp:122
--Type <RET> for more, q to quit, c to continue without paging--
#27 0x00007ffff735b9f4 in wxEventLoopBase::Run (this=0x555555b94050) at ../src/common/evtloopcmn.cpp:87
#28 0x00007ffff731dce6 in wxAppConsoleBase::MainLoop (this=0x5555556056b0) at ../src/common/appbase.cpp:395
#29 0x00007ffff731da2b in wxAppConsoleBase::OnRun (this=0x5555556056b0) at ../src/common/appbase.cpp:317
#30 0x00007ffff796f347 in wxAppBase::OnRun (this=0x5555556056b0) at ../src/common/appcmn.cpp:334
#31 0x00007ffff7395279 in wxEntry (argc=@0x7ffff755e744: 1, argv=0x555555605490) at ../src/common/init.cpp:497
#32 0x00007ffff7395352 in wxEntry (argc=@0x7fffffffdbcc: 1, argv=0x7fffffffdcc8) at ../src/common/init.cpp:509
#33 0x0000555555577cac in main (argc=1, argv=0x7fffffffdcc8) at ../../../samples/docview/docview.cpp:74
(gdb)
I don't know if this is also addressed. Just wanted to mention it.
If its not - its probably late anyway and most likely will need to be addressed in the new PR.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
AliKet left a comment (wxWidgets/wxWidgets#25332)That issue is addressed in this commit
FYI, there are a lot of bugs/improvements in wxQt 3.3 compared to previous versions, and upgrading to it is more than welcome. And if you could, please test with this PR and report any issues before it get merged. TIA!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25332)Just to be sure: this is ready to be merged, isn't it?
Thanks!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
AliKet left a comment (wxWidgets/wxWidgets#25332)Just to be sure: this is ready to be merged, isn't it?
Thanks!
Yes, TIA.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Merged #25332 into master.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.