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 <mitch.curtis@qt.io>
This commit is contained in:
Igor Bugaev 2022-06-24 11:21:16 +02:00
parent bb337a0e1c
commit 80166d58a1
4 changed files with 486 additions and 23 deletions

View File

@ -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<QQuickSplitHandleAttached*>(
@ -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<QMouseEvent *>(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<QMouseEvent *>(event);
const QPointF point = mapFromItem(item, mouseEvent->position());
d->handleRelease(point, mouseEvent->timestamp());
}
else if (event->type() == QEvent::MouseMove) {
QMouseEvent *mouseEvent = static_cast<QMouseEvent *>(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;
}

View File

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

View File

@ -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);

View File

@ -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")