From 12b9b1c79ec29a9560cc9bf28908d1f50ccafc41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20L=C3=BCbking?= Date: Sat, 13 Aug 2011 16:39:58 +0200 Subject: [PATCH] fix xsync protocol BUG: 160393 REVIEW: 102311 --- client.cpp | 60 +++++++++++++++++++++++++++++++++++---------------- client.h | 19 ++++++++-------- composite.cpp | 25 ++++++++++----------- events.cpp | 20 +++++++++-------- geometry.cpp | 54 +++++++++++++++++++++++----------------------- manage.cpp | 2 ++ 6 files changed, 104 insertions(+), 76 deletions(-) diff --git a/client.cpp b/client.cpp index 9d86cc4fed..a6f0618641 100644 --- a/client.cpp +++ b/client.cpp @@ -114,12 +114,6 @@ Client::Client(Workspace* ws) , block_geometry_updates(0) , pending_geometry_update(PendingGeometryNone) , shade_geometry_change(false) -#ifdef HAVE_XSYNC - , sync_counter(None) - , sync_alarm(None) -#endif - , sync_timeout(NULL) - , sync_resize_pending(false) , border_left(0) , border_right(0) , border_top(0) @@ -141,6 +135,11 @@ Client::Client(Workspace* ws) #ifdef KWIN_BUILD_SCRIPTING scriptCache = new QHash(); #endif +#ifdef HAVE_XSYNC + syncRequest.counter = syncRequest.alarm = None; + syncRequest.timeout = syncRequest.failsafeTimeout = NULL; + syncRequest.isPending = false; +#endif // Set the initial mapping state mapping_state = Withdrawn; @@ -216,8 +215,8 @@ Client::~Client() { //SWrapper::Client::clientRelease(this); #ifdef HAVE_XSYNC - if (sync_alarm != None) - XSyncDestroyAlarm(display(), sync_alarm); + if (syncRequest.alarm != None) + XSyncDestroyAlarm(display(), syncRequest.alarm); #endif assert(!moveResizeMode); assert(client == None); @@ -2037,19 +2036,19 @@ void Client::getSyncCounter() 0, 1, false, XA_CARDINAL, &retType, &formatRet, &nItemRet, &byteRet, &propRet); if (ret == Success && formatRet == 32) { - sync_counter = *(long*)(propRet); - XSyncIntToValue(&sync_counter_value, 0); + syncRequest.counter = *(long*)(propRet); + XSyncIntToValue(&syncRequest.value, 0); XSyncValue zero; XSyncIntToValue(&zero, 0); - XSyncSetCounter(display(), sync_counter, zero); - if (sync_alarm == None) { + XSyncSetCounter(display(), syncRequest.counter, zero); + if (syncRequest.alarm == None) { XSyncAlarmAttributes attrs; - attrs.trigger.counter = sync_counter; + attrs.trigger.counter = syncRequest.counter; attrs.trigger.value_type = XSyncRelative; attrs.trigger.test_type = XSyncPositiveTransition; XSyncIntToValue(&attrs.trigger.wait_value, 1); XSyncIntToValue(&attrs.delta, 1); - sync_alarm = XSyncCreateAlarm(display(), + syncRequest.alarm = XSyncCreateAlarm(display(), XSyncCACounter | XSyncCAValueType | XSyncCATestType | XSyncCADelta | XSyncCAValue, &attrs); } @@ -2066,8 +2065,17 @@ void Client::getSyncCounter() void Client::sendSyncRequest() { #ifdef HAVE_XSYNC - if (sync_counter == None) - return; + if (syncRequest.counter == None || syncRequest.isPending) + return; // do NOT, NEVER send a sync request when there's one on the stack. the clients will just stop respoding. FOREVER! ... + + if (!syncRequest.failsafeTimeout) { + syncRequest.failsafeTimeout = new QTimer(this); + connect(syncRequest.failsafeTimeout, SIGNAL(timeout()), SLOT(removeSyncSupport())); + syncRequest.failsafeTimeout->setSingleShot(true); + } + // if there's no response within 10 seconds, sth. went wrong and we remove XSYNC support from this client. + // see events.cpp Client::syncEvent() + syncRequest.failsafeTimeout->start(ready_for_painting ? 10000 : 1000); // We increment before the notify so that after the notify // syncCounterSerial will equal the value we are expecting @@ -2076,7 +2084,7 @@ void Client::sendSyncRequest() XSyncValue one; XSyncIntToValue(&one, 1); #undef XSyncValueAdd // It causes a warning :-/ - XSyncValueAdd(&sync_counter_value, sync_counter_value, one, &overflow); + XSyncValueAdd(&syncRequest.value, syncRequest.value, one, &overflow); // Send the message to client XEvent ev; @@ -2086,14 +2094,28 @@ void Client::sendSyncRequest() ev.xclient.message_type = atoms->wm_protocols; ev.xclient.data.l[0] = atoms->net_wm_sync_request; ev.xclient.data.l[1] = xTime(); - ev.xclient.data.l[2] = XSyncValueLow32(sync_counter_value); - ev.xclient.data.l[3] = XSyncValueHigh32(sync_counter_value); + ev.xclient.data.l[2] = XSyncValueLow32(syncRequest.value); + ev.xclient.data.l[3] = XSyncValueHigh32(syncRequest.value); ev.xclient.data.l[4] = 0; + syncRequest.isPending = true; XSendEvent(display(), window(), False, NoEventMask, &ev); XSync(display(), false); #endif } +void Client::removeSyncSupport() +{ + if (!ready_for_painting) { + ready_for_painting = true; + addRepaintFull(); + return; + } + syncRequest.isPending = false; + syncRequest.counter = syncRequest.alarm = None; + delete syncRequest.timeout; delete syncRequest.failsafeTimeout; + syncRequest.timeout = syncRequest.failsafeTimeout = NULL; +} + bool Client::wantsTabFocus() const { return (isNormalWindow() || isDialog()) && wantsInput(); diff --git a/client.h b/client.h index 7796c44a69..66b9c466b0 100644 --- a/client.h +++ b/client.h @@ -471,11 +471,12 @@ protected: virtual bool shouldUnredirect() const; private slots: + void delayedSetShortcut(); + void performMoveResize(); + void removeSyncSupport(); void pingTimeout(); void processKillerExited(); void demandAttentionKNotify(); - void syncTimeout(); - void delayedSetShortcut(); void repaintDecorationPending(); //Signals for the scripting interface @@ -528,7 +529,6 @@ private: void blockGeometryUpdates(bool block); void getSyncCounter(); void sendSyncRequest(); - bool startMoveResize(); void finishMoveResize(bool cancel); void leaveMoveResize(); @@ -541,7 +541,6 @@ private: void ungrabButton(int mod); void resetMaximize(); void resizeDecoration(const QSize& s); - void performMoveResize(); void pingWindow(); void killProcess(bool ask, Time timestamp = CurrentTime); @@ -691,12 +690,14 @@ private: QRect deco_rect_before_block; bool shade_geometry_change; #ifdef HAVE_XSYNC - XSyncCounter sync_counter; - XSyncValue sync_counter_value; - XSyncAlarm sync_alarm; + struct { + XSyncCounter counter; + XSyncValue value; + XSyncAlarm alarm; + QTimer *timeout, *failsafeTimeout; + bool isPending; + } syncRequest; #endif - QTimer* sync_timeout; - bool sync_resize_pending; int border_left, border_right, border_top, border_bottom; int padding_left, padding_right, padding_top, padding_bottom; QRegion _mask; diff --git a/composite.cpp b/composite.cpp index 2b8ea6f0e6..7295349892 100644 --- a/composite.cpp +++ b/composite.cpp @@ -373,22 +373,20 @@ void Workspace::performCompositing() } // create a list of all windows in the stacking order ToplevelList windows = xStackingOrder(); - foreach (EffectWindow * c, static_cast< EffectsHandlerImpl* >(effects)->elevatedWindows()) { + foreach (EffectWindow *c, static_cast< EffectsHandlerImpl* >(effects)->elevatedWindows()) { Toplevel* t = static_cast< EffectWindowImpl* >(c)->window(); windows.removeAll(t); windows.append(t); } -#if 0 + // skip windows that are not yet ready for being painted - ToplevelList tmp = windows; - windows.clear(); - // There is a bug somewhere that prevents this from working properly (#160393), but additionally + // TODO ? // this cannot be used so carelessly - needs protections against broken clients, the window // should not get focus before it's displayed, handle unredirected windows properly and so on. - foreach (Toplevel * c, tmp) - if (c->readyForPainting()) - windows.append(c); -#endif + foreach (Toplevel *t, windows) + if (!t->readyForPainting()) + windows.removeAll(t); + QRegion repaints = repaints_region; // clear all repaints, so that post-pass can add repaints for the next repaint repaints_region = QRegion(); @@ -635,13 +633,16 @@ void Toplevel::damageNotifyEvent(XDamageNotifyEvent* e) void Client::damageNotifyEvent(XDamageNotifyEvent* e) { - Toplevel::damageNotifyEvent(e); #ifdef HAVE_XSYNC - if (sync_counter == None) // cannot detect complete redraw, consider done now + if (syncRequest.isPending && isResize()) + return; + if (syncRequest.counter == None) // cannot detect complete redraw, consider done now ready_for_painting = true; #else - ready_for_painting = true; // no sync at all, consider done now + ready_for_painting = true; #endif + + Toplevel::damageNotifyEvent(e); } void Toplevel::addDamage(const QRect& r) diff --git a/events.cpp b/events.cpp index dd2c3a4b97..9843f53da0 100644 --- a/events.cpp +++ b/events.cpp @@ -473,9 +473,9 @@ bool Workspace::workspaceEvent(XEvent * e) } else if (e->type == Extensions::syncAlarmNotifyEvent() && Extensions::syncAvailable()) { #ifdef HAVE_XSYNC foreach (Client * c, clients) - c->syncEvent(reinterpret_cast< XSyncAlarmNotifyEvent* >(e)); + c->syncEvent(reinterpret_cast< XSyncAlarmNotifyEvent* >(e)); foreach (Client * c, desktops) - c->syncEvent(reinterpret_cast< XSyncAlarmNotifyEvent* >(e)); + c->syncEvent(reinterpret_cast< XSyncAlarmNotifyEvent* >(e)); #endif } break; @@ -1549,15 +1549,17 @@ void Client::keyPressEvent(uint key_code) #ifdef HAVE_XSYNC void Client::syncEvent(XSyncAlarmNotifyEvent* e) { - if (e->alarm == sync_alarm && XSyncValueEqual(e->counter_value, sync_counter_value)) { + if (e->alarm == syncRequest.alarm && XSyncValueEqual(e->counter_value, syncRequest.value)) { ready_for_painting = true; + syncRequest.isPending = false; + if (syncRequest.failsafeTimeout) + syncRequest.failsafeTimeout->stop(); if (isResize()) { - delete sync_timeout; - sync_timeout = NULL; - if (sync_resize_pending) - performMoveResize(); - sync_resize_pending = false; - } + if (syncRequest.timeout) + syncRequest.timeout->stop(); + performMoveResize(); + } else + addRepaintFull(); } } #endif diff --git a/geometry.cpp b/geometry.cpp index 0e42f6f071..0f4224d91f 100644 --- a/geometry.cpp +++ b/geometry.cpp @@ -1893,7 +1893,10 @@ void Client::setGeometry(int x, int y, int w, int h, ForceGeometry_t force, bool QSize cs = clientSize(); XMoveResizeWindow(display(), wrapperId(), clientPos().x(), clientPos().y(), cs.width(), cs.height()); - XMoveResizeWindow(display(), window(), 0, 0, cs.width(), cs.height()); +#ifdef HAVE_XSYNC + if (!isResize() || syncRequest.counter == None) +#endif + XMoveResizeWindow(display(), window(), 0, 0, cs.width(), cs.height()); } updateShape(); } else @@ -2678,8 +2681,8 @@ void Client::leaveMoveResize() move_resize_grab_window = None; workspace()->setClientIsMoving(0); moveResizeMode = false; - delete sync_timeout; - sync_timeout = NULL; + delete syncRequest.timeout; + syncRequest.timeout = NULL; #ifdef KWIN_BUILD_SCREENEDGES if (options->electricBorders() == Options::ElectricMoveOnly || options->electricBorderMaximize() || @@ -2760,6 +2763,9 @@ void Client::delayedMoveResize() void Client::handleMoveResize(int x, int y, int x_root, int y_root) { + if (syncRequest.isPending && isResize()) + return; // we're still waiting for the client or the timeout + if ((mode == PositionCenter && !isMovableAcrossScreens()) || (mode != PositionCenter && (isShade() || !isResizable()))) return; @@ -3011,14 +3017,21 @@ void Client::handleMoveResize(int x, int y, int x_root, int y_root) } else abort(); - if (isResize()) { - if (sync_timeout != NULL) { - sync_resize_pending = true; - return; - } - } + if (!update) + return; - if (update) +#ifdef HAVE_XSYNC + if (isResize() && syncRequest.counter != None) { + if (!syncRequest.timeout) { + syncRequest.timeout = new QTimer(this); + connect(syncRequest.timeout, SIGNAL(timeout()), SLOT(performMoveResize())); + syncRequest.timeout->setSingleShot(true); + } + syncRequest.timeout->start(250); + sendSyncRequest(); + XMoveResizeWindow(display(), window(), 0, 0, moveResizeGeom.width() - (border_left + border_right), moveResizeGeom.height() - (border_top + border_bottom)); + } else +#endif performMoveResize(); if (isMove()) { @@ -3033,31 +3046,18 @@ void Client::handleMoveResize(int x, int y, int x_root, int y_root) void Client::performMoveResize() { -#ifdef HAVE_XSYNC - if (isResize() && sync_counter != None && !sync_resize_pending) { - sync_timeout = new QTimer(this); - connect(sync_timeout, SIGNAL(timeout()), SLOT(syncTimeout())); - sync_timeout->setSingleShot(true); - sync_timeout->start(500); - sendSyncRequest(); - } -#endif #ifdef KWIN_BUILD_TILING if (!workspace()->tiling()->isEnabled()) +#endif setGeometry(moveResizeGeom); +#ifdef HAVE_XSYNC + if (isResize() && syncRequest.counter != None) + addRepaintFull(); #endif positionGeometryTip(); emit clientStepUserMovedResized(this, moveResizeGeom); } -void Client::syncTimeout() -{ - sync_timeout->deleteLater(); - sync_timeout = NULL; - if (sync_resize_pending) - performMoveResize(); -} - void Client::setElectricBorderMode(QuickTileMode mode) { if (mode != QuickTileMaximize) { diff --git a/manage.cpp b/manage.cpp index 47c8814510..222f64d3d3 100644 --- a/manage.cpp +++ b/manage.cpp @@ -501,6 +501,8 @@ bool Client::manage(Window w, bool isMapped) // Sending ConfigureNotify is done when setting mapping state below, // Getting the first sync response means window is ready for compositing sendSyncRequest(); + else + ready_for_painting = true; // set to true in case compositing is turned on later. bug #160393 if (isShown(true) && !doNotShow) { if (isDialog())