From 86cf366a30c178006ee23f0fbf5bee1748a26b57 Mon Sep 17 00:00:00 2001 From: Oliver Wolff Date: Thu, 31 Jan 2019 13:10:50 +0100 Subject: [PATCH 01/17] winrt: Use ES3 ANGLE code path when blitting widgets We run into validation issues when using the ES2 code path when blitting widgets on winrt. By using ES3 we not only avoid this issue, but there might also be performance gains. We now call window()->format() instead of window()->requestedFormat as the latter will not respect the values that were set on initialization of the native window (which is done in QWinRTWindow's constructor). Change-Id: I5ed7a9326691375f9c9cb5d8d22ee8d1b643fbd0 Reviewed-by: Andre de la Rocha Reviewed-by: Miguel Costa --- src/plugins/platforms/winrt/qwinrtbackingstore.cpp | 7 +++---- src/plugins/platforms/winrt/qwinrtwindow.cpp | 2 ++ 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/plugins/platforms/winrt/qwinrtbackingstore.cpp b/src/plugins/platforms/winrt/qwinrtbackingstore.cpp index c23d48b2dd..fbf611d7f7 100644 --- a/src/plugins/platforms/winrt/qwinrtbackingstore.cpp +++ b/src/plugins/platforms/winrt/qwinrtbackingstore.cpp @@ -45,8 +45,7 @@ #include #include -#include -#include +#include QT_BEGIN_NAMESPACE @@ -83,7 +82,7 @@ bool QWinRTBackingStore::initialize() return true; d->context.reset(new QOpenGLContext); - QSurfaceFormat format = window()->requestedFormat(); + QSurfaceFormat format = window()->format(); d->context->setFormat(format); d->context->setScreen(window()->screen()); if (!d->context->create()) @@ -138,7 +137,7 @@ void QWinRTBackingStore::flush(QWindow *window, const QRegion ®ion, const QPo const int y2 = y1 + bounds.height(); const int x1 = bounds.x(); const int x2 = x1 + bounds.width(); - glBlitFramebufferANGLE(x1, y1, x2, y2, + glBlitFramebuffer(x1, y1, x2, y2, x1, d->size.height() - y1, x2, d->size.height() - y2, GL_COLOR_BUFFER_BIT, GL_NEAREST); diff --git a/src/plugins/platforms/winrt/qwinrtwindow.cpp b/src/plugins/platforms/winrt/qwinrtwindow.cpp index 29d234d276..83c3715bfd 100644 --- a/src/plugins/platforms/winrt/qwinrtwindow.cpp +++ b/src/plugins/platforms/winrt/qwinrtwindow.cpp @@ -112,6 +112,8 @@ QWinRTWindow::QWinRTWindow(QWindow *window) d->screen = static_cast(screen()); handleContentOrientationChange(window->contentOrientation()); + d->surfaceFormat.setMajorVersion(3); + d->surfaceFormat.setMinorVersion(0); d->surfaceFormat.setAlphaBufferSize(0); d->surfaceFormat.setRedBufferSize(8); d->surfaceFormat.setGreenBufferSize(8); From 1036c450860fcc6eb1b5fd702f8ca83972170bb7 Mon Sep 17 00:00:00 2001 From: Oliver Wolff Date: Wed, 30 Jan 2019 10:05:31 +0100 Subject: [PATCH 02/17] Revert "winrt: Skip context validation in ANGLE" Our current ANGLE version (chromium/3280) relies on validation to be done when doing the rendering, as the validation at the same time completes the caching. Skipping the validation caused asserts and rendering issues. The part of the validation that failed before is now deactivated in Qt's copy of ANGLE as it is not relevant for our use case, so that validation can be re-enabled now. This reverts commit a1dec825f9aa9d7f86d740fd420f1c084463f507. Fixes: QTBUG-73317 Change-Id: I5fd176eaa0bc28d93ca93019b7092211fe5bcce5 Reviewed-by: Miguel Costa Reviewed-by: Andre de la Rocha --- src/plugins/platforms/winrt/qwinrteglcontext.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/plugins/platforms/winrt/qwinrteglcontext.cpp b/src/plugins/platforms/winrt/qwinrteglcontext.cpp index eeb79be2e6..5ce2671bf1 100644 --- a/src/plugins/platforms/winrt/qwinrteglcontext.cpp +++ b/src/plugins/platforms/winrt/qwinrteglcontext.cpp @@ -138,7 +138,6 @@ void QWinRTEGLContext::initialize() EGL_CONTEXT_CLIENT_VERSION, d->format.majorVersion(), EGL_CONTEXT_MINOR_VERSION_KHR, d->format.minorVersion(), EGL_CONTEXT_FLAGS_KHR, flags, - EGL_CONTEXT_OPENGL_NO_ERROR_KHR, true, EGL_NONE }; d->eglContext = eglCreateContext(g->eglDisplay, d->eglConfig, d->eglShareContext, attributes); From 932b13d3ec76eedfc0e76e81c05b2d54552c0715 Mon Sep 17 00:00:00 2001 From: Oliver Wolff Date: Mon, 4 Feb 2019 11:23:16 +0100 Subject: [PATCH 03/17] windows: Support OpenGL ES versions > 3.0 with ANGLE Even though our ANGLE versions only partially supports OpenGL ES > 3.0, there are users who want to use functionality that is available. By setting major and minor version we can support this use case. Fixes: QTBUG-72762 Change-Id: I9a1d3009355693baa971deb3c4bbf14c595edf0b Reviewed-by: Andre de la Rocha Reviewed-by: Miguel Costa --- src/plugins/platforms/windows/qwindowseglcontext.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/plugins/platforms/windows/qwindowseglcontext.cpp b/src/plugins/platforms/windows/qwindowseglcontext.cpp index 70ba2784e9..063e81150e 100644 --- a/src/plugins/platforms/windows/qwindowseglcontext.cpp +++ b/src/plugins/platforms/windows/qwindowseglcontext.cpp @@ -393,8 +393,14 @@ QWindowsEGLContext::QWindowsEGLContext(QWindowsEGLStaticContext *staticContext, m_shareContext = share ? static_cast(share)->m_eglContext : nullptr; QVector contextAttrs; - contextAttrs.append(EGL_CONTEXT_CLIENT_VERSION); - contextAttrs.append(m_format.majorVersion()); + const int major = m_format.majorVersion(); + const int minor = m_format.minorVersion(); + if (major > 3 || (major == 3 && minor > 0)) + qWarning("QWindowsEGLContext: ANGLE only partially supports OpenGL ES > 3.0"); + contextAttrs.append(EGL_CONTEXT_MAJOR_VERSION); + contextAttrs.append(major); + contextAttrs.append(EGL_CONTEXT_MINOR_VERSION); + contextAttrs.append(minor); contextAttrs.append(EGL_NONE); QWindowsEGLStaticContext::libEGL.eglBindAPI(m_api); From 5f384bd39c54b6bd60e71af1e3ac1b8168a33ed9 Mon Sep 17 00:00:00 2001 From: Oliver Wolff Date: Mon, 4 Feb 2019 12:15:54 +0100 Subject: [PATCH 04/17] winrt: Warn if OpenGL ES version is set to a value > 3.0 Our bundled ANGLE library only partially supports OpenGL ES > 3.0 so warn users that there might be dragons. Change-Id: I16711fe9f449e85dd8b2369e1fcec6c9f81d5ae0 Reviewed-by: Andre de la Rocha Reviewed-by: Miguel Costa --- src/plugins/platforms/winrt/qwinrteglcontext.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/plugins/platforms/winrt/qwinrteglcontext.cpp b/src/plugins/platforms/winrt/qwinrteglcontext.cpp index 5ce2671bf1..aa64ac1f99 100644 --- a/src/plugins/platforms/winrt/qwinrteglcontext.cpp +++ b/src/plugins/platforms/winrt/qwinrteglcontext.cpp @@ -134,6 +134,10 @@ void QWinRTEGLContext::initialize() const EGLint flags = d->format.testOption(QSurfaceFormat::DebugContext) ? EGL_CONTEXT_OPENGL_DEBUG_BIT_KHR : 0; + const int major = d->format.majorVersion(); + const int minor = d->format.minorVersion(); + if (major > 3 || (major == 3 && minor > 0)) + qWarning("QWinRTEGLContext: ANGLE only partially supports OpenGL ES > 3.0"); const EGLint attributes[] = { EGL_CONTEXT_CLIENT_VERSION, d->format.majorVersion(), EGL_CONTEXT_MINOR_VERSION_KHR, d->format.minorVersion(), From bf7458c76fcf821637368d05938823e42fbb28d5 Mon Sep 17 00:00:00 2001 From: Oliver Wolff Date: Wed, 6 Feb 2019 11:16:32 +0100 Subject: [PATCH 05/17] winrt qpa: Remove last windows phone leftovers WINAPI_PARTITION_PHONE_APP is defined for all our winrt mkspecs nowadays so the code can be used unconditionally. Change-Id: I4f2b60a0b9bba5b407ebbc213c44a0e5b4057855 Reviewed-by: Maurice Kalinowski --- .../platforms/winrt/qwinrtinputcontext.cpp | 4 --- .../platforms/winrt/qwinrtinputcontext.h | 2 -- src/plugins/platforms/winrt/qwinrtscreen.cpp | 29 ------------------- src/plugins/platforms/winrt/qwinrtscreen.h | 5 ---- 4 files changed, 40 deletions(-) diff --git a/src/plugins/platforms/winrt/qwinrtinputcontext.cpp b/src/plugins/platforms/winrt/qwinrtinputcontext.cpp index f7e91bb047..5ae94ba613 100644 --- a/src/plugins/platforms/winrt/qwinrtinputcontext.cpp +++ b/src/plugins/platforms/winrt/qwinrtinputcontext.cpp @@ -158,8 +158,6 @@ HRESULT QWinRTInputContext::handleVisibilityChange(IInputPane *pane) return S_OK; } -#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) - static HRESULT getInputPane(ComPtr *inputPane2) { ComPtr factory; @@ -221,6 +219,4 @@ void QWinRTInputContext::hideInputPanel() }); } -#endif // WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) - QT_END_NAMESPACE diff --git a/src/plugins/platforms/winrt/qwinrtinputcontext.h b/src/plugins/platforms/winrt/qwinrtinputcontext.h index 13a0088ddc..59db90231f 100644 --- a/src/plugins/platforms/winrt/qwinrtinputcontext.h +++ b/src/plugins/platforms/winrt/qwinrtinputcontext.h @@ -74,10 +74,8 @@ public: bool isInputPanelVisible() const override; -#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) void showInputPanel() override; void hideInputPanel() override; -#endif private slots: void updateScreenCursorRect(); diff --git a/src/plugins/platforms/winrt/qwinrtscreen.cpp b/src/plugins/platforms/winrt/qwinrtscreen.cpp index bd2bbcb81c..7f1854c601 100644 --- a/src/plugins/platforms/winrt/qwinrtscreen.cpp +++ b/src/plugins/platforms/winrt/qwinrtscreen.cpp @@ -91,13 +91,10 @@ typedef ITypedEventHandler CharacterRe typedef ITypedEventHandler InputEnabledHandler; typedef ITypedEventHandler KeyHandler; typedef ITypedEventHandler PointerHandler; -typedef ITypedEventHandler SizeChangedHandler; typedef ITypedEventHandler VisibilityChangedHandler; typedef ITypedEventHandler DisplayInformationHandler; typedef ITypedEventHandler RedirectHandler; -#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) typedef ITypedEventHandler VisibleBoundsChangedHandler; -#endif // WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) QT_BEGIN_NAMESPACE @@ -479,10 +476,8 @@ typedef HRESULT (__stdcall IDisplayInformation::*DisplayCallbackRemover)(EventRe uint qHash(DisplayCallbackRemover key) { void *ptr = *(void **)(&key); return qHash(ptr); } typedef HRESULT (__stdcall ICorePointerRedirector::*RedirectorCallbackRemover)(EventRegistrationToken); uint qHash(RedirectorCallbackRemover key) { void *ptr = *(void **)(&key); return qHash(ptr); } -#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) typedef HRESULT (__stdcall IApplicationView2::*ApplicationView2CallbackRemover)(EventRegistrationToken); uint qHash(ApplicationView2CallbackRemover key) { void *ptr = *(void **)(&key); return qHash(ptr); } -#endif // WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) class QWinRTScreenPrivate { @@ -509,10 +504,8 @@ public: QHash windowTokens; QHash displayTokens; QHash redirectTokens; -#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) QHash view2Tokens; ComPtr view2; -#endif // WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) QAtomicPointer mouseGrabWindow; QAtomicPointer keyboardGrabWindow; QWindow *currentPressWindow = nullptr; @@ -603,10 +596,8 @@ QWinRTScreen::QWinRTScreen() d->cursor.reset(new QWinRTCursor); -#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) hr = d->view.As(&d->view2); Q_ASSERT_SUCCEEDED(hr); -#endif // WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) } QWinRTScreen::~QWinRTScreen() @@ -630,12 +621,10 @@ QWinRTScreen::~QWinRTScreen() hr = (d->redirect.Get()->*i.key())(i.value()); Q_ASSERT_SUCCEEDED(hr); } -#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) for (QHash::const_iterator i = d->view2Tokens.begin(); i != d->view2Tokens.end(); ++i) { hr = (d->view2.Get()->*i.key())(i.value()); Q_ASSERT_SUCCEEDED(hr); } -#endif // WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) return hr; }); RETURN_VOID_IF_FAILED("Failed to unregister screen event callbacks"); @@ -777,14 +766,8 @@ void QWinRTScreen::initialize() Q_ASSERT_SUCCEEDED(hr); hr = d->coreWindow->add_PointerWheelChanged(Callback(this, &QWinRTScreen::onPointerUpdated).Get(), &d->windowTokens[&ICoreWindow::remove_PointerWheelChanged]); Q_ASSERT_SUCCEEDED(hr); -#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) hr = d->view2->add_VisibleBoundsChanged(Callback(this, &QWinRTScreen::onWindowSizeChanged).Get(), &d->view2Tokens[&IApplicationView2::remove_VisibleBoundsChanged]); Q_ASSERT_SUCCEEDED(hr); -#else - hr = d->coreWindow->add_SizeChanged(Callback(this, &QWinRTScreen::onWindowSizeChanged).Get(), &d->windowTokens[&ICoreWindow::remove_SizeChanged]); - Q_ASSERT_SUCCEEDED(hr) -#endif // WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) - hr = d->coreWindow->add_Activated(Callback(this, &QWinRTScreen::onActivated).Get(), &d->windowTokens[&ICoreWindow::remove_Activated]); Q_ASSERT_SUCCEEDED(hr); hr = d->coreWindow->add_Closed(Callback(this, &QWinRTScreen::onClosed).Get(), &d->windowTokens[&ICoreWindow::remove_Closed]); @@ -820,7 +803,6 @@ void QWinRTScreen::setKeyboardRect(const QRectF &keyboardRect) return; } d->logicalRect = QRectF(windowSize.X, windowSize.Y, windowSize.Width, windowSize.Height); -#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) Rect visibleRect; hr = d->view2->get_VisibleBounds(&visibleRect); if (FAILED(hr)) { @@ -828,9 +810,6 @@ void QWinRTScreen::setKeyboardRect(const QRectF &keyboardRect) return; } visibleRectF = QRectF(visibleRect.X, visibleRect.Y, visibleRect.Width, visibleRect.Height); -#else - visibleRectF = d->logicalRect; -#endif // WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) // if keyboard is snapped to the bottom of the screen and would cover the cursor the content is // moved up to make it visible if (keyboardRect.intersects(mCursorRect) @@ -1528,11 +1507,7 @@ HRESULT QWinRTScreen::onRedirectReleased(ICorePointerRedirector *, IPointerEvent return onPointerUpdated(nullptr, args); } -#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) HRESULT QWinRTScreen::onWindowSizeChanged(IApplicationView *, IInspectable *) -#else -HRESULT QWinRTScreen::onWindowSizeChanged(ICoreWindow *, IWindowSizeChangedEventArgs *) -#endif // WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) { Q_D(QWinRTScreen); @@ -1543,14 +1518,10 @@ HRESULT QWinRTScreen::onWindowSizeChanged(ICoreWindow *, IWindowSizeChangedEvent RETURN_OK_IF_FAILED("Failed to get window bounds"); d->logicalRect = QRectF(windowSize.X, windowSize.Y, windowSize.Width, windowSize.Height); -#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) Rect visibleRect; hr = d->view2->get_VisibleBounds(&visibleRect); RETURN_OK_IF_FAILED("Failed to get window visible bounds"); d->visibleRect = QRectF(visibleRect.X, visibleRect.Y, visibleRect.Width, visibleRect.Height); -#else - d->visibleRect = QRectF(windowSize.X, windowSize.Y, windowSize.Width, windowSize.Height); -#endif // WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) qCDebug(lcQpaWindows) << __FUNCTION__ << d->logicalRect; QWindowSystemInterface::handleScreenGeometryChange(screen(), geometry(), availableGeometry()); diff --git a/src/plugins/platforms/winrt/qwinrtscreen.h b/src/plugins/platforms/winrt/qwinrtscreen.h index cde148a638..e28cfd8cc8 100644 --- a/src/plugins/platforms/winrt/qwinrtscreen.h +++ b/src/plugins/platforms/winrt/qwinrtscreen.h @@ -59,7 +59,6 @@ namespace ABI { struct IPointerEventArgs; struct IVisibilityChangedEventArgs; struct IWindowActivatedEventArgs; - struct IWindowSizeChangedEventArgs; } namespace Xaml { struct IDependencyObject; @@ -154,11 +153,7 @@ private: HRESULT onOrientationChanged(ABI::Windows::Graphics::Display::IDisplayInformation *, IInspectable *); HRESULT onDpiChanged(ABI::Windows::Graphics::Display::IDisplayInformation *, IInspectable *); -#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_PHONE_APP) HRESULT onWindowSizeChanged(ABI::Windows::UI::ViewManagement::IApplicationView *, IInspectable *); -#else - HRESULT onWindowSizeChanged(ABI::Windows::UI::Core::ICoreWindow *, ABI::Windows::UI::Core::IWindowSizeChangedEventArgs *); -#endif HRESULT onRedirectReleased(ABI::Windows::UI::Core::ICorePointerRedirector *, ABI::Windows::UI::Core::IPointerEventArgs *); QScopedPointer d_ptr; From b611eb81c822ed2bcd3107ba098b56952ae0685c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Morten=20Johan=20S=C3=B8rvig?= Date: Tue, 5 Feb 2019 11:43:04 +0100 Subject: [PATCH 06/17] Fix QDeadlineTimer::Forever case in QWaitCondition The timeout will never be larger than numeric_limits::max(), especially on platforms with 32-bit longs. Instead, test if the timeout is exactly numeric_limits::max(), which matches the ULONG_MAX value which is documented to indicate no timeout. Change-Id: Ib663eddb5703797c50c04fd4eae60bd64f379d1c Reviewed-by: Thiago Macieira Reviewed-by: Edward Welbourne --- src/corelib/thread/qwaitcondition_unix.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/corelib/thread/qwaitcondition_unix.cpp b/src/corelib/thread/qwaitcondition_unix.cpp index c93328b4bc..0ba90763cf 100644 --- a/src/corelib/thread/qwaitcondition_unix.cpp +++ b/src/corelib/thread/qwaitcondition_unix.cpp @@ -204,7 +204,7 @@ void QWaitCondition::wakeAll() bool QWaitCondition::wait(QMutex *mutex, unsigned long time) { - if (quint64(time) > quint64(std::numeric_limits::max())) + if (time == std::numeric_limits::max()) return wait(mutex, QDeadlineTimer(QDeadlineTimer::Forever)); return wait(mutex, QDeadlineTimer(time)); } @@ -231,6 +231,8 @@ bool QWaitCondition::wait(QMutex *mutex, QDeadlineTimer deadline) bool QWaitCondition::wait(QReadWriteLock *readWriteLock, unsigned long time) { + if (time == std::numeric_limits::max()) + return wait(readWriteLock, QDeadlineTimer(QDeadlineTimer::Forever)); return wait(readWriteLock, QDeadlineTimer(time)); } From c066656aff4841f9095e77754fa7533f7bbbb66a Mon Sep 17 00:00:00 2001 From: Edward Welbourne Date: Thu, 7 Feb 2019 17:04:49 +0100 Subject: [PATCH 07/17] Avoid read-outside-array error by QStringRef over-reach Constructing a QStringRef directly from the string, offset and a length is UB if the offset + length exceeds the string's length. Thanks to Robert Loehning and libFuzzer for finding this. QString::midRef (as correctly used in both changed uses of QStringRef, since 432d3b69629) takes care of that for us. Changed one UB case and a matching but correct case, for consistency. In the process, deduplicate a QStringList look-up. Added tests to exercise the code (but the one that exercises the formerly UB case doesn't crash before the fix, so isn't very useful; the invalid read is only outside the array it's scanning, not outside allocated memory). Change-Id: I7051bbbc0267dd7ec0a8f75eee2034d0b7eb75a2 Reviewed-by: Anton Kudryavtsev Reviewed-by: Thiago Macieira --- src/corelib/tools/qdatetimeparser.cpp | 11 ++++++----- .../auto/corelib/tools/qdatetime/tst_qdatetime.cpp | 13 +++++++++++++ 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/corelib/tools/qdatetimeparser.cpp b/src/corelib/tools/qdatetimeparser.cpp index e6afd510fd..e8470f6cde 100644 --- a/src/corelib/tools/qdatetimeparser.cpp +++ b/src/corelib/tools/qdatetimeparser.cpp @@ -1125,13 +1125,14 @@ QDateTimeParser::scanString(const QDateTime &defaultValue, for (int index = 0; index < sectionNodesCount; ++index) { Q_ASSERT(state != Invalid); - if (QStringRef(input, pos, separators.at(index).size()) != separators.at(index)) { - QDTPDEBUG << "invalid because" << input->midRef(pos, separators.at(index).size()) - << "!=" << separators.at(index) + const QString &separator = separators.at(index); + if (input->midRef(pos, separator.size()) != separator) { + QDTPDEBUG << "invalid because" << input->midRef(pos, separator.size()) + << "!=" << separator << index << pos << currentSectionIndex; return StateNode(); } - pos += separators.at(index).size(); + pos += separator.size(); sectionNodes[index].pos = pos; int *current = 0; const SectionNode sn = sectionNodes.at(index); @@ -1227,7 +1228,7 @@ QDateTimeParser::scanString(const QDateTime &defaultValue, isSet |= sn.type; } - if (QStringRef(input, pos, input->size() - pos) != separators.last()) { + if (input->midRef(pos) != separators.last()) { QDTPDEBUG << "invalid because" << input->midRef(pos) << "!=" << separators.last() << pos; return StateNode(); diff --git a/tests/auto/corelib/tools/qdatetime/tst_qdatetime.cpp b/tests/auto/corelib/tools/qdatetime/tst_qdatetime.cpp index 943805e228..b128ccebc5 100644 --- a/tests/auto/corelib/tools/qdatetime/tst_qdatetime.cpp +++ b/tests/auto/corelib/tools/qdatetime/tst_qdatetime.cpp @@ -2401,6 +2401,19 @@ void tst_QDateTime::fromStringStringFormat_data() QTest::newRow("late") << QString("9999-12-31T23:59:59.999Z") << QString("yyyy-MM-ddThh:mm:ss.zZ") << QDateTime(QDate(9999, 12, 31), QTime(23, 59, 59, 999)); + // Separators match /([^aAdhHMmstyz]*)/ + QTest::newRow("oddly-separated") // To show broken-separator's format is valid. + << QStringLiteral("2018 wilful long working block relief 12-19T21:09 cruel blurb encore flux") + << QStringLiteral("yyyy wilful long working block relief MM-ddThh:mm cruel blurb encore flux") + << QDateTime(QDate(2018, 12, 19), QTime(21, 9)); + QTest::newRow("broken-separator") + << QStringLiteral("2018 wilful") + << QStringLiteral("yyyy wilful long working block relief MM-ddThh:mm cruel blurb encore flux") + << invalidDateTime(); + QTest::newRow("broken-terminator") + << QStringLiteral("2018 wilful long working block relief 12-19T21:09 cruel") + << QStringLiteral("yyyy wilful long working block relief MM-ddThh:mm cruel blurb encore flux") + << invalidDateTime(); } void tst_QDateTime::fromStringStringFormat() From 4247d7c5a0c9a5133245b935eef017149f49de87 Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Wed, 23 Jan 2019 14:12:47 +0100 Subject: [PATCH 08/17] Fix QTextTable:insertRows() for tables with spanning cells Don't resize the height of cells spanning several columns multiple times. Fixes: QTBUG-36713 Change-Id: I5eb45892f2008e6a4f85745b56efd04323e25673 Reviewed-by: Eskil Abrahamsen Blomfeldt --- src/gui/text/qtexttable.cpp | 12 ++++++++---- tests/auto/gui/text/qtexttable/tst_qtexttable.cpp | 6 ++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/gui/text/qtexttable.cpp b/src/gui/text/qtexttable.cpp index 9639c18d2b..39f26d5d42 100644 --- a/src/gui/text/qtexttable.cpp +++ b/src/gui/text/qtexttable.cpp @@ -696,18 +696,22 @@ void QTextTable::insertRows(int pos, int num) int extended = 0; int insert_before = 0; if (pos > 0 && pos < d->nRows) { + int lastCell = -1; for (int i = 0; i < d->nCols; ++i) { int cell = d->grid[pos*d->nCols + i]; if (cell == d->grid[(pos-1)*d->nCols+i]) { // cell spans the insertion place, extend it - QTextDocumentPrivate::FragmentIterator it(&p->fragmentMap(), cell); - QTextCharFormat fmt = c->charFormat(it->format); - fmt.setTableCellRowSpan(fmt.tableCellRowSpan() + num); - p->setCharFormat(it.position(), 1, fmt); + if (cell != lastCell) { + QTextDocumentPrivate::FragmentIterator it(&p->fragmentMap(), cell); + QTextCharFormat fmt = c->charFormat(it->format); + fmt.setTableCellRowSpan(fmt.tableCellRowSpan() + num); + p->setCharFormat(it.position(), 1, fmt); + } extended++; } else if (!insert_before) { insert_before = cell; } + lastCell = cell; } } else { insert_before = (pos == 0 ? d->grid[0] : d->fragment_end); diff --git a/tests/auto/gui/text/qtexttable/tst_qtexttable.cpp b/tests/auto/gui/text/qtexttable/tst_qtexttable.cpp index 29eef506bf..22f00c677d 100644 --- a/tests/auto/gui/text/qtexttable/tst_qtexttable.cpp +++ b/tests/auto/gui/text/qtexttable/tst_qtexttable.cpp @@ -431,6 +431,12 @@ void tst_QTextTable::insertRows() table->insertRows(5, 2); QCOMPARE(table->rows(), 7); + table = cursor.insertTable(5,5); + table->mergeCells(0,0,3,3); + table->insertRows(2,1); + + QCOMPARE(table->rows(), 6); + } void tst_QTextTable::deleteInTable() From 79f2a9e666a241c5baba1b9bf35c12be4cefcc26 Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Fri, 25 Jan 2019 10:17:18 +0100 Subject: [PATCH 09/17] Fix an assertion in the BiDi algorithm The algorithm has been treating DirB inconsistently so far. initScriptAnalysisAndIsolatePairs was treating it differently than generateDireationalRuns leading to assertions. It wasn't visible in our test data, as DirB is in almost all cases the paragraph separator, where we split strings anyway. Change-Id: I7dc0e7bbcf30ee84d8781ea06097da023e371f05 Fixes: QTBUG-73238 Reviewed-by: Robert Loehning Reviewed-by: Eskil Abrahamsen Blomfeldt --- src/gui/text/qtextengine.cpp | 42 ++++++++++++------- .../other/qcomplextext/tst_qcomplextext.cpp | 6 +++ 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/src/gui/text/qtextengine.cpp b/src/gui/text/qtextengine.cpp index 9ed497839c..f305b9b7dc 100644 --- a/src/gui/text/qtextengine.cpp +++ b/src/gui/text/qtextengine.cpp @@ -252,8 +252,6 @@ struct QBidiAlgorithm { void initScriptAnalysisAndIsolatePairs(Vector &isolatePairs) { - isolatePairs.append({ -1, length }); // treat the whole string as one isolate - int isolateStack[128]; int isolateLevel = 0; // load directions of string, and determine isolate pairs @@ -304,6 +302,14 @@ struct QBidiAlgorithm { case QChar::DirS: case QChar::DirB: analysis[pos].bidiFlags = QScriptAnalysis::BidiResetToParagraphLevel; + if (uc == QChar::ParagraphSeparator) { + // close all open isolates as we start a new paragraph + while (isolateLevel > 0) { + --isolateLevel; + if (isolateLevel < 128) + isolatePairs[isolateStack[isolateLevel]].end = pos; + } + } break; default: break; @@ -434,21 +440,21 @@ struct QBidiAlgorithm { doEmbed(true, true, false); break; case QChar::DirLRI: - ++isolatePairPosition; Q_ASSERT(isolatePairs.at(isolatePairPosition).start == i); doEmbed(false, false, true); + ++isolatePairPosition; break; case QChar::DirRLI: - ++isolatePairPosition; Q_ASSERT(isolatePairs.at(isolatePairPosition).start == i); doEmbed(true, false, true); + ++isolatePairPosition; break; case QChar::DirFSI: { - ++isolatePairPosition; const auto &pair = isolatePairs.at(isolatePairPosition); Q_ASSERT(pair.start == i); bool isRtl = QStringView(text + pair.start + 1, pair.end - pair.start - 1).isRightToLeft(); doEmbed(isRtl, false, true); + ++isolatePairPosition; break; } @@ -492,16 +498,24 @@ struct QBidiAlgorithm { analysis[i].bidiDirection = (level & 1) ? QChar::DirR : QChar::DirL; break; case QChar::DirB: - // paragraph separator, go down to base direction - appendRun(i - 1); - while (stack.counter > 1) { - // there might be remaining isolates on the stack that are missing a PDI. Those need to get - // a continuation indicating to take the eos from the end of the string (ie. the paragraph level) - const auto &t = stack.top(); - if (t.isIsolate) { - runs[t.runBeforeIsolate].continuation = -2; + // paragraph separator, go down to base direction, reset all state + if (text[i].unicode() == QChar::ParagraphSeparator) { + appendRun(i - 1); + while (stack.counter > 1) { + // there might be remaining isolates on the stack that are missing a PDI. Those need to get + // a continuation indicating to take the eos from the end of the string (ie. the paragraph level) + const auto &t = stack.top(); + if (t.isIsolate) { + runs[t.runBeforeIsolate].continuation = -2; + } + --stack.counter; } - --stack.counter; + continuationFrom = -1; + lastRunWithContent = -1; + validIsolateCount = 0; + overflowIsolateCount = 0; + overflowEmbeddingCount = 0; + level = baseLevel; } break; default: diff --git a/tests/auto/other/qcomplextext/tst_qcomplextext.cpp b/tests/auto/other/qcomplextext/tst_qcomplextext.cpp index 0116e546a0..c328776089 100644 --- a/tests/auto/other/qcomplextext/tst_qcomplextext.cpp +++ b/tests/auto/other/qcomplextext/tst_qcomplextext.cpp @@ -68,6 +68,12 @@ void tst_QComplexText::bidiReorderString_data() << (int) data->basicDir; data++; } + + QString isolateAndBoundary = QString(QChar(0x2068 /* DirFSI */)) + QChar(0x1c /* DirB */) + QChar(0x2069 /* DirPDI */); + QTest::newRow( "isolateAndBoundary" ) + << QString::fromUtf8( data->logical ) + << QString::fromUtf8( data->visual ) + << (int) QChar::DirL; } void tst_QComplexText::bidiReorderString() From 035d80407b693662c209cd4f156085d583ad428c Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Fri, 25 Jan 2019 10:49:34 +0100 Subject: [PATCH 10/17] Fix two smaller bugs in the BiDi engine Remove wrong code changing the Bido level of line separators. This lead to wrong ordering of the string in case the line separator was meant to be ignored and the string should be rendered in one line. Line breaks are anyways already reset to the paragraph level by the algorithm and reordering is done on a line by line basis, so this will work correctly when doing proper line breaking. Secondly fix a small bug found while testing the above change, where we wouldn't set the correct levels for boundary neutrals and explicit embedding chars because we did that processing before we were fully done with the BiDi algorithm. Change-Id: Id88f91cd58d2ab29be864aef34ca1727c1586611 Reviewed-by: Konstantin Ritt Reviewed-by: Eskil Abrahamsen Blomfeldt --- src/gui/text/qtextengine.cpp | 34 +++++++++---------- .../other/qcomplextext/bidireorderstring.h | 2 +- .../other/qcomplextext/tst_qcomplextext.cpp | 7 +++- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/src/gui/text/qtextengine.cpp b/src/gui/text/qtextengine.cpp index f305b9b7dc..a83ef95c79 100644 --- a/src/gui/text/qtextengine.cpp +++ b/src/gui/text/qtextengine.cpp @@ -1108,6 +1108,22 @@ struct QBidiAlgorithm { resolveImplicitLevels(runs); } + BIDI_DEBUG() << "Rule L1:"; + // Rule L1: + bool resetLevel = true; + for (int i = length - 1; i >= 0; --i) { + if (analysis[i].bidiFlags & QScriptAnalysis::BidiResetToParagraphLevel) { + BIDI_DEBUG() << "resetting pos" << i << "to baselevel"; + analysis[i].bidiLevel = baseLevel; + resetLevel = true; + } else if (resetLevel && analysis[i].bidiFlags & QScriptAnalysis::BidiMaybeResetToParagraphLevel) { + BIDI_DEBUG() << "resetting pos" << i << "to baselevel (maybereset flag)"; + analysis[i].bidiLevel = baseLevel; + } else { + resetLevel = false; + } + } + // set directions for BN to the minimum of adjacent chars // This makes is possible to be conformant with the Bidi algorithm even though we don't // remove BN and explicit embedding chars from the stream of characters to reorder @@ -1139,22 +1155,6 @@ struct QBidiAlgorithm { } } - BIDI_DEBUG() << "Rule L1:"; - // Rule L1: - bool resetLevel = true; - for (int i = length - 1; i >= 0; --i) { - if (analysis[i].bidiFlags & QScriptAnalysis::BidiResetToParagraphLevel) { - BIDI_DEBUG() << "resetting pos" << i << "to baselevel"; - analysis[i].bidiLevel = baseLevel; - resetLevel = true; - } else if (resetLevel && analysis[i].bidiFlags & QScriptAnalysis::BidiMaybeResetToParagraphLevel) { - BIDI_DEBUG() << "resetting pos" << i << "to baselevel (maybereset flag)"; - analysis[i].bidiLevel = baseLevel; - } else { - resetLevel = false; - } - } - if (BidiDebugEnabled) { BIDI_DEBUG() << "final resolved levels:"; for (int i = 0; i < length; ++i) @@ -2087,8 +2087,6 @@ void QTextEngine::itemize() const analysis->flags = QScriptAnalysis::Object; break; case QChar::LineSeparator: - if (analysis->bidiLevel % 2) - --analysis->bidiLevel; analysis->flags = QScriptAnalysis::LineOrParagraphSeparator; if (option.flags() & QTextOption::ShowLineAndParagraphSeparators) { const int offset = uc - string; diff --git a/tests/auto/other/qcomplextext/bidireorderstring.h b/tests/auto/other/qcomplextext/bidireorderstring.h index a7401d2c18..b537bf45e4 100644 --- a/tests/auto/other/qcomplextext/bidireorderstring.h +++ b/tests/auto/other/qcomplextext/bidireorderstring.h @@ -78,7 +78,7 @@ const LV logical_visual[] = { { "data42", "foo\nfoo", "foo\nfoo", QChar::DirL }, { "data43", "\327\251\327\234\327\225\327\235\n\327\251\327\234\327\225\327\235", "\327\235\327\225\327\234\327\251\n\327\235\327\225\327\234\327\251", QChar::DirR }, { "data44", "foo\n\327\251\327\234\327\225\327\235", "foo\n\327\235\327\225\327\234\327\251", QChar::DirL }, - { "data45", "\327\251\327\234\327\225\327\235\nfoo", "\327\235\327\225\327\234\327\251\nfoo", QChar::DirR }, + { "data45", "\327\251\327\234\327\225\327\235\nfoo", "foo\n\327\235\327\225\327\234\327\251", QChar::DirR }, { "data46", "\330\250 1.23 \330\250", "\330\250 1.23 \330\250", QChar::DirR }, { "data47", "\331\204\330\250 1.23 \331\202\330\250", "\330\250\331\202 1.23 \330\250\331\204", QChar::DirR }, { "data48", "\330\250 1.2 \330\250", "\330\250 1.2 \330\250", QChar::DirR }, diff --git a/tests/auto/other/qcomplextext/tst_qcomplextext.cpp b/tests/auto/other/qcomplextext/tst_qcomplextext.cpp index c328776089..9ca61a69b4 100644 --- a/tests/auto/other/qcomplextext/tst_qcomplextext.cpp +++ b/tests/auto/other/qcomplextext/tst_qcomplextext.cpp @@ -66,6 +66,11 @@ void tst_QComplexText::bidiReorderString_data() << QString::fromUtf8( data->logical ) << QString::fromUtf8( data->visual ) << (int) data->basicDir; + + QTest::newRow( QByteArray(data->name) + " (doubled)" ) + << (QString::fromUtf8( data->logical ) + QChar(QChar::ParagraphSeparator) + QString::fromUtf8( data->logical )) + << (QString::fromUtf8( data->visual ) + QChar(QChar::ParagraphSeparator) + QString::fromUtf8( data->visual )) + << (int) data->basicDir; data++; } @@ -432,7 +437,7 @@ ushort unicodeForDirection(const QByteArray &direction) { "ET", 0x24 }, { "AN", 0x660 }, { "CS", 0x2c }, - { "B", QChar::ParagraphSeparator }, + { "B", '\n' }, { "S", 0x9 }, { "WS", 0x20 }, { "ON", 0x2a }, From 6f9e444c28c0095595b1b61cb0f399ac2790716b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Sun, 16 Dec 2018 19:54:26 +0100 Subject: [PATCH 11/17] harbuzzng: Remove assumption about Core Text working in 96 DPI Core Text doesn't actually have a concept of DPI internally, as it doesn't rasterize anything by itself, it just generates vector paths that get passed along to Core Graphics. In practice this means Core Text operates in the classical macOS logical DPI of 72, with one typographic point corresponding to one point in the Core Graphics coordinate system, which for a normal bitmap context then corresponds to one pixel -- or two pixels for a "retina" context with a 2x scale transform. Scaling the font point sizes given to HarfBuzz to an assumed DPI of 96 is problematic with this in mind, as fonts with optical features such as 'trak' tables for tracking, or color glyphs, will then base the metrics off of the wrong point size compared to what the client asked for. This in turn causes mismatches between the metrics of the shaped text and the actual rasterization, which doesn't include the 72 to 96 DPI scaling. If a 96 DPI is needed, such as on the Web, the scaling should be done outside of HarfBuzz, allowing the client to keep the DPI of the shaping in sync with the rasterization. The recommended way to do that is by scaling the font point size, not by applying a transform to the target Core Graphics context, to let Core Text choose the right optical features of the target point size, as described in WWDC 2015 session 804: https://developer.apple.com/videos/play/wwdc2015/804/ GitHub-PR: https://github.com/harfbuzz/harfbuzz/pull/1484 Change-Id: I830f0cd7a82552422bbe09226e2d571e246fe3f4 Reviewed-by: Allan Sandfeld Jensen --- src/3rdparty/harfbuzz-ng/src/hb-coretext.cc | 25 ++++----------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/src/3rdparty/harfbuzz-ng/src/hb-coretext.cc b/src/3rdparty/harfbuzz-ng/src/hb-coretext.cc index 9431ba5fe1..d64cb7edbd 100644 --- a/src/3rdparty/harfbuzz-ng/src/hb-coretext.cc +++ b/src/3rdparty/harfbuzz-ng/src/hb-coretext.cc @@ -52,24 +52,6 @@ struct CoreTextFontEngineData { /* https://developer.apple.com/documentation/coretext/1508745-ctfontcreatewithgraphicsfont */ #define HB_CORETEXT_DEFAULT_FONT_SIZE 12.f -static CGFloat -coretext_font_size_from_ptem (float ptem) -{ - /* CoreText points are CSS pixels (96 per inch), - * NOT typographic points (72 per inch). - * - * https://developer.apple.com/library/content/documentation/GraphicsAnimation/Conceptual/HighResolutionOSX/Explained/Explained.html - */ - ptem *= 96.f / 72.f; - return ptem <= 0.f ? HB_CORETEXT_DEFAULT_FONT_SIZE : ptem; -} -static float -coretext_font_size_to_ptem (CGFloat size) -{ - size *= 72.f / 96.f; - return size <= 0.f ? 0 : size; -} - static void release_table_data (void *user_data) { @@ -104,7 +86,7 @@ _hb_cg_font_release (void *data) HB_SHAPER_DATA_ENSURE_DEFINE(coretext, face) HB_SHAPER_DATA_ENSURE_DEFINE_WITH_CONDITION(coretext, font, - fabs (CTFontGetSize((CTFontRef) data) - coretext_font_size_from_ptem (font->ptem)) <= .5 + fabs (CTFontGetSize ((CTFontRef) data) - font->ptem) <= .5 ) static CTFontDescriptorRef @@ -308,7 +290,8 @@ _hb_coretext_shaper_font_data_create (hb_font_t *font) if (unlikely (!hb_coretext_shaper_face_data_ensure (face))) return nullptr; CGFontRef cg_font = (CGFontRef) HB_SHAPER_DATA_GET (face); - CTFontRef ct_font = create_ct_font (cg_font, coretext_font_size_from_ptem (font->ptem)); + CGFloat font_size = font->ptem <= 0.f ? HB_CORETEXT_DEFAULT_FONT_SIZE : font->ptem; + CTFontRef ct_font = create_ct_font (cg_font, font_size); if (unlikely (!ct_font)) { @@ -340,7 +323,7 @@ hb_coretext_font_create (CTFontRef ct_font) if (unlikely (hb_object_is_inert (font))) return font; - hb_font_set_ptem (font, coretext_font_size_to_ptem (CTFontGetSize(ct_font))); + hb_font_set_ptem (font, CTFontGetSize (ct_font)); /* Let there be dragons here... */ HB_SHAPER_DATA_GET (font) = (hb_coretext_shaper_font_data_t *) CFRetain (ct_font); From 4697467e9888d7631841af6be436ed58e41f4759 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Fri, 8 Feb 2019 15:01:28 +0100 Subject: [PATCH 12/17] macOS: Explain QNSViewMouseMoveHelper for future generations Change-Id: I61f38ee38d5afb657cd8b76b2b9dba1dac7167b4 Reviewed-by: Timur Pocheptsov --- src/plugins/platforms/cocoa/qnsview_mouse.mm | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/plugins/platforms/cocoa/qnsview_mouse.mm b/src/plugins/platforms/cocoa/qnsview_mouse.mm index 3d6471005d..563a66515a 100644 --- a/src/plugins/platforms/cocoa/qnsview_mouse.mm +++ b/src/plugins/platforms/cocoa/qnsview_mouse.mm @@ -39,6 +39,22 @@ // This file is included from qnsview.mm, and only used to organize the code +/* + The reason for using this helper is to ensure that QNSView doesn't implement + the NSResponder callbacks for mouseEntered, mouseExited, and mouseMoved. + + If it did, we would get mouse events though the responder chain as well, + for example if a subview has a tracking area of its own and calls super + in the handler, which results in forwarding the event though the responder + chain. The same applies if NSWindow.acceptsMouseMovedEvents is YES. + + By having a helper as the target for our tracking areas, we know for sure + that the events we are getting stem from our own tracking areas. + + FIXME: Ideally we wouldn't need this workaround, and would correctly + interact with the responder chain by e.g. calling super if Qt does not + accept the mouse event +*/ @implementation QT_MANGLE_NAMESPACE(QNSViewMouseMoveHelper) { QNSView *view; } From 21e25ff38babc6dad57a56c758d05997c16eb111 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Mon, 4 Feb 2019 23:39:42 +0100 Subject: [PATCH 13/17] macOS: Simplify mouse tracking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don't need to react to updateTrackingAreas, as we only have a single tracking area that we can add once and forget. By asking AppKit to track all events in the visible rect, we can also pass a zero-rect for the tracking area. Change-Id: I2545712adc49b51904d5adc11f1faca36901b49d Reviewed-by: Tor Arne Vestbø --- src/plugins/platforms/cocoa/qnsview.mm | 8 +--- src/plugins/platforms/cocoa/qnsview_mouse.mm | 50 +++++++++----------- 2 files changed, 25 insertions(+), 33 deletions(-) diff --git a/src/plugins/platforms/cocoa/qnsview.mm b/src/plugins/platforms/cocoa/qnsview.mm index 7f826942f3..17063f6e92 100644 --- a/src/plugins/platforms/cocoa/qnsview.mm +++ b/src/plugins/platforms/cocoa/qnsview.mm @@ -85,6 +85,7 @@ @end @interface QT_MANGLE_NAMESPACE(QNSView) (Mouse) +- (void)initMouse; - (NSPoint)screenMousePoint:(NSEvent *)theEvent; - (void)mouseMovedImpl:(NSEvent *)theEvent; - (void)mouseEnteredImpl:(NSEvent *)theEvent; @@ -114,7 +115,6 @@ @implementation QT_MANGLE_NAMESPACE(QNSView) { QPointer m_platformWindow; - NSTrackingArea *m_trackingArea; Qt::MouseButtons m_buttons; Qt::MouseButtons m_acceptedMouseDowns; Qt::MouseButtons m_frameStrutButtons; @@ -150,7 +150,6 @@ m_currentlyInterpretedKeyEvent = nil; m_dontOverrideCtrlLMB = qt_mac_resolveOption(false, platformWindow->window(), "_q_platform_MacDontOverrideCtrlLMB", "QT_MAC_DONT_OVERRIDE_CTRL_LMB"); - m_trackingArea = nil; self.focusRingType = NSFocusRingTypeNone; self.cursor = nil; @@ -159,6 +158,7 @@ self.previousWindow = nil; [self initDrawing]; + [self initMouse]; [self registerDragTypes]; [[NSNotificationCenter defaultCenter] addObserver:self @@ -173,10 +173,6 @@ { qCDebug(lcQpaWindow) << "Deallocating" << self; - if (m_trackingArea) { - [self removeTrackingArea:m_trackingArea]; - [m_trackingArea release]; - } [m_inputSource release]; [[NSNotificationCenter defaultCenter] removeObserver:self]; [m_mouseMoveHelper release]; diff --git a/src/plugins/platforms/cocoa/qnsview_mouse.mm b/src/plugins/platforms/cocoa/qnsview_mouse.mm index 563a66515a..49c94f18db 100644 --- a/src/plugins/platforms/cocoa/qnsview_mouse.mm +++ b/src/plugins/platforms/cocoa/qnsview_mouse.mm @@ -151,6 +151,28 @@ @implementation QT_MANGLE_NAMESPACE(QNSView) (Mouse) +- (void)initMouse +{ + NSUInteger trackingOptions = NSTrackingActiveInActiveApp + | NSTrackingMouseEnteredAndExited | NSTrackingCursorUpdate; + + // Ideally, NSTrackingMouseMoved should be turned on only if QWidget::mouseTracking + // is enabled, hover is on, or a tool tip is set. Unfortunately, Qt will send "tooltip" + // events on mouse moves, so we need to turn it on in ALL case. That means EVERY QWindow + // gets to pay the cost of mouse moves delivered to it (Apple recommends keeping it OFF + // because there is a performance hit). + trackingOptions |= NSTrackingMouseMoved; + + // Using NSTrackingInVisibleRect means AppKit will automatically synchronize the + // tracking rect with changes in the view's visible area, so leave it undefined. + trackingOptions |= NSTrackingInVisibleRect; + static const NSRect trackingRect = NSZeroRect; + + QMacAutoReleasePool pool; + [self addTrackingArea:[[[NSTrackingArea alloc] initWithRect:trackingRect + options:trackingOptions owner:m_mouseMoveHelper userInfo:nil] autorelease]]; +} + - (BOOL)acceptsFirstMouse:(NSEvent *)theEvent { Q_UNUSED(theEvent) @@ -422,35 +444,9 @@ [super otherMouseUp:theEvent]; } -- (void)updateTrackingAreas -{ - [super updateTrackingAreas]; - - QMacAutoReleasePool pool; - - // NSTrackingInVisibleRect keeps care of updating once the tracking is set up, so bail out early - if (m_trackingArea && [[self trackingAreas] containsObject:m_trackingArea]) - return; - - // Ideally, we shouldn't have NSTrackingMouseMoved events included below, it should - // only be turned on if mouseTracking, hover is on or a tool tip is set. - // Unfortunately, Qt will send "tooltip" events on mouse moves, so we need to - // turn it on in ALL case. That means EVERY QWindow gets to pay the cost of - // mouse moves delivered to it (Apple recommends keeping it OFF because there - // is a performance hit). So it goes. - NSUInteger trackingOptions = NSTrackingMouseEnteredAndExited | NSTrackingActiveInActiveApp - | NSTrackingInVisibleRect | NSTrackingMouseMoved | NSTrackingCursorUpdate; - [m_trackingArea release]; - m_trackingArea = [[NSTrackingArea alloc] initWithRect:[self frame] - options:trackingOptions - owner:m_mouseMoveHelper - userInfo:nil]; - [self addTrackingArea:m_trackingArea]; -} - - (void)cursorUpdate:(NSEvent *)theEvent { - qCDebug(lcQpaMouse) << "[QNSView cursorUpdate:]" << self.cursor; + qCDebug(lcQpaMouse) << "Updating cursor for" << self << "to" << self.cursor; // Note: We do not get this callback when moving from a subview that // uses the legacy cursorRect API, so the cursor is reset to the arrow From 17e51585702b51c4b854ec32a0cb9339ab34528f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Mon, 4 Feb 2019 15:53:01 +0100 Subject: [PATCH 14/17] macOS: Treat default swapInterval (-1) as vsync-enabled display-link MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: I6d3d241d3813bfac36155ad219d4a338cb1ef6f7 Reviewed-by: Tor Arne Vestbø --- src/plugins/platforms/cocoa/qcocoascreen.mm | 4 ++-- src/plugins/platforms/cocoa/qcocoawindow.h | 1 + src/plugins/platforms/cocoa/qcocoawindow.mm | 13 +++++++++---- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/plugins/platforms/cocoa/qcocoascreen.mm b/src/plugins/platforms/cocoa/qcocoascreen.mm index afe14e623c..36c11ba8af 100644 --- a/src/plugins/platforms/cocoa/qcocoascreen.mm +++ b/src/plugins/platforms/cocoa/qcocoascreen.mm @@ -330,7 +330,7 @@ void QCocoaScreen::deliverUpdateRequests() auto windows = QGuiApplication::allWindows(); for (int i = 0; i < windows.size(); ++i) { QWindow *window = windows.at(i); - QPlatformWindow *platformWindow = window->handle(); + auto *platformWindow = static_cast(window->handle()); if (!platformWindow) continue; @@ -341,7 +341,7 @@ void QCocoaScreen::deliverUpdateRequests() continue; // Skip windows that are not doing update requests via display link - if (!(window->format().swapInterval() > 0)) + if (!platformWindow->updatesWithDisplayLink()) continue; platformWindow->deliverUpdateRequest(); diff --git a/src/plugins/platforms/cocoa/qcocoawindow.h b/src/plugins/platforms/cocoa/qcocoawindow.h index 8f1bdb8af0..0a913ef66e 100644 --- a/src/plugins/platforms/cocoa/qcocoawindow.h +++ b/src/plugins/platforms/cocoa/qcocoawindow.h @@ -129,6 +129,7 @@ public: bool isForeignWindow() const override; void requestUpdate() override; + bool updatesWithDisplayLink() const; void deliverUpdateRequest() override; void requestActivateWindow() override; diff --git a/src/plugins/platforms/cocoa/qcocoawindow.mm b/src/plugins/platforms/cocoa/qcocoawindow.mm index a1bed3db45..792993afbf 100644 --- a/src/plugins/platforms/cocoa/qcocoawindow.mm +++ b/src/plugins/platforms/cocoa/qcocoawindow.mm @@ -1473,11 +1473,10 @@ void QCocoaWindow::recreateWindowIfNeeded() void QCocoaWindow::requestUpdate() { - const int swapInterval = format().swapInterval(); - qCDebug(lcQpaDrawing) << "QCocoaWindow::requestUpdate" << window() << "swapInterval" << swapInterval; + qCDebug(lcQpaDrawing) << "QCocoaWindow::requestUpdate" << window() + << "using" << (updatesWithDisplayLink() ? "display-link" : "timer"); - if (swapInterval > 0) { - // Vsync is enabled, deliver via CVDisplayLink + if (updatesWithDisplayLink()) { static_cast(screen())->requestUpdate(); } else { // Fall back to the un-throttled timer-based callback @@ -1485,6 +1484,12 @@ void QCocoaWindow::requestUpdate() } } +bool QCocoaWindow::updatesWithDisplayLink() const +{ + // Update via CVDisplayLink if Vsync is enabled + return format().swapInterval() != 0; +} + void QCocoaWindow::deliverUpdateRequest() { // Don't send update requests for views that need display, as the update From 9f22ac0aa0254f20f9b26aec7b124d74141fdfcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Wed, 30 Jan 2019 16:29:59 +0100 Subject: [PATCH 15/17] macOS: Don't send redundant geometry change events for top level windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Top level windows already get their geometry changes via windowDidMove and windowDidResize. Change-Id: Ie6370aa290ef48c8b3ac770e77adb57ce43cbb47 Reviewed-by: Tor Arne Vestbø --- src/plugins/platforms/cocoa/qcocoawindow.mm | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/plugins/platforms/cocoa/qcocoawindow.mm b/src/plugins/platforms/cocoa/qcocoawindow.mm index 792993afbf..724c9485bd 100644 --- a/src/plugins/platforms/cocoa/qcocoawindow.mm +++ b/src/plugins/platforms/cocoa/qcocoawindow.mm @@ -1101,6 +1101,9 @@ void QCocoaWindow::setEmbeddedInForeignView() void QCocoaWindow::viewDidChangeFrame() { + if (isContentView()) + return; // Handled below + handleGeometryChange(); } From 3b87ecc06a4056de48ce7fa4d69ca71422cc1170 Mon Sep 17 00:00:00 2001 From: Yuhang Zhao <2546789017@qq.com> Date: Mon, 11 Feb 2019 20:39:36 +0800 Subject: [PATCH 16/17] Win32 makefile for qmake: Use proper linker Use xilink for ICC and lld-link for Clang. Change-Id: I13c74339ae9e3e5c97210afd20a53c7e474b873b Reviewed-by: Friedemann Kleint Reviewed-by: Thiago Macieira --- qmake/Makefile.win32 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qmake/Makefile.win32 b/qmake/Makefile.win32 index ed738e0c02..3f13df884a 100644 --- a/qmake/Makefile.win32 +++ b/qmake/Makefile.win32 @@ -12,11 +12,11 @@ QMKSRC = $(SOURCE_PATH)\qmake # !if "$(QMAKESPEC)" == "win32-icc" CXX = icl -LINKER = link +LINKER = xilink CFLAGS_EXTRA = /Zc:forScope /Qstd=c++11 !elseif "$(QMAKESPEC)" == "win32-clang-msvc" CXX = clang-cl -LINKER = link +LINKER = lld-link CFLAGS_EXTRA = -fms-compatibility-version=19.00.23506 -Wno-microsoft-enum-value !else CXX = cl From fd88c152db0949e47613858a914a6ae4a825781d Mon Sep 17 00:00:00 2001 From: Eric Lemanissier Date: Sun, 3 Feb 2019 14:42:27 +0100 Subject: [PATCH 17/17] configure: use proper separator for mingw libraries Change-Id: Ic328691fe2f08e918c1bb67910521d85b274a8fd Fixes: QTBUG-73466 Reviewed-by: Kai Koehne Reviewed-by: Joerg Bornemann --- mkspecs/features/toolchain.prf | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mkspecs/features/toolchain.prf b/mkspecs/features/toolchain.prf index 2a7cbabc54..9c3a64aa8b 100644 --- a/mkspecs/features/toolchain.prf +++ b/mkspecs/features/toolchain.prf @@ -229,7 +229,10 @@ isEmpty($${target_prefix}.INCDIRS) { line ~= s/^[ \\t]*// # remove leading spaces contains(line, "LIBRARY_PATH=.*") { line ~= s/^LIBRARY_PATH=// # remove leading LIBRARY_PATH= - paths = $$split(line, $$QMAKE_DIRLIST_SEP) + equals(QMAKE_HOST.os, Windows): \ + paths = $$split(line, ;) + else: \ + paths = $$split(line, $$QMAKE_DIRLIST_SEP) for (path, paths): \ QMAKE_DEFAULT_LIBDIRS += $$clean_path($$path) } else: contains(line, "Library search paths:") {