Skip to content

Conversation

@wineee
Copy link
Member

@wineee wineee commented Apr 23, 2025

Log: xsettings daemon not setup in time, may not get correct Xft.dpi, add listener to it

close: #352

Log: xsettings daemon not setup in time, may not get correct `Xft.dpi`, add listener to it
@deepin-ci-robot
Copy link
Contributor

deepin pr auto review

代码审查意见:

  1. main_x11.cpp文件中,新增的#if (QT_VERSION >= QT_VERSION_CHECK(6, 0, 0))条件编译指令用于检查Qt版本。这是一个好的做法,可以确保代码在不同版本的Qt上都能正确编译。但是,建议在注释中说明为什么需要这个条件编译,以及这个改动对Qt 6版本的影响。

  2. windowstyle/windowstylemanager.cpp文件中,对m_scale变量的赋值进行了修改,增加了对QScreen::logicalDotsPerInchChanged信号的连接。这是一个好的做法,可以动态调整缩放比例以适应不同屏幕的DPI。但是,这里存在一个问题:在信号槽函数中,QGuiApplication::primaryScreen()被多次调用,这可能会导致性能问题。建议将QGuiApplication::primaryScreen()的结果存储在一个局部变量中,以避免重复调用。

  3. windowstyle/windowstylemanager.h文件中,新增了一个信号sigScaleChanged()。这是一个好的做法,可以通知其他对象缩放比例发生了变化。但是,建议在信号注释中说明这个信号的具体用途,以及它如何影响其他部分的代码。

  4. windowstyle/windowstylemanager.cpp文件中,新增的信号槽连接使用了Lambda表达式。这是一个好的做法,可以避免在信号槽函数中直接访问类的成员变量。但是,Lambda表达式中的this指针可能会导致误解,因为它指向的是Lambda表达式创建时的上下文对象,而不是WindowStyleManager对象。建议在Lambda表达式中使用[this]捕获列表,以确保this指针指向正确的对象。

  5. windowstyle/windowstylemanager.cpp文件中,注释中提到了xsettings daemon,但没有提供足够的上下文信息。建议在注释中说明xsettings daemon的作用,以及为什么需要监听它的DPI设置。

总体来说,代码改动是合理的,但需要注意性能和代码可读性方面的问题。

@deepin-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: justforlxz, wineee

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

@justforlxz justforlxz merged commit 7b760da into linuxdeepin:master Apr 23, 2025
6 of 9 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