feat: add card surface plugin interface for QML-based plugins#474
Open
wjyrich wants to merge 1 commit into
Open
feat: add card surface plugin interface for QML-based plugins#474wjyrich wants to merge 1 commit into
wjyrich wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wjyrich 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 GuideAdds a QML-based card surface plugin interface (PluginsItemInterfaceV3) and a CardPluginItem loader path, wires it into the existing plugin manager/loader to create and manage Wayland-backed QQuickView card surfaces, and updates the brightness dock plugin to implement V3 with a QML card, while preserving backward compatibility for V2/legacy plugins. Sequence diagram for creating a QML card surface on itemAddedsequenceDiagram
participant PluginManager
participant BrightnessPlugin
participant WidgetPlugin
participant CardPluginItem
participant EmbedPlugin
PluginManager->>BrightnessPlugin: loadPlugin(pluginFilePath)
BrightnessPlugin->>PluginManager: instance implements PluginsItemInterfaceV3
PluginManager->>WidgetPlugin: new WidgetPlugin(pluginsItemInterface)
BrightnessPlugin->>WidgetPlugin: itemAdded(this, cardItemKey())
WidgetPlugin->>WidgetPlugin: createCardItemIfNeeded(itemInter, itemKey)
WidgetPlugin->>CardPluginItem: new CardPluginItem(cardInterface, itemKey, this)
CardPluginItem->>CardPluginItem: init()
CardPluginItem->>CardPluginItem: QQuickView setSource(cardQmlSource())
WidgetPlugin->>EmbedPlugin: Plugin::EmbedPlugin::get(CardPluginItem.window())
EmbedPlugin->>EmbedPlugin: setPluginType(Plugin::EmbedPlugin::Card)
EmbedPlugin->>WidgetPlugin: dockColorThemeChanged(uint32_t)
WidgetPlugin->>CardPluginItem: setDockColorTheme(int)
EmbedPlugin->>CardPluginItem: eventGeometry(QRect)
CardPluginItem->>CardPluginItem: resize(QSize)
WidgetPlugin->>CardPluginItem: show()
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, and left some high level feedback:
- CardPluginItem instances are constructed with WidgetPlugin as parent and also deleted via qDeleteAll(m_cardItems) in the destructor, which can lead to double deletion; either drop the parent relationship or remove qDeleteAll and rely on QObject ownership.
- In createCardItemIfNeeded(), when cardItem->init() or window() fails you still return true, which prevents the normal widget path from being created; consider returning false on failure so the plugin can gracefully fall back to the non-card implementation.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- CardPluginItem instances are constructed with WidgetPlugin as parent and also deleted via qDeleteAll(m_cardItems) in the destructor, which can lead to double deletion; either drop the parent relationship or remove qDeleteAll and rely on QObject ownership.
- In createCardItemIfNeeded(), when cardItem->init() or window() fails you still return true, which prevents the normal widget path from being created; consider returning false on failure so the plugin can gracefully fall back to the non-card implementation.
## Individual Comments
### Comment 1
<location path="src/loader/widgetplugin.cpp" line_range="382" />
<code_context>
return Plugin::EmbedPlugin::get(widget->windowHandle());
}
+bool WidgetPlugin::createCardItemIfNeeded(PluginsItemInterface *itemInter, const QString &itemKey)
+{
+ auto cardInterface = dynamic_cast<PluginsItemInterfaceV3 *>(itemInter);
</code_context>
<issue_to_address>
**issue (complexity):** Consider separating card-item classification from lifecycle management and making the card helpers void and explicitly branched in itemAdded/itemRemoved to clarify control flow and responsibilities.
The added card path does increase complexity, mainly through the overloaded `createCardItemIfNeeded` and its hidden control‑flow contract. You can simplify the logic and make `itemAdded`/`itemRemoved` easier to reason about by:
1. **Separate classification from lifecycle**
Extract a small helper that decides “is this a card item?” and use it to branch explicitly in `itemAdded`/`itemRemoved`. Then make the lifecycle helper `void` so its return value no longer encodes behavior.
```cpp
// New helper
bool WidgetPlugin::isCardItem(PluginsItemInterface *itemInter, const QString &itemKey) const
{
auto cardInterface = dynamic_cast<PluginsItemInterfaceV3 *>(itemInter);
return cardInterface && cardInterface->cardItemKey() == itemKey;
}
// Adjusted itemAdded
void WidgetPlugin::itemAdded(PluginsItemInterface * const itemInter, const QString &itemKey)
{
qDebug() << "itemAdded:" << itemKey;
if (isCardItem(itemInter, itemKey)) {
ensureCardItem(itemInter, itemKey);
return;
}
auto flag = getPluginFlags();
if (flag & Dock::Type_Quick) {
// existing quick path...
}
// existing normal widget path...
}
```
2. **Make card lifecycle explicit and non‑boolean**
Rename `createCardItemIfNeeded` to something like `ensureCardItem` and change the signature to `void`. Keep the semantics identical: normal widget path should never run for card items.
```cpp
// Refactored from createCardItemIfNeeded
void WidgetPlugin::ensureCardItem(PluginsItemInterface *itemInter, const QString &itemKey)
{
auto cardInterface = static_cast<PluginsItemInterfaceV3 *>(itemInter);
if (auto existing = m_cardItems.value(itemKey)) {
existing->show();
return;
}
auto cardItem = new CardPluginItem(cardInterface, itemKey, this);
if (!cardItem->init() || !cardItem->window()) {
cardItem->deleteLater();
qWarning() << "create card plugin surface failed" << itemInter->pluginName() << itemKey;
return; // still block normal widget path
}
auto plugin = Plugin::EmbedPlugin::get(cardItem->window());
plugin->setPluginFlags(getPluginFlags());
plugin->setPluginId(itemInter->pluginName());
plugin->setDisplayName(itemInter->pluginDisplayName());
plugin->setItemKey(itemKey);
plugin->setPluginType(Plugin::EmbedPlugin::Card);
plugin->setPluginSizePolicy(itemInter->pluginSizePolicy());
connect(plugin, &Plugin::EmbedPlugin::dockColorThemeChanged,
this, &WidgetPlugin::onDockColorThemeChanged, Qt::UniqueConnection);
connect(plugin, &Plugin::EmbedPlugin::dockColorThemeChanged,
cardItem, [cardItem](uint32_t colorTheme) {
cardItem->setDockColorTheme(static_cast<int>(colorTheme));
});
connect(plugin, &Plugin::EmbedPlugin::eventGeometry,
cardItem, [cardItem](const QRect &geometry) {
cardItem->resize(geometry.size());
});
m_cardItems.insert(itemKey, cardItem);
cardItem->show();
}
```
3. **Mirror the explicit branching in `itemRemoved`**
Keep the current behavior, but make the “card vs normal widget” decision obvious:
```cpp
void WidgetPlugin::itemRemoved(PluginsItemInterface * const itemInter, const QString &itemKey)
{
Q_UNUSED(itemInter);
if (auto cardItem = m_cardItems.take(itemKey)) {
cardItem->hide();
cardItem->deleteLater();
return;
}
auto widget = m_pluginsItemInterface->itemWidget(itemKey);
if (widget && widget->window() && widget->window()->windowHandle()) {
widget->window()->windowHandle()->hide();
}
auto quickPanel = m_pluginsItemInterface->itemWidget(Dock::QUICK_ITEM_KEY);
if (quickPanel && quickPanel->window() && quickPanel->window()->windowHandle()) {
quickPanel->window()->windowHandle()->hide();
}
}
```
These changes keep all current functionality (including “card creation failure blocks normal widget path”) but make the control flow and responsibilities clearer: classification (`isCardItem`), lifecycle (`ensureCardItem`), and normal widget handling are separated and easier to follow.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| return Plugin::EmbedPlugin::get(widget->windowHandle()); | ||
| } | ||
|
|
||
| bool WidgetPlugin::createCardItemIfNeeded(PluginsItemInterface *itemInter, const QString &itemKey) |
There was a problem hiding this comment.
issue (complexity): Consider separating card-item classification from lifecycle management and making the card helpers void and explicitly branched in itemAdded/itemRemoved to clarify control flow and responsibilities.
The added card path does increase complexity, mainly through the overloaded createCardItemIfNeeded and its hidden control‑flow contract. You can simplify the logic and make itemAdded/itemRemoved easier to reason about by:
-
Separate classification from lifecycle
Extract a small helper that decides “is this a card item?” and use it to branch explicitly initemAdded/itemRemoved. Then make the lifecycle helpervoidso its return value no longer encodes behavior.// New helper bool WidgetPlugin::isCardItem(PluginsItemInterface *itemInter, const QString &itemKey) const { auto cardInterface = dynamic_cast<PluginsItemInterfaceV3 *>(itemInter); return cardInterface && cardInterface->cardItemKey() == itemKey; } // Adjusted itemAdded void WidgetPlugin::itemAdded(PluginsItemInterface * const itemInter, const QString &itemKey) { qDebug() << "itemAdded:" << itemKey; if (isCardItem(itemInter, itemKey)) { ensureCardItem(itemInter, itemKey); return; } auto flag = getPluginFlags(); if (flag & Dock::Type_Quick) { // existing quick path... } // existing normal widget path... }
-
Make card lifecycle explicit and non‑boolean
RenamecreateCardItemIfNeededto something likeensureCardItemand change the signature tovoid. Keep the semantics identical: normal widget path should never run for card items.// Refactored from createCardItemIfNeeded void WidgetPlugin::ensureCardItem(PluginsItemInterface *itemInter, const QString &itemKey) { auto cardInterface = static_cast<PluginsItemInterfaceV3 *>(itemInter); if (auto existing = m_cardItems.value(itemKey)) { existing->show(); return; } auto cardItem = new CardPluginItem(cardInterface, itemKey, this); if (!cardItem->init() || !cardItem->window()) { cardItem->deleteLater(); qWarning() << "create card plugin surface failed" << itemInter->pluginName() << itemKey; return; // still block normal widget path } auto plugin = Plugin::EmbedPlugin::get(cardItem->window()); plugin->setPluginFlags(getPluginFlags()); plugin->setPluginId(itemInter->pluginName()); plugin->setDisplayName(itemInter->pluginDisplayName()); plugin->setItemKey(itemKey); plugin->setPluginType(Plugin::EmbedPlugin::Card); plugin->setPluginSizePolicy(itemInter->pluginSizePolicy()); connect(plugin, &Plugin::EmbedPlugin::dockColorThemeChanged, this, &WidgetPlugin::onDockColorThemeChanged, Qt::UniqueConnection); connect(plugin, &Plugin::EmbedPlugin::dockColorThemeChanged, cardItem, [cardItem](uint32_t colorTheme) { cardItem->setDockColorTheme(static_cast<int>(colorTheme)); }); connect(plugin, &Plugin::EmbedPlugin::eventGeometry, cardItem, [cardItem](const QRect &geometry) { cardItem->resize(geometry.size()); }); m_cardItems.insert(itemKey, cardItem); cardItem->show(); }
-
Mirror the explicit branching in
itemRemoved
Keep the current behavior, but make the “card vs normal widget” decision obvious:void WidgetPlugin::itemRemoved(PluginsItemInterface * const itemInter, const QString &itemKey) { Q_UNUSED(itemInter); if (auto cardItem = m_cardItems.take(itemKey)) { cardItem->hide(); cardItem->deleteLater(); return; } auto widget = m_pluginsItemInterface->itemWidget(itemKey); if (widget && widget->window() && widget->window()->windowHandle()) { widget->window()->windowHandle()->hide(); } auto quickPanel = m_pluginsItemInterface->itemWidget(Dock::QUICK_ITEM_KEY); if (quickPanel && quickPanel->window() && quickPanel->window()->windowHandle()) { quickPanel->window()->windowHandle()->hide(); } }
These changes keep all current functionality (including “card creation failure blocks normal widget path”) but make the control flow and responsibilities clearer: classification (isCardItem), lifecycle (ensureCardItem), and normal widget handling are separated and easier to follow.
18202781743
reviewed
Jul 7, 2026
| void WidgetPlugin::itemAdded(PluginsItemInterface * const itemInter, const QString &itemKey) | ||
| { | ||
| qDebug() << "itemAdded:" << itemKey; | ||
| if (createCardItemIfNeeded(itemInter, itemKey)) { |
18202781743
reviewed
Jul 7, 2026
| * The loader creates a QQuickView for this URL and exposes it to the dock | ||
| * compositor as a Wayland surface. | ||
| */ | ||
| virtual QUrl cardQmlSource() const |
Contributor
There was a problem hiding this comment.
这里返回一个qwindow是不是更好，让应用控制，这样也能支持qml和qwidget，
|
TAG Bot New tag: 2.0.36 |
wjyrich
force-pushed
the
CardPlugin
branch
2 times, most recently
from
db8987a to
6c683d2
Compare
UI Add a new plugin interface version (V3) supporting card surface rendering via native QWindow, enabling richer plugin UI like media player controls. Implement the media plugin card feature with QML- based music player card, previous song support, and improved metadata handling. - Add PluginsItemInterfaceV3 with cardItemKey() and cardWindow() for native window rendering - Add Attribute_HasCard plugin flag to identify card-capable plugins - Implement CardPluginItem in loader to manage card windows as EmbedPlugin - Add media card QML UI with playback controls, artwork display, and metadata - Add previous track support and playback toggle to MediaController - Improve MPRIS metadata handling (artist list, title, artwork path) - Fix plugin unload issues with QPointer guard in EmbedPlugin - Update media plugin to use V3 interface and create card surface - Add play-previous SVG icon and update CMakeLists for QtQuick dependencies Log: Added card surface support for dock plugins and music player card UI Influence: 1. Test media plugin card appears when music player is active 2. Verify card artwork display with various mpris:artUrl sources 3. Test playback controls (play/pause, next, previous) via card 4. Verify card auto-hides when plugin/player is removed 5. Test plugin loading with V2 backwards compatibility 6. Test card window resize and positioning in dock 7. Verify metadata display (title, artist) update in real-time 8. Test hover behavior and control animations in card UI 9. Test clean plugin unload and resource cleanup 10. Verify card works with different system themes (light/dark) feat: 为任务栏插件添加卡片表面支持及音乐播放器卡片UI 新增插件接口版本V3，支持通过原生QWindow渲染卡片表面，实现更丰富的插件 UI（如媒体播放控制）。实现媒体插件的卡片功能，包含基于QML的音乐播放器卡 片、上一曲支持和改进的元数据处理。 - 新增PluginsItemInterfaceV3接口，提供cardItemKey()和cardWindow()实现原 生窗口渲染 - 新增Attribute_HasCard插件标志，用于标识支持卡片的插件 - 在加载器中实现CardPluginItem，管理作为EmbedPlugin的卡片窗口 - 添加媒体卡片QML UI，包含播放控制、封面显示和元数据展示 - 在MediaController中添加上一曲支持和播放切换功能 - 改进MPRIS元数据处理（艺术家列表、标题、封面路径） - 使用QPointer守卫修复EmbedPlugin中的插件卸载问题 - 更新媒体插件使用V3接口并创建卡片表面 - 添加上一曲SVG图标并更新CMakeLists添加QtQuick依赖 Log: 新增任务栏插件卡片表面功能和音乐播放器卡片UI Influence: 1. 测试音乐播放器活动时媒体插件卡片正确显示 2. 验证卡片封面图在不同mpris:artUrl源下的显示效果 3. 测试通过卡片进行播放控制（播放/暂停、下一曲、上一曲） 4. 验证插件/播放器移除时卡片自动隐藏 5. 测试V2版本插件的向后兼容加载 6. 测试卡片窗口在任务栏中的尺寸调整和定位 7. 验证元数据（标题、艺术家）实时更新 8. 测试卡片UI的悬停行为和控件动画 9. 测试插件的干净卸载和资源清理 10. 验证卡片在不同系统主题（浅色/深色）下的显示效果
deepin pr auto review★ 总体评分：95分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // src/tray-wayland-integration/plugin.cpp
// 修复 Use-After-Free 缺陷，使用 QPointer 守护对象生命周期
QPointer<EmbedPlugin> pluginGuard(plugin);
QObject::connect(window, &QWindow::visibleChanged, window, [window, pluginGuard] (bool visible) {
if (!visible) {
if (!pluginGuard) {
s_map.remove(window);
return;
}
pluginGuard->deleteLater();
s_map.remove(window);
}
}); |
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
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.
Introduce PluginsItemInterfaceV3 extending V2 with virtual functions for card item key, QML source, icon source and preferred size. Implement CardPluginItem class and integrate into loader to support creating card QQuickView surfaces. Brightness plugin updated to V3 with a simple QML card showing icon, title and slider.
Log: Added card surface support for dock plugins with QML implementation
Influence:
feat: 添加基于 QML 的卡片表面插件接口
引入 PluginsItemInterfaceV3 扩展 V2，增加卡片项键、QML 源、图标源和首 选大小的虚函数。实现 CardPluginItem 类并集成到加载器中，支持创建卡片
QQuickView 表面。亮度插件更新至 V3，使用简单的 QML 卡片显示图标、标题和
滑块。
Log: 新增卡片表面支持，插件可使用 QML 实现
Influence:
Summary by Sourcery
Introduce a V3 plugins item interface and loader support for QML-based card surfaces, and adopt it in the brightness dock plugin.
New Features:
Enhancements:
Build: