Skip to content

feat: add xsettings plugin implementation - #35

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
caixr23:master
Dec 4, 2025
Merged

deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
caixr23:master

Conversation

@caixr23

@caixr23 caixr23 commented Dec 3, 2025

Copy link
Copy Markdown
Contributor
  1. Added new xsettings plugin module for deepin-service-manager
  2. Implemented XSettings protocol support with DConfig integration
  3. Added XCB utilities for X11 property management
  4. Implemented scale factor management and DPI calculation
  5. Added Firefox DPI configuration support
  6. Included Plymouth theme scaling support
  7. Added comprehensive configuration file handling

Log: Added XSettings plugin for system configuration management

Influence:

  1. Test XSettings property retrieval and modification via D-Bus
  2. Verify scale factor changes affect system DPI and cursor size
  3. Check Firefox DPI configuration updates
  4. Test Plymouth theme scaling during boot
  5. Verify XResources updates for cursor theme and DPI settings
  6. Test individual screen scaling factors configuration
  7. Validate configuration file persistence and error handling

feat: 添加 xsettings 插件实现

  1. 新增 deepin-service-manager 的 xsettings 插件模块
  2. 实现 XSettings 协议支持并与 DConfig 集成
  3. 添加 XCB 工具类用于 X11 属性管理
  4. 实现缩放因子管理和 DPI 计算
  5. 添加 Firefox DPI 配置支持
  6. 包含 Plymouth 主题缩放支持
  7. 添加完整的配置文件处理功能

Log: 新增系统配置管理的 XSettings 插件

Influence:

  1. 测试通过 D-Bus 获取和修改 XSettings 属性
  2. 验证缩放因子变化对系统 DPI 和光标大小的影响
  3. 检查 Firefox DPI 配置更新
  4. 测试启动时 Plymouth 主题缩放效果
  5. 验证光标主题和 DPI 设置的 XResources 更新
  6. 测试独立屏幕缩放因子配置
  7. 验证配置文件持久化和错误处理

PMS: TASK-381269

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @caixr23, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@caixr23
caixr23 force-pushed the master branch 4 times, most recently from 30890ee to b2924f6 Compare December 4, 2025 07:23
1. Added new xsettings plugin module for deepin-service-manager
2. Implemented XSettings protocol support with DConfig integration
3. Added XCB utilities for X11 property management
4. Implemented scale factor management and DPI calculation
5. Added Firefox DPI configuration support
6. Included Plymouth theme scaling support
7. Added comprehensive configuration file handling

Log: Added XSettings plugin for system configuration management

Influence:
1. Test XSettings property retrieval and modification via D-Bus
2. Verify scale factor changes affect system DPI and cursor size
3. Check Firefox DPI configuration updates
4. Test Plymouth theme scaling during boot
5. Verify XResources updates for cursor theme and DPI settings
6. Test individual screen scaling factors configuration
7. Validate configuration file persistence and error handling

feat: 添加 xsettings 插件实现

1. 新增 deepin-service-manager 的 xsettings 插件模块
2. 实现 XSettings 协议支持并与 DConfig 集成
3. 添加 XCB 工具类用于 X11 属性管理
4. 实现缩放因子管理和 DPI 计算
5. 添加 Firefox DPI 配置支持
6. 包含 Plymouth 主题缩放支持
7. 添加完整的配置文件处理功能

Log: 新增系统配置管理的 XSettings 插件

Influence:
1. 测试通过 D-Bus 获取和修改 XSettings 属性
2. 验证缩放因子变化对系统 DPI 和光标大小的影响
3. 检查 Firefox DPI 配置更新
4. 测试启动时 Plymouth 主题缩放效果
5. 验证光标主题和 DPI 设置的 XResources 更新
6. 测试独立屏幕缩放因子配置
7. 验证配置文件持久化和错误处理

PMS: TASK-381269
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

我来对这个代码变更进行审查。这是一个新的xsettings插件的实现,主要包含配置管理、X11设置处理、缩放因子管理等功能。我将从多个角度进行分析:

  1. 代码结构和组织

  2. 内存管理

  3. 错误处理

  4. 线程安全

  5. 性能考虑

  6. 安全性

  7. 代码结构和组织:

  • 优点:

    • 代码模块化良好,职责划分清晰
    • 使用了智能指针管理对象生命周期
    • 配置文件和代码分离
  • 改进建议:

    • dconfinfos.cpp中的构造函数初始化列表过长,建议拆分成多个初始化函数
    • 一些常量定义散布在不同文件中,建议集中到一个配置文件中
  1. 内存管理:
  • 优点:

    • 使用QSharedPointer管理对象生命周期
    • RAII原则应用得当
  • 潜在问题:

    • xsettingsmanager.cpp中的QThreadPool::globalInstance()->start()没有等待线程完成,可能在程序退出时造成问题
    • XcbUtils中的单例实现没有考虑线程安全
  1. 错误处理:
  • 优点:

    • 大部分函数都有错误检查
    • 使用了Qt的错误处理机制
  • 改进建议:

    • 一些错误处理过于简单,如xcbutils.cpp中的错误处理只是打印警告
    • 建议增加更详细的错误日志,便于调试
  1. 线程安全:
  • 问题:
    • XcbUtils单例不是线程安全的
    • DconfInfos中的数据访问没有加锁保护
    • updateFirefoxDPI()在单独线程中执行,但没有同步机制
  1. 性能考虑:
  • 优点:

    • 使用了缓存机制
    • 合理使用了Qt的信号槽机制
  • 改进建议:

    • dconfinfos.cpp中的线性查找可以优化为使用QMap
    • 文件读写操作可以考虑使用异步IO
  1. 安全性:
  • 优点:

    • 使用了Qt的类型系统
    • 对输入数据进行了验证
  • 改进建议:

    • 文件操作需要增加权限检查
    • DBus接口需要增加权限验证
    • 配置文件解析需要增加格式验证

具体改进建议:

  1. 修复线程安全问题:
// XcbUtils.h
class XcbUtils {
private:
    static std::mutex instanceMutex;
    static XcbUtils* instance;
public:
    static XcbUtils& getInstance() {
        std::lock_guard<std::mutex> lock(instanceMutex);
        if (!instance) {
            instance = new XcbUtils();
        }
        return *instance;
    }
};
  1. 优化DconfInfos的查找:
class DconfInfos {
private:
    QMap<QString, QSharedPointer<DconfInfo>> dconfKeyMap;
    QMap<QString, QSharedPointer<DconfInfo>> xsKeyMap;
public:
    QSharedPointer<DconfInfo> getByDconfKey(const QString &dconfKey) {
        return dconfKeyMap.value(dconfKey);
    }
};
  1. 增加错误处理:
bool XSettingsManager::updateFirefoxDPI() {
    try {
        QString homeDir = Utils::getUserHomeDir();
        if (homeDir.isEmpty()) {
            qWarning() << "Failed to get user home directory";
            return false;
        }
        // ... 其他代码
    } catch (const std::exception& e) {
        qCritical() << "Error updating Firefox DPI:" << e.what();
        return false;
    }
    return true;
}
  1. 增加权限检查:
bool XSettingsManager::setString(const QString &prop, const QString &v) {
    if (!hasWritePermission()) {
        qWarning() << "No write permission for property:" << prop;
        return false;
    }
    // ... 其他代码
}
  1. 优化文件操作:
void XSettingsManager::updateGreeterQtTheme(KeyFile &keyFile) {
    QFile file("/tmp/startdde-qt-theme-");
    if (!file.open(QIODevice::ReadWrite)) {
        qWarning() << "Failed to open temporary file:" << file.errorString();
        return;
    }
    
    // 使用QSaveFile确保原子性写入
    QSaveFile saveFile(file.fileName());
    if (!saveFile.open(QIODevice::WriteOnly)) {
        qWarning() << "Failed to open save file:" << saveFile.errorString();
        return;
    }
    
    // ... 写入操作
    if (!saveFile.commit()) {
        qWarning() << "Failed to save file:" << saveFile.errorString();
        return;
    }
}
  1. 增加配置验证:
bool XSettingsManager::validateConfig(const QString& configPath) {
    QFileInfo info(configPath);
    if (!info.exists() || !info.isReadable()) {
        qWarning() << "Config file is not accessible:" << configPath;
        return false;
    }
    
    if (info.size() > MAX_CONFIG_SIZE) {
        qWarning() << "Config file too large:" << configPath;
        return false;
    }
    
    return true;
}
  1. 优化资源管理:
class XSettingsManager {
private:
    QThreadPool workerPool;
public:
    XSettingsManager(QObject *parent) 
        : QObject(parent)
        , workerPool(this) // 设置父对象,确保自动清理
    {
        workerPool.setMaxThreadCount(1);
        // ... 其他初始化代码
    }
    
    ~XSettingsManager() {
        workerPool.waitForDone(5000); // 等待任务完成
    }
};
  1. 增加日志记录:
class XSettingsManager {
private:
    void logOperation(const QString& operation, const QString& details) {
        QLoggingCategory category("xsettings");
        qCDebug(category) << operation << ":" << details;
    }
};

这些改进建议主要关注代码的健壮性、安全性和性能。建议分阶段实施这些改进,优先处理线程安全和错误处理相关的问题。

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: caixr23, fly602

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@caixr23

caixr23 commented Dec 4, 2025

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Dec 4, 2025

Copy link
Copy Markdown

This pr force merged! (status: blocked)

@deepin-bot
deepin-bot Bot merged commit 85402be into linuxdeepin:master Dec 4, 2025
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants