feat: add cursor size scaling with system scale factor - #36
Merged
Merged
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideImplements cursor size scaling based on the system scale factor by introducing a base cursor size setting, wiring it into the dconfig/xsettings flow, and ensuring gtk cursor theme size updates dynamically when scale or cursor settings change. Sequence diagram for cursor size update on scale factor changesequenceDiagram
actor User
participant DisplaySettingsUI
participant XSettingsManager
participant DConfig
participant GTKEnvironment
User->>DisplaySettingsUI: Change display scale
DisplaySettingsUI->>XSettingsManager: setSingleScaleFactor(scale, emitSignal)
activate XSettingsManager
XSettingsManager->>DConfig: getValue(gtk-cursor-theme-size-base)
DConfig-->>XSettingsManager: baseCursorSizeInt, ok
XSettingsManager->>XSettingsManager: if !ok or baseCursorSizeInt < 0
XSettingsManager->>XSettingsManager: baseCursorSizeInt = BASE_CURSORSIZE
XSettingsManager->>XSettingsManager: cursorSize = int(baseCursorSizeInt * scale)
XSettingsManager->>DConfig: setValue(dcKeyGtkCursorThemeSize, cursorSize)
deactivate XSettingsManager
DConfig-->>GTKEnvironment: gtk-cursor-theme-size changed
GTKEnvironment->>GTKEnvironment: Recalculate and apply cursor size
Sequence diagram for DConfig cursor setting change handlingsequenceDiagram
participant DConfig
participant XSettingsManager
participant XResources
DConfig-->>XSettingsManager: handleDConfigChangedCb(key)
activate XSettingsManager
XSettingsManager->>XSettingsManager: if key in excludedkeys(xft-dpi, scale-factor, window-scale)
XSettingsManager-->>DConfig: return (ignored)
deactivate XSettingsManager
DConfig-->>XSettingsManager: handleDConfigChangedCb(gtk-cursor-theme-name)
activate XSettingsManager
XSettingsManager->>XSettingsManager: key == gtk-cursor-theme-name
XSettingsManager->>XResources: updateXResources()
deactivate XSettingsManager
DConfig-->>XSettingsManager: handleDConfigChangedCb(gtk-cursor-theme-size)
activate XSettingsManager
XSettingsManager->>XSettingsManager: key == gtk-cursor-theme-size
XSettingsManager->>XResources: updateXResources()
deactivate XSettingsManager
DConfig-->>XSettingsManager: handleDConfigChangedCb(gtk-cursor-theme-size-base)
activate XSettingsManager
XSettingsManager->>DConfig: getValue(gtk-cursor-theme-size-base)
DConfig-->>XSettingsManager: cursorSizeBase
XSettingsManager->>DConfig: setValue(gtk-cursor-theme-size, cursorSizeBase)
XSettingsManager-->>DConfig: return (no further processing)
deactivate XSettingsManager
Updated class diagram for XSettingsManager cursor scaling logicclassDiagram
class XSettingsManager {
- DConfigSettings* m_settingDconfig
- DconfInfoStore m_dconfInfos
+ void handleDConfigChangedCb(QString key)
+ void setSingleScaleFactor(double scale, bool emitSignal)
+ void setString(QString prop, QString v)
+ void setGSettingsByXProp(QString prop, XsValue value)
+ void updateXResources()
}
class DConfigSettings {
+ QVariant value(QString key)
+ int value(QString key, bool* ok)
+ void setValue(QString key, QVariant value)
}
class DconfInfoStore {
+ QSharedPointer~DconfInfo~ getByDconfKey(QString key)
}
class DconfInfo {
+ QString dconfKey
+ QString xsettingsKey
}
XSettingsManager --> DConfigSettings : uses m_settingDconfig
XSettingsManager --> DconfInfoStore : uses m_dconfInfos
DconfInfoStore --> DconfInfo : returns
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- When handling
gtk-cursor-theme-size-baseinhandleDConfigChangedCb, you updategtk-cursor-theme-sizebut don’t callupdateXResources(), which may prevent the cursor size change from taking effect immediately; consider triggering the same refresh path as forgtk-cursor-theme-sizeupdates. - In
setSingleScaleFactor, the validationif (!ok || baseCursorSizeInt < 0)allows a base cursor size of 0, which will result in a cursor size of 0; consider treating non-positive values as invalid (e.g.<= 0) to fall back toBASE_CURSORSIZE.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When handling `gtk-cursor-theme-size-base` in `handleDConfigChangedCb`, you update `gtk-cursor-theme-size` but don’t call `updateXResources()`, which may prevent the cursor size change from taking effect immediately; consider triggering the same refresh path as for `gtk-cursor-theme-size` updates.
- In `setSingleScaleFactor`, the validation `if (!ok || baseCursorSizeInt < 0)` allows a base cursor size of 0, which will result in a cursor size of 0; consider treating non-positive values as invalid (e.g. `<= 0`) to fall back to `BASE_CURSORSIZE`.
## Individual Comments
### Comment 1
<location> `src/plugin-qt/xsettings/impl/xsettingsmanager.cpp:530-535` </location>
<code_context>
m_settingDconfig->setValue(dcKeyWindowScale, windowScale);
}
-
+ bool ok = false;
+ int baseCursorSizeInt = m_settingDconfig->value("gtk-cursor-theme-size-base").toInt(&ok);
+ if (!ok || baseCursorSizeInt < 0) {
+ baseCursorSizeInt = BASE_CURSORSIZE;
+ }
+ int cursorSize = static_cast<int>(baseCursorSizeInt * scale);
m_settingDconfig->setValue(dcKeyGtkCursorThemeSize, cursorSize);
}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Allowing a base cursor size of 0 may unintentionally produce a zero-size cursor.
Because `baseCursorSizeInt == 0` passes the current check, `cursorSize` can be computed as 0 for normal `scale` values, which is likely undesirable and may break cursor visibility/layout. If `0` is not intended to be valid, treat it as invalid in the condition (e.g., `<= 0`) or otherwise explicitly guard against `0` before computing `cursorSize`.
```suggestion
bool ok = false;
int baseCursorSizeInt = m_settingDconfig->value("gtk-cursor-theme-size-base").toInt(&ok);
// Treat non-positive values (including 0) as invalid to avoid producing an invisible cursor.
if (!ok || baseCursorSizeInt <= 0) {
baseCursorSizeInt = BASE_CURSORSIZE;
}
int cursorSize = static_cast<int>(baseCursorSizeInt * scale);
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Added support for cursor size to scale proportionally with system scaling factor Introduced a new base cursor size configuration item "gtk-cursor-theme- size-base" Removed "gtk-cursor-theme-size" from excluded keys to allow dynamic updates Cursor size is now calculated as base size multiplied by system scale factor This ensures cursor size remains appropriately sized across different display scales Log: Added cursor size scaling feature that adjusts cursor size based on system scaling factor Influence: 1. Test cursor size changes when adjusting system scaling factor 2. Verify base cursor size configuration works correctly 3. Check cursor size updates in real-time when scaling factor changes 4. Test compatibility with existing cursor theme settings 5. Verify cursor size calculation accuracy across different scaling factors feat: 增加光标大小随系统缩放比例调整功能 添加了光标大小随系统缩放比例按比例调整的支持 引入了新的基础光标大小配置项"gtk-cursor-theme-size-base" 将"gtk-cursor-theme-size"从排除列表中移除以支持动态更新 光标大小现在计算为基础大小乘以系统缩放比例 确保光标在不同显示比例下保持合适的大小 Log: 新增光标大小缩放功能，根据系统缩放比例调整光标大小 Influence: 1. 测试调整系统缩放比例时光标大小的变化 2. 验证基础光标大小配置是否正确工作 3. 检查缩放比例改变时光标大小的实时更新 4. 测试与现有光标主题设置的兼容性 5. 验证不同缩放比例下光标大小计算的准确性 PMS: STORY-39797
deepin pr auto review我来对这个代码变更进行详细审查：
b) 常量定义：
static const QString DCONF_KEY_CURSOR_SIZE_BASE = "gtk-cursor-theme-size-base";
static const QSet<QString> excludedKeys = {"xft-dpi", "scale-factor", "window-scale"};
if (excludedKeys.contains(key)) {
return;
}
if (scale <= 0 || scale > 10) { // 设置合理的缩放范围
return;
}b) 数据验证：
if (!ok || baseCursorSizeInt <= 0 || baseCursorSizeInt > 128) { // 设置合理的上限
baseCursorSizeInt = BASE_CURSORSIZE;
}
b) 代码注释：
// 这些键值需要特殊处理，因为它们会影响系统UI的基本显示
static const QStringList excludedKeys = {"xft-dpi", "scale-factor", "window-scale"};
这些改进建议主要关注代码的可维护性、性能和安全性。整体来说，这个变更实现了光标大小的基础值和实际值的分离，是一个合理的设计改进。 |
mhduiy
approved these changes
Dec 4, 2025
Contributor
|
/forcemerge |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: caixr23, mhduiy The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
This pr force merged! (status: blocked) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added support for cursor size to scale proportionally with system scaling factor
Introduced a new base cursor size configuration item "gtk-cursor-theme- size-base"
Removed "gtk-cursor-theme-size" from excluded keys to allow dynamic updates
Cursor size is now calculated as base size multiplied by system scale factor
This ensures cursor size remains appropriately sized across different display scales
Log: Added cursor size scaling feature that adjusts cursor size based on system scaling factor
Influence:
feat: 增加光标大小随系统缩放比例调整功能
添加了光标大小随系统缩放比例按比例调整的支持
引入了新的基础光标大小配置项"gtk-cursor-theme-size-base"
将"gtk-cursor-theme-size"从排除列表中移除以支持动态更新
光标大小现在计算为基础大小乘以系统缩放比例
确保光标在不同显示比例下保持合适的大小
Log: 新增光标大小缩放功能，根据系统缩放比例调整光标大小
Influence:
PMS: STORY-39797
Summary by Sourcery
Adjust cursor handling to support scaling cursor size based on a configurable base size and the system scale factor.
New Features:
Enhancements: