-
Notifications
You must be signed in to change notification settings - Fork 34
fix: improve surface proxy handling for splash screens #673
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
Conversation
|
Skipping CI for Draft Pull Request. |
Reviewer's GuideRefines surface proxy and wrapper behavior to better handle prelaunch splash screens, synchronize proxy delegates/flags with dynamic surface item changes, and improve window title display/fallbacks in the task switcher. Sequence diagram for proxy delegate and flag synchronizationsequenceDiagram
participant Client as ClientCode
participant Proxy as SurfaceProxy
participant SourceWrap as SourceSurfaceWrapper
participant ProxyWrap as ProxySurfaceWrapper
participant SourceItem as SourceWSurfaceItem
participant ProxyItem as ProxyWSurfaceItem
Client->>Proxy: setSurface(SourceWrap)
activate Proxy
Proxy->>Proxy: disconnect m_sourceConnections
Proxy->>ProxyWrap: surfaceItem()
alt proxy item exists
Proxy->>SourceWrap: surfaceItem()
SourceWrap-->>Proxy: SourceItem or null
Proxy->>Proxy: updateProxyFlagsAndDelegate()
Proxy->>ProxyItem: setFlags(live ? RejectEvent : RejectEvent|NonLive)
alt SourceItem exists
Proxy->>ProxyItem: setDelegate(SourceItem.delegate)
end
Proxy->>ProxyWrap: connect(surfaceItemChanged, updateProxyFlagsAndDelegate)
alt SourceItem exists
Proxy->>SourceItem: connect(delegateChanged, updateProxyFlagsAndDelegate)
else SourceItem missing
Proxy->>SourceWrap: connect(surfaceItemChanged, lambda)
Note over Proxy,SourceWrap: On first SourceWrap.surfaceItemChanged,<br/>updateProxyFlagsAndDelegate and connect SourceItem.delegateChanged
end
Proxy->>SourceWrap: connect(destroyed, setSurface(nullptr))
end
deactivate Proxy
SourceWrap-->>Proxy: surfaceItemChanged
activate Proxy
Proxy->>Proxy: updateProxyFlagsAndDelegate()
Proxy->>ProxyItem: setFlags(live ? RejectEvent : RejectEvent|NonLive)
Proxy->>SourceWrap: surfaceItem()
SourceWrap-->>Proxy: SourceItem
Proxy->>ProxyItem: setDelegate(SourceItem.delegate)
deactivate Proxy
SourceItem-->>Proxy: delegateChanged
activate Proxy
Proxy->>Proxy: updateProxyFlagsAndDelegate()
Proxy->>ProxyItem: setDelegate(SourceItem.delegate)
deactivate Proxy
Sequence diagram for prelaunch splash to real window transition in SurfaceWrappersequenceDiagram
participant Client as ClientCode
participant Orig as OriginalSurfaceWrapper
participant Clone as NewSurfaceWrapper
participant Engine as SurfaceEngine
participant Splash as PrelaunchSplashItem
Client->>Orig: create window (no shellSurface)
Note over Orig: m_shellSurface is null initially
Client->>Clone: SurfaceWrapper(Orig, parent, Engine)
activate Clone
alt original has shellSurface
Clone->>Clone: setup()
else no shellSurface
Clone->>Orig: implicitWidth(), implicitHeight()
Orig-->>Clone: width, height
Clone->>Clone: setImplicitSize(width, height)
Orig->>Orig: prelaunchSplash()
Orig-->>Clone: existingSplash or null
alt existing prelaunch splash
Clone->>Orig: prelaunchSplash().property(iconBuffer)
else no existing splash
Clone->>Orig: prelaunchSplash() == null
Orig-->>Clone: null
Note over Clone: iconVar is invalid
end
Clone->>Engine: createPrelaunchSplash(Clone, Orig.radius(), iconBuffer)
Engine-->>Clone: Splash
Clone->>Clone: m_prelaunchSplash = Splash
Clone->>Clone: setNoDecoration(false)
Clone->>Orig: connect(surfaceItemChanged, lambda)
end
deactivate Clone
Orig-->>Clone: surfaceItemChanged (shellSurface now available)
activate Clone
Clone->>Orig: read m_shellSurface, m_type
Clone->>Clone: m_shellSurface = Orig.m_shellSurface
Clone->>Clone: m_type = Orig.m_type
alt m_prelaunchSplash exists
Clone->>Splash: deleteLater()
Clone->>Clone: m_prelaunchSplash = nullptr
end
Clone->>Clone: setup()
deactivate Clone
Class diagram for updated SurfaceProxy and SurfaceWrapper behaviorclassDiagram
class SurfaceProxy {
-SurfaceWrapper* m_sourceSurface
-SurfaceWrapper* m_proxySurface
-QList~QMetaObject_Connection~ m_sourceConnections
-bool m_live
+void setSurface(SurfaceWrapper* newSurface)
-void updateProxyFlagsAndDelegate()
}
class SurfaceWrapper {
-QWShellSurface* m_shellSurface
-QObject* m_prelaunchSplash
-SurfaceEngine* m_engine
-SurfaceType m_type
+SurfaceWrapper(SurfaceWrapper* original, QObject* parent, SurfaceEngine* engine)
+QQuickItem* prelaunchSplash()
+qreal radius()
+void setup()
+void setImplicitSize(qreal width, qreal height)
+void setNoDecoration(bool noDecoration)
+WSurfaceItem* surfaceItem()
<<signal>> surfaceItemChanged()
<<signal>> noTitleBarChanged()
}
class WSurfaceItem {
<<QObject>>
+enum Flag
+void setFlags(Flag flags)
+void setDelegate(QObject* delegate)
<<signal>> delegateChanged()
}
class SurfaceEngine {
+QQuickItem* createPrelaunchSplash(SurfaceWrapper* wrapper, qreal radius, qw_buffer* iconBuffer)
}
SurfaceProxy --> SurfaceWrapper : m_sourceSurface
SurfaceProxy --> SurfaceWrapper : m_proxySurface
SurfaceProxy ..> WSurfaceItem : updates flags
SurfaceProxy ..> WSurfaceItem : mirrors delegate
SurfaceWrapper ..> WSurfaceItem : owns surfaceItem
SurfaceWrapper ..> SurfaceEngine : uses
SurfaceWrapper ..> QQuickItem : m_prelaunchSplash
Flow diagram for SwitchViewDelegate title selection logicflowchart TD
A[Start title binding] --> B[Get wrapper = windowItem.surface]
B --> C[Get xdg = wrapper.shellSurface]
C --> D{xdg exists?}
D -->|No| E[title = empty string]
D -->|Yes| F[title = xdg.title]
E --> G{title length > 0?}
F --> G{title length > 0?}
G -->|Yes| H[Use title]
G -->|No| I[Use wrapper.appId]
H --> J[Display in task switcher]
I --> J[Display in task switcher]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wineee 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 |
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.
Pull request overview
This PR improves splash screen and surface proxy handling for window management stability. It addresses cases where shellSurface might be null during window creation, adds better delegate synchronization for proxy surfaces, and enhances prelaunch splash screen support for proxy wrappers.
Key changes:
- Added null-safe title display in task switcher with fallback to appId when title is empty or shellSurface is null
- Refactored surface proxy delegate synchronization to handle late surface item creation and respond to both proxy and source surface item changes
- Enhanced SurfaceWrapper proxy constructor to create prelaunch splash screens when the original surface lacks a shellSurface
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/core/qml/SwitchViewDelegate.qml |
Added null-safety check for shellSurface and fallback to appId when window title is empty |
src/surface/surfaceproxy.cpp |
Refactored delegate sync with lambda-based approach, replaced foreach with range-based for loop, added handling for late surface item creation |
src/surface/surfacewrapper.cpp |
Added prelaunch splash screen support in proxy constructor when original surface has no shellSurface, with connection to handle delayed surface item creation |
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 found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/surface/surfaceproxy.cpp:52` </location>
<code_context>
- item->setFlags(WSurfaceItem::RejectEvent | WSurfaceItem::NonLive);
- }
- item->setDelegate(m_sourceSurface->surfaceItem()->delegate());
+ auto updateProxyFlagsAndDelegate = [this]() {
+ auto proxyItem = m_proxySurface ? m_proxySurface->surfaceItem() : nullptr;
+ auto sourceItem = m_sourceSurface ? m_sourceSurface->surfaceItem() : nullptr;
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the new lambda-based wiring into named helper methods to simplify how proxy flags and delegates are synchronized and connected.
You can keep the new behavior while reducing the complexity by:
1. Moving `updateProxyFlagsAndDelegate` into a private member function.
2. Centralizing the connection setup for `surfaceItem`/`delegateChanged` into a helper that doesn’t mutate `m_sourceConnections` from inside lambdas.
3. Separating “flags sync” from “delegate sync” behavior if needed.
### 1. Move `updateProxyFlagsAndDelegate` to a member function
This removes the capturing lambda and makes the intent clearer:
```cpp
// In SurfaceProxy private section:
void SurfaceProxy::updateProxyFlagsAndDelegate()
{
auto proxyItem = m_proxySurface ? m_proxySurface->surfaceItem() : nullptr;
auto sourceItem = m_sourceSurface ? m_sourceSurface->surfaceItem() : nullptr;
if (!proxyItem)
return;
const auto flags = m_live ? WSurfaceItem::RejectEvent
: (WSurfaceItem::RejectEvent | WSurfaceItem::NonLive);
proxyItem->setFlags(flags);
if (sourceItem)
proxyItem->setDelegate(sourceItem->delegate());
}
```
Then in `setSurface`:
```cpp
updateProxyFlagsAndDelegate();
m_sourceConnections << connect(m_proxySurface,
&SurfaceWrapper::surfaceItemChanged,
this,
&SurfaceProxy::updateProxyFlagsAndDelegate);
```
### 2. Centralize source item / delegate wiring
Instead of adding connections from inside a lambda, push that logic into a helper that you call when the source’s `surfaceItem` becomes available:
```cpp
// In SurfaceProxy:
void SurfaceProxy::bindSourceItem()
{
auto sourceItem = m_sourceSurface ? m_sourceSurface->surfaceItem() : nullptr;
if (!sourceItem)
return;
// Optional: store these separately if you want to clear/rebind them
m_sourceConnections << connect(sourceItem,
&WSurfaceItem::delegateChanged,
this,
&SurfaceProxy::updateProxyFlagsAndDelegate);
// Ensure initial sync
updateProxyFlagsAndDelegate();
}
```
Then the wiring in `setSurface` becomes linear and non‑self‑modifying:
```cpp
// After m_proxySurface is created:
updateProxyFlagsAndDelegate();
m_sourceConnections << connect(m_proxySurface,
&SurfaceWrapper::surfaceItemChanged,
this,
&SurfaceProxy::updateProxyFlagsAndDelegate);
m_sourceConnections << connect(m_sourceSurface,
&SurfaceWrapper::surfaceItemChanged,
this,
&SurfaceProxy::bindSourceItem);
// If surfaceItem already exists, bind immediately:
bindSourceItem();
```
This preserves the behavior:
- Flags are updated whenever the proxy’s `surfaceItem` changes.
- Delegate is mirrored whenever:
- The source’s `surfaceItem` appears (via `bindSourceItem`),
- The source item’s delegate changes (`delegateChanged`).
But it removes:
- The lambda that mutates `m_sourceConnections` from inside a callback.
- The overloaded lambda responsibility (now in a named method).
- The scattered conditional logic around `m_proxySurface`/`m_sourceSurface`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Fixed window title display in SwitchViewDelegate to handle cases where shellSurface might be null. Added fallback to use appId when title is empty. Refactored surface proxy connection management to use range-based for loop and improved delegate synchronization. Enhanced SurfaceWrapper constructor to properly handle prelaunch splash screens and surface item changes. Log: Improved window title display and window management stability Influence: 1. Test window switching with applications that have empty titles 2. Verify window titles display correctly in task switcher 3. Test window proxy behavior during live/non-live transitions 4. Verify delegate synchronization between source and proxy surfaces 5. Test prelaunch splash screen handling for new windows fix: 改进窗口标题显示和表面代理处理 修复了SwitchViewDelegate中窗口标题显示问题,处理shellSurface可能为空的情 况。当标题为空时使用appId作为回退。重构了表面代理连接管理,使用基于范围 的for循环并改进了委托同步。增强了SurfaceWrapper构造函数以正确处理预启动 闪屏和表面项变化。 Log: 改进窗口标题显示和窗口管理稳定性 Influence: 1. 测试多任务视图中显示闪屏 2. 验证任务切换器中显示闪屏
Fixed window title display in SwitchViewDelegate to handle cases where shellSurface might be null. Added fallback to use appId when title is empty. Refactored surface proxy connection management to use range-based for loop and improved delegate synchronization. Enhanced SurfaceWrapper constructor to properly handle prelaunch splash screens and surface item changes.
Log: Improved window title display and window management stability
Influence:
fix: 改进窗口标题显示和表面代理处理
修复了SwitchViewDelegate中窗口标题显示问题,处理shellSurface可能为空的情 况。当标题为空时使用appId作为回退。重构了表面代理连接管理,使用基于范围
的for循环并改进了委托同步。增强了SurfaceWrapper构造函数以正确处理预启动
闪屏和表面项变化。
Log: 改进窗口标题显示和窗口管理稳定性
Influence:
Summary by Sourcery
Improve window title handling in the task switcher and make surface proxy/splash screen management more robust.
Bug Fixes:
Enhancements: