Getting ready to Qt 6 (3/n). Do not use QKeyEvent copy constructor #580
Getting ready to Qt 6 (3/n). Do not use QKeyEvent copy constructor #580hebasto merged 1 commit intobitcoin-core:masterfrom
QKeyEvent copy constructor #580Conversation
shaavan
left a comment
shaavan
left a comment
There was a problem hiding this comment.
ACK d543515
- I was able to verify that with Qt 6, the copying of QEvent has been disabled. See for reference {second and third paragraph}.
- Therefore, instead of copying the key event, this PR uses its data to create a new event.
nit:
As a minor nit, I would suggest creating a variable type instead of calling keyevt→type() multiple times.
QEvent::Type type = keyevt->type()
|
Why are we creating a new event instead of forwarding the original one on? Seems like this PR would simply be bypassing the behaviour Qt is trying to prevent... |
"forwarding the original one on" ends with double deletion of an |
|
Hmm, how about using https://doc.qt.io/qt-5/qcoreapplication.html#sendEvent ? |
This change is preparation for Qt 6, and it fixes an experimental build with Qt 6.2.4 as copying of `QEvent` has been disabled in Qt 6.0.0 (see 19f9b0d5f54379151eb71e98555b203ad6756276 upstream commit).
shaavan
left a comment
shaavan
left a comment
There was a problem hiding this comment.
reACK 3ec6504
Changes since my last review:
- Instead of creating a new event for each call of postEvent, send Event is used with keyevt event.
Since sendEvent does not delete the event argument after the completion of its execution, the same keyevt could be used multiple times instead of creating a separate event for each instance of using postEvent. Therefore I agree with this change.
References:
- postEvent: https://doc.qt.io/qt-6/qcoreapplication.html#postEvent
- sendEvent: https://doc.qt.io/qt-6/qcoreapplication.html#sendEvent
- Comparison between the two: https://doc.qt.io/archives/qq/qq11-events.html (under section Synthetic Events)
…QKeyEvent` copy constructor 3ec6504 qt: Do not use `QKeyEvent` copy constructor (Hennadii Stepanov) Pull request description: This PR is preparation for [Qt 6](bitcoin#24798), and it fixes an experimental build with Qt 6.2.4 as copying of `QEvent` has been [disabled](qt/qtbase@19f9b0d) in Qt 6.0.0. ACKs for top commit: w0xlt: tACK bitcoin-core/gui@3ec6504 on Ubuntu 21.10, Qt 5.15.2 shaavan: reACK 3ec6504 Tree-SHA512: 583a9dad0c621d9f02f77ccaa9f55ee79e12e3c47f418911ef2dfe0de357d772d1928ae3ec19b6f0c0674da858bab9d4542a26cc14b06ed921370dfeabd1c194
4 participants
This PR is preparation for Qt 6, and it fixes an experimental build with Qt 6.2.4 as copying of
QEventhas been disabled in Qt 6.0.0.