From 80166d58a18370bd5a2c6c61d2519011cfd65721 Mon Sep 17 00:00:00 2001 From: Igor Bugaev Date: Fri, 24 Jun 2022 11:21:16 +0200 Subject: [PATCH] Fix SplitView containmentMask hit testing If someone catches mouse events under a SplitView handle, and that handle uses containmentMask to extend hit testing, the mouse events outside the handle's bounds are not handled by the SplitView. Because of this containmentMask is not useful for hit testing. [ChangeLog][QtQuick][SplitView] SplitView now correctly handles mouse events when using containmentMask. Task-number: QTBUG-82678 Fixes: QTBUG-87119 Fixes: QTBUG-97385 Pick-to: 6.2 6.3 6.4 Change-Id: I3f00a73f9d80f22575545dd8556af6c755aadaa6 Reviewed-by: Mitch Curtis --- src/quicktemplates2/qquicksplitview.cpp | 75 ++- src/quicktemplates2/qquicksplitview_p.h | 1 + src/quicktemplates2/qquicksplitview_p_p.h | 3 + .../controls/data/tst_splitview.qml | 430 ++++++++++++++++++ 4 files changed, 486 insertions(+), 23 deletions(-) diff --git a/src/quicktemplates2/qquicksplitview.cpp b/src/quicktemplates2/qquicksplitview.cpp index feef9d1cb1..c0f894ffa5 100644 --- a/src/quicktemplates2/qquicksplitview.cpp +++ b/src/quicktemplates2/qquicksplitview.cpp @@ -718,7 +718,12 @@ void QQuickSplitViewPrivate::createHandleItem(int index) m_handleItems.insert(index, handleItem); handleItem->setParentItem(q); - + // Handles must have priority for press events, so we need to set this. + handleItem->setAcceptedMouseButtons(Qt::LeftButton); + handleItem->setKeepMouseGrab(true); +#if QT_CONFIG(cursor) + updateCursorHandle(handleItem); +#endif m_handle->completeCreate(); resizeHandle(handleItem); } @@ -850,6 +855,13 @@ void QQuickSplitViewPrivate::resizeHandles() resizeHandle(handleItem); } +#if QT_CONFIG(cursor) +void QQuickSplitViewPrivate::updateCursorHandle(QQuickItem *handleItem) +{ + handleItem->setCursor(isHorizontal() ? Qt::SplitHCursor : Qt::SplitVCursor); +} +#endif + void QQuickSplitViewPrivate::updateHandleVisibilities() { // If this is the first item that is visible, we won't have any @@ -886,7 +898,6 @@ void QQuickSplitViewPrivate::updateHandleVisibilities() void QQuickSplitViewPrivate::updateHoveredHandle(QQuickItem *hoveredItem) { - Q_Q(QQuickSplitView); qCDebug(qlcQQuickSplitViewMouse) << "updating hovered handle after" << hoveredItem << "was hovered"; const int oldHoveredHandleIndex = m_hoveredHandleIndex; @@ -911,13 +922,6 @@ void QQuickSplitViewPrivate::updateHoveredHandle(QQuickItem *hoveredItem) } else { qCDebug(qlcQQuickSplitViewMouse) << "either there is no hovered item or" << hoveredItem << "is not a handle"; } - -#if QT_CONFIG(cursor) - if (m_hoveredHandleIndex != -1) - q->setCursor(m_orientation == Qt::Horizontal ? Qt::SplitHCursor : Qt::SplitVCursor); - else - q->setCursor(Qt::ArrowCursor); -#endif } void QQuickSplitViewPrivate::setResizing(bool resizing) @@ -977,8 +981,6 @@ bool QQuickSplitViewPrivate::handlePress(const QPointF &point, ulong timestamp) m_rightOrBottomItemSizeBeforePress = isHorizontal ? rightOrBottomItem->width() : rightOrBottomItem->height(); m_handlePosBeforePress = pressedItem->position(); - // Avoid e.g. Flickable stealing our drag if we're inside it. - q->setKeepMouseGrab(true); // Force the attached object to be created since we rely on it. QQuickSplitHandleAttached *handleAttached = qobject_cast( @@ -1013,7 +1015,6 @@ bool QQuickSplitViewPrivate::handleMove(const QPointF &point, ulong timestamp) bool QQuickSplitViewPrivate::handleRelease(const QPointF &point, ulong timestamp) { - Q_Q(QQuickSplitView); QQuickContainerPrivate::handleRelease(point, timestamp); if (m_pressedHandleIndex != -1) { @@ -1031,7 +1032,6 @@ bool QQuickSplitViewPrivate::handleRelease(const QPointF &point, ulong timestamp m_handlePosBeforePress = QPointF(); m_leftOrTopItemSizeBeforePress = 0.0; m_rightOrBottomItemSizeBeforePress = 0.0; - q->setKeepMouseGrab(false); return true; } @@ -1086,7 +1086,6 @@ QQuickSplitView::QQuickSplitView(QQuickItem *parent) Q_D(QQuickSplitView); d->changeTypes |= QQuickItemPrivate::Visibility; - setAcceptedMouseButtons(Qt::LeftButton); setFiltersChildMouseEvents(true); } @@ -1096,7 +1095,6 @@ QQuickSplitView::QQuickSplitView(QQuickSplitViewPrivate &dd, QQuickItem *parent) Q_D(QQuickSplitView); d->changeTypes |= QQuickItemPrivate::Visibility; - setAcceptedMouseButtons(Qt::LeftButton); setFiltersChildMouseEvents(true); } @@ -1134,6 +1132,10 @@ void QQuickSplitView::setOrientation(Qt::Orientation orientation) d->m_orientation = orientation; d->resizeHandles(); +#if QT_CONFIG(cursor) + for (QQuickItem *handleItem : d->m_handleItems) + d->updateCursorHandle(handleItem); +#endif d->requestLayout(); emit orientationChanged(); } @@ -1369,18 +1371,46 @@ void QQuickSplitView::hoverMoveEvent(QHoverEvent *event) d->updateHoveredHandle(hoveredItem); } +void QQuickSplitView::hoverLeaveEvent(QHoverEvent *event) +{ + Q_UNUSED(event); + Q_D(QQuickSplitView); + // If SplitView is no longer hovered (e.g. visible set to false), clear handle hovered value + d->updateHoveredHandle(nullptr); +} + bool QQuickSplitView::childMouseEventFilter(QQuickItem *item, QEvent *event) { Q_D(QQuickSplitView); qCDebug(qlcQQuickSplitViewMouse) << "childMouseEventFilter called with" << item << event; - if (event->type() != QEvent::HoverEnter) - return false; - // If a child item received a hover enter event, then it means our handle is no longer hovered. - // Handles should be purely visual and not accept hover events, - // so we should never get hover events for them here. - d->updateHoveredHandle(nullptr); - return false; + if (event->type() == QEvent::MouseButtonPress) { + QMouseEvent *mouseEvent = static_cast(event); + const QPointF point = mapFromItem(item, mouseEvent->position()); + d->handlePress(point, mouseEvent->timestamp()); + + // Keep the mouse grab if this item belongs to the handle, + // otherwise this event can be stolen e.g. Flickable if we're inside it. + if (d->m_pressedHandleIndex != -1) + item->setKeepMouseGrab(true); + } + else if (event->type() == QEvent::MouseButtonRelease) { + QMouseEvent *mouseEvent = static_cast(event); + const QPointF point = mapFromItem(item, mouseEvent->position()); + d->handleRelease(point, mouseEvent->timestamp()); + } + else if (event->type() == QEvent::MouseMove) { + QMouseEvent *mouseEvent = static_cast(event); + const QPointF point = mapFromItem(item, mouseEvent->position()); + d->handleMove(point, mouseEvent->timestamp()); + } + + // If this event belongs to the handle, filter it. (d->m_pressedHandleIndex != -1) means that + // we press or move the handle, so we don't need to propagate it further. + if (d->m_pressedHandleIndex != -1) + return true; + + return QQuickContainer::childMouseEventFilter(item, event); } void QQuickSplitView::geometryChange(const QRectF &newGeometry, const QRectF &oldGeometry) @@ -1454,7 +1484,6 @@ void QQuickSplitView::itemRemoved(int index, QQuickItem *item) handleAttachedPrivate->setPressed(false); } - setKeepMouseGrab(false); d->m_hoveredHandleIndex = -1; d->m_pressedHandleIndex = -1; } diff --git a/src/quicktemplates2/qquicksplitview_p.h b/src/quicktemplates2/qquicksplitview_p.h index 10e6f4d295..5d8fc93849 100644 --- a/src/quicktemplates2/qquicksplitview_p.h +++ b/src/quicktemplates2/qquicksplitview_p.h @@ -69,6 +69,7 @@ protected: void componentComplete() override; void hoverMoveEvent(QHoverEvent *event) override; + void hoverLeaveEvent(QHoverEvent *event) override; bool childMouseEventFilter(QQuickItem *item, QEvent *event) override; void geometryChange(const QRectF &newGeometry, const QRectF &oldGeometry) override; diff --git a/src/quicktemplates2/qquicksplitview_p_p.h b/src/quicktemplates2/qquicksplitview_p_p.h index 4c2ed48348..86316ecfe9 100644 --- a/src/quicktemplates2/qquicksplitview_p_p.h +++ b/src/quicktemplates2/qquicksplitview_p_p.h @@ -40,6 +40,9 @@ public: void destroyHandles(); void resizeHandle(QQuickItem *handleItem); void resizeHandles(); +#if QT_CONFIG(cursor) + void updateCursorHandle(QQuickItem *handleItem); +#endif void updateHandleVisibilities(); void updateHoveredHandle(QQuickItem *hoveredItem); void setResizing(bool resizing); diff --git a/tests/auto/quickcontrols2/controls/data/tst_splitview.qml b/tests/auto/quickcontrols2/controls/data/tst_splitview.qml index 548f906731..add4d7b7ec 100644 --- a/tests/auto/quickcontrols2/controls/data/tst_splitview.qml +++ b/tests/auto/quickcontrols2/controls/data/tst_splitview.qml @@ -1836,6 +1836,436 @@ TestCase { verify(firstItem.height > firstItemOriginalHeight) } + Component { + id: splitViewHandleContainmentMaskComponent + + MouseArea { + property alias mouseArea1: mouseArea1 + property alias mouseArea2: mouseArea2 + property alias splitView: splitView + + anchors.fill: parent + hoverEnabled: true + acceptedButtons: Qt.LeftButton + + Rectangle { + anchors.fill: parent + color: 'green' + opacity: 0.3 + } + + SplitView { + id: splitView + + anchors { + fill: parent + margins: 100 + } + + handle: Rectangle { + id: handleRoot + + readonly property bool containsMouse: SplitHandle.hovered + readonly property int defaultSize: 2 + + implicitWidth: splitView.orientation === Qt.Horizontal ? handleRoot.defaultSize : splitView.width + implicitHeight: splitView.orientation === Qt.Vertical ? handleRoot.defaultSize : splitView.height + + color: 'red' + objectName: "handle" + + Text { + objectName: "handleText_" + text + text: parent.x + "," + parent.y + " " + parent.width + "x" + parent.height + color: "black" + anchors.centerIn: parent + rotation: 90 + } + + containmentMask: Item { + readonly property real extraOverflow: 20 + + x: splitView.orientation === Qt.Horizontal ? -extraOverflow : 0 + y: splitView.orientation === Qt.Horizontal ? 0 : -extraOverflow + width: splitView.orientation === Qt.Horizontal ? handleRoot.defaultSize + (extraOverflow * 2): handleRoot.width + height: splitView.orientation === Qt.Horizontal ? handleRoot.height : handleRoot.defaultSize + (extraOverflow * 2) + } + } + + MouseArea { + id: mouseArea1 + + SplitView.fillHeight: splitView.orientation === Qt.Horizontal + SplitView.fillWidth: splitView.orientation === Qt.Vertical + SplitView.preferredWidth: splitView.orientation === Qt.Horizontal ? parent.width / 2 : undefined + SplitView.preferredHeight: splitView.orientation === Qt.Vertical ? parent.height / 2 : undefined + hoverEnabled: true + + Rectangle { + anchors.fill: parent + color: 'cyan' + opacity: 0.3 + } + } + + MouseArea { + id: mouseArea2 + + SplitView.fillHeight: splitView.orientation === Qt.Horizontal + SplitView.fillWidth: splitView.orientation === Qt.Vertical + SplitView.preferredWidth: splitView.orientation === Qt.Horizontal ? parent.width / 2 : undefined + SplitView.preferredHeight: splitView.orientation === Qt.Vertical ? parent.height / 2 : undefined + hoverEnabled: true + + Rectangle { + anchors.fill: parent + color: 'cyan' + opacity: 0.3 + } + } + } + } + } + + function test_handleContainmentMask_data() { + const data = [ + { + tag: "handleContainmentMaskHorizontalLeftEdgeDragRight", + orientation: Qt.Horizontal, + press: { + x: (handle) => handle.containmentMask.x, + y: (handle) => handle.height / 2 + }, + dx: 25, + dy: 0, + }, + { + tag: "handleContainmentMaskHorizontalRightEdgeDragLeft", + orientation: Qt.Horizontal, + press: { + x: (handle) => handle.containmentMask.x, + y: (handle) => handle.height / 2 + }, + dx: -50, + dy: 0 + }, + { + tag: "handleContainmentMaskHorizontalTopEdgeDragRight", + orientation: Qt.Horizontal, + press: { + x: (handle) => handle.containmentMask.x + handle.containmentMask.width - 1, + y: (handle) => handle.height / 2 + }, + dx: 25, + dy: 0, + }, + { + tag: "handleContainmentMaskHorizontalBottomEdgeDragLeft", + orientation: Qt.Horizontal, + press: { + x: (handle) => handle.containmentMask.x + handle.containmentMask.width - 1, + y: (handle) => handle.containmentMask.y + }, + dx: -50, + dy: 0 + }, + { + tag: "handleContainmentMaskVerticalTopEdgeDragUp", + orientation: Qt.Vertical, + press: { + x: (handle) => handle.width / 2, + y: (handle) => handle.containmentMask.y + }, + dx: 0, + dy: -40, + }, + { + tag: "handleContainmentMaskVerticalTopEdgeDragDown", + orientation: Qt.Vertical, + press: { + x: (handle) => handle.width / 2, + y: (handle) => handle.containmentMask.y + }, + dx: 0, + dy: 70 + }, + { + tag: "handleContainmentMaskVerticalBottomEdgeDragUp", + orientation: Qt.Vertical, + press: { + x: (handle) => handle.width / 2, + y: (handle) => handle.containmentMask.y + handle.containmentMask.height - 1 + }, + dx: 0, + dy: -40, + }, + { + tag: "handleContainmentMaskVerticalBottomEdgeDragDown", + orientation: Qt.Vertical, + press: { + x: (handle) => handle.width / 2, + y: (handle) => handle.containmentMask.y + handle.containmentMask.height - 1 + }, + dx: 0, + dy: 70 + } + ] + + return data + } + + function test_handleContainmentMask(data) { + const control = createTemporaryObject(splitViewHandleContainmentMaskComponent, testCase) + verify(control) + const splitView = control.splitView + splitView.orientation = data.orientation + + const handle = findHandles(splitView)[0] + if (splitView.orientation === Qt.Vertical) + handle.height = handle.defaultHeight + + verify(isPolishScheduled(splitView)) + verify(waitForItemPolished(splitView)) + + const firstItem = control.mouseArea1 + const secondItem = control.mouseArea2 + + const backgroundMouseAreaPress = signalSpyComponent.createObject(control, + { target: control, signalName: "onPressed" }) + + const mouseArea1Press = signalSpyComponent.createObject(firstItem, + { target: firstItem, signalName: "onPressed" }) + + const mouseArea2Press = signalSpyComponent.createObject(secondItem, + { target: secondItem, signalName: "onPressed" }) + + verify(backgroundMouseAreaPress.valid) + verify(mouseArea1Press.valid) + verify(mouseArea2Press.valid) + + const firstItemWidthBeforeDrag = firstItem.width + const secondItemWidthBeforeDrag = secondItem.width + const firstItemHeightBeforeDrag = firstItem.height + const secondItemHeightBeforeDrag = secondItem.height + + const dx = data.dx + const dy = data.dy + const pressX = data.press.x(handle) + const pressY = data.press.y(handle) + + mousePress(handle, pressX, pressY, Qt.LeftButton) + mouseMove(handle, pressX + dx, pressY + dy, -1, Qt.LeftButton) + mouseRelease(handle, pressX + dx, pressY + dy, Qt.LeftButton) + + compare(firstItem.width, firstItemWidthBeforeDrag + dx) + compare(secondItem.width, secondItemWidthBeforeDrag - dx) + compare(firstItem.height, firstItemHeightBeforeDrag + dy) + compare(secondItem.height, secondItemHeightBeforeDrag - dy) + + compare(backgroundMouseAreaPress.count, 0) + compare(mouseArea1Press.count, 0) + compare(mouseArea2Press.count, 0) + } + + function test_handleContainmentMaskHovered_data() { + const data = [ + { + tag: "firstItemHorizontalHover", + orientation: Qt.Horizontal, + press: { + "x": (item) => item.width / 2, + "y": (item) => item.height / 2 + }, + hoverItem: "firstItem", + expectedHover: { + "firstItem": true, + "handle": false, + "secondItem": false + } + }, + { + tag: "handleHorizontalHoverOnTheLeft", + orientation: Qt.Horizontal, + press: { + "x": (item) => item.containmentMask.x, + "y": (item) => item.height / 2 + }, + hoverItem: "handle", + expectedHover: { + "firstItem": true, + "handle": true, + "secondItem": false + } + }, + { + tag: "handleHorizontalHoverOnTheCenter", + orientation: Qt.Horizontal, + press: { + "x": (item) => item.width / 2, + "y": (item) => item.height / 2 + }, + hoverItem: "handle", + expectedHover: { + "firstItem": false, + "handle": true, + "secondItem": false + } + }, + { + tag: "handleHorizontalHoverOnTheRight", + orientation: Qt.Horizontal, + press: { + "x": (item) => item.containmentMask.x + item.containmentMask.width - 1, + "y": (item) => item.height / 2 + }, + hoverItem: "handle", + expectedHover: { + "firstItem": false, + "handle": true, + "secondItem": true + } + }, + { + tag: "secondItemHorizontalHover", + orientation: Qt.Horizontal, + press: { + "x": (item) => item.width / 2, + "y": (item) => item.height / 2 + }, + hoverItem: "secondItem", + expectedHover: { + "firstItem": false, + "handle": false, + "secondItem": true + } + }, + { + tag: "firstItemVerticalHover", + orientation: Qt.Vertical, + press: { + "x": (item) => item.width / 2, + "y": (item) => item.height / 2 + }, + hoverItem: "firstItem", + expectedHover: { + "firstItem": true, + "handle": false, + "secondItem": false + } + }, + { + tag: "handleVerticalHoverOnTheTop", + orientation: Qt.Vertical, + press: { + "x": (item) => item.width / 2, + "y": (item) => item.containmentMask.y + }, + hoverItem: "handle", + expectedHover: { + "firstItem": true, + "handle": true, + "secondItem": false + } + }, + { + tag: "handleVerticalHoverOnTheCenter", + orientation: Qt.Vertical, + press: { + "x": (item) => item.width / 2, + "y": (item) => item.height / 2 + }, + hoverItem: "handle", + expectedHover: { + "firstItem": false, + "handle": true, + "secondItem": false + } + }, + { + tag: "handleVerticalHoverOnTheBottom", + orientation: Qt.Vertical, + press: { + "x": (item) => item.width / 2, + "y": (item) => item.containmentMask.y + item.containmentMask.height - 1 + }, + hoverItem: "handle", + expectedHover: { + "firstItem": false, + "handle": true, + "secondItem": true + } + }, + { + tag: "secondItemVerticalHover", + orientation: Qt.Vertical, + press: { + "x": (item) => item.width / 2, + "y": (item) => item.height / 2 + }, + hoverItem: "secondItem", + expectedHover: { + "firstItem": false, + "handle": false, + "secondItem": true + } + } + ] + + return data + } + + function test_handleContainmentMaskHovered(data) { + if ((Qt.platform.pluginName === "offscreen") || (Qt.platform.pluginName === "minimal")) + skip("Mouse hovering not functional on offscreen/minimal platforms") + + const control = createTemporaryObject(splitViewHandleContainmentMaskComponent, testCase) + verify(control) + const splitView = control.splitView + splitView.orientation = data.orientation + + const handle = findHandles(splitView)[0] + if (splitView.orientation === Qt.Vertical) + handle.height = handle.defaultHeight + + verify(isPolishScheduled(splitView)) + verify(waitForItemPolished(splitView)) + + const firstItem = control.mouseArea1 + const secondItem = control.mouseArea2 + + verify(!firstItem.containsMouse) + verify(!secondItem.containsMouse) + verify(!handle.containsMouse) + + const actualItem = { + "firstItem": firstItem, + "secondItem": secondItem, + "handle": handle + }[data.hoverItem] + + const pressX = data.press.x(actualItem) + const pressY = data.press.y(actualItem) + + // Test fails if we don't do two moves for some reason... + mouseMove(actualItem, pressX, pressY) + mouseMove(actualItem, pressX, pressY) + + compare(firstItem.containsMouse, data.expectedHover.firstItem) + compare(secondItem.containsMouse, data.expectedHover.secondItem) + compare(handle.containsMouse, data.expectedHover.handle) + + // Hide SplitView, then all children shouldn't be hovered + control.visible = false + + verify(isPolishScheduled(splitView)) + verify(waitForItemPolished(splitView)) + + verify(!control.containsMouse) + verify(!firstItem.containsMouse) + verify(!secondItem.containsMouse) + verify(!handle.containsMouse) + } + function test_hoveredPressed() { if ((Qt.platform.pluginName === "offscreen") || (Qt.platform.pluginName === "minimal")) skip("Mouse hovering not functional on offscreen/minimal platforms")