Bug 496646 - [Desktop Grid] Regression in dropped window animations
Summary: [Desktop Grid] Regression in dropped window animations
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: effects-overview (show other bugs)
Version: git master
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: KWin default assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-11-24 20:24 UTC by Blazer Silving
Modified: 2024-12-01 20:55 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Normal behavior of the desktop grid (1.51 MB, video/mp4)
2024-11-24 20:26 UTC, Blazer Silving
Details
Abnormal behavior of the desktop grid (2.15 MB, video/mp4)
2024-11-24 20:26 UTC, Blazer Silving
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Blazer Silving 2024-11-24 20:24:42 UTC
This directly follows BUG 495444 and https://invent.kde.org/plasma/kwin/-/commit/2e0b33eae4588a829e64495efbb27a25da311190

SUMMARY
With the Overview's Desktop Bar being addressed in the previous case, the fix to WindowHeapDelegate.qml regressed window placement on the expanded Desktop Grid in the Overview effect. 

Windows dragged into the target desktops stop animating and do not ease into their defined position. 
Can reproduce on X11 and Wayland with 4 desktops in 2x2 grid, recordings attached.

STEPS TO REPRODUCE
1. Sign into new session (X11/Wayland)
2. Engage Overview (Meta+W) and add four Virtual Desktops in the Desktop Bar.
3. In Virtual Desktop KCM, increase Row count to 2.
3. Drag and drop a window to any desktop using the expanded Desktop Grid.
4. Observe the window behavior, where the dropped windows do not animate when they should.

OBSERVED RESULT
Windows stop animating when dropped in the Desktop Grid. 

EXPECTED RESULT
Windows should "ease" over to their designated place. 

SOFTWARE/OS VERSIONS
KDE Plasma Version: master branch
KDE Frameworks Version: 6.7.0
Qt Version: 6.8.0

ADDITIONAL INFORMATION

I tried to move the code fix from WindowHeapDelegate.qml to DesktopBar.qml (in DropArea) or main.qml of Overview, but realized rather quick it doesn't work this way, the behavior is driven here by WindowHeapDelegate. 

I then tried to restrict the fix using an extra if() check in WindowHeapDelegate.qml to only apply to windows dropped in the Desktop Bar's container, but I don't think this is possible due to the QML context not being nested as parent/child. I'm reading in general that chaining contexts like this is not best practice, anyway.
Comment 1 Blazer Silving 2024-11-24 20:26:18 UTC
Created attachment 176089 [details]
Normal behavior of the desktop grid
Comment 2 Blazer Silving 2024-11-24 20:26:48 UTC
Created attachment 176090 [details]
Abnormal behavior of the desktop grid
Comment 3 Blazer Silving 2024-11-24 20:47:03 UTC
Adding Nate and Marco to CC, thanks y'all!
Comment 4 David Edmundson 2024-11-25 12:16:56 UTC
I had a look at the newly merged patch:

This looks wrong:                if (newGlobalRect.x < thumb.windowHeap.x ||

newGlobalRect.x is (presumably) global.

windowHeap.x is relative to the parent of thumb.windowHeap

That's mixing co-ordinate spaces.
Comment 5 Blazer Silving 2024-11-25 16:35:22 UTC
https://invent.kde.org/plasma/kwin/-/commit/31c4b8ac555289309341b03bf813b4ac45d2e12d
This later patch defines heapRect to reference thumb.windowHeap, but still runs it against "oldGlobalRect" and issue persists. It doesn't seem the coordinate space/objects used by Overview is accessible in the WindowHeapDelgate scope though? 

I wonder again if the fix may be better approached in the DropArea block of DesktopBar.qml: https://invent.kde.org/plasma/kwin/-/blob/master/src/plugins/overview/qml/DesktopBar.qml?ref_type=heads#L177
Comment 6 Bug Janitor Service 2024-11-26 10:50:23 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/kwin/-/merge_requests/6833
Comment 7 Marco Martin 2024-11-26 13:03:03 UTC
Git commit 0e9f1e721c7b45b6b7d31a44df925483c1ec1155 by Marco Martin.
Committed on 26/11/2024 at 10:50.
Pushed by mart into branch 'master'.

effects/overview: Animate if the thumbnail is dropped in an heap

If the new position the thumbnail was dropped intersects the geometry of
an heap in any way, then execute the animation, otherwise skip it
(the case of drop on a desktop thumbnails in the overview)
previous patch attempted that but with not completely correct logic

M  +2    -4    src/plugins/private/qml/WindowHeapDelegate.qml

https://invent.kde.org/plasma/kwin/-/commit/0e9f1e721c7b45b6b7d31a44df925483c1ec1155
Comment 8 Blazer Silving 2024-11-26 13:23:34 UTC
Sorry to report: 
https://invent.kde.org/plasma/kwin/-/commit/0e9f1e721c7b45b6b7d31a44df925483c1ec1155 fixes the Desktop Grid animations, but the Desktop Bar regresses back to BUG 495444 with the gliding windows on drop.
Comment 9 Marco Martin 2024-11-26 13:34:50 UTC
Git commit c402845961fa10185d7c28a49a9b72e7581258f8 by Marco Martin.
Committed on 26/11/2024 at 13:03.
Pushed by mart into branch 'Plasma/6.2'.

effects/overview: Animate if the thumbnail is dropped in an heap

If the new position the thumbnail was dropped intersects the geometry of
an heap in any way, then execute the animation, otherwise skip it
(the case of drop on a desktop thumbnails in the overview)
previous patch attempted that but with not completely correct logic


(cherry picked from commit 0e9f1e721c7b45b6b7d31a44df925483c1ec1155)

0e9f1e72 effects/overview: Animate if the thumbnail is dropped in an heap

Co-authored-by: Marco Martin <notmart@gmail.com>

M  +2    -4    src/plugins/private/qml/WindowHeapDelegate.qml

https://invent.kde.org/plasma/kwin/-/commit/c402845961fa10185d7c28a49a9b72e7581258f8
Comment 10 Blazer Silving 2024-12-01 20:55:46 UTC
Seeing as https://invent.kde.org/plasma/kwin/-/commit/c402845961fa10185d7c28a49a9b72e7581258f8 for this ticket fixes the overall drop issue, but regresses back to BUG 495444, we can close this one and re-open the former.