feat: add brightness control for Treeland compositor #2955
+320
−969
Conversation
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
Reviewer's GuideAdds Treeland-specific per-output brightness control based on a new color-control Wayland protocol, wires it into the display worker and registry/output abstractions, and fixes screen dimension reporting to account for output scale factors. Sequence diagram for Treeland per-output brightness update and syncsequenceDiagram
actor User
participant DisplayUI
participant DisplayWorker
participant Monitor
participant TreeLandOutputManager
participant ColorControl as treeland_output_color_control_v1
participant TreelandCompositor
User->>DisplayUI: Adjust monitor brightness slider
DisplayUI->>DisplayWorker: setMonitorBrightness(monitor, newBrightness)
DisplayWorker->>DisplayWorker: lookup ColorControl in m_control_monitors
alt ColorControl found
DisplayWorker->>TreeLandOutputManager: setBrightness(ColorControl, newBrightness * 100.0)
TreeLandOutputManager->>ColorControl: treeland_output_color_control_v1_set_brightness()
TreeLandOutputManager->>ColorControl: treeland_output_color_control_v1_commit()
ColorControl->>TreelandCompositor: commit brightness request
TreelandCompositor-->>ColorControl: brightness event (current brightness)
ColorControl-->>TreeLandOutputManager: handleBrightness(data, obj, value)
TreeLandOutputManager-->>DisplayWorker: brightnessChanged(ColorControl, value)
DisplayWorker->>DisplayWorker: onBrightnessChanged(ColorControl, value)
DisplayWorker->>Monitor: setBrightness(value / 100.0)
else ColorControl not found
DisplayWorker->>DisplayWorker: No-op (no color control for monitor)
end
Updated class diagram for Treeland output brightness control integrationclassDiagram
class DisplayWorker {
- WQt_Registry* m_reg
- QMap~Monitor*, WQt_OutputHead*~ m_wl_monitors
- QMap~Monitor*, treeland_output_color_control_v1*~ m_control_monitors
+ onInterfaceRegistered(interface: WQt_Registry_Interface) void
+ onWlMonitorListChanged() void
+ wlMonitorAdded(head: WQt_OutputHead*) void
+ wlMonitorRemoved(head: WQt_OutputHead*) void
+ wlOutputAdded(output: WQt_Output*) void
+ wlOutputRemoved(output: WQt_Output*) void
+ updateControl() void
+ setMonitorBrightness(mon: Monitor*, brightness: double) void
+ onBrightnessChanged(colorControl: treeland_output_color_control_v1*, brightness: double) void
}
class TreeLandOutputManager {
- treeland_output_manager_v1* mObj
+ setPrimaryOutput(name: const char*) void
+ getColorControl(output: wl_output*) treeland_output_color_control_v1*
+ setBrightness(colorControl: treeland_output_color_control_v1*, brightness: double) void
+ destroyColorControl(colorControl: treeland_output_color_control_v1*) void
+ primaryOutputChanged(outputName: const char*) void
+ brightnessChanged(colorControl: treeland_output_color_control_v1*, brightness: double) void
+ colorTemperatureChanged(colorControl: treeland_output_color_control_v1*, temperature: uint) void
- handlePrimaryOutput(data: void*, mgr: treeland_output_manager_v1*, output_name: const char*) void
- handleResult(data: void*, colorControl: treeland_output_color_control_v1*, success: uint32_t) void
- handleColorTemperature(data: void*, colorControl: treeland_output_color_control_v1*, temperature: uint32_t) void
- handleBrightness(data: void*, colorControl: treeland_output_color_control_v1*, brightness: wl_fixed_t) void
- static treeland_output_manager_v1_listener mListener
- static treeland_output_color_control_v1_listener mColorControlListener
}
class Output {
- bool mDone
+ scale() int32_t
+ isReady() bool
+ waitForReady() void
+ get() wl_output*
+ done() void
}
class Registry {
+ setup() void
+ outputManager() OutputManager*
+ treeLandOutputManager() TreeLandOutputManager*
+ waylandOutputs() QList~WQt_Output*~
+ interfaceRegistered(interface: WQt_Registry_Interface) void
}
class Monitor {
+ name() QString
+ w() int
+ h() int
+ scale() int
+ setBrightness(value: double) void
}
DisplayWorker --> Registry : uses
DisplayWorker --> TreeLandOutputManager : uses
DisplayWorker --> Monitor : maps via m_wl_monitors and m_control_monitors
DisplayWorker --> Output : uses in updateControl()
Registry --> Output : manages list of Wayland outputs
Registry --> TreeLandOutputManager : creates and owns
class treeland_output_manager_v1 {
}
class treeland_output_color_control_v1 {
}
treeland_output_manager_v1 <.. TreeLandOutputManager : wraps
treeland_output_color_control_v1 <.. TreeLandOutputManager : creates and controls
Monitor "1" --> "0..1" treeland_output_color_control_v1 : brightness control handle
Class diagram for updated Treeland Wayland protocol interfacesclassDiagram
class treeland_output_manager_v1 {
<<interface>>
+ set_primary_output(output: string) void
+ get_color_control(id: treeland_output_color_control_v1*, output: wl_output*) void
+ destroy() void
+ primary_output(output_name: string)
}
class treeland_output_color_control_v1 {
<<interface>>
+ set_color_temperature(temperature: uint) void
+ set_brightness(brightness: wl_fixed_t) void
+ commit() void
+ destroy() void
+ result(success: uint)
+ color_temperature(temperature: uint)
+ brightness(brightness: wl_fixed_t)
}
class treeland_output_color_control_v1_error {
<<enum>>
invalid_color_temperature = 1
invalid_brightness = 2
}
treeland_output_manager_v1 --> treeland_output_color_control_v1 : creates per wl_output
treeland_output_color_control_v1 ..> treeland_output_color_control_v1_error : uses error codes
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 - I've found 3 issues, and left some high level feedback:
- onWlMonitorListChanged() wires outputAdded/outputRemoved signals every time it runs, which can lead to duplicate connections and repeated callbacks; consider moving these connects to a one-time initialization path instead.
- wlOutputRemoved() is currently empty while you create per-output color controls in updateControl(); you likely need to destroy the associated color_control object and remove its entry from m_control_monitors when an output is removed to avoid leaks and stale mappings.
- TreeLandOutputManager::handleResult() is a no-op even though the protocol exposes success/failure for commits; consider emitting a signal or handling non-success values so callers can react to failed brightness/color changes instead of assuming they always succeed.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- onWlMonitorListChanged() wires outputAdded/outputRemoved signals every time it runs, which can lead to duplicate connections and repeated callbacks; consider moving these connects to a one-time initialization path instead.
- wlOutputRemoved() is currently empty while you create per-output color controls in updateControl(); you likely need to destroy the associated color_control object and remove its entry from m_control_monitors when an output is removed to avoid leaks and stale mappings.
- TreeLandOutputManager::handleResult() is a no-op even though the protocol exposes success/failure for commits; consider emitting a signal or handling non-success values so callers can react to failed brightness/color changes instead of assuming they always succeed.
## Individual Comments
### Comment 1
<location> `src/plugin-display/operation/private/displayworker.cpp:231-235` </location>
<code_context>
if (isNew)
wlMonitorAdded(head);
}
+ for(auto output:m_reg->waylandOutputs()){
+ wlOutputAdded(output);
+ }
+ connect(m_reg,&WQt::Registry::outputAdded,this,&DisplayWorker::wlOutputAdded);
+ connect(m_reg,&WQt::Registry::outputRemoved,this,&DisplayWorker::wlOutputRemoved);
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid reconnecting registry signals and reprocessing outputs every time onWlMonitorListChanged runs.
Because onWlMonitorListChanged iterates over all outputs and then connects to outputAdded/outputRemoved on m_reg, if this method is called more than once it will re-run wlOutputAdded on existing outputs and create duplicate signal connections. That will cause each outputAdded/outputRemoved event to trigger handlers multiple times. Please move the signal connections to a one-time initialization point (e.g., the Treeland constructor branch or onInterfaceRegistered) and either perform the initial output scan only once or guard both the scan and connections with a flag.
</issue_to_address>
### Comment 2
<location> `src/plugin-display/wayland/libwayqt/TreeLandOutputManager.cpp:49-52` </location>
<code_context>
+ }
+}
+
+void WQt::TreeLandOutputManager::destroyColorControl(treeland_output_color_control_v1 *treeland_output_color_control_v1)
+{
+ treeland_output_color_control_v1_destroy(treeland_output_color_control_v1);
+}
+
</code_context>
<issue_to_address>
**suggestion:** Guard destroyColorControl against null pointers for safety and consistency.
`setBrightness` guards `treeland_output_color_control_v1` against null, but this function does not. Even if current callers never pass null, adding the same guard here would make the API safer and consistent:
```cpp
if (!treeland_output_color_control_v1)
return;
treeland_output_color_control_v1_destroy(treeland_output_color_control_v1);
```
```suggestion
void WQt::TreeLandOutputManager::destroyColorControl(treeland_output_color_control_v1 *treeland_output_color_control_v1)
{
if (!treeland_output_color_control_v1) {
return;
}
treeland_output_color_control_v1_destroy(treeland_output_color_control_v1);
}
```
</issue_to_address>
### Comment 3
<location> `src/plugin-display/wayland/libwayqt/protocols/treeland-output-management.xml:74-75` </location>
<code_context>
+ Color temperature is defined as the corresponding temperature (in Kelvin) of the current white point
+ of the display on a Planckian locus.
+ With the current implementation, the neutral temperature is 6600K.
+ This event is sent once after the treeland_output_color_control_v1 object is created,
+ or right after when a color temperature change for the output is successfully commited.
+ </description>
+ <arg name="temperature" type="uint" summary="current color temperature in Kelvin"/>
</code_context>
<issue_to_address>
**nitpick (typo):** Fix minor wording and spelling issues in the protocol documentation.
Two small issues here: "commited" → "committed", and "or right after when" reads awkwardly. Consider something like:
```xml
This event is sent once after the treeland_output_color_control_v1 object is created,
or immediately after a color temperature change for the output is successfully committed.
```
Suggested implementation:
```
With the current implementation, the neutral temperature is 6600K.
This event is sent once after the treeland_output_color_control_v1 object is created,
or immediately after a color temperature change for the output is successfully committed.
```
```
Brightness is valued in the range [0.0, 100.0].
This event is sent once after the treeland_output_color_control_v1 object is created,
or immediately after a brightness change for the output is successfully committed.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
231
to
235
| for(auto output:m_reg->waylandOutputs()){ | ||
| wlOutputAdded(output); | ||
| } | ||
| connect(m_reg,&WQt::Registry::outputAdded,this,&DisplayWorker::wlOutputAdded); | ||
| connect(m_reg,&WQt::Registry::outputRemoved,this,&DisplayWorker::wlOutputRemoved); |
There was a problem hiding this comment.
issue (bug_risk): Avoid reconnecting registry signals and reprocessing outputs every time onWlMonitorListChanged runs.
Because onWlMonitorListChanged iterates over all outputs and then connects to outputAdded/outputRemoved on m_reg, if this method is called more than once it will re-run wlOutputAdded on existing outputs and create duplicate signal connections. That will cause each outputAdded/outputRemoved event to trigger handlers multiple times. Please move the signal connections to a one-time initialization point (e.g., the Treeland constructor branch or onInterfaceRegistered) and either perform the initial output scan only once or guard both the scan and connections with a flag.
Comment on lines
+49
to
+52
| void WQt::TreeLandOutputManager::destroyColorControl(treeland_output_color_control_v1 *treeland_output_color_control_v1) | ||
| { | ||
| treeland_output_color_control_v1_destroy(treeland_output_color_control_v1); | ||
| } |
There was a problem hiding this comment.
suggestion: Guard destroyColorControl against null pointers for safety and consistency.
setBrightness guards treeland_output_color_control_v1 against null, but this function does not. Even if current callers never pass null, adding the same guard here would make the API safer and consistent:
if (!treeland_output_color_control_v1)
return;
treeland_output_color_control_v1_destroy(treeland_output_color_control_v1);
Suggested change
| void WQt::TreeLandOutputManager::destroyColorControl(treeland_output_color_control_v1 *treeland_output_color_control_v1) | |
| { | |
| treeland_output_color_control_v1_destroy(treeland_output_color_control_v1); | |
| } | |
| void WQt::TreeLandOutputManager::destroyColorControl(treeland_output_color_control_v1 *treeland_output_color_control_v1) | |
| { | |
| if (!treeland_output_color_control_v1) { | |
| return; | |
| } | |
| treeland_output_color_control_v1_destroy(treeland_output_color_control_v1); | |
| } |
Comment on lines
74
to
75
| This event is sent once after the treeland_output_color_control_v1 object is created, | ||
| or right after when a color temperature change for the output is successfully commited. |
There was a problem hiding this comment.
nitpick (typo): Fix minor wording and spelling issues in the protocol documentation.
Two small issues here: "commited" → "committed", and "or right after when" reads awkwardly. Consider something like:
This event is sent once after the treeland_output_color_control_v1 object is created,
or immediately after a color temperature change for the output is successfully committed.Suggested implementation:
With the current implementation, the neutral temperature is 6600K.
This event is sent once after the treeland_output_color_control_v1 object is created,
or immediately after a color temperature change for the output is successfully committed.
Brightness is valued in the range [0.0, 100.0].
This event is sent once after the treeland_output_color_control_v1 object is created,
or immediately after a brightness change for the output is successfully committed.
caixr23
force-pushed
the
bug-display
branch
2 times, most recently
from
d0174e4 to
89093be
Compare
mhduiy
requested changes
Jan 27, 2026
src/plugin-display/wayland/libwayqt/protocols/treeland-output-management.xml
Outdated
Show resolved
Hide resolved
caixr23
force-pushed
the
bug-display
branch
5 times, most recently
from
2539921 to
c247aa5
Compare
mhduiy
reviewed
Jan 28, 2026
|
|
||
| ws_generate_local(client | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/wayland/libwayqt/protocols/treeland-output-management.xml | ||
| ${TREELAND_PROTOCOLS_DATA_DIR}/treeland-output-manager-v1.xml |
Contributor
There was a problem hiding this comment.
Enable_TreelandSupport 没有包裹这里的胶水代码生成，和cpp的treelandoutputmanager，就没啥意义，要么删掉Enable_TreelandSupport，要么把这些地方都包裹上
|
TAG Bot New tag: 6.1.70 |
caixr23
force-pushed
the
bug-display
branch
2 times, most recently
from
e1c7ce7 to
3eb30bf
Compare
1. Implement color control protocol support for brightness adjustment 2. Add scale factor consideration in screen dimension calculations 3. Improve Wayland output management with proper event handling 4. Add brightness synchronization between compositor and display settings Log: Added brightness control support for Treeland compositor Influence: 1. Test brightness adjustment on monitors with Treeland compositor 2. Verify screen resolution calculations account for scale factor 3. Check monitor detection and removal with brightness controls 4. Test multi-monitor setup with individual brightness settings 5. Verify brightness changes persist across display configuration changes feat: 为 Treeland 合成器添加亮度控制功能 1. 实现颜色控制协议支持亮度调节 2. 在屏幕尺寸计算中考虑缩放因子 3. 改进 Wayland 输出管理，完善事件处理 4. 添加合成器与显示设置之间的亮度同步 Log: 新增 Treeland 合成器的亮度控制支持 Influence: 1. 在 Treeland 合成器环境下测试显示器亮度调节 2. 验证屏幕分辨率计算是否考虑缩放因子 3. 检查带亮度控制的显示器检测和移除功能 4. 测试多显示器环境下的独立亮度设置 5. 验证亮度变更在显示配置更改后是否持久化 PMS: BUG-344299
deepin pr auto reviewGit Diff 代码审查报告1. 总体评估这次提交主要涉及 Wayland 显示管理相关的代码变更，包括：
整体代码质量较好，但存在一些可以改进的地方。 2. 语法与逻辑问题2.1 代码风格不一致// src/plugin-display/wayland/libwayqt/TreeLandOutputManager.h
void setBrightness(treeland_output_color_control_v1 * treeland_output_color_control_v1,const double brightness);参数名与类型名相同，且参数前有多余空格，建议修改为： void setBrightness(treeland_output_color_control_v1 *control, const double brightness);2.2 条件判断优化// src/plugin-display/operation/dccscreen.cpp
return d_ptrDccScreen->monitor()->scale() > 0 ? (d_ptrDccScreen->monitor()->w() / d_ptrDccScreen->monitor()->scale()) : d_ptrDccScreen->monitor()->w();这段代码在 width() 和 height() 方法中重复出现，建议提取为私有方法： int DccScreen::scaledValue(int value) const
{
int scale = d_ptrDccScreen->monitor()->scale();
return scale > 0 ? value / scale : value;
}2.3 逻辑顺序问题// src/plugin-display/operation/private/displayworker.cpp
void DisplayWorker::onInterfaceRegistered(WQt::Registry::Interface interface)
{
switch (interface) {
case WQt::Registry::OutputManagerInterface: {
auto *opMgr = m_reg->outputManager();
if (!opMgr) {
qCFatal(DdcDisplayWorker) << "Unable to start the output manager";
}
connect(opMgr, &WQt::OutputManager::done, this, &DisplayWorker::onWlOutputManagerDone);
} break;在检查 opMgr 是否为空之前，应该先确保 m_reg 不为空，否则可能导致空指针解引用。 3. 代码质量问题3.1 魔法数字// src/plugin-display/operation/private/displayworker.cpp
brightness = qBound(0.0, brightness / 100.0, 1.0);硬编码的 100.0 应该定义为常量： static constexpr double BRIGHTNESS_CONVERSION_FACTOR = 100.0;3.2 错误处理不完整// src/plugin-display/wayland/libwayqt/TreeLandOutputManager.cpp
void WQt::TreeLandOutputManager::handleResult(void *data, treeland_output_color_control_v1 *treeland_output_color_control_v1, uint32_t success)
{
// TODO: handleResult
}TODO 注释表明错误处理未实现，应该添加适当的错误处理逻辑。 3.3 资源管理// src/plugin-display/wayland/libwayqt/TreeLandOutputManager.cpp
treeland_output_color_control_v1 *WQt::TreeLandOutputManager::getColorControl(struct wl_output *output)
{
auto colorControl = treeland_output_manager_v1_get_color_control(mObj, output);
if (colorControl) {
treeland_output_color_control_v1_add_listener(colorControl, &mColorControlListener, this);
}
return colorControl;
}返回的 colorControl 对象的生命周期管理不明确，应该明确说明由谁负责释放。 4. 代码性能问题4.1 不必要的循环等待// src/plugin-display/wayland/libwayqt/Output.cpp
void WQt::Output::waitForReady()
{
if (mDone) {
return;
}
do {
QThread::usleep(100);
qApp->processEvents();
} while (not mDone);
}虽然这段代码在 diff 中被删除了，但原来的实现存在性能问题。新实现的 isReady() 方法更好，但应该考虑添加异步通知机制。 4.2 重复查找// src/plugin-display/operation/private/displayworker.cpp
void DisplayWorker::updateControl()
{
for (auto output : m_reg->waylandOutputs()) {
if (output->isReady()) {
for (auto it(m_wl_monitors.cbegin()); it != m_wl_monitors.cend(); ++it) {
if (it.key()->name() == output->name()) {
if (!m_control_monitors.contains(it.key())) {
auto control = m_reg->treeLandOutputManager()->getColorControl(output->get());
m_control_monitors.insert(it.key(), control);
}
break;
}
}
}
}
}嵌套循环可能导致性能问题，特别是当显示器数量较多时。可以考虑使用哈希表来优化查找。 5. 代码安全问题5.1 空指针检查// src/plugin-display/operation/private/displayworker.cpp
void DisplayWorker::setMonitorBrightness(Monitor *mon, const double brightness)
{
if (WQt::Utils::isTreeland()) {
#ifndef DCC_DISABLE_ROTATE
auto *gammaConfig = m_wl_gammaConfig->value(mon);
gammaConfig->brightness = brightness;
gammaEffect->setConfiguration(*gammaConfig);
#else
for (auto it(m_control_monitors.cbegin()); it != m_control_monitors.cend(); ++it) {
if (it.key() == mon) {
m_reg->treeLandOutputManager()->setBrightness(it.value(), brightness * 100.0);
break;
}
}
#endif
}在访问 m_reg->treeLandOutputManager() 之前，应该检查 m_reg 是否为空。 5.2 边界检查// src/plugin-display/operation/private/displayworker.cpp
void DisplayWorker::onBrightnessChanged(const treeland_output_color_control_v1 *colorControl, double brightness)
{
brightness = qBound(0.0, brightness / 100.0, 1.0);
for (auto it(m_control_monitors.cbegin()); it != m_control_monitors.cend(); ++it) {
if (it.value() == colorControl) {
it.key()->setBrightness(brightness);
return;
}
}
}虽然使用了 qBound 限制亮度值，但没有检查 colorControl 是否为空指针。 6. 改进建议
7. 总结这次提交主要改进了 Wayland 显示管理功能，特别是亮度控制方面。代码整体质量较好，但在错误处理、资源管理和性能优化方面还有改进空间。建议在合并前解决上述问题，以提高代码的健壮性和可维护性。 |
mhduiy
approved these changes
Jan 28, 2026
|
[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 |
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.
Log: Added brightness control support for Treeland compositor
Influence:
feat: 为 Treeland 合成器添加亮度控制功能
Log: 新增 Treeland 合成器的亮度控制支持
Influence:
PMS: BUG-344299
Summary by Sourcery
Introduce Treeland compositor color control protocol support and wire it into display handling for brightness-aware output management.
New Features:
Bug Fixes:
Enhancements: