From 64b10c852f4b8662d1cfbeb2a5a5032782616485 Mon Sep 17 00:00:00 2001 From: DTK Agent Date: Tue, 8 Sep 2026 21:07:31 +0800 Subject: [PATCH] fix: break QML Palette windowText binding loop in DTK controls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 控制中心中操作 Button 时出现 "QML Palette: Binding loop detected for property windowText" (Button.qml:27:5) 警告。经分析存在两条独立的 binding loop，一并修复。 环路 A — ColorSelector 反馈链 (dquickcontrolpalette.cpp): palette.windowText: D.ColorSelector.textColor 求值 → 写 windowText → QQuickPalette::changed → updateControlTheme() → updateAllColorProperties() → updatePropertyFromName("textColor") → getColorOf() → 读含 windowText 的完整 palette → isEqual(Inactive, Active) 读 windowText 致 inactive 闸门翻转 → 若 textColor 值变 → textColorChanged → windowText 绑定重入 → 报环 修复: - 新增 _d_getControlPaletteForResolve()，typed/主题读点只读 Window/Highlight/HighlightedText 三角色，不读 WindowText - 新增 _d_controlPaletteHasInactiveState()，isEqual 排除 WindowText， 防止 inactive 闸门因 DTK 自反写入而翻转 - updateControlTheme() 入口加去重守卫，防止 changed 与 paletteChanged 双触发导致重复 updateAllColorProperties() 环路 B — makeIconPalette 读回 (dqmlglobalobject.cpp + QML): palette.windowText: D.ColorSelector.textColor 求值 → 写 windowText → QQuickPalette::changed → paletteChanged [Qt 内部] → D.DciIcon.palette: D.DTK.makeIconPalette(palette) 重新求值 → makeIconPalette() 内部 palette->windowText() 读 windowText → 此时 windowText 绑定仍在求值栈上 → 报环 修复: - 新增 makeIconPalette(const QQuickPalette*, const QColor&) 重载， 前景色由 QML 显式传入，不再从 C++ 读 palette->windowText() - 8 个控件 (Button/ToolButton/MenuItem/TitleBar/ActionButton/ ItemDelegate/SpinBoxIndicator/Switch) 的 makeIconPalette 调用 传入与 palette.windowText 绑定等价的前景色参数 受影响控件: Button, ToolButton, MenuItem, TitleBar, ActionButton, ItemDelegate, SpinBoxIndicator, Switch Log: 修复 QML Palette windowText binding loop Change-Id: Ie3123e24698a4094aec48d1f7602268b --- qt6/src/qml/ActionButton.qml | 2 +- qt6/src/qml/Button.qml | 2 +- qt6/src/qml/ItemDelegate.qml | 4 +- qt6/src/qml/MenuItem.qml | 2 +- qt6/src/qml/SpinBoxIndicator.qml | 2 +- qt6/src/qml/Switch.qml | 2 +- qt6/src/qml/TitleBar.qml | 2 +- qt6/src/qml/ToolButton.qml | 2 +- src/private/dqmlglobalobject.cpp | 16 +++ src/private/dqmlglobalobject_p.h | 9 ++ src/private/dquickcontrolpalette.cpp | 172 ++++++++++++++++++++++++++- src/private/dquickcontrolpalette_p.h | 8 ++ 12 files changed, 211 insertions(+), 12 deletions(-) diff --git a/qt6/src/qml/ActionButton.qml b/qt6/src/qml/ActionButton.qml index 082b72bad..b1e316ac7 100644 --- a/qt6/src/qml/ActionButton.qml +++ b/qt6/src/qml/ActionButton.qml @@ -20,7 +20,7 @@ T.Button { height: DS.Style.button.iconSize } contentItem: D.DciIcon { - palette: D.DTK.makeIconPalette(control.palette) + palette: D.DTK.makeIconPalette(control.palette, control.pressed ? control.D.ColorSelector.textColor : undefined) mode: control.D.ColorSelector.controlState theme: control.D.ColorSelector.controlTheme name: control.icon.name diff --git a/qt6/src/qml/Button.qml b/qt6/src/qml/Button.qml index 60f842335..dd043a0dc 100644 --- a/qt6/src/qml/Button.qml +++ b/qt6/src/qml/Button.qml @@ -23,7 +23,7 @@ T.Button { opacity: D.ColorSelector.controlState === D.DTK.DisabledState ? 0.4 : 1 D.DciIcon.mode: D.ColorSelector.controlState D.DciIcon.theme: D.ColorSelector.controlTheme - D.DciIcon.palette: D.DTK.makeIconPalette(palette) + D.DciIcon.palette: D.DTK.makeIconPalette(palette, D.ColorSelector.textColor) palette.windowText: D.ColorSelector.textColor icon { width: DS.Style.button.iconSize diff --git a/qt6/src/qml/ItemDelegate.qml b/qt6/src/qml/ItemDelegate.qml index 3f9af5204..23c475657 100644 --- a/qt6/src/qml/ItemDelegate.qml +++ b/qt6/src/qml/ItemDelegate.qml @@ -50,7 +50,7 @@ T.ItemDelegate { D.DciIcon.mode: D.ColorSelector.controlState D.DciIcon.theme: D.ColorSelector.controlTheme - D.DciIcon.palette: D.DTK.makeIconPalette(palette) + D.DciIcon.palette: D.DTK.makeIconPalette(palette, checked && !control.cascadeSelected && (D.DTK.hasAnimation ? control.backgroundVisible && !dragActive : true) ? D.ColorSelector.checkedTextColor : undefined) icon { width: DS.Style.itemDelegate.iconSize height: DS.Style.itemDelegate.iconSize @@ -64,7 +64,7 @@ T.ItemDelegate { active: control.indicatorVisible && control.checked sourceComponent: D.DciIcon { - palette: D.DTK.makeIconPalette(control.palette) + palette: D.DTK.makeIconPalette(control.palette, control.D.ColorSelector.checkedTextColor) mode: control.D.ColorSelector.controlState theme: control.D.ColorSelector.controlTheme fallbackToQIcon: false diff --git a/qt6/src/qml/MenuItem.qml b/qt6/src/qml/MenuItem.qml index ea6d6e8a3..be1579132 100644 --- a/qt6/src/qml/MenuItem.qml +++ b/qt6/src/qml/MenuItem.qml @@ -30,7 +30,7 @@ T.MenuItem { palette.windowText: D.ColorSelector.textColor D.DciIcon.mode: D.ColorSelector.controlState D.DciIcon.theme: D.ColorSelector.controlTheme - D.DciIcon.palette: D.DTK.makeIconPalette(palette) + D.DciIcon.palette: D.DTK.makeIconPalette(palette, D.ColorSelector.textColor) contentItem: D.IconLabel { readonly property real arrowPadding: control.subMenu && control.arrow ? control.arrow.width + control.spacing : 0 readonly property real indicatorPadding: control.useIndicatorPadding && control.indicator ? control.indicator.width + control.spacing : 0 diff --git a/qt6/src/qml/SpinBoxIndicator.qml b/qt6/src/qml/SpinBoxIndicator.qml index dbe538d72..a9d7cc791 100644 --- a/qt6/src/qml/SpinBoxIndicator.qml +++ b/qt6/src/qml/SpinBoxIndicator.qml @@ -31,7 +31,7 @@ Control { D.DciIcon { id: icon sourceSize.width: DS.Style.spinBox.indicator.iconSize - palette: D.DTK.makeIconPalette(control.palette) + palette: D.DTK.makeIconPalette(control.palette, control.D.ColorSelector.inactiveBackgroundColor) name: direction === SpinBoxIndicator.IndicatorDirection.UpIndicator ? "entry_spinbox_up" : "entry_spinbox_down" mode: control.D.ColorSelector.controlState theme: control.D.ColorSelector.controlTheme diff --git a/qt6/src/qml/Switch.qml b/qt6/src/qml/Switch.qml index a79a8623f..1c12b95fe 100644 --- a/qt6/src/qml/Switch.qml +++ b/qt6/src/qml/Switch.qml @@ -74,7 +74,7 @@ T.Switch { sourceSize: Qt.size(DS.Style.switchButton.indicatorWidth, DS.Style.switchButton.indicatorWidth) opacity: control.D.ColorSelector.controlState === D.DTK.DisabledState && control.checked ? 0.4 : 1 - palette: DTK.makeIconPalette(control.palette) + palette: DTK.makeIconPalette(control.palette, control.palette.windowText) mode: control.D.ColorSelector.controlState theme: control.D.ColorSelector.controlTheme fallbackToQIcon: false diff --git a/qt6/src/qml/TitleBar.qml b/qt6/src/qml/TitleBar.qml index 9291d3fb2..2faad59f7 100644 --- a/qt6/src/qml/TitleBar.qml +++ b/qt6/src/qml/TitleBar.qml @@ -122,7 +122,7 @@ Item { } Layout.alignment: Qt.AlignLeft visible: name - palette: D.DTK.makeIconPalette(control.palette) + palette: D.DTK.makeIconPalette(control.palette, D.ColorSelector.textColor) mode: control.D.ColorSelector.controlState theme: control.D.ColorSelector.controlTheme } diff --git a/qt6/src/qml/ToolButton.qml b/qt6/src/qml/ToolButton.qml index ad7ef9dc0..4dc528857 100644 --- a/qt6/src/qml/ToolButton.qml +++ b/qt6/src/qml/ToolButton.qml @@ -27,7 +27,7 @@ T.ToolButton { opacity: D.ColorSelector.controlState === D.DTK.DisabledState ? 0.4 : 1 D.DciIcon.mode: D.ColorSelector.controlState D.DciIcon.theme: D.ColorSelector.controlTheme - D.DciIcon.palette: D.DTK.makeIconPalette(palette) + D.DciIcon.palette: D.DTK.makeIconPalette(palette, D.ColorSelector.textColor) palette.windowText: D.ColorSelector.textColor D.ColorSelector.family: D.Palette.CrystalColor display: D.IconLabel.TextUnderIcon diff --git a/src/private/dqmlglobalobject.cpp b/src/private/dqmlglobalobject.cpp index 643e888c0..a826be737 100644 --- a/src/private/dqmlglobalobject.cpp +++ b/src/private/dqmlglobalobject.cpp @@ -440,6 +440,22 @@ DDciIconPalette DQMLGlobalObject::makeIconPalette(const QQuickPalette *palette) iconPalette.setHighlightForeground(palette->highlightedText()); return iconPalette; } + +DDciIconPalette DQMLGlobalObject::makeIconPalette(const QQuickPalette *palette, const QColor &foreground) +{ + DDciIconPalette iconPalette; + // Foreground is passed explicitly from QML (the same value bound to palette.windowText), + // so we never read palette->windowText() here — that C++ read was the fourth windowText + // read point that caused the binding loop. When foreground is invalid (QML passed + // undefined / reset), fall back to palette->windowText(): in that case the windowText + // binding evaluates to undefined (a reset), so it is not in a write state and reading + // it cannot re-enter the binding. + iconPalette.setForeground(foreground.isValid() ? foreground : palette->windowText()); + iconPalette.setBackground(palette->window()); + iconPalette.setHighlight(palette->highlight()); + iconPalette.setHighlightForeground(palette->highlightedText()); + return iconPalette; +} #endif bool DQMLGlobalObject::sendMessage(QObject *target, const QString &content, const QString &iconName, int duration, const QString &msgId) diff --git a/src/private/dqmlglobalobject_p.h b/src/private/dqmlglobalobject_p.h index e2d24d55b..cb514d0f0 100644 --- a/src/private/dqmlglobalobject_p.h +++ b/src/private/dqmlglobalobject_p.h @@ -228,6 +228,15 @@ class DQMLGlobalObject : public QObject, public DTK_CORE_NAMESPACE::DObject Q_INVOKABLE DTK_GUI_NAMESPACE::DDciIconPalette makeIconPalette(const QPalette &palette); #else Q_INVOKABLE DTK_GUI_NAMESPACE::DDciIconPalette makeIconPalette(const QQuickPalette *palette); + // Overload that takes the foreground color explicitly from QML instead of reading + // palette->windowText() in C++. This breaks the binding loop: when palette.windowText + // is bound to D.ColorSelector.textColor, reading it back via C++ during the + // paletteChanged-triggered re-evaluation of makeIconPalette re-enters the windowText + // binding. By passing the same value from QML (which depends on textColorChanged, not + // paletteChanged), the loop is broken. When foreground is invalid (QML passes undefined + // / reset), falls back to palette->windowText() — safe because the windowText binding + // evaluates to undefined (reset), not a write. + Q_INVOKABLE DTK_GUI_NAMESPACE::DDciIconPalette makeIconPalette(const QQuickPalette *palette, const QColor &foreground); #endif Q_INVOKABLE bool sendMessage(QObject *target, const QString &content, const QString &iconName = QString(), int duration = 4000, const QString &msgId = QString()); diff --git a/src/private/dquickcontrolpalette.cpp b/src/private/dquickcontrolpalette.cpp index a8ea3dcc0..9648e1360 100644 --- a/src/private/dquickcontrolpalette.cpp +++ b/src/private/dquickcontrolpalette.cpp @@ -80,6 +80,124 @@ static inline QPalette _d_getControlPalette(QQuickItem *item) { #endif } +#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) +// Build a palette used only for DTK color resolution: theme detection (which needs +// the Window role) and typed-color resolving (which needs Highlight / +// HighlightedText, see DColor::toColor). +// +// It must NOT be obtained via QQuickPalette::toQPalette(). That function reads +// back every palette role, including the ones DTK itself overrides from QML +// (e.g. `palette.windowText: D.ColorSelector.textColor` on Button / ToolButton / +// MenuItem / ...). Reading such a role re-triggers its QML binding, which then +// resolves the typed color through this very code path and reads the role again +// -- a self-referential cycle that Qt6 reports as +// "Binding loop detected for property windowText". +// +// Resolution only ever needs roles that DTK does not override from QML, so we copy +// just those -- one role at a time, which never touches the overridden roles -- +// and fall back to the DTK standard palette of the current control theme for the +// rest. Each role is taken from the Active group, matching the previous +// `toQPalette().color(role)` behaviour (toQPalette() yields a palette whose +// currentColorGroup stays Active, regardless of the control's state). +static QPalette _d_getControlPaletteForResolve(QQuickItem *item, DGuiApplicationHelper::ColorType theme) +{ + QPalette palette = DGuiApplicationHelper::standardPalette(theme); + if (item) { + if (const QQuickPalette *pa = item->property("palette").value()) { + if (QQuickColorGroup *active = pa->active()) { + palette.setColor(QPalette::Window, active->window()); + palette.setColor(QPalette::Highlight, active->highlight()); + palette.setColor(QPalette::HighlightedText, active->highlightedText()); + } + } + } + return palette; +} + +// Tell whether the control's palette actually has a distinct Inactive group, i.e. +// whether it has a real inactive state. Used by getColorOf() to decide whether to +// blend the inactive mask color. +// +// Like _d_getControlPaletteForResolve(), this must NOT go through +// QQuickPalette::toQPalette(): that reads back every role -- including +// `windowText`, which DTK overrides from QML (`palette.windowText: +// D.ColorSelector.*`) -- and would re-trigger that QML binding while it is still +// on the evaluation stack (getColorOf() runs inside `palette.windowText`'s +// binding), producing the same "Binding loop detected for windowText" as the +// typed-color path. This is the third windowText read point on the getColorOf() +// call path and must be broken together with the other two. +// +// We copy every role EXCEPT WindowText, one role at a time, from the control's +// resolved active()/inactive() groups into a fresh QPalette and compare the two +// groups with isEqual(). WindowText is left at its default value, which is +// identical in both groups of a default-constructed QPalette, so it never +// affects the result. Excluding WindowText is not only required to break the +// loop but also semantically correct here: DTK binds `palette.windowText` to a +// D.ColorSelector value, so any active/inactive difference in windowText is an +// artifact of that binding rather than a real inactive state of the control, +// and must not flip the inactive-blend gate (otherwise the inactive text would +// always be blended and look disabled). +// +// A DTK standard palette is deliberately NOT used as the base: its Inactive +// group is already blended (generatePaletteColor_helper), which would make +// Inactive != Active unconditionally and defeat the "no real inactive state" +// gate. +static bool _d_controlPaletteHasInactiveState(QQuickItem *item) +{ + if (!item) + return false; + const QQuickPalette *pa = item->property("palette").value(); + if (!pa) + return false; + QQuickColorGroup *activeGroup = pa->active(); + QQuickColorGroup *inactiveGroup = pa->inactive(); + if (!activeGroup || !inactiveGroup) + return false; + + // QQuickColorGroup::color(ColorRole) is private, so read each role through its + // public per-role accessor. + auto colorOf = [](QQuickColorGroup *g, QPalette::ColorRole r) -> QColor { + switch (r) { + case QPalette::WindowText: return g->windowText(); + case QPalette::Button: return g->button(); + case QPalette::Light: return g->light(); + case QPalette::Midlight: return g->midlight(); + case QPalette::Dark: return g->dark(); + case QPalette::Mid: return g->mid(); + case QPalette::Text: return g->text(); + case QPalette::BrightText: return g->brightText(); + case QPalette::ButtonText: return g->buttonText(); + case QPalette::Base: return g->base(); + case QPalette::Window: return g->window(); + case QPalette::Shadow: return g->shadow(); + case QPalette::Highlight: return g->highlight(); + case QPalette::HighlightedText: return g->highlightedText(); + case QPalette::Link: return g->link(); + case QPalette::LinkVisited: return g->linkVisited(); + case QPalette::AlternateBase: return g->alternateBase(); + case QPalette::NoRole: return QColor(); // no public accessor; equal by default in both groups + 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(); + }; + + QPalette compare; + for (int role = QPalette::WindowText; role < QPalette::NColorRoles; ++role) { + if (role == QPalette::WindowText || role == QPalette::NoRole) + continue; // windowText: DTK overrides it from QML (binding artifact, not a real inactive state). + // NoRole: no public QQuickColorGroup accessor; left at default (equal in both groups). + const auto cr = static_cast(role); + compare.setColor(QPalette::Active, cr, colorOf(activeGroup, cr)); + compare.setColor(QPalette::Inactive, cr, colorOf(inactiveGroup, cr)); + } + return !compare.isEqual(QPalette::Inactive, QPalette::Active); +} +#endif + static QMetaProperty findMetaPropertyFromSignalIndex(const QObject *obj, int signal_index) { QMetaProperty itemProperty; if (signal_index < 0) @@ -838,8 +956,19 @@ QColor DQuickControlColorSelector::getColorOf(const DQuickControlPalette *palett QColor colorValue; if (targetColor.isTypedColor()) { - if (m_control) + if (m_control) { +#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) + // Resolve typed colors (Highlight / HighlightedText) against a palette + // that only carries the non-DTK-overridden roles, so we never read back + // `windowText` (which DTK binds to this very selector) and retrigger its + // QML binding -- the root cause of the "Binding loop detected for + // windowText" warning. Qt5 keeps the original path (no QQuickPalette, + // no loop). + colorValue = targetColor.toColor(_d_getControlPaletteForResolve(m_control, state->controlTheme)); +#else colorValue = targetColor.toColor(_d_getControlPalette(m_control)); +#endif + } } else { colorValue = targetColor.color(); } @@ -855,11 +984,23 @@ QColor DQuickControlColorSelector::getColorOf(const DQuickControlPalette *palett bool shouldBlendInactive = useInactiveColor && state->controlState == DQMLGlobalObject::InactiveState; if (shouldBlendInactive) { if (m_control) { - // If the control's inactive palette is same as active palette, it means the control does not have a real - // inactive state, we should not blend the color with inactive mask color, otherwise it will cause the + // If the control's inactive palette is same as active palette, it means the control does not have a real + // inactive state, we should not blend the color with inactive mask color, otherwise it will cause the // color looks like disabled and hard to recognize. +#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) + // Compare the control's Active/Inactive palette groups without going through + // QQuickPalette::toQPalette() (the third windowText read point on this + // getColorOf() call path -- it would read back the DTK-overwritten + // `windowText` and re-trigger its QML binding, i.e. the same binding loop as + // the typed-color path). The comparison excludes `windowText`: it is the + // only role DTK overrides from QML, so any active/inactive difference there + // is a binding artifact rather than a real inactive state and must not flip + // this gate. See _d_controlPaletteHasInactiveState() for the full rationale. + shouldBlendInactive = _d_controlPaletteHasInactiveState(m_control); +#else const auto qpalette = _d_getControlPalette(m_control); shouldBlendInactive = !qpalette.isEqual(QPalette::Inactive, QPalette::Active); +#endif } } if (shouldBlendInactive) { @@ -1015,12 +1156,34 @@ void DQuickControlColorSelector::updateControlTheme() if (!m_control) return; +#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) + // Deduplicate: a single QQuickPalette::changed emission reaches + // updateControlTheme() through two connections — the direct + // changed→updateControlTheme (line ~590) and the indirect + // changed→paletteChanged→updateControlTheme (line ~597, where paletteChanged + // is Qt6's internal forwarding of changed). Without this guard, one palette + // update triggers two full updateAllColorProperties() passes. The guard + // ensures only one pass per synchronous event-loop iteration. + if (m_updateControlThemeGuard) + return; + m_updateControlThemeGuard = true; + + // Detect the control theme from its Window color without going through + // QQuickPalette::toQPalette(), which would read back the DTK-overwritten + // `windowText` and retrigger its QML binding (binding loop). The Window role + // is never overridden by DTK, so reading it alone is both correct and safe. + const QColor windowColor = _d_getControlPaletteForResolve(m_control, m_state->controlTheme).color(QPalette::Window); +#else const QPalette pa = _d_getControlPalette(m_control); const QColor windowColor = pa.color(QPalette::Window); +#endif if (!windowColor.isValid()) { // When the palette changed, should update the properties if it's DColor type is variant color. updateAllColorProperties(); +#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) + m_updateControlThemeGuard = false; +#endif return; } @@ -1030,6 +1193,9 @@ void DQuickControlColorSelector::updateControlTheme() // When the palette changed, should update the properties if it's DColor type is variant color. updateAllColorProperties(); } +#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) + m_updateControlThemeGuard = false; +#endif } bool DQuickControlColorSelector::updateControlState() diff --git a/src/private/dquickcontrolpalette_p.h b/src/private/dquickcontrolpalette_p.h index fb1631989..74b905499 100644 --- a/src/private/dquickcontrolpalette_p.h +++ b/src/private/dquickcontrolpalette_p.h @@ -420,6 +420,14 @@ class DQuickControlColorSelector : public QObject }; QScopedPointer m_state; QList m_itemParentChangeConnections; +#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) + // Guard against double-invocation of updateControlTheme(): a single + // QQuickPalette::changed emission reaches updateControlTheme() through two + // paths — the direct changed→updateControlTheme connection and the indirect + // changed→paletteChanged→updateControlTheme connection (Qt6 internal). The + // guard ensures only one full re-evaluation per event-loop iteration. + bool m_updateControlThemeGuard = false; +#endif }; DQUICK_END_NAMESPACE