Skip to content

sync: from linuxdeepin/dde-session-shell #489

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yixinshark merged 1 commit into from
Feb 26, 2026
Merged

sync: from linuxdeepin/dde-session-shell #489

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 57 additions & 2 deletions src/global_util/multiscreenmanager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,16 @@

#include <QApplication>
#include <QDebug>
#include <QJsonDocument>

Check warning on line 11 in src/global_util/multiscreenmanager.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <QJsonDocument> not found. Please note: Cppcheck does not need standard library headers to get proper results.
#include <QJsonObject>

Check warning on line 12 in src/global_util/multiscreenmanager.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <QJsonObject> not found. Please note: Cppcheck does not need standard library headers to get proper results.

#ifndef ENABLE_DSS_SNIPE
#include <QX11Info>

Check warning on line 15 in src/global_util/multiscreenmanager.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <QX11Info> not found. Please note: Cppcheck does not need standard library headers to get proper results.
#else
#include <xcb/xproto.h>

Check warning on line 17 in src/global_util/multiscreenmanager.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <xcb/xproto.h> not found. Please note: Cppcheck does not need standard library headers to get proper results.
#endif
#include <xcb/xcb.h>

Check warning on line 19 in src/global_util/multiscreenmanager.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <xcb/xcb.h> not found. Please note: Cppcheck does not need standard library headers to get proper results.

#include "dbusconstant.h"

MultiScreenManager::MultiScreenManager(QObject *parent)
Expand Down Expand Up @@ -114,6 +121,11 @@
return;
}

qCInfo(DDE_SHELL) << "onScreenAdded processing, screen:" << screen
<< ", name:" << screen->name()
<< ", geometry:" << screen->geometry()
<< ", existing frames count:" << m_frames.size();

QWidget* w = nullptr;
if (m_isCopyMode) {
// 如果m_frames不为空则直接退出
Expand Down Expand Up @@ -244,10 +256,53 @@

void MultiScreenManager::checkLockFrameLocation()
Copy link

@sourcery-ai sourcery-ai bot Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider extracting the XCB connection retrieval and geometry-checking logic into reusable helper functions so checkLockFrameLocation stays focused on high-level behavior.

You can keep all the new checks/logging but reduce complexity by extracting the XCB-specific and geometry logic into small helpers, and centralizing XCB connection acquisition (also avoids duplication with fullscreenbackground.cpp).

1. Centralize XCB connection retrieval

Create a small helper (in a shared .cpp or anonymous namespace) and use it both here and in fullscreenbackground.cpp:

// e.g. in a common helper .cpp, or at top of this .cpp in an anonymous namespace
static xcb_connection_t *getXcbConnection()
{
    if (QGuiApplication::platformName().startsWith("wayland", Qt::CaseInsensitive))
        return nullptr;

#ifndef ENABLE_DSS_SNIPE
    return QX11Info::connection();
#else
    auto *x11App = qGuiApp->nativeInterface<QNativeInterface::QX11Application>();
    return x11App ? x11App->connection() : nullptr;
#endif
}

Then checkLockFrameLocation becomes simpler:

void MultiScreenManager::checkLockFrameLocation()
{
    xcb_connection_t *connection = getXcbConnection();

    for (QScreen *screen : m_frames.keys()) {
        if (!screen)
            continue;

        QWidget *frame = m_frames.value(screen);
        qCInfo(DDE_SHELL) << "Check lock frame location, screen:" << screen
                          << ", screen name:" << screen->name()
                          << ", screen geometry:" << screen->geometry()
                          << ", lockframe:" << frame
                          << ", Qt frame geometry:" << frame->geometry();

        if (connection && frame)
            checkAndLogXcbGeometry(connection, screen, frame);
    }
}

2. Extract geometry calculation + logging

Move the DPR/geometry math and logging into a focused helper:

static void checkAndLogXcbGeometry(xcb_connection_t *connection, QScreen *screen, QWidget *frame)
{
    auto cookie = xcb_get_geometry(connection, static_cast<xcb_window_t>(frame->winId()));
    auto *reply = xcb_get_geometry_reply(connection, cookie, nullptr);
    if (!reply) {
        qCWarning(DDE_SHELL) << "  Failed to get XCB geometry for frame:" << frame;
        return;
    }

    QRect xcbGeometry(reply->x, reply->y, reply->width, reply->height);
    free(reply);

    const qreal dpr = frame->devicePixelRatioF();
    const QRect screenGeo = screen->geometry();
    QRect expectedPhysical(screenGeo.x(), screenGeo.y(),
                           qRound(screenGeo.width() * dpr),
                           qRound(screenGeo.height() * dpr));

    const bool positionMatch = (xcbGeometry == expectedPhysical);

    qCInfo(DDE_SHELL) << "  XCB actual geometry(physical):" << xcbGeometry
                      << ", expected(physical):" << expectedPhysical
                      << ", dpr:" << dpr
                      << ", match:" << positionMatch;

    if (!positionMatch) {
        qCWarning(DDE_SHELL) << "  *** POSITION MISMATCH DETECTED! ***"
                             << "XCB geometry:" << xcbGeometry
                             << "expected:" << expectedPhysical
                             << "for screen" << screen->name();
    }
}

This keeps all current behavior and logging, but makes checkLockFrameLocation read as high-level intent while encapsulating platform-specific and geometry details.

{
xcb_connection_t *connection = nullptr;
if (!QGuiApplication::platformName().startsWith("wayland", Qt::CaseInsensitive)) {
#ifndef ENABLE_DSS_SNIPE
connection = QX11Info::connection();
#else
auto *x11App = qGuiApp->nativeInterface<QNativeInterface::QX11Application>();
if (x11App)
connection = x11App->connection();
#endif
}

for (QScreen *screen : m_frames.keys()) {
if (screen) {
qCInfo(DDE_SHELL) << "Check lock frame location, screen:" << screen << ", location:" << screen->geometry()
<< ", lockframe:" << m_frames.value(screen) << ", location:" << m_frames.value(screen)->geometry();
QWidget *frame = m_frames.value(screen);
Comment on lines 271 to +272
Copy link

@sourcery-ai sourcery-ai bot Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Check that the frame pointer is non-null before dereferencing it.

In this loop you only check screen, but then dereference frame (e.g. frame->geometry()) without ensuring it’s non-null. If m_frames can contain nullptr, this can crash. Please add a null check for frame (e.g. if (!frame) continue; or if (screen && frame) before use).

qCInfo(DDE_SHELL) << "Check lock frame location, screen:" << screen
<< ", screen name:" << screen->name()
<< ", screen geometry:" << screen->geometry()
<< ", lockframe:" << frame
<< ", Qt frame geometry:" << frame->geometry();

// 通过 XCB 检查窗口在 X Server 中的实际位置
// 注意：X11 下窗口位置不乘 DPR（与 RandR 物理坐标一致），窗口大小乘 DPR
if (connection && frame) {
auto cookie = xcb_get_geometry(connection, static_cast<xcb_window_t>(frame->winId()));
auto *reply = xcb_get_geometry_reply(connection, cookie, nullptr);
if (reply) {
QRect xcbGeometry(reply->x, reply->y, reply->width, reply->height);
const qreal dpr = frame->devicePixelRatioF();
QRect expectedPhysical(screen->geometry().x(), screen->geometry().y(),
qRound(screen->geometry().width() * dpr),
qRound(screen->geometry().height() * dpr));
bool positionMatch = (xcbGeometry == expectedPhysical);
qCInfo(DDE_SHELL) << " XCB actual geometry(physical):" << xcbGeometry
<< ", expected(physical):" << expectedPhysical
<< ", dpr:" << dpr
<< ", match:" << positionMatch;
if (!positionMatch) {
qCWarning(DDE_SHELL) << " *** POSITION MISMATCH DETECTED! ***"
<< "XCB geometry:" << xcbGeometry
<< "expected:" << expectedPhysical
<< "for screen" << screen->name();
}
free(reply);
} else {
qCWarning(DDE_SHELL) << " Failed to get XCB geometry for frame:" << frame;
}
}
}
}
}
Expand Down
69 changes: 66 additions & 3 deletions src/widgets/fullscreenbackground.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,17 @@
#include <QKeyEvent>
#include <QPainter>
#include <QScreen>
#include <QTimer>

Check warning on line 21 in src/widgets/fullscreenbackground.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <QTimer> not found. Please note: Cppcheck does not need standard library headers to get proper results.
#include <QWindow>

Check warning on line 22 in src/widgets/fullscreenbackground.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <QWindow> not found. Please note: Cppcheck does not need standard library headers to get proper results.

#ifndef ENABLE_DSS_SNIPE
#include <QX11Info>

Check warning on line 25 in src/widgets/fullscreenbackground.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <QX11Info> not found. Please note: Cppcheck does not need standard library headers to get proper results.
#else
#include <xcb/xproto.h>
#endif
#include <xcb/xcb.h>

Check warning on line 29 in src/widgets/fullscreenbackground.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <xcb/xcb.h> not found. Please note: Cppcheck does not need standard library headers to get proper results.

#include "dbusconstant.h"

Check warning on line 31 in src/widgets/fullscreenbackground.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: "dbusconstant.h" not found.

Q_LOGGING_CATEGORY(DDE_SS, "dss.active")

Expand Down Expand Up @@ -74,9 +81,53 @@
connect(m_resetGeometryTimer, &QTimer::timeout, this, [this] {
const auto &currentGeometry = geometry();
if (currentGeometry != m_geometryRect) {
qCDebug(DDE_SHELL) << "Current geometry:" << currentGeometry <<"setGeometry:" << m_geometryRect;
qCWarning(DDE_SHELL) << "Geometry mismatch detected! Qt cached geometry:" << currentGeometry
<< ", target geometry:" << m_geometryRect << ", this:" << this;
setGeometry(m_geometryRect);
}

// 通过 XCB 检查窗口在 X Server 中的实际位置
// 注意：X11 坐标系中，窗口位置不乘 DPR（与 X RandR 一致），窗口大小乘 DPR
if (!m_model->isUseWayland() && windowHandle()) {
Copy link

@sourcery-ai sourcery-ai bot Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider extracting the X11/XCB connection and geometry reconciliation logic into helper functions and centralizing the repeated logging to keep the timer callback and geometry-related methods simpler and easier to read.

You can keep the new behavior but reduce complexity by extracting the XCB/X11 handling and geometry reconciliation into small helpers, and by centralizing the verbose logging.

1. Extract X11 connection acquisition

The timer lambda and possibly other files duplicate the #ifndef ENABLE_DSS_SNIPE + QX11Info vs QNativeInterface::QX11Application logic. Move that into a small helper:

// In a suitable .cpp/.h (e.g. fullscreenbackground.cpp or a small X11 helper)
static xcb_connection_t *x11Connection()
{
    xcb_connection_t *connection = nullptr;
#ifndef ENABLE_DSS_SNIPE
    connection = QX11Info::connection();
#else
    auto *x11App = qGuiApp->nativeInterface<QNativeInterface::QX11Application>();
    if (x11App)
        connection = x11App->connection();
#endif
    return connection;
}

Then use it in the timer:

if (!m_model->isUseWayland() && windowHandle()) {
    if (auto *connection = x11Connection()) {
        auto cookie = xcb_get_geometry(connection, static_cast<xcb_window_t>(winId()));
        auto *reply = xcb_get_geometry_reply(connection, cookie, nullptr);
        // ...
    }
}

This removes preprocessor noise from the lambda and avoids duplication in other places.

2. Extract XCB geometry reconciliation

The DPR handling, XCB get_geometry, and resize-to-0 workaround can be moved out of the lambda into a dedicated method. That keeps the timer callback focused on “check geometry + fix if needed”.

// In FullScreenBackground
void FullScreenBackground::ensureXcbGeometryMatches()
{
    if (m_model->isUseWayland() || !windowHandle())
        return;

    xcb_connection_t *connection = x11Connection();
    if (!connection)
        return;

    auto cookie = xcb_get_geometry(connection, static_cast<xcb_window_t>(winId()));
    auto *reply = xcb_get_geometry_reply(connection, cookie, nullptr);
    if (!reply)
        return;

    const QRect xcbGeometry(reply->x, reply->y, reply->width, reply->height);
    free(reply);

    const qreal dpr = devicePixelRatioF();
    const QRect expectedPhysical(
        m_geometryRect.x(),
        m_geometryRect.y(),
        qRound(m_geometryRect.width() * dpr),
        qRound(m_geometryRect.height() * dpr));

    if (xcbGeometry == expectedPhysical) {
        qCInfo(DDE_SHELL) << "XCB position match, no need to set geometry";
        return;
    }

    qCWarning(DDE_SHELL) << "XCB position mismatch! XCB geometry(physical):" << xcbGeometry
                         << ", expected(physical):" << expectedPhysical
                         << ", target(logical):" << m_geometryRect
                         << ", dpr:" << dpr
                         << ", this:" << this
                         << ", screen:" << (m_screen ? m_screen->name() : "null");

    if (!m_screen.isNull() && windowHandle()->screen() != m_screen)
        windowHandle()->setScreen(m_screen);

    setGeometry(0, 0, 0, 0);
    setGeometry(m_geometryRect);
}

Timer becomes much clearer:

connect(m_resetGeometryTimer, &QTimer::timeout, this, [this] {
    const auto &currentGeometry = geometry();
    if (currentGeometry != m_geometryRect) {
        qCWarning(DDE_SHELL) << "Geometry mismatch detected! Qt cached geometry:" << currentGeometry
                             << ", target geometry:" << m_geometryRect << ", this:" << this;
        setGeometry(m_geometryRect);
    }

    ensureXcbGeometryMatches();
});

3. Centralize repetitive logging

setddeGeometry, updateGeometry, and the timer lambda all log similar information about the frame, screen, and window handle. You can reduce repetition and visual noise by introducing a small logging helper:

void FullScreenBackground::logGeometryInfo(const char *prefix, const QRect &target) const
{
    qCInfo(DDE_SHELL) << prefix
                      << "this:" << this
                      << "target:" << target
                      << "current geometry:" << geometry()
                      << "screen:" << (m_screen ? m_screen->name() : "null")
                      << "windowHandle screen:" << (windowHandle() ? windowHandle()->screen()->name() : "null")
                      << "dpr:" << devicePixelRatioF();
}

Use it where logging is currently verbose:

void FullScreenBackground::setddeGeometry(const QRect &rect)
{
    logGeometryInfo("setddeGeometry", rect);
    setGeometry(rect);
    m_geometryRect = rect;
    m_resetGeometryTimer->start(200);
    QTimer::singleShot(400 * 5, m_resetGeometryTimer, &QTimer::stop);
}

And in updateGeometry / timer as needed, with short prefixes. This keeps all current information but makes the control flow easier to read and maintain.

xcb_connection_t *connection = nullptr;
#ifndef ENABLE_DSS_SNIPE
connection = QX11Info::connection();
#else
auto *x11App = qGuiApp->nativeInterface<QNativeInterface::QX11Application>();
if (x11App)
connection = x11App->connection();
#endif
if (connection) {
auto cookie = xcb_get_geometry(connection, static_cast<xcb_window_t>(winId()));
auto *reply = xcb_get_geometry_reply(connection, cookie, nullptr);
if (reply) {
QRect xcbGeometry(reply->x, reply->y, reply->width, reply->height);
const qreal dpr = devicePixelRatioF();
QRect expectedPhysical(m_geometryRect.x(), m_geometryRect.y(),
qRound(m_geometryRect.width() * dpr),
qRound(m_geometryRect.height() * dpr));
if (xcbGeometry != expectedPhysical) {
qCWarning(DDE_SHELL) << "XCB position mismatch! XCB geometry(physical):" << xcbGeometry
<< ", expected(physical):" << expectedPhysical
<< ", target(logical):" << m_geometryRect
<< ", dpr:" << dpr
<< ", this:" << this
<< ", screen:" << (m_screen ? m_screen->name() : "null");
// Qt 的 geometry() 缓存可能已经是正确值，但实际 X 窗口未移动。
// 强制重新绑定 screen 并调用 setGeometry，让 Qt 重新发送 XCB configure request。
if (!m_screen.isNull() && windowHandle()->screen() != m_screen) {
windowHandle()->setScreen(m_screen);
}
// 先 resize 到 0,0 再设目标值，破坏 Qt 的 "geometry 未变" 优化，强制重新下发
setGeometry(0, 0, 0, 0);
setGeometry(m_geometryRect);
} else {
qCInfo(DDE_SHELL) << "XCB position match, no need to set geometry";
}
free(reply);
}
}
}
});

connect(m_model, &SessionBaseModel::shutdownkModeChanged, this, [this] (bool value){
Expand Down Expand Up @@ -453,14 +504,20 @@
}

if (!m_screen.isNull()) {
if(m_model->isUseWayland())
// X11 下也需要将 QWindow 关联到正确的 QScreen，否则 Qt 可能将窗口归属到错误的 screen
if (windowHandle() && windowHandle()->screen() != m_screen) {
qCInfo(DDE_SHELL) << "bindWindowToScreen, windowHandle screen:" << windowHandle()->screen()->name()
Comment on lines +508 to +509
Copy link

@sourcery-ai sourcery-ai bot Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Guard against null QScreen when logging windowHandle()->screen()->name().

windowHandle()->screen() may be null (e.g. during screen changes or early initialization), so calling .name() directly risks a null dereference in this logging path. Capture the screen in a local pointer and check it before accessing name(), for example:

if (auto *win = windowHandle()) {
    QScreen *curScreen = win->screen();
    qCInfo(DDE_SHELL) << "bindWindowToScreen, windowHandle screen:"
                      << (curScreen ? curScreen->name() : "null")
                      << ", target screen:" << m_screen->name() << ", this:" << this;
    if (curScreen != m_screen)
        win->setScreen(m_screen);
}

<< ", target screen:" << m_screen->name() << ", this:" << this;
windowHandle()->setScreen(m_screen);
}
setddeGeometry(m_screen->geometry());

qCInfo(DDE_SHELL) << "Update geometry, screen:" << m_screen
<< ", screen name:" << m_screen->name()
<< ", screen geometry:" << m_screen->geometry()
<< ", lockFrame:" << this
<< ", frame geometry:" << this->geometry();
<< ", frame geometry:" << this->geometry()
<< ", windowHandle screen:" << (windowHandle() ? windowHandle()->screen()->name() : "null");
} else {
qCWarning(DDE_SHELL) << "Screen is nullptr";
}
Expand Down Expand Up @@ -735,6 +792,12 @@
//增加一个定时器，每隔50ms再设置一次Geometry，避免出现xorg初始化未完成的情况，导致界面显示不全
void FullScreenBackground::setddeGeometry(const QRect &rect)
{
qCInfo(DDE_SHELL) << "setddeGeometry called, this:" << this
<< ", target rect:" << rect
<< ", current geometry:" << geometry()
<< ", screen:" << (m_screen ? m_screen->name() : "null")
<< ", windowHandle screen:" << (windowHandle() ? windowHandle()->screen()->name() : "null")
Copy link

@sourcery-ai sourcery-ai bot Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Also guard windowHandle()->screen() in logging to prevent crashes when screen is null.

The current ternary only guards windowHandle(), not windowHandle()->screen(). If a window exists but has no screen yet, screen()->name() will dereference null and crash in this logging code. Please also guard the screen pointer, e.g. via a local QScreen *winScreen and logging winScreen ? winScreen->name() : "null".

<< ", dpr:" << devicePixelRatioF();
setGeometry(rect);
m_geometryRect = rect;
m_resetGeometryTimer->start(200);
Expand Down
Loading