Bug 439689

Summary: Maximize effect: no more crossfade for window content
Product: [Plasma] kwin Reporter: Antonio Orefice <kokoko3k>
Component: effects-window-managementAssignee: KWin default assignee <kwin-bugs-null>
Status: RESOLVED FIXED    
Severity: normal CC: agurenko, nate, openmindead, postix, sokann
Priority: HI Keywords: regression
Version: 5.23.0   
Target Milestone: ---   
Platform: Other   
OS: Linux   
See Also: https://bugs.kde.org/show_bug.cgi?id=435423
Latest Commit: Version Fixed In: 5.26
Attachments: maximize/minimize, no crossfade video.

Description Antonio Orefice 2021-07-09 17:17:22 UTC
SUMMARY
In the past when un/maximizing windows there was a nice crossfade effect betweeb the old window content and the target one.
Now as soon as the animation starts, the new window content is displayed immediately and the old one is gone immediately, no more crossfade.

STEPS TO REPRODUCE
1. activate maximize effect
2. slow down amination speed
3. (un)maximize a window
4. notice no crossfade

OBSERVED RESULT


EXPECTED RESULT


SOFTWARE/OS VERSIONS
Windows: 
macOS: 
Linux/KDE Plasma: 
(available in About System)
KDE Plasma Version: 
KDE Frameworks Version: 
Qt Version: 

ADDITIONAL INFORMATION
Comment 1 Nate Graham 2021-08-02 14:33:59 UTC
Cannot reproduce. Can you attach a screen recording of the issue?
Comment 2 Antonio Orefice 2021-08-02 17:19:07 UTC
Created attachment 140468 [details]
maximize/minimize, no crossfade video.

Oh weird, it is broken on 3 systems here.

Fun fact:
the first morphing to broke was the one referenced long time ago here:
https://bugs.kde.org/show_bug.cgi?id=435423
...in the sense that popups did not crossfade anymore, while window maximizing still worked.
After some kwin release, the issue affected maximize animation too, so i tend to exclude drivers/hardware problems.
Are you sure you cannot notice anything even with animations slowed down as in the video attached?
I tried to install an older kwin version to rule out things, but unfortunately old kwin versions do not build on newer plasma.

There are flashes/glitches in the video attached; they do depend on ffmpeg capturing while compositing and vsync is active (and it is not possible to disable vsync in kwin anymore...), don't mind them.
Comment 3 Antonio Orefice 2021-08-02 17:27:13 UTC
PS:
I've still a *not* upgraded system where maximize effect still does crossfades, while morphing popups does not, if it could be useful somehow.
Comment 4 Nate Graham 2021-08-02 17:56:34 UTC
Oh you mean for the content, not the window size! Sorry for being dumb. Can reproduce.
Comment 5 Antonio Orefice 2021-08-03 16:14:49 UTC
First Broken version seems to be 5.21.90
I've had hard tme trying to bisect offending commit; but unfortunately all i've discovered so far is the following:
Working till commit:  Feb/15/21 22d386cd xwayland: Improve handling of Xwayland restarts
[..]
Broken at least from commit: apr/08/21 340c8b59 Update virtual desktops KCM docs

Between them there is a black hole due to me having build problems.

I've also changed the reported version to 5.21.90, because it worked till 5.21.5 which i've now downgraded to.
Comment 6 Antonio Orefice 2021-08-04 10:43:48 UTC
Bisected. The offending commit is:

koko@Gozer# git bisect good
47113e09b8a80497463725a795728a34e9db940c is the first bad commit
commit 47113e09b8a80497463725a795728a34e9db940c
Author: Vlad Zahorodnii <vlad.zahorodnii@kde.org>
Date:   Thu Feb 4 11:07:20 2021 +0200

    scene: Introduce window items
    
    Currently, dealing with sub-surfaces is very difficult due to the scene
    design being heavily influenced by X11 requirements.
    
    The goal of this change is to re-work scene abstractions to make improving
    the wayland support easier.
    
    The Item class is based on the QQuickItem class. My hope is that one day
    we will be able to transition to QtQuick for painting scene, but in
    meanwhile it makes more sense to have a minimalistic internal item class.
    
    The WindowItem class represents a window. The SurfaceItem class represents
    the contents of either an X11, or a Wayland, or an internal surface. The
    DecorationItem and the ShadowItem class represent the server-side deco and
    drop-shadow, respectively.
    
    At the moment, the SurfaceItem is bound to the scene window, but the long
    term plan is to break that connection so we could re-use the SurfaceItem
    for things such as software cursors and drag-and-drop additional icons.
    
    One of the responsibilities of the Item is to schedule repaints as needed.
    Ideally, there shouldn't be any addRepaint() calls in the core code. The
    Item class schedules repaints on geometry updates. In the future, it also
    has to request an update if its opacity or visibility changes.

 src/CMakeLists.txt                             |   8 +
 src/abstract_client.cpp                        |   8 +-
 src/abstract_client.h                          |   5 +-
 src/composite.cpp                              |  20 +-
 src/decorationitem.cpp                         |  35 ++
 src/decorationitem.h                           |  36 ++
 src/effects.cpp                                |   3 +-
 src/events.cpp                                 |   4 -
 src/internal_client.cpp                        |   5 +-
 src/item.cpp                                   | 345 ++++++++++++++++++++
 src/item.h                                     | 142 ++++++++
 src/plugins/scenes/opengl/scene_opengl.cpp     | 280 ++++++++--------
 src/plugins/scenes/opengl/scene_opengl.h       |   6 +-
 src/plugins/scenes/qpainter/scene_qpainter.cpp |  46 ++-
 src/plugins/scenes/qpainter/scene_qpainter.h   |   5 +-
 src/plugins/scenes/xrender/scene_xrender.cpp   |  26 +-
 src/scene.cpp                                  | 433 ++++++++-----------------
 src/scene.h                                    | 181 ++---------
 src/shadowitem.cpp                             |  46 +++
 src/shadowitem.h                               |  35 ++
 src/surfaceitem.cpp                            | 119 +++++++
 src/surfaceitem.h                              |  55 ++++
 src/surfaceitem_internal.cpp                   |  46 +++
 src/surfaceitem_internal.h                     |  34 ++
 src/surfaceitem_wayland.cpp                    | 151 +++++++++
 src/surfaceitem_wayland.h                      |  61 ++++
 src/surfaceitem_x11.cpp                        | 141 ++++++++
 src/surfaceitem_x11.h                          |  47 +++
 src/toplevel.cpp                               | 251 ++++----------
 src/toplevel.h                                 |  42 +--
 src/unmanaged.cpp                              |  53 +--
 src/unmanaged.h                                |   4 +-
 src/windowitem.cpp                             | 143 ++++++++
 src/windowitem.h                               |  90 +++++
 src/x11client.cpp                              |  31 +-
 src/x11client.h                                |   3 +-
 src/xwaylandclient.cpp                         |  19 +-
 37 files changed, 2015 insertions(+), 944 deletions(-)
 create mode 100644 src/decorationitem.cpp
 create mode 100644 src/decorationitem.h
 create mode 100644 src/item.cpp
 create mode 100644 src/item.h
 create mode 100644 src/shadowitem.cpp
 create mode 100644 src/shadowitem.h
 create mode 100644 src/surfaceitem.cpp
 create mode 100644 src/surfaceitem.h
 create mode 100644 src/surfaceitem_internal.cpp
 create mode 100644 src/surfaceitem_internal.h
 create mode 100644 src/surfaceitem_wayland.cpp
 create mode 100644 src/surfaceitem_wayland.h
 create mode 100644 src/surfaceitem_x11.cpp
 create mode 100644 src/surfaceitem_x11.h
 create mode 100644 src/windowitem.cpp
 create mode 100644 src/windowitem.h
Comment 7 Vlad Zahorodnii 2021-08-04 12:13:08 UTC
Thanks for bisecting. Cross-fade animation is implemented as a hack so I guess it's not surprising that it got broken... :(
Comment 8 Bug Janitor Service 2021-08-04 12:43:26 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/kwin/-/merge_requests/1221
Comment 9 Antonio Orefice 2021-08-04 13:25:29 UTC
Thank YOU very much for your efforts.
However, the same commit affects: https://bugs.kde.org/show_bug.cgi?id=438458 which, by gut feeling, it should not be due to previous pixmap.
Comment 10 Vlad Zahorodnii 2021-08-04 13:37:08 UTC
Right, I still need to figure out what might be causing that bug.
Comment 11 Vlad Zahorodnii 2021-08-04 14:32:23 UTC
Git commit f432ba7821cea2302d040c8bfc7e3cb7d9540874 by Vlad Zahorodnii.
Committed on 04/08/2021 at 12:44.
Pushed by ngraham into branch 'Plasma/5.22'.

Avoid discarding previous pixmap

When resizing, the SurfaceItem will discard the current pixmap until a
new one is created. If the newly created pixmap is valid, the previous
pixmap will be discarded in SurfaceItem::updatePixmap().

However, the previous pixmap can be discarded only if it's unreferenced,
in other words, no effect needs the previous pixmap.

M  +3    -1    src/surfaceitem.cpp

https://invent.kde.org/plasma/kwin/commit/f432ba7821cea2302d040c8bfc7e3cb7d9540874
Comment 12 Sok Ann Yap 2021-09-15 08:14:40 UTC
With commit f432ba78, chromium/chrome/vivaldi would get stuck when trying to enter fullscreen (e.g. by pressing F11) with Ozone/Wayland, as described in https://bugs.chromium.org/p/chromium/issues/detail?id=1248049
Comment 13 Vlad Zahorodnii 2021-09-15 10:53:55 UTC
Thanks for reporting. I'll check if it's indeed a kwin bug and comment on the chromium bug report.
Comment 14 Antonio Orefice 2021-10-19 07:25:39 UTC
Unfortunately, the bug is back on kwin 5.23
Comment 15 Antonio Orefice 2021-10-19 07:34:45 UTC
Maybe related, or not, we lost xfade on window resize too when using the resize effect that scales the window texture.
I managed to bring xfade for (un)maximize effect by downgrading kwin,kdecoration and kwayland-server back to 5.22.5.
Comment 16 Antonio Orefice 2021-10-19 11:03:19 UTC
f432ba78  | GOOD
ebc785167 | BAD
I'm unable to build in between, sorry.
Comment 17 Vlad Zahorodnii 2021-10-19 11:06:35 UTC
Yeah, crossfade doesn't work atm, we will try to get it working again in 5.24.
Comment 18 Vladimir Yerilov 2022-01-30 16:56:30 UTC
Fall apart for closing windows also doesn't work on 5.23
Comment 19 Antonio Orefice 2022-02-08 15:04:02 UTC
Hi, i've seen kwin 5.24 is out.
Any news on this?
Comment 20 Bug Janitor Service 2022-07-29 11:03:02 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/kwin/-/merge_requests/2753
Comment 21 Marco Martin 2022-09-07 10:16:49 UTC
Git commit 31d1f885ce184b54690d44c5fcd4bb7384f6f0a3 by Marco Martin.
Committed on 07/09/2022 at 10:16.
Pushed by mart into branch 'master'.

Restore the crossfade effect

This enables again the crossfade between the old window picture and the new one in the maximize and morphingpopup effects.
It does that with the OffScreenEffect redirect() feature.
Related: bug 435423

M  +1    -0    autotests/integration/effects/maximize_animation_test.cpp
M  +22   -0    src/effects.cpp
M  +1    -0    src/effects.h
M  +10   -22   src/effects/blendchanges/blendchanges.cpp
M  +3    -3    src/effects/blendchanges/blendchanges.h
M  +25   -12   src/effects/maximize/package/contents/code/maximize.js
M  +23   -17   src/effects/morphingpopups/package/contents/code/morphingpopups.js
M  +2    -0    src/events.cpp
M  +2    -0    src/internalwindow.cpp
M  +12   -5    src/libkwineffects/kwinanimationeffect.cpp
M  +1    -1    src/libkwineffects/kwinanimationeffect.h
M  +29   -0    src/libkwineffects/kwineffects.h
M  +156  -51   src/libkwineffects/kwinoffscreeneffect.cpp
M  +46   -13   src/libkwineffects/kwinoffscreeneffect.h
M  +1    -0    src/window.cpp
M  +6    -0    src/window.h
M  +1    -0    src/x11window.cpp
M  +2    -0    src/xdgshellwindow.cpp

https://invent.kde.org/plasma/kwin/commit/31d1f885ce184b54690d44c5fcd4bb7384f6f0a3