fix(palette): break QML Palette windowText binding loop from the read side - #684
Draft
52cyb wants to merge 1 commit into
Draft
fix(palette): break QML Palette windowText binding loop from the read side#68452cyb wants to merge 1 commit into
52cyb wants to merge 1 commit into
Conversation
… side 1. Add Qt6-only `_d_getControlPaletteForResolve()`: resolve typed colors (Highlight / HighlightedText) from the stable application palette, and read only the Window role (theme detection) from the control -- never via QQuickPalette::toQPalette(), which reads back the DTK-overridden `palette.windowText` and re-triggers its QML binding 2. Add Qt6-only `_d_controlPaletteHasInactiveState()`: compare Active vs Inactive groups role-by-role excluding `windowText` (a binding artifact, not a real inactive-state difference), avoiding the third windowText read-back point in getColorOf() 3. Replace the three read points (typed-color branch / inactive gate / updateControlTheme theme detection) to use the new safe helpers 4. Add a re-entrancy guard (`m_updateDepth`) in updateControlTheme() to skip the redundant recompute triggered by the `palette.windowText` binding writing the palette Influence: 1. Open control center; no more "Binding loop detected for property windowText" when operating Button / MenuItem / ItemDelegate 2. Verify checked/highlighted text and icon foreground still turn white (HighlightedText) in light theme, and accent-color changes are followed 3. Verify normal state keeps the default foreground color 4. Verify light/dark theme switching 5. Verify inactive state (window unfocused) text is not wrongly blended fix(palette): 从读侧断开 QML Palette windowText 绑定环 1. 新增仅 Qt6 的 `_d_getControlPaletteForResolve()`：类型化颜色（Highlight/ HighlightedText）从稳定的应用调色板解析，仅 Window 角色（主题探测）从控件读取 —— 不再走 QQuickPalette::toQPalette()，避免读回被 DTK 覆盖的 `palette.windowText` 而重触发其 QML 绑定 2. 新增仅 Qt6 的 `_d_controlPaletteHasInactiveState()`：逐角色比较 Active/Inactive 组并排除 `windowText`（绑定产物，非真实 inactive 差异），消除 getColorOf() 中第三个 windowText 读回点 3. 三处读点（类型化分支 / inactive 门控 / updateControlTheme 主题探测）改用新的安全函数 4. 在 updateControlTheme() 增加重入守卫（`m_updateDepth`），跳过由 `palette.windowText` 绑定写 palette 触发的冗余重算 Influence: 1. 打开控制中心，操作 Button / MenuItem / ItemDelegate 不再出现 "Binding loop detected for property windowText" 2. 验证浅色主题下选中/高亮文字与图标前景仍为白色（HighlightedText），强调色变化跟随 3. 验证普通态保持默认前景色 4. 验证亮/暗主题切换 5. 验证窗口失焦 inactive 态文字不被误混合 PMS: Task-392413
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 52cyb 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 |
Reviewer's GuideThe PR breaks the Qt 6 palette.windowText binding loop by eliminating unsafe palette read-backs, using targeted palette resolution and inactive-state checks, and guarding synchronous re-entrant theme updates while retaining the existing Qt 5 behavior. Sequence diagram for safe Qt6 palette color resolutionsequenceDiagram
participant Selector as DQuickControlColorSelector
participant Control as QQuickControl
participant Palette as QQuickPalette
participant AppPalette as ApplicationPalette
Selector->>AppPalette: applicationPalette(theme)
AppPalette-->>Selector: Stable accent and typed-color roles
Selector->>Control: property("palette")
Control-->>Selector: QQuickPalette
Selector->>Palette: active()->window()
Palette-->>Selector: Window role
Selector->>Selector: targetColor.toColor(resolvedPalette)
opt InactiveState
Selector->>Palette: Compare Active/Inactive roles
Note over Selector,Palette: Exclude WindowText binding artifact
Palette-->>Selector: Real inactive-state difference
end
Sequence diagram for guarded palette theme updatessequenceDiagram
participant Palette as QQuickPalette
participant Selector as DQuickControlColorSelector
participant ColorProps as ColorProperties
Palette->>Selector: updateControlTheme()
Selector->>Selector: Check m_updateDepth
Selector->>Selector: Increment m_updateDepth
Selector->>Selector: Read Window role via _d_getControlPaletteForResolve()
Selector->>ColorProps: updateAllColorProperties()
ColorProps->>Selector: Increment m_updateDepth
Selector->>Palette: Write palette.windowText
Palette-->>Selector: Synchronous changed signal
Selector->>Selector: Skip re-entrant updateControlTheme()
ColorProps->>Selector: Decrement m_updateDepth
Selector->>Selector: Decrement m_updateDepth
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 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/private/dquickcontrolpalette.cpp" line_range="144-145" />
<code_context>
+ case QPalette::ToolTipBase: return g->toolTipBase();
+ case QPalette::ToolTipText: return g->toolTipText();
+ case QPalette::PlaceholderText: return g->placeholderText();
+ case QPalette::Accent: return g->accent();
+ case QPalette::NColorRoles: break;
+ }
+ return QColor();
</code_context>
<issue_to_address>
**issue (bug_risk):** The Qt6-only helper does not compile with Qt versions where `QPalette::Accent` and `QQuickColorGroup::accent()` are unavailable, including the Qt 6.2-era versions this project otherwise explicitly supports. The unguarded enum cases prevent building those configurations.
**Triggers:** When building against Qt 6.2 or another Qt6 version before the Accent palette role was introduced.
**Suggested fix:** Guard the Accent case and accessor with the Qt version that introduced them, or omit that role for older Qt versions.
</issue_to_address>
Comment on lines
+144
to
+145
| case QPalette::Accent: return g->accent(); | ||
| case QPalette::NColorRoles: break; |
There was a problem hiding this comment.
issue (bug_risk): The Qt6-only helper does not compile with Qt versions where QPalette::Accent and QQuickColorGroup::accent() are unavailable, including the Qt 6.2-era versions this project otherwise explicitly supports. The unguarded enum cases prevent building those configurations.
Triggers: When building against Qt 6.2 or another Qt6 version before the Accent palette role was introduced.
Suggested fix: Guard the Accent case and accessor with the Qt version that introduced them, or omit that role for older Qt versions.
Contributor
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
📋 代码变更概要
变更说明： 修复方案包括：
🔍 详细分析1. 语法逻辑 ✓评分： 25/25 分 潜在问题： 分析说明：
2. 代码质量 ✓评分： 24/25 分 潜在问题：
建议： struct UpdateDepthGuard {
int& depth;
explicit UpdateDepthGuard(int& d) : depth(d) { ++depth; }
~UpdateDepthGuard() { --depth; }
Q_DISABLE_COPY(UpdateDepthGuard)
};
// 在 updateControlTheme() 中使用：
void DQuickControlColorSelector::updateControlTheme()
{
if (!m_control)
return;
if (m_updateDepth > 0)
return;
UpdateDepthGuard guard(m_updateDepth);
// ... 函数体 ...
// 无需手动 --m_updateDepth，guard 析构时自动递减
}3. 代码性能 ✓评分： 20/20 分 潜在问题： 分析说明：
4. 代码安全 ✓评分： 30/30 分
安全漏洞详情： 漏洞对比统计：新增漏洞 0 个，减少漏洞 0 个，持平 0 个 分析说明： 💡 改进建议代码示例// RAII 作用域守卫，替代手动 m_updateDepth 递增/递减
struct UpdateDepthGuard {
int& depth;
explicit UpdateDepthGuard(int& d) : depth(d) { ++depth; }
~UpdateDepthGuard() { --depth; }
Q_DISABLE_COPY(UpdateDepthGuard)
};
// 在头文件中添加（dquickcontrolpalette_p.h）
class DQuickControlColorSelector : public QObject
{
// ...
int m_updateDepth = 0;
};
// updateControlTheme() 使用 RAII 守卫
void DQuickControlColorSelector::updateControlTheme()
{
if (!m_control)
return;
if (m_updateDepth > 0)
return;
UpdateDepthGuard guard(m_updateDepth); // 自动管理深度
#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
const QPalette pa = _d_getControlPaletteForResolve(m_control, m_state->controlTheme);
const QColor windowColor = pa.color(QPalette::Window);
#else
const QPalette pa = _d_getControlPalette(m_control);
const QColor windowColor = pa.color(QPalette::Window);
#endif
if (!windowColor.isValid()) {
updateAllColorProperties();
return; // guard 自动递减
}
const auto themeType = DGuiApplicationHelper::toColorType(windowColor);
if (!setControlTheme(themeType)) {
updateAllColorProperties();
}
// guard 自动递减
}
// updateAllColorProperties() 使用 RAII 守卫
void DQuickControlColorSelector::updateAllColorProperties()
{
UpdateDepthGuard guard(m_updateDepth);
for (int i = 0; i < m_metaObject->count(); ++i) {
auto p = m_metaObject->name(i);
if (p.isEmpty())
continue;
updatePropertyFromName(p);
}
// guard 自动递减
}本报告由 AI 代码审查工具自动生成 |
52cyb
marked this pull request as draft
September 10, 2026 07:10
Contributor
|
TAG Bot New tag: 6.7.49 |
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
2 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.
_d_getControlPaletteForResolve(): resolve typed colors (Highlight / HighlightedText) from the stable application palette, and read only the Window role (theme detection) from the control -- never via QQuickPalette::toQPalette(), which reads back the DTK-overriddenpalette.windowTextand re-triggers its QML binding_d_controlPaletteHasInactiveState(): compare Active vs Inactive groups role-by-role excludingwindowText(a binding artifact, not a real inactive-state difference), avoiding the third windowText read-back point in getColorOf()m_updateDepth) in updateControlTheme() to skip the redundant recompute triggered by thepalette.windowTextbinding writing the paletteInfluence:
fix(palette): 从读侧断开 QML Palette windowText 绑定环
_d_getControlPaletteForResolve()：类型化颜色（Highlight/ HighlightedText）从稳定的应用调色板解析，仅 Window 角色（主题探测）从控件读取 —— 不再走 QQuickPalette::toQPalette()，避免读回被 DTK 覆盖的palette.windowText而重触发其 QML 绑定_d_controlPaletteHasInactiveState()：逐角色比较 Active/Inactive 组并排除windowText（绑定产物，非真实 inactive 差异），消除 getColorOf() 中第三个 windowText 读回点m_updateDepth），跳过由palette.windowText绑定写 palette 触发的冗余重算Influence:
PMS: Task-392413
Summary by Sourcery
Break Qt 6 palette windowText binding loops from the read side while preserving correct control colors across themes and inactive states.
Bug Fixes:
Enhancements: