-
Notifications
You must be signed in to change notification settings - Fork 45
fix: skip inactive color when active and inactive colors are equal #573
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds a runtime check before using the inactive color group so that inactive-color processing is skipped when a control’s QPalette has identical active and inactive color groups, preventing unnecessary color transformations and potential visual inconsistencies. Sequence diagram for inactive color selection with QPalette equality checksequenceDiagram
participant Control as DQuickControl
participant ColorSelector as DQuickControlColorSelector
participant Helper as DGuiApplicationHelper
participant QPal as QPalette
Control->>ColorSelector: getColorOf(palette)
ColorSelector->>Helper: testAttribute(UseInactiveColorGroup)
Helper-->>ColorSelector: useInactiveColor
alt UseInactiveColorGroup is true
ColorSelector->>Control: _d_getControlPalette(control)
Control-->>ColorSelector: qpalette
ColorSelector->>QPal: isEqual(Inactive, Active)
QPal-->>ColorSelector: areEqual
alt Inactive and Active groups are equal
ColorSelector->>ColorSelector: useInactiveColor = false
else Groups differ
ColorSelector->>ColorSelector: useInactiveColor = true
end
else UseInactiveColorGroup is false
ColorSelector->>ColorSelector: useInactiveColor = false
end
alt useInactiveColor and controlState is InactiveState
ColorSelector->>Helper: standardPalette(controlTheme)
Helper-->>ColorSelector: standardPalette
ColorSelector->>Helper: paletteColor(Window)
Helper-->>ColorSelector: inactiveWindowColor
ColorSelector-->>Control: return inactiveWindowColor
else
ColorSelector-->>Control: return activeColorOrOther
end
Class diagram for updated color selection logic in DQuickControlColorSelectorclassDiagram
class DQuickControlColorSelector {
- DQuickControl m_control
QColor getColorOf(DQuickControlPalette palette)
}
class DQuickControlPalette {
// Palette data for quick controls
}
class DGuiApplicationHelper {
+ static bool testAttribute(int attribute)
+ static QPalette standardPalette(int controlTheme)
<<static>> UseInactiveColorGroup
}
class QPalette {
+ bool isEqual(QPalette::ColorGroup group1, QPalette::ColorGroup group2)
+ QColor color(QPalette::ColorRole role)
<<enum>> ColorGroup
<<enum>> ColorRole
<<enum>> Inactive
<<enum>> Active
<<enum>> Window
}
class DQMLGlobalObject {
<<enum>> InactiveState
}
DQuickControlColorSelector --> DQuickControlPalette : uses
DQuickControlColorSelector --> DGuiApplicationHelper : calls
DQuickControlColorSelector --> QPalette : compares groups
DQuickControlColorSelector --> DQMLGlobalObject : reads controlState
DGuiApplicationHelper --> QPalette : returns standardPalette
Flow diagram for deciding whether to use inactive color groupflowchart TD
A[Start getColorOf] --> B[Call testAttribute UseInactiveColorGroup]
B --> C{Attribute enabled?}
C -- No --> D[useInactiveColor = false]
D --> G[Check controlState and return color]
C -- Yes --> E{m_control exists?}
E -- No --> G
E -- Yes --> F[Get qpalette from _d_getControlPalette]
F --> H[Call qpalette.isEqual Inactive Active]
H --> I{Inactive equals Active?}
I -- Yes --> J[useInactiveColor = false]
I -- No --> K[useInactiveColor = true]
J --> G
K --> G
G --> L{useInactiveColor and controlState is InactiveState?}
L -- Yes --> M[Use standardPalette Inactive colors]
L -- No --> N[Use active or default color]
M --> O[Return selected color]
N --> O[Return selected color]
O --> P[End]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review这段代码主要是在获取控件颜色时,增加了对 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
改进建议综合上述分析,代码逻辑正确且安全。为了进一步提升代码质量,建议做如下微调:
改进后的代码示例: // items with inactive state should use inactive color, it likes dtkgui's generatePaletteColor_helper.
// 检查是否使用非激活颜色。如果控件存在,还需检查控件自身的非激活与激活调色板是否一致,
// 若一致则无需特殊处理非激活状态。
bool useInactiveColor = DGuiApplicationHelper::testAttribute(DGuiApplicationHelper::UseInactiveColorGroup);
if (useInactiveColor) {
if (m_control) {
const auto qpalette = _d_getControlPalette(m_control);
// 如果控件的非激活色和激活色定义相同,则不应用非激活色逻辑
useInactiveColor = !qpalette.isEqual(QPalette::Inactive, QPalette::Active);
}
}
if (useInactiveColor && state->controlState == DQMLGlobalObject::InactiveState) {
const auto &palette = DGuiApplicationHelper::standardPalette(state->controlTheme);
const auto &windowColor = palette.color(QPalette::Window);
// ... 后续代码总结: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've left some high level feedback:
- The previous
static const auto useInactiveColorwas evaluated once, but nowtestAttributeand_d_getControlPaletterun on every call togetColorOf; consider caching these results (e.g., per-control or via a small helper) if this path is performance-sensitive. - The new
isEqual(QPalette::Inactive, QPalette::Active)check compares full palette groups, which may be heavier than necessary; if only specific roles drive the inactive-color behavior here, comparing just those roles could simplify the logic and reduce overhead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The previous `static const auto useInactiveColor` was evaluated once, but now `testAttribute` and `_d_getControlPalette` run on every call to `getColorOf`; consider caching these results (e.g., per-control or via a small helper) if this path is performance-sensitive.
- The new `isEqual(QPalette::Inactive, QPalette::Active)` check compares full palette groups, which may be heavier than necessary; if only specific roles drive the inactive-color behavior here, comparing just those roles could simplify the logic and reduce overhead.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
When the inactive and active colors in QPalette are identical, the system should not apply inactive color processing to avoid unnecessary color calculations and potential visual inconsistencies. This change adds a check to bypass inactive color usage when the QPalette's inactive and active color groups are equal, ensuring more efficient rendering and consistent visual behavior. Log: Fixed an issue where inactive color processing was incorrectly applied when active and inactive colors were identical, improving rendering efficiency and visual consistency. Influence: 1. Test UI components with identical active and inactive QPalette colors to ensure they render correctly 2. Verify that components with different active/inactive colors still properly apply inactive colors when appropriate 3. Check performance impact when switching between color states 4. Test with various control types to ensure consistent behavior across the application 5. Verify that the fix doesn't break existing inactive state visual effects fix: 当QPalette中非活动色和活动色相同时跳过非活动色处理 当QPalette中的非活动色和活动色相同时,系统不应应用非活动色处理,以避免 不必要的颜色计算和潜在的视觉不一致。此更改添加了一个检查,当QPalette的非 活动色和活动色组相等时,跳过非活动色的使用,确保更高效的渲染和一致的视觉 行为。 Log: 修复了当活动色和非活动色相同时错误应用非活动色处理的问题,提高了渲 染效率和视觉一致性。 Influence: 1. 测试具有相同活动和非活动QPalette颜色的UI组件,确保它们正确渲染 2. 验证具有不同活动/非活动颜色的组件在适当时仍能正确应用非活动色 3. 检查在不同颜色状态之间切换时的性能影响 4. 测试各种控件类型,确保在整个应用程序中行为一致 5. 验证此修复不会破坏现有的非活动状态视觉效果 PMS: BUG-298841
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia 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 |
When the inactive and active colors in QPalette are identical, the
system should not apply inactive color processing to avoid unnecessary
color calculations and potential visual inconsistencies. This change
adds a check to bypass inactive color usage when the QPalette's inactive
and active color groups are equal, ensuring more efficient rendering and
consistent visual behavior.
Log: Fixed an issue where inactive color processing was incorrectly
applied when active and inactive colors were identical, improving
rendering efficiency and visual consistency.
Influence:
to ensure they render correctly
properly apply inactive colors when appropriate
the application
effects
fix: 当QPalette中非活动色和活动色相同时跳过非活动色处理
当QPalette中的非活动色和活动色相同时,系统不应应用非活动色处理,以避免
不必要的颜色计算和潜在的视觉不一致。此更改添加了一个检查,当QPalette的非
活动色和活动色组相等时,跳过非活动色的使用,确保更高效的渲染和一致的视觉
行为。
Log: 修复了当活动色和非活动色相同时错误应用非活动色处理的问题,提高了渲
染效率和视觉一致性。
Influence:
PMS: BUG-298841
Summary by Sourcery
Bug Fixes: