feat: sync Xresouces and xsettings scale with treeland scale changes#622
Merged
zccrs merged 1 commit intolinuxdeepin:masterfrom Dec 2, 2025
Merged
feat: sync Xresouces and xsettings scale with treeland scale changes#622zccrs merged 1 commit intolinuxdeepin:masterfrom
zccrs merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's GuideThis PR introduces a full XSettings/XResource framework and SettingManager to propagate DPI and theme changes to X11 clients dynamically, integrates it into the XWayland session lifecycle, replaces legacy manual ResourceManager atom updates, and updates the build and logging configurations accordingly. Sequence diagram for dynamic scale propagation to X11 clients via SettingManagersequenceDiagram
participant "Wayland Output Window"
participant "Helper"
participant "Session"
participant "SettingManager"
participant "XResource/XSettings"
participant "X11/XWayland Clients"
"Wayland Output Window"->>"Helper": effectiveDevicePixelRatioChanged(dpr)
"Helper"->>"Session": (signal)
"Session"->>"SettingManager": setGlobalScale(dpr)
"SettingManager"->>"XResource/XSettings": setPropertyValue(DPI, scale)
"SettingManager"->>"XResource/XSettings": apply()
"XResource/XSettings"->>"X11/XWayland Clients": DPI/theme/scale updated (no restart needed)
ER diagram for XSettings and XResource property propagationerDiagram
SESSION ||--o| SETTING_MANAGER : manages
SETTING_MANAGER ||--|{ XSETTINGS : updates
SETTING_MANAGER ||--|{ XRESOURCE : updates
XSETTINGS ||--|{ XSETTINGS_PROPERTY_VALUE : contains
XRESOURCE ||--|{ RESOURCE_PROPERTY : contains
XSETTINGS_PROPERTY_VALUE {
int last_change_serial
QVariant value
}
RESOURCE_PROPERTY {
QVariant value
}
Class diagram for new XSettings/XResource/SettingManager frameworkclassDiagram
class AbstractSettings {
+initialized() bool
+isEmpty() bool
+contains(property: QByteArray) bool
+getPropertyValue(property: QByteArray) QVariant
+setPropertyValue(property: QByteArray, value: QVariant)
+propertyList() QByteArrayList
+apply()
<<signal>> PropertyChanged(property, value)
<<signal>> PropertyAdded(property, value)
<<signal>> PropertyRemoveed(property, value)
-xcb_connection_t *m_connection
-xcb_atom_t m_atom
}
class XSettings {
+initialized() bool
+isEmpty() bool
+contains(property: QByteArray) bool
+getPropertyValue(property: QByteArray) QVariant
+setPropertyValue(property: QByteArray, value: QVariant)
+propertyList() QByteArrayList
+apply()
+toString(key: XSettingsKey) QString
-std::vector<xcb_window_t> m_windows
-QMap<QByteArray, XSettingsPropertyValue> m_settings
-xcb_atom_t m_notifyAtom
-xcb_atom_t m_signalAtom
-int m_serial
}
class XResource {
+initialized() bool
+isEmpty() bool
+contains(property: QByteArray) bool
+getPropertyValue(property: QByteArray) QVariant
+setPropertyValue(property: QByteArray, value: QVariant)
+propertyList() QByteArrayList
+apply()
+toString(key: XResourceKey) QString
-xcb_window_t m_root
-QMap<QByteArray, QVariant> m_resources
}
class SettingManager {
+setGTKTheme(themeName: QString)
+GTKTheme() QString
+setFont(name: QString)
+font() QString
+setIconTheme(theme: QString)
+iconTheme() QString
+setSoudTheme(theme: QString)
+soundTheme() QString
+setCursorTheme(theme: QString)
+cursorTheme() QString
+setCursorSize(value: qreal)
+cursorSize() qreal
+setDoubleClickInterval(interval: int)
+setGlobalScale(scale: qreal)
+globalScale() qreal
+apply()
-XResource *m_resource
-XSettings *m_settings
}
AbstractSettings <|-- XSettings
AbstractSettings <|-- XResource
SettingManager o-- XSettings
SettingManager o-- XResource
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:
- SettingManager::cursorSize retrieves the wrong property key (Gtk_CursorThemeName) instead of Xcursor_Size—please correct it to use the proper Xcursor_Size enum.
- There's a typo in SettingManager::setSoudTheme; it should be setSoundTheme for consistency and clarity.
- Session threads for the SettingManager are started but never explicitly stopped or cleaned up on session destruction, potentially leaking threads; consider stopping the thread and cleaning up in Session’s destructor.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- SettingManager::cursorSize retrieves the wrong property key (Gtk_CursorThemeName) instead of Xcursor_Size—please correct it to use the proper Xcursor_Size enum.
- There's a typo in SettingManager::setSoudTheme; it should be setSoundTheme for consistency and clarity.
- Session threads for the SettingManager are started but never explicitly stopped or cleaned up on session destruction, potentially leaking threads; consider stopping the thread and cleaning up in Session’s destructor.
## Individual Comments
### Comment 1
<location> `src/xsettings/settingmanager.cpp:54` </location>
<code_context>
+ return m_settings->getPropertyValue(XSettings::toString(XSettings::Gtk_IconThemeName).toLatin1()).toString();
+}
+
+void SettingManager::setSoudTheme(const QString &theme)
+{
+ m_resource->setPropertyValue(XResource::toString(XResource::Net_SoundThemeName).toLatin1(), theme);
</code_context>
<issue_to_address>
**suggestion (typo):** Typo in method name: 'setSoudTheme' should be 'setSoundTheme'.
Please rename the method to 'setSoundTheme' for clarity and consistency.
Suggested implementation:
```cpp
void SettingManager::setSoundTheme(const QString &theme)
{
m_resource->setPropertyValue(XResource::toString(XResource::Net_SoundThemeName).toLatin1(), theme);
m_settings->setPropertyValue(XSettings::toString(XSettings::Net_SoundThemeName).toLatin1(), theme);
}
```
You will also need to:
1. Update the method declaration in the corresponding header file (`settingmanager.h`) from `setSoudTheme` to `setSoundTheme`.
2. Update any calls to `setSoudTheme` elsewhere in the codebase to use the new name `setSoundTheme`.
</issue_to_address>
### Comment 2
<location> `src/xsettings/settingmanager.cpp:82` </location>
<code_context>
+ m_settings->setPropertyValue(XSettings::toString(XSettings::Xcursor_Size).toLatin1(), value);
+}
+
+qreal SettingManager::cursorSize() const
+{
+ return m_settings->getPropertyValue(XSettings::toString(XSettings::Gtk_CursorThemeName).toLatin1()).toReal();
</code_context>
<issue_to_address>
**issue (bug_risk):** cursorSize() returns Gtk_CursorThemeName value instead of Xcursor_Size.
The getter should retrieve XSettings::Xcursor_Size or Gtk_CursorThemeSize to ensure consistency with the setter and expected behavior.
</issue_to_address>
### Comment 3
<location> `src/xsettings/settingmanager.cpp:92` </location>
<code_context>
+ m_settings->setPropertyValue(XSettings::toString(XSettings::Net_DoubleClickTime).toLatin1(), interval);
+}
+
+void SettingManager::setGlobalScale(qreal scale)
+{
+ m_resource->setPropertyValue(XResource::toString(XResource::Xft_DPI).toLatin1(), scale * BASE_DPI);
</code_context>
<issue_to_address>
**question:** Potential mismatch in Xft_DPI value between XResource and XSettings.
XResource uses BASE_DPI, while XSettings uses XSETTINGS_BASE_DPI_FIXED for scaling. This could cause inconsistent DPI values; please align the calculations or clarify the rationale.
</issue_to_address>
### Comment 4
<location> `src/xsettings/xsettings.cpp:159-168` </location>
<code_context>
+ xSettings.append((char*)&number_of_settings, sizeof(number_of_settings)); //N_SETTINGS
</code_context>
<issue_to_address>
**issue (bug_risk):** number_of_settings is decremented for invalid values but still appended before filtering.
Filter out invalid values before updating and appending number_of_settings to ensure consistency.
</issue_to_address>
### Comment 5
<location> `src/xsettings/xsettings.cpp:275-200` </location>
<code_context>
+ local_offset += 4;
+
+ QVariant value;
+ if (type == XSettingsTypeString) {
+ VALIDATE_LENGTH(4);
+ int value_length = ADJUST_BO(byteOrder, qint32, data + offset + local_offset);
+ local_offset+=4;
+ VALIDATE_LENGTH(value_length);
+ QByteArray value_string(data + offset + local_offset, value_length);
+ value.setValue(value_string);
+ local_offset += round_to_nearest_multiple_of_4(value_length);
+ } else if (type == XSettingsTypeInteger) {
+ VALIDATE_LENGTH(4);
+ int value_length = ADJUST_BO(byteOrder, qint32, data + offset + local_offset);
+ local_offset += 4;
+ value.setValue(value_length);
+ } else if (type == XSettingsTypeColor) {
+ VALIDATE_LENGTH(2*4);
+ quint16 red = ADJUST_BO(byteOrder, quint16, data + offset + local_offset);
</code_context>
<issue_to_address>
**nitpick:** Parsing for XSettingsTypeInteger uses value_length as the value.
Consider renaming value_length to value or integer_value for better clarity, as it represents the actual value rather than a length.
</issue_to_address>
### Comment 6
<location> `src/xsettings/xresource.cpp:126` </location>
<code_context>
+ free(reply);
+
+ const QList<QByteArray> lines = text.split('\n');
+ for (const QByteArray &line : lines) {
+ int sep = line.indexOf(':');
+ if (sep > 0) {
</code_context>
<issue_to_address>
**suggestion:** Parsing of XResource lines does not handle escaped colons.
Splitting on the first colon fails if colons are present in the value. Please update the parsing to correctly handle escaped colons in the input.
</issue_to_address>
### Comment 7
<location> `src/xsettings/abstractsettings.h:31` </location>
<code_context>
+Q_SIGNALS:
+ void PropertyChanged(const QByteArray &property, const QVariant &value);
+ void PropertyAdded(const QByteArray &property, const QVariant &value);
+ void PropertyRemoveed(const QByteArray &property, const QVariant &value);
+
+protected:
</code_context>
<issue_to_address>
**suggestion (typo):** Typo in signal name: 'PropertyRemoveed' should be 'PropertyRemoved'.
Please update the signal name to 'PropertyRemoved' to correct the typo.
```suggestion
void PropertyRemoved(const QByteArray &property, const QVariant &value);
```
</issue_to_address>
### Comment 8
<location> `src/xsettings/xsettings.h:107` </location>
<code_context>
+{
+ Q_OBJECT
+public:
+ enum XSettingsKey {
+ Unknown = 0,
+
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the key definitions using the X-Macro pattern to centralize metadata and automate enum and lookup generation.
Here’s one way to collapse those hundreds of lines into a single maintainable list by using the “X‐Macro” pattern plus a small generated lookup table. You keep one file of definitions, and generate both your enum and your string‐mapping from it.
1. Create a new file `XSettingsKeys.def` with one line per key:
```c
// XSettingsKeys.def
// key, property‐name, default‐QVariant, type
XDEF(Unknown, "Unknown", QVariant(), Integer)
XDEF(Xft_DPI, "Xft/DPI", QVariant(196608), Integer)
XDEF(Xft_Antialias, "Xft/Antialias", QVariant(1), Integer)
// …and so on for all your keys…
```
2. In your header replace the big enum with:
```c++
// XSettings.h
#pragma once
#include "abstractsettings.h"
#include <QMetaEnum>
class XSettings : public AbstractSettings {
Q_OBJECT
public:
enum XSettingsKey {
#define XDEF(key, prop, def, type) key,
#include "XSettingsKeys.def"
#undef XDEF
};
Q_ENUM(XSettingsKey)
static QString toString(XSettingsKey key) {
static const char* names[] = {
#define XDEF(key, prop, def, type) prop,
#include "XSettingsKeys.def"
#undef XDEF
};
return names[key];
}
// …
};
```
3. (Optionally) In your `.cpp` you can build a default‐value lookup:
```c++
// XSettings.cpp
const QVariant& XSettings::defaultValue(XSettingsKey key) {
static const QVariant defaults[] = {
#define XDEF(key, prop, def, type) def,
#include "XSettingsKeys.def"
#undef XDEF
};
return defaults[key];
}
```
Benefits:
- All your key-metadata lives in one small file.
- Adding/removing a key is a one-line edit.
- You still have an enum, string lookup, and defaults in sync.
- You avoid hundreds of manual case statements or comment table rows.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a dynamic XSettings subsystem that synchronizes X11 application settings (DPI, scaling, themes) with Treeland's display scaling changes, allowing XWayland clients to automatically adapt to scale changes without restart.
Key Changes
- Implements a new settings management architecture with
AbstractSettingsbase class,XSettingsandXResourceimplementations, and a unifiedSettingManagerAPI - Integrates settings synchronization into the session lifecycle, running on a dedicated thread and updating automatically when display scale changes
- Removes the previous manual
RESOURCE_MANAGERatom update approach in favor of the comprehensive xsettings-based solution
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| src/xsettings/abstractsettings.h/cpp | Base class defining the settings interface for property management and X11 communication |
| src/xsettings/xsettings.h/cpp | XSettings protocol implementation handling XSETTINGS manager window, property serialization, and client notification |
| src/xsettings/xresource.h/cpp | X Resources (RESOURCE_MANAGER) implementation for legacy X11 application compatibility |
| src/xsettings/settingmanager.h/cpp | High-level API coordinating XSettings and XResource updates for themes, fonts, and scaling |
| src/seat/helper.h/cpp | Session integration creating SettingManager instances and connecting to scale change signals |
| src/core/shellhandler.cpp | Removes old manual RESOURCE_MANAGER atom update code replaced by new subsystem |
| src/common/treelandlogging.h/cpp | Adds treeland.xsettings logging category |
| src/CMakeLists.txt | Registers new xsettings source files in the build system |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
TAG Bot New tag: 0.7.8 |
zccrs
requested changes
Nov 24, 2025
zzxyb
force-pushed
the
scale
branch
3 times, most recently
from
2c19e05 to
b789d53
Compare
zccrs
requested changes
Dec 1, 2025
- allows X11 applications running under XWayland to automatically follow the new max scale without needing to restart. - improves user experience and visual consistency between native Wayland and X11 clients when display scaling is adjusted. Log:
zccrs
approved these changes
Dec 2, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: zccrs, zzxyb 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 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
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.
Log:
Summary by Sourcery
Implement a dynamic xsettings subsystem that syncs X resources and XSettings with Treeland’s display scaling, allowing X11 applications under XWayland to automatically adjust DPI and scale without restart.
New Features:
Enhancements:
Build: