xcb: fix various bugs with _NET_WM_MOVERESIZE

1) After a37785ec76 it become
apparent that we don't get mouse release event from X server
when system move/resize ends (because WM is grabbing the pointer).
The old code (before a37785ec) would wrongly deduce mouse move
as mouse release, which is why the issue was not seen before.
The solution is to subscribe to slave device events.

2) This patch also amends 2488f34ecf as
that patch was solving the issue only for 1/3 of the supported DEs.
It worked with KWin, but not with Unity and Gnome. Its worth
noting that it also worked with two other WMs that I tested -
openbox and awesomewm. The way forward is to detect when system
move/resize was started as a result of touch event and let the
QSizeGrip do the move/resize instead of WMs that are known to
have bugs.

With this patch we also need to adjust the event compression algorithm
to not treat all XI_TouchUpdate events equally. For XI_Motion we don't
care if the event that we process comes from a master or a slave device,
so we can process them as equal.

Task-number: QTBUG-68501
Task-number: QTBUG-51385
Task-number: QTBUG-32476
Change-Id: Iab4e79a289d7bc0fe26f7ae2cff7c562f51a3334
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
This commit is contained in:
Gatis Paeglis 2018-06-05 14:38:01 +02:00
parent 27f1f84c1c
commit 3bc0f1724a
5 changed files with 132 additions and 12 deletions

View File

@ -586,6 +586,8 @@ QXcbConnection::QXcbConnection(QXcbNativeInterface *nativeInterface, bool canGra
m_setup = xcb_get_setup(xcb_connection());
m_xdgCurrentDesktop = qgetenv("XDG_CURRENT_DESKTOP").toLower();
initializeAllAtoms();
if (!qEnvironmentVariableIsSet("QT_XCB_NO_MITSHM"))
@ -1716,7 +1718,8 @@ bool QXcbConnection::compressEvent(xcb_generic_event_t *event, int currentIndex,
continue;
if (isXIType(next, m_xiOpCode, XI_TouchUpdate)) {
xXIDeviceEvent *xiDeviceNextEvent = reinterpret_cast<xXIDeviceEvent *>(next);
if (id == xiDeviceNextEvent->detail % INT_MAX)
if (id == xiDeviceNextEvent->detail % INT_MAX &&
xiDeviceNextEvent->deviceid == xiDeviceEvent->deviceid)
return true;
}
}

View File

@ -514,6 +514,9 @@ public:
void grabServer();
void ungrabServer();
bool isUnity() const { return m_xdgCurrentDesktop == "unity"; }
bool isGnome() const { return m_xdgCurrentDesktop == "gnome"; }
QXcbNativeInterface *nativeInterface() const { return m_nativeInterface; }
QXcbSystemTrayTracker *systemTrayTracker() const;
@ -534,6 +537,7 @@ public:
#endif
#ifdef XCB_USE_XINPUT22
bool startSystemMoveResizeForTouchBegin(xcb_window_t window, const QPoint &point, int corner);
void abortSystemMoveResizeForTouch();
bool isTouchScreen(int id);
#endif
#endif
@ -578,6 +582,7 @@ private:
bool m_xi2Enabled = false;
#if QT_CONFIG(xinput2)
QVector<int> m_floatingSlaveDevices;
int m_xi2Minor = -1;
void initializeXInput2();
void xi2SetupDevice(void *info, bool removeExisting = true);
@ -736,6 +741,8 @@ private:
bool m_peekerIndexCacheDirty = false;
QHash<qint32, qint32> m_peekerToCachedIndex;
friend class QXcbEventReader;
QByteArray m_xdgCurrentDesktop;
};
#if QT_CONFIG(xinput2)
#if QT_CONFIG(tabletevent)

View File

@ -127,7 +127,7 @@ void QXcbConnection::xi2SelectDeviceEvents(xcb_window_t window)
XIEventMask mask;
mask.mask_len = sizeof(bitMask);
mask.mask = xiBitMask;
mask.deviceid = XIAllMasterDevices;
mask.deviceid = XIAllDevices;
Display *dpy = static_cast<Display *>(m_xlib_display);
Status result = XISelectEvents(dpy, window, &mask, 1);
if (result == Success)
@ -315,6 +315,7 @@ void QXcbConnection::xi2SetupDevices()
#endif
m_scrollingDevices.clear();
m_touchDevices.clear();
m_floatingSlaveDevices.clear();
Display *xDisplay = static_cast<Display *>(m_xlib_display);
int deviceCount = 0;
@ -322,6 +323,10 @@ void QXcbConnection::xi2SetupDevices()
m_xiMasterPointerIds.clear();
for (int i = 0; i < deviceCount; ++i) {
XIDeviceInfo deviceInfo = devices[i];
if (deviceInfo.use == XIFloatingSlave) {
m_floatingSlaveDevices.append(deviceInfo.deviceid);
continue;
}
if (deviceInfo.use == XIMasterPointer) {
m_xiMasterPointerIds.append(deviceInfo.deviceid);
continue;
@ -550,6 +555,72 @@ static inline qreal fixed1616ToReal(FP1616 val)
}
#endif // defined(XCB_USE_XINPUT21) || QT_CONFIG(tabletevent)
namespace {
/*! \internal
Qt listens for XIAllDevices to avoid losing mouse events. This function
ensures that we don't process the same event twice: from a slave device and
then again from a master device.
In a normal use case (e.g. mouse press and release inside a window), we will
drop events from master devices as duplicates. Other advantage of processing
events from slave devices is that they don't share button state. All buttons
on a master device share the state.
Examples of special cases:
- During system move/resize, window manager (_NET_WM_MOVERESIZE) grabs the
master pointer, in this case we process the matching release from the slave
device. A master device event is not sent by the server, hence no duplicate
event to drop. If we listened for XIAllMasterDevices instead, we would never
see a release event in this case.
- If we dismiss a context menu by clicking somewhere outside a Qt application,
we will process the mouse press from the master pointer as that is the
device we are grabbing. We are not grabbing slave devices (grabbing on the
slave device is buggy according to 19d289ab1b5bde3e136765e5432b5c7d004df3a4).
And since the event occurs outside our window, the slave device event is
not sent to us by the server, hence no duplicate event to drop.
*/
bool isDuplicateEvent(xcb_ge_event_t *event)
{
struct qXIEvent {
bool isValid = false;
uint16_t sourceid;
uint8_t evtype;
uint32_t detail;
int32_t root_x;
int32_t root_y;
};
static qXIEvent lastSeenEvent;
bool isDuplicate = false;
auto xiDeviceEvent = reinterpret_cast<xXIDeviceEvent *>(event);
if (lastSeenEvent.isValid) {
isDuplicate = lastSeenEvent.sourceid == xiDeviceEvent->sourceid &&
lastSeenEvent.evtype == xiDeviceEvent->evtype &&
lastSeenEvent.detail == xiDeviceEvent->detail &&
lastSeenEvent.root_x == xiDeviceEvent->root_x &&
lastSeenEvent.root_y == xiDeviceEvent->root_y;
} else {
lastSeenEvent.isValid = true;
}
lastSeenEvent.sourceid = xiDeviceEvent->sourceid;
lastSeenEvent.evtype = xiDeviceEvent->evtype;
lastSeenEvent.detail = xiDeviceEvent->detail;
lastSeenEvent.root_x = xiDeviceEvent->root_x;
lastSeenEvent.root_y = xiDeviceEvent->root_y;
if (isDuplicate)
// This sanity check ensures that special cases like QTBUG-59277 keep working.
lastSeenEvent.isValid = false; // An event can be a duplicate only once.
return isDuplicate;
}
} // namespace
void QXcbConnection::xi2HandleEvent(xcb_ge_event_t *event)
{
xi2PrepareXIGenericDeviceEvent(event);
@ -559,10 +630,14 @@ void QXcbConnection::xi2HandleEvent(xcb_ge_event_t *event)
xXIEnterEvent *xiEnterEvent = 0;
QXcbWindowEventListener *eventListener = 0;
bool isTouchEvent = true;
switch (xiEvent->evtype) {
case XI_ButtonPress:
case XI_ButtonRelease:
case XI_Motion:
isTouchEvent = false;
if (!xi2MouseEventsDisabled() && isDuplicateEvent(event))
return;
#ifdef XCB_USE_XINPUT22
case XI_TouchBegin:
case XI_TouchUpdate:
@ -570,6 +645,18 @@ void QXcbConnection::xi2HandleEvent(xcb_ge_event_t *event)
#endif
{
xiDeviceEvent = reinterpret_cast<xXIDeviceEvent *>(event);
if (m_floatingSlaveDevices.contains(xiDeviceEvent->sourceid))
return; // Not interested in floating slave device events, only in attached slaves.
bool isSlaveEvent = xiDeviceEvent->deviceid == xiDeviceEvent->sourceid;
if (!xi2MouseEventsDisabled() && isTouchEvent && isSlaveEvent) {
// For touch events we want events only from master devices, at least
// currently there is no apparent reason why we would need to consider
// events from slave devices.
return;
}
eventListener = windowEventListenerFromId(xiDeviceEvent->event);
sourceDeviceId = xiDeviceEvent->sourceid; // use the actual device id instead of the master
break;
@ -845,6 +932,11 @@ bool QXcbConnection::startSystemMoveResizeForTouchBegin(xcb_window_t window, con
}
return false;
}
void QXcbConnection::abortSystemMoveResizeForTouch()
{
m_startSystemMoveResizeInfo.window = XCB_NONE;
}
#endif // XCB_USE_XINPUT22
bool QXcbConnection::xi2SetMouseGrabEnabled(xcb_window_t w, bool grab)

View File

@ -2635,15 +2635,34 @@ bool QXcbWindow::startSystemMoveResize(const QPoint &pos, int corner)
const xcb_atom_t moveResize = connection()->atom(QXcbAtom::_NET_WM_MOVERESIZE);
if (!connection()->wmSupport()->isSupportedByWM(moveResize))
return false;
const QPoint globalPos = QHighDpi::toNativePixels(window()->mapToGlobal(pos), window()->screen());
#ifdef XCB_USE_XINPUT22
if (connection()->startSystemMoveResizeForTouchBegin(m_window, globalPos, corner))
return true;
#endif
return doStartSystemMoveResize(globalPos, corner);
}
bool QXcbWindow::doStartSystemMoveResize(const QPoint &globalPos, int corner)
const QPoint globalPos = QHighDpi::toNativePixels(window()->mapToGlobal(pos), window()->screen());
bool startedByTouch = false;
#ifdef XCB_USE_XINPUT22
// ### FIXME QTBUG-53389
startedByTouch = connection()->startSystemMoveResizeForTouchBegin(m_window, globalPos, corner);
#endif
if (startedByTouch) {
if (connection()->isUnity() || connection()->isGnome()) {
// These desktops fail to move/resize via _NET_WM_MOVERESIZE (WM bug?).
connection()->abortSystemMoveResizeForTouch();
return false;
}
// KWin, Openbox, AwesomeWM have been tested to work with _NET_WM_MOVERESIZE.
} else { // Started by mouse press.
if (!connection()->hasXInput2() || connection()->xi2MouseEventsDisabled()) {
// Without XI2 we can't get button press/move/release events.
return false;
}
if (connection()->isUnity())
return false; // _NET_WM_MOVERESIZE on this WM is bouncy (WM bug?).
doStartSystemMoveResize(globalPos, corner);
}
return true;
}
void QXcbWindow::doStartSystemMoveResize(const QPoint &globalPos, int corner)
{
const xcb_atom_t moveResize = connection()->atom(QXcbAtom::_NET_WM_MOVERESIZE);
xcb_client_message_event_t xev;
@ -2670,7 +2689,6 @@ bool QXcbWindow::doStartSystemMoveResize(const QPoint &globalPos, int corner)
xcb_send_event(connection()->xcb_connection(), false, xcbScreen()->root(),
XCB_EVENT_MASK_SUBSTRUCTURE_REDIRECT | XCB_EVENT_MASK_SUBSTRUCTURE_NOTIFY,
(const char *)&xev);
return true;
}
// Sends an XEmbed message.

View File

@ -177,7 +177,7 @@ public:
QXcbScreen *xcbScreen() const;
bool startSystemMoveResize(const QPoint &pos, int corner);
bool doStartSystemMoveResize(const QPoint &globalPos, int corner);
void doStartSystemMoveResize(const QPoint &globalPos, int corner);
virtual void create();
virtual void destroy();