From 54427623716a42ade3c8c33eb8a6b1e132e2bdb4 Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Wed, 28 Oct 2020 09:03:09 +0200 Subject: [PATCH] platforms/drm: Use a software cursor if the cursor image is too big When dragging files on the desktop, the cursor image might be just too big for the cursor plane, in which case we need to abandon hardware cursors for a brief moment and use a software cursor. Once the files have been dropped and the cursor image is small enough, we can go back to using hw cursors. BUG: 424589 --- platform.cpp | 33 ++++++++++-- platform.h | 18 +++++++ plugins/platforms/drm/drm_backend.cpp | 53 +++++++++++-------- plugins/platforms/drm/drm_backend.h | 3 +- plugins/platforms/drm/drm_gpu.cpp | 2 +- plugins/platforms/drm/drm_output.cpp | 35 ++++++++---- plugins/platforms/drm/drm_output.h | 2 +- plugins/platforms/fbdev/fb_backend.cpp | 2 +- plugins/platforms/virtual/virtual_backend.cpp | 2 +- 9 files changed, 110 insertions(+), 40 deletions(-) diff --git a/platform.cpp b/platform.cpp index 3923e8aafe..7660913bb7 100644 --- a/platform.cpp +++ b/platform.cpp @@ -37,7 +37,7 @@ Platform::Platform(QObject *parent) : QObject(parent) , m_eglDisplay(EGL_NO_DISPLAY) { - setSoftwareCursor(false); + setSoftwareCursorForced(false); m_colorCorrect = new ColorCorrect::Manager(this); connect(Cursors::self(), &Cursors::currentCursorRendered, this, &Platform::cursorRendered); } @@ -188,13 +188,11 @@ bool Platform::usesSoftwareCursor() const void Platform::setSoftwareCursor(bool set) { - if (qEnvironmentVariableIsSet("KWIN_FORCE_SW_CURSOR")) { - set = true; - } if (m_softwareCursor == set) { return; } m_softwareCursor = set; + doSetSoftwareCursor(); if (m_softwareCursor) { connect(Cursors::self(), &Cursors::positionChanged, this, &Platform::triggerCursorRepaint); connect(Cursors::self(), &Cursors::currentCursorChanged, this, &Platform::triggerCursorRepaint); @@ -205,6 +203,33 @@ void Platform::setSoftwareCursor(bool set) triggerCursorRepaint(); } +void Platform::doSetSoftwareCursor() +{ +} + +bool Platform::isSoftwareCursorForced() const +{ + return m_softwareCursorForced; +} + +void Platform::setSoftwareCursorForced(bool forced) +{ + if (qEnvironmentVariableIsSet("KWIN_FORCE_SW_CURSOR")) { + forced = true; + } + if (m_softwareCursorForced == forced) { + return; + } + m_softwareCursorForced = forced; + if (m_softwareCursorForced) { + setSoftwareCursor(true); + } else { + // Do not unset the software cursor yet, the platform will choose the right + // moment when it can be done. There is still a chance that we must continue + // using the software cursor. + } +} + void Platform::triggerCursorRepaint() { if (!Compositor::self()) { diff --git a/platform.h b/platform.h index e69b5bc63d..c12a2a94ab 100644 --- a/platform.h +++ b/platform.h @@ -284,6 +284,21 @@ public: */ bool usesSoftwareCursor() const; + /** + * Returns @c true if the software cursor is being forced; otherwise returns @c false. + * + * Note that the value returned by this function not always matches usesSoftwareCursor(). + * If this function returns @c true, then it is guaranteed that the compositor will + * use the software cursor. However, this doesn't apply vice versa. + * + * If the compositor uses a software cursor, this function may return @c false. This + * is typically the case if the current cursor image can't be displayed using hardware + * cursors, for example due to buffer size limitations, etc. + * + * @see usesSoftwareCursor() + */ + bool isSoftwareCursorForced() const; + /** * Returns a PlatformCursorImage. By default this is created by softwareCursor and * softwareCursorHotspot. An implementing subclass can use this to provide a better @@ -487,6 +502,7 @@ Q_SIGNALS: protected: explicit Platform(QObject *parent = nullptr); void setSoftwareCursor(bool set); + void setSoftwareCursorForced(bool forced); void repaint(const QRect &rect); void setReady(bool ready); QSize initialWindowSize() const { @@ -532,10 +548,12 @@ protected: * @see showCursor */ virtual void doShowCursor(); + virtual void doSetSoftwareCursor(); private: void triggerCursorRepaint(); bool m_softwareCursor = false; + bool m_softwareCursorForced = false; struct { QRect lastRenderedGeometry; } m_cursor; diff --git a/plugins/platforms/drm/drm_backend.cpp b/plugins/platforms/drm/drm_backend.cpp index fceeaddaef..a2b70eeb42 100644 --- a/plugins/platforms/drm/drm_backend.cpp +++ b/plugins/platforms/drm/drm_backend.cpp @@ -506,7 +506,7 @@ void DrmBackend::initCursor() break; } } - setSoftwareCursor(needsSoftwareCursor); + setSoftwareCursorForced(needsSoftwareCursor); #endif if (waylandServer()->seat()->hasPointer()) { @@ -529,41 +529,42 @@ void DrmBackend::initCursor() connect(Cursors::self(), &Cursors::positionChanged, this, &DrmBackend::moveCursor); } -void DrmBackend::setCursor() -{ - for (auto it = m_outputs.constBegin(); it != m_outputs.constEnd(); ++it) { - if (!(*it)->showCursor()) { - setSoftwareCursor(true); - } - } -} - void DrmBackend::updateCursor() { - if (usesSoftwareCursor()) { - return; - } - if (isCursorHidden()) { + if (isSoftwareCursorForced() || isCursorHidden()) { return; } auto cursor = Cursors::self()->currentCursor(); - const QImage &cursorImage = cursor->image(); - if (cursorImage.isNull()) { + if (cursor->image().isNull()) { doHideCursor(); return; } - for (auto it = m_outputs.constBegin(); it != m_outputs.constEnd(); ++it) { - (*it)->updateCursor(); + + bool success = true; + + for (DrmOutput *output : qAsConst(m_outputs)) { + success = output->updateCursor(); + if (!success) { + qCDebug(KWIN_DRM) << "Failed to update cursor on output" << output->name(); + break; + } + success = output->showCursor(); + if (!success) { + qCDebug(KWIN_DRM) << "Failed to show cursor on output" << output->name(); + break; + } + output->moveCursor(); } - setCursor(); - - moveCursor(); + setSoftwareCursor(!success); } void DrmBackend::doShowCursor() { + if (usesSoftwareCursor()) { + return; + } updateCursor(); } @@ -577,6 +578,16 @@ void DrmBackend::doHideCursor() } } +void DrmBackend::doSetSoftwareCursor() +{ + if (isCursorHidden() || !usesSoftwareCursor()) { + return; + } + for (auto it = m_outputs.constBegin(); it != m_outputs.constEnd(); ++it) { + (*it)->hideCursor(); + } +} + void DrmBackend::moveCursor() { if (isCursorHidden() || usesSoftwareCursor()) { diff --git a/plugins/platforms/drm/drm_backend.h b/plugins/platforms/drm/drm_backend.h index 772de609cb..14f0de1db6 100644 --- a/plugins/platforms/drm/drm_backend.h +++ b/plugins/platforms/drm/drm_backend.h @@ -88,6 +88,8 @@ public Q_SLOTS: protected: void doHideCursor() override; void doShowCursor() override; + void doSetSoftwareCursor() override; + private: friend class DrmGpu; void addOutput(DrmOutput* output); @@ -98,7 +100,6 @@ private: void reactivate(); void deactivate(); bool updateOutputs(); - void setCursor(); void updateCursor(); void moveCursor(); void initCursor(); diff --git a/plugins/platforms/drm/drm_gpu.cpp b/plugins/platforms/drm/drm_gpu.cpp index f15b82d9f9..ce82c4f732 100644 --- a/plugins/platforms/drm/drm_gpu.cpp +++ b/plugins/platforms/drm/drm_gpu.cpp @@ -252,7 +252,7 @@ bool DrmGpu::updateOutputs() continue; } if (!output->initCursor(m_cursorSize)) { - m_backend->setSoftwareCursor(true); + m_backend->setSoftwareCursorForced(true); } qCDebug(KWIN_DRM) << "Found new output with uuid" << output->uuid(); diff --git a/plugins/platforms/drm/drm_output.cpp b/plugins/platforms/drm/drm_output.cpp index da83da5661..33f940148f 100644 --- a/plugins/platforms/drm/drm_output.cpp +++ b/plugins/platforms/drm/drm_output.cpp @@ -112,11 +112,6 @@ bool DrmOutput::showCursor() return false; } - if (Q_UNLIKELY(m_backend->usesSoftwareCursor())) { - qCCritical(KWIN_DRM) << "DrmOutput::showCursor should never be called when software cursor is enabled"; - return true; - } - const bool ret = showCursor(m_cursor[m_cursorIndex].data()); if (!ret) { qCDebug(KWIN_DRM) << "DrmOutput::showCursor(DrmDumbBuffer) failed"; @@ -131,25 +126,45 @@ bool DrmOutput::showCursor() return ret; } -void DrmOutput::updateCursor() +static bool isCursorSpriteCompatible(const QImage *buffer, const QImage *sprite) +{ + // Note that we need compare the rects in the device independent pixels because the + // buffer and the cursor sprite image may have different scale factors. + const QRect bufferRect(QPoint(0, 0), buffer->size() / buffer->devicePixelRatio()); + const QRect spriteRect(QPoint(0, 0), sprite->size() / sprite->devicePixelRatio()); + + return bufferRect.contains(spriteRect); +} + +bool DrmOutput::updateCursor() { if (m_deleted) { - return; + return false; } const Cursor *cursor = Cursors::self()->currentCursor(); const QImage cursorImage = cursor->image(); if (cursorImage.isNull()) { - return; + return false; } - m_hasNewCursor = true; + QImage *c = m_cursor[m_cursorIndex]->image(); + c->setDevicePixelRatio(scale()); + + if (!isCursorSpriteCompatible(c, &cursorImage)) { + // If the cursor image is too big, fall back to rendering the software cursor. + return false; + } + + m_hasNewCursor = true; c->fill(Qt::transparent); QPainter p; p.begin(c); - p.setWorldTransform(logicalToNativeMatrix(cursor->rect(), scale(), transform()).toTransform()); + p.setWorldTransform(logicalToNativeMatrix(cursor->rect(), 1, transform()).toTransform()); p.drawImage(QPoint(0, 0), cursorImage); p.end(); + + return true; } void DrmOutput::moveCursor() diff --git a/plugins/platforms/drm/drm_output.h b/plugins/platforms/drm/drm_output.h index 8db75a0c89..db96846699 100644 --- a/plugins/platforms/drm/drm_output.h +++ b/plugins/platforms/drm/drm_output.h @@ -45,7 +45,7 @@ public: bool showCursor(DrmDumbBuffer *buffer); bool showCursor(); bool hideCursor(); - void updateCursor(); + bool updateCursor(); void moveCursor(); bool init(drmModeConnector *connector); bool present(DrmBuffer *buffer); diff --git a/plugins/platforms/fbdev/fb_backend.cpp b/plugins/platforms/fbdev/fb_backend.cpp index c7059b6850..33833b3b22 100644 --- a/plugins/platforms/fbdev/fb_backend.cpp +++ b/plugins/platforms/fbdev/fb_backend.cpp @@ -67,7 +67,7 @@ QPainterBackend *FramebufferBackend::createQPainterBackend() void FramebufferBackend::init() { - setSoftwareCursor(true); + setSoftwareCursorForced(true); LogindIntegration *logind = LogindIntegration::self(); auto takeControl = [logind, this]() { if (logind->hasSessionControl()) { diff --git a/plugins/platforms/virtual/virtual_backend.cpp b/plugins/platforms/virtual/virtual_backend.cpp index 9626acdf60..45696eec3b 100644 --- a/plugins/platforms/virtual/virtual_backend.cpp +++ b/plugins/platforms/virtual/virtual_backend.cpp @@ -59,7 +59,7 @@ void VirtualBackend::init() m_enabledOutputs << dummyOutput ; } - setSoftwareCursor(true); + setSoftwareCursorForced(true); setReady(true); waylandServer()->seat()->setHasPointer(true); waylandServer()->seat()->setHasKeyboard(true);