Skip to content

Conversation

AliKet
Copy link
Contributor

@AliKet AliKet commented Apr 15, 2025

No description provided.

@AliKet

This comment was marked as outdated.

Copy link
Contributor

@vadz vadz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@vadz
Copy link
Contributor

vadz commented Apr 15, 2025

@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.

@AliKet

This comment was marked as outdated.

@AliKet

This comment was marked as outdated.

@AliKet AliKet force-pushed the qt-mdi-fixes branch 5 times, most recently from 49b7cc0 to bec80c1 Compare April 17, 2025 14:16
@AliKet

This comment was marked as outdated.

@jdpurcell
Copy link

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.

@AliKet AliKet force-pushed the qt-mdi-fixes branch 3 times, most recently from 953767f to 956fde4 Compare April 19, 2025 15:16
@AliKet

This comment was marked as outdated.

Copy link
Contributor

@vadz vadz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@AliKet AliKet force-pushed the qt-mdi-fixes branch 2 times, most recently from 649ee26 to c9fda6e Compare April 21, 2025 17:49
@vadz
Copy link
Contributor

vadz commented Apr 21, 2025

Just in case you missed it, there is now a failure in EventPropagationTestCase.

vadz pushed a commit that referenced this pull request Apr 21, 2025
@AliKet AliKet force-pushed the qt-mdi-fixes branch 3 times, most recently from 5648148 to bb469cf Compare April 22, 2025 15:23
aliket83 added 11 commits April 22, 2025 22:41
If m_qtWindow is an instance of QAbstractScrollArea then the painter
should paint on the viewport and not on the widget itself.

Closes wxWidgets#25226
The QMainWindow widget will be created by wxFrame which wxMDIParentFrame derives from.

This commit also contains some code formatting for consistency with wxWidgets style.
No real changes, this function will be used in the upcomming commits to avoid
explicit static_cast<QMdiArea*> when working with m_clientWindow->GetHandle()
wxMDIClientWindow derives from wxFrame for its interface only for use in MDI
and is not intended to be used as a real TLW.
Some of the code is taken from wxMSW port with minor changes.
This helps users to choose the preferred DI used by wxMDIParentFrame:
- Layout::MDI (the default under Windows)
- Layout::Tabbed (the default under non-Windows platforms)

Currently the default is to use MDI on Windows and TDI elsewhere.
@AliKet AliKet force-pushed the qt-mdi-fixes branch 2 times, most recently from 533cff8 to 6ebea3e Compare April 23, 2025 11:45
@AliKet

This comment was marked as outdated.

@AliKet AliKet force-pushed the qt-mdi-fixes branch 2 times, most recently from 250c0d4 to ec3e698 Compare April 24, 2025 14:07
@AliKet

This comment was marked as outdated.

Because using ASSERT_MENU_EVENT_RESULT, which uses wxMenuBase::SendEvent()
under the hood, fails with wxQt on Linux. While the former seems to work on
all platforms.
@oneeyeman1
Copy link
Contributor

@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.

@AliKet
Copy link
Contributor Author

AliKet commented Apr 25, 2025

@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.

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!

@vadz
Copy link
Contributor

vadz commented Apr 29, 2025

Just to be sure: this is ready to be merged, isn't it?

Thanks!

@AliKet
Copy link
Contributor Author

AliKet commented Apr 30, 2025

Just to be sure: this is ready to be merged, isn't it?

Thanks!

Yes, TIA.

@vadz vadz merged commit d7a696d into wxWidgets:master Apr 30, 2025
41 checks passed
@AliKet AliKet deleted the qt-mdi-fixes branch May 17, 2025 23:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants