From c6580ce69650000fda71ddea582798cce080e675 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20L=C3=BCbking?= Date: Mon, 23 Sep 2013 23:04:17 +0200 Subject: [PATCH 1/5] add int Screens::intersecting(QRect) REVIEW: 112910 --- screens.cpp | 11 +++++++++++ screens.h | 2 ++ 2 files changed, 13 insertions(+) diff --git a/screens.cpp b/screens.cpp index 23caf59352..c351c7e12e 100644 --- a/screens.cpp +++ b/screens.cpp @@ -117,6 +117,17 @@ int Screens::current() const return m_current; } +int Screens::intersecting(const QRect &r) const +{ + int cnt = 0; + for (int i = 0; i < count(); ++i) { + if (geometry(i).intersects(r)) { + ++cnt; + } + } + return cnt; +} + DesktopWidgetScreens::DesktopWidgetScreens(QObject *parent) : Screens(parent) , m_desktop(QApplication::desktop()) diff --git a/screens.h b/screens.h index f05f818519..0ba11d1aa9 100644 --- a/screens.h +++ b/screens.h @@ -68,6 +68,8 @@ public: inline bool isChanging() { return m_changedTimer->isActive(); } + int intersecting(const QRect &r) const; + public Q_SLOTS: void reconfigure(); From 06b482f72552175d171a3250ac1dcb8176f8d255 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20L=C3=BCbking?= Date: Mon, 16 Sep 2013 16:26:21 +0200 Subject: [PATCH 2/5] pack to client, not deco this makes packing following the snapping behavior REVIEW: 112807 --- client.cpp | 2 +- client.h | 2 +- placement.cpp | 29 +++++++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/client.cpp b/client.cpp index e8e20d5ba2..4b88f22a24 100644 --- a/client.cpp +++ b/client.cpp @@ -2430,7 +2430,7 @@ QRect Client::decorationRect() const } } -KDecorationDefines::Position Client::titlebarPosition() +KDecorationDefines::Position Client::titlebarPosition() const { Position titlePos = PositionCenter; // PositionTop is returned by the default implementation // this will hint errors in the metaobject usage ;-) diff --git a/client.h b/client.h index 40811f2e39..a2b3365e3d 100644 --- a/client.h +++ b/client.h @@ -603,7 +603,7 @@ public: bool decorationHasAlpha() const; - Position titlebarPosition(); + Position titlebarPosition() const; enum CoordinateMode { DecorationRelative, // Relative to the top left corner of the decoration diff --git a/placement.cpp b/placement.cpp index 493ae158c7..2f5f222aa9 100644 --- a/placement.cpp +++ b/placement.cpp @@ -33,6 +33,7 @@ along with this program. If not, see . #include "client.h" #include "options.h" #include "rules.h" +#include "screens.h" #endif namespace KWin @@ -836,6 +837,13 @@ int Workspace::packPositionLeft(const Client* cl, int oldx, bool left_edge) cons if (oldx <= newx) // try another Xinerama screen newx = clientArea(MaximizeArea, QPoint(cl->geometry().left() - 1, cl->geometry().center().y()), cl->desktop()).left(); + if (cl->titlebarPosition() != KDecorationDefines::PositionLeft) { + QRect geo = cl->geometry(); + int rgt = newx - cl->clientPos().x(); + geo.moveRight(rgt); + if (screens()->intersecting(geo) < 2) + newx = rgt; + } if (oldx <= newx) return oldx; for (ClientList::ConstIterator it = clients.constBegin(), end = clients.constEnd(); it != end; ++it) { @@ -856,6 +864,13 @@ int Workspace::packPositionRight(const Client* cl, int oldx, bool right_edge) co if (oldx >= newx) // try another Xinerama screen newx = clientArea(MaximizeArea, QPoint(cl->geometry().right() + 1, cl->geometry().center().y()), cl->desktop()).right(); + if (cl->titlebarPosition() != KDecorationDefines::PositionRight) { + QRect geo = cl->geometry(); + int rgt = newx + cl->width() - (cl->clientSize().width() + cl->clientPos().x()); + geo.moveRight(rgt); + if (screens()->intersecting(geo) < 2) + newx = rgt; + } if (oldx >= newx) return oldx; for (ClientList::ConstIterator it = clients.constBegin(), end = clients.constEnd(); it != end; ++it) { @@ -876,6 +891,13 @@ int Workspace::packPositionUp(const Client* cl, int oldy, bool top_edge) const if (oldy <= newy) // try another Xinerama screen newy = clientArea(MaximizeArea, QPoint(cl->geometry().center().x(), cl->geometry().top() - 1), cl->desktop()).top(); + if (cl->titlebarPosition() != KDecorationDefines::PositionTop) { + QRect geo = cl->geometry(); + int top = newy - cl->clientPos().y(); + geo.moveTop(top); + if (screens()->intersecting(geo) < 2) + newy = top; + } if (oldy <= newy) return oldy; for (ClientList::ConstIterator it = clients.constBegin(), end = clients.constEnd(); it != end; ++it) { @@ -896,6 +918,13 @@ int Workspace::packPositionDown(const Client* cl, int oldy, bool bottom_edge) co if (oldy >= newy) // try another Xinerama screen newy = clientArea(MaximizeArea, QPoint(cl->geometry().center().x(), cl->geometry().bottom() + 1), cl->desktop()).bottom(); + if (cl->titlebarPosition() != KDecorationDefines::PositionBottom) { + QRect geo = cl->geometry(); + int btm = newy + cl->height() - (cl->clientSize().height() + cl->clientPos().y()); + geo.moveBottom(btm); + if (screens()->intersecting(geo) < 2) + newy = btm; + } if (oldy >= newy) return oldy; for (ClientList::ConstIterator it = clients.constBegin(), end = clients.constEnd(); it != end; ++it) { From 4c170a2fdbcf42ed5ff3dac04e9a0423c1c0b76d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20L=C3=BCbking?= Date: Thu, 15 Aug 2013 11:54:46 +0200 Subject: [PATCH 3/5] snap to deco, not client on inner screen borders client snapping is ultimately reasoned by fitt's law and inner borders are not infinite BUG: 323504 FIXED-IN: 4.11 REVIEW: 112103 --- geometry.cpp | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/geometry.cpp b/geometry.cpp index 6d71f27d04..7064566a5d 100644 --- a/geometry.cpp +++ b/geometry.cpp @@ -400,8 +400,9 @@ QPoint Workspace::adjustClientPosition(Client* c, QPoint pos, bool unrestricted, if (options->windowSnapZone() || !borderSnapZone.isNull() || options->centerSnapZone()) { const bool sOWO = options->isSnapOnlyWhenOverlapping(); + const int screen = screens()->number(pos + c->rect().center()); if (maxRect.isNull()) - maxRect = clientArea(MovementArea, pos + c->rect().center(), c->desktop()); + maxRect = clientArea(MovementArea, screen, c->desktop()); const int xmin = maxRect.left(); const int xmax = maxRect.right() + 1; //desk size const int ymin = maxRect.top(); @@ -424,21 +425,25 @@ QPoint Workspace::adjustClientPosition(Client* c, QPoint pos, bool unrestricted, const int snapX = borderSnapZone.width() * snapAdjust; //snap trigger const int snapY = borderSnapZone.height() * snapAdjust; if (snapX || snapY) { + QRect geo = c->geometry(); const QPoint cp = c->clientPos(); - const QSize cs = c->geometry().size() - c->clientSize(); + const QSize cs = geo.size() - c->clientSize(); int padding[4] = { cp.x(), cs.width() - cp.x(), cp.y(), cs.height() - cp.y() }; - // snap to titlebar + // snap to titlebar / snap to window borders on inner screen edges Position titlePos = c->titlebarPosition(); - if (titlePos == PositionLeft || (c->maximizeMode() & MaximizeHorizontal)) + if (padding[0] && (titlePos == PositionLeft || (c->maximizeMode() & MaximizeHorizontal) || + screens()->intersecting(geo.translated(maxRect.x() - (padding[0] + geo.x()), 0)) > 1)) padding[0] = 0; - if (titlePos == PositionRight || (c->maximizeMode() & MaximizeHorizontal)) + if (padding[1] && (titlePos == PositionRight || (c->maximizeMode() & MaximizeHorizontal) || + screens()->intersecting(geo.translated(maxRect.right() + padding[1] - geo.right(), 0)) > 1)) padding[1] = 0; - if (titlePos == PositionTop || (c->maximizeMode() & MaximizeVertical)) + if (padding[2] && (titlePos == PositionTop || (c->maximizeMode() & MaximizeVertical) || + screens()->intersecting(geo.translated(0, maxRect.y() - (padding[2] + geo.y()))) > 1)) padding[2] = 0; - if (titlePos == PositionBottom || (c->maximizeMode() & MaximizeVertical)) + if (padding[3] && (titlePos == PositionBottom || (c->maximizeMode() & MaximizeVertical) || + screens()->intersecting(geo.translated(0, maxRect.bottom() + padding[3] - geo.bottom())) > 1)) padding[3] = 0; - if ((sOWO ? (cx < xmin) : true) && (qAbs(xmin - cx) < snapX)) { deltaX = xmin - cx; nx = xmin - padding[0]; From cb88bc884885d6d841132e81b1d4fa4aba7cef4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20L=C3=BCbking?= Date: Wed, 11 Sep 2013 22:50:32 +0200 Subject: [PATCH 4/5] keepInArea, client geometry containment condition Since windows can place the decoration outside the screen this needs to be still a valid condition when checking whether we've to keep in area In addition: calculate tx, ty and perform one move call (include rule check and XMoveWindow unless there's a geometry blocker ...) BUG: 324792 FIXED-IN: 4.11.2 REVIEW: 112805 --- geometry.cpp | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/geometry.cpp b/geometry.cpp index 7064566a5d..07be3cd9db 100644 --- a/geometry.cpp +++ b/geometry.cpp @@ -852,19 +852,19 @@ void Client::keepInArea(QRect area, bool partial) if (area.width() < width() || area.height() < height()) resizeWithChecks(qMin(area.width(), width()), qMin(area.height(), height())); } + int tx = x(), ty = y(); if (geometry().right() > area.right() && width() <= area.width()) - move(area.right() - width() + 1, y()); + tx = area.right() - width() + 1; if (geometry().bottom() > area.bottom() && height() <= area.height()) - move(x(), area.bottom() - height() + 1); + ty = area.bottom() - height() + 1; if (!area.contains(geometry().topLeft())) { - int tx = x(); - int ty = y(); if (tx < area.x()) tx = area.x(); if (ty < area.y()) ty = area.y(); - move(tx, ty); } + if (tx != x() || ty != y()) + move(tx, ty); } /*! @@ -1491,16 +1491,16 @@ void Client::getWmNormalHints() // update to match restrictions QSize new_size = adjustedSize(); if (new_size != size() && !isFullScreen()) { - QRect orig_geometry = geometry(); + QRect origClientGeometry(pos() + clientPos(), clientSize()); resizeWithChecks(new_size); if ((!isSpecialWindow() || isToolbar()) && !isFullScreen()) { // try to keep the window in its xinerama screen if possible, // if that fails at least keep it visible somewhere QRect area = workspace()->clientArea(MovementArea, this); - if (area.contains(orig_geometry)) + if (area.contains(origClientGeometry)) keepInArea(area); area = workspace()->clientArea(WorkArea, this); - if (area.contains(orig_geometry)) + if (area.contains(origClientGeometry)) keepInArea(area); } } @@ -1682,7 +1682,7 @@ void Client::configureRequest(int value_mask, int rx, int ry, int rw, int rh, in if (newScreen != rules()->checkScreen(newScreen)) return; // not allowed by rule - QRect orig_geometry = geometry(); + QRect origClientGeometry(pos() + clientPos(), clientSize()); GeometryUpdatesBlocker blocker(this); move(new_pos); plainResize(ns); @@ -1690,7 +1690,7 @@ void Client::configureRequest(int value_mask, int rx, int ry, int rw, int rh, in updateFullScreenHack(QRect(new_pos, QSize(nw, nh))); QRect area = workspace()->clientArea(WorkArea, this); if (!from_tool && (!isSpecialWindow() || isToolbar()) && !isFullScreen() - && area.contains(orig_geometry)) + && area.contains(origClientGeometry)) keepInArea(area); // this is part of the kicker-xinerama-hack... it should be @@ -1712,7 +1712,7 @@ void Client::configureRequest(int value_mask, int rx, int ry, int rw, int rh, in QSize ns = sizeForClientSize(QSize(nw, nh)); if (ns != size()) { // don't restore if some app sets its own size again - QRect orig_geometry = geometry(); + QRect origClientGeometry(pos() + clientPos(), clientSize()); GeometryUpdatesBlocker blocker(this); int save_gravity = xSizeHint.win_gravity; xSizeHint.win_gravity = gravity; @@ -1723,10 +1723,10 @@ void Client::configureRequest(int value_mask, int rx, int ry, int rw, int rh, in // try to keep the window in its xinerama screen if possible, // if that fails at least keep it visible somewhere QRect area = workspace()->clientArea(MovementArea, this); - if (area.contains(orig_geometry)) + if (area.contains(origClientGeometry)) keepInArea(area); area = workspace()->clientArea(WorkArea, this); - if (area.contains(orig_geometry)) + if (area.contains(origClientGeometry)) keepInArea(area); } } From e63e5f5712328b3b4e4f81a7e94c222b4501f9a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20L=C3=BCbking?= Date: Fri, 30 Aug 2013 01:30:13 +0200 Subject: [PATCH 5/5] fix scheduling the repaints repaints caused by effects so far polluted the timing calculations since they started the timer on the old vsync offset This (together with undercut timing) lead to multiple frames in the buffer queue, and ultimately to a blocking swap For unsynced painting, it simply caused wrong timings - leading to "well, kinda around 60Hz - could be 75 as just well". REVIEW: 112368 CCBUG: 322060 that part is fixed in 4.11.2 --- composite.cpp | 33 +++++++++++++++++++++++++++------ eglonxbackend.cpp | 10 ++++++++++ glxbackend.cpp | 10 ++++++++++ 3 files changed, 47 insertions(+), 6 deletions(-) diff --git a/composite.cpp b/composite.cpp index de674b6c91..4eb8e46044 100644 --- a/composite.cpp +++ b/composite.cpp @@ -270,7 +270,6 @@ void Compositor::slotCompositingOptionsInitialized() } // render at least once - compositeTimer.stop(); performCompositing(); } @@ -548,7 +547,6 @@ void Compositor::addRepaintFull() void Compositor::timerEvent(QTimerEvent *te) { if (te->timerId() == compositeTimer.timerId()) { - compositeTimer.stop(); performCompositing(); } else QObject::timerEvent(te); @@ -596,10 +594,12 @@ void Compositor::performCompositing() if (repaints_region.isEmpty() && !windowRepaintsPending()) { m_scene->idle(); + m_timeSinceLastVBlank = fpsInterval - (options->vBlankTime() + 1); // means "start now" // Note: It would seem here we should undo suspended unredirect, but when scenes need // it for some reason, e.g. transformations or translucency, the next pass that does not // need this anymore and paints normally will also reset the suspended unredirect. // Otherwise the window would not be painted normally anyway. + compositeTimer.stop(); return; } @@ -617,6 +617,8 @@ void Compositor::performCompositing() m_timeSinceLastVBlank = m_scene->paint(repaints, windows); + compositeTimer.stop(); // stop here to ensure *we* cause the next repaint schedule - not some effect through m_scene->paint() + // Trigger at least one more pass even if there would be nothing to paint, so that scene->idle() // is called the next time. If there would be nothing pending, it will not restart the timer and // scheduleRepaint() would restart it again somewhen later, called from functions that @@ -676,10 +678,29 @@ void Compositor::setCompositeTimer() waitTime = nanoToMilli(padding - options->vBlankTime()); } } - else // w/o vsync we just jump to the next demanded tick - // the "1" will ensure we don't block out the eventloop - the system's just not faster - // "0" would be sufficient, but the compositor isn't the WMs only task - waitTime = (m_timeSinceLastVBlank > fpsInterval) ? 1 : nanoToMilli(fpsInterval - m_timeSinceLastVBlank); + else { // w/o blocking vsync we just jump to the next demanded tick + if (fpsInterval > m_timeSinceLastVBlank) { + waitTime = nanoToMilli(fpsInterval - m_timeSinceLastVBlank); + if (!waitTime) { + waitTime = 1; // will ensure we don't block out the eventloop - the system's just not faster ... + } + }/* else if (m_scene->syncsToVBlank() && m_timeSinceLastVBlank - fpsInterval < (vBlankInterval<<1)) { + // NOTICE - "for later" ------------------------------------------------------------------ + // It can happen that we push two frames within one refresh cycle. + // Swapping will then block even with triple buffering when the GPU does not discard but + // queues frames + // now here's the mean part: if we take that as "OMG, we're late - next frame ASAP", + // there'll immediately be 2 frames in the pipe, swapping will block, we think we're + // late ... ewww + // so instead we pad to the clock again and add 2ms safety to ensure the pipe is really + // free + // NOTICE: obviously m_timeSinceLastVBlank can be too big because we're too slow as well + // So if this code was enabled, we'd needlessly half the framerate once more (15 instead of 30) + waitTime = nanoToMilli(vBlankInterval - (m_timeSinceLastVBlank - fpsInterval)%vBlankInterval) + 2; + }*/ else { + waitTime = 1; // ... "0" would be sufficient, but the compositor isn't the WMs only task + } + } compositeTimer.start(qMin(waitTime, 250u), this); // force 4fps minimum } diff --git a/eglonxbackend.cpp b/eglonxbackend.cpp index ab692d7cb5..39dd82419b 100644 --- a/eglonxbackend.cpp +++ b/eglonxbackend.cpp @@ -26,6 +26,8 @@ along with this program. If not, see . #include // KDE #include +// system +#include namespace KWin { @@ -320,6 +322,14 @@ SceneOpenGL::TexturePrivate *EglOnXBackend::createBackendTexture(SceneOpenGL::Te void EglOnXBackend::prepareRenderingFrame() { + if (gs_tripleBufferNeedsDetection) { + // the composite timer floors the repaint frequency. This can pollute our triple buffering + // detection because the glXSwapBuffers call for the new frame has to wait until the pending + // one scanned out. + // So we compensate for that by waiting an extra milisecond to give the driver the chance to + // fllush the buffer queue + usleep(1000); + } present(); startRenderTimer(); eglWaitNative(EGL_CORE_NATIVE_ENGINE); diff --git a/glxbackend.cpp b/glxbackend.cpp index 9e3064c092..f1b9a6a598 100644 --- a/glxbackend.cpp +++ b/glxbackend.cpp @@ -35,6 +35,8 @@ along with this program. If not, see . // KDE #include #include +// system +#include namespace KWin { @@ -493,6 +495,14 @@ SceneOpenGL::TexturePrivate *GlxBackend::createBackendTexture(SceneOpenGL::Textu void GlxBackend::prepareRenderingFrame() { + if (gs_tripleBufferNeedsDetection) { + // the composite timer floors the repaint frequency. This can pollute our triple buffering + // detection because the glXSwapBuffers call for the new frame has to wait until the pending + // one scanned out. + // So we compensate for that by waiting an extra milisecond to give the driver the chance to + // fllush the buffer queue + usleep(1000); + } present(); startRenderTimer(); glXWaitX();