Bug 450582 - Some change between 5.24.0 and 5.24.1 broke windows shade/shutter feature
Summary: Some change between 5.24.0 and 5.24.1 broke windows shade/shutter feature
Status: CLOSED FIXED
Alias: None
Product: kwin
Classification: Unclassified
Component: general (show other bugs)
Version: 5.24.1
Platform: openSUSE RPMs Linux
: VHI normal with 3 votes (vote)
Target Milestone: ---
Assignee: KWin default assignee
URL:
Keywords: regression
: 450641 450777 456220 457745 (view as bug list)
Depends on:
Blocks:
 
Reported: 2022-02-19 18:22 UTC by Hans-Peter Jansen
Modified: 2022-08-11 17:54 UTC (History)
19 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.25.4


Attachments
Screenshot Fensterverhalten.png (139.57 KB, image/png)
2022-02-21 13:16 UTC, Daniel Bruckner
Details
A couple of "shuttered" windows (144.76 KB, image/png)
2022-02-21 14:11 UTC, Hans-Peter Jansen
Details
Attach revert for reference (3.02 KB, patch)
2022-05-29 13:47 UTC, Hans-Peter Jansen
Details
Revert forward ported to 5.25.0 makes window shutter usable again with current kwin (2.68 KB, patch)
2022-06-19 17:33 UTC, Hans-Peter Jansen
Details
Shaded windows after a forceful kwin restart with kwin_x11 --replace. (74.60 KB, image/png)
2022-07-23 14:10 UTC, Hans-Peter Jansen
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Hans-Peter Jansen 2022-02-19 18:22:36 UTC
I'm using windows shutter ("Fensterheber") as a double click action since ages in KDE, hence also on my Tumbleweed X11 desktop. The update from 5.24.0 to 5.24.1 broke it. Retracting worked as usual, but when opening again, the window is resized to the smallest possible vertical shape, while the horizontal size isn't changed (as expected). 

The expected behavior is to restore the window to the original size. 

With a rebuilt 5.24.0 package https://build.opensuse.org/package/show/home:frispete:Tumbleweed/kwin5 on top of an ordinary update to 5.24.1, the behavior is fixed again.

If you expect a certain commit is the culprit for this behavior, I can rebuild the package with it reverted to test/prove it.

Operating System: openSUSE Tumbleweed 20220217
KDE Plasma Version: 5.24.1
KDE Frameworks Version: 5.90.0
Qt Version: 5.15.2
Kernel Version: 5.16.10-2-preempt (64-bit)
Graphics Platform: X11
Processors: 24 × AMD Ryzen 9 3900X 12-Core Processor
Memory: 62.7 GiB of RAM
Graphics Processor: NVIDIA GeForce RTX 3060/PCIe/SSE2
Comment 1 Behzad A 2022-02-21 12:11:34 UTC
*** Bug 450641 has been marked as a duplicate of this bug. ***
Comment 2 Vlad Zahorodnii 2022-02-21 12:16:57 UTC
What's window shutter?
Comment 3 Daniel Bruckner 2022-02-21 13:16:49 UTC
Created attachment 147001 [details]
Screenshot Fensterverhalten.png

In German it's called "Fensterheber"

Am 21.02.22 um 13:16 schrieb Vlad Zahorodnii:
> https://bugs.kde.org/show_bug.cgi?id=450582
>
> --- Comment #2 from Vlad Zahorodnii <vlad.zahorodnii@kde.org> ---
> What's window shutter?
>
Comment 4 Hans-Peter Jansen 2022-02-21 14:08:42 UTC
Hi Vlad, 

in cars, this is called electric windows, don't know, how this translates in KDE. 

I've been using the "electric windows/windows shutter" function as double click action on window title bars. 
When activated, it acts as a toggle to retract the window to a single bar and open it to the original size again.
Comment 5 Hans-Peter Jansen 2022-02-21 14:11:19 UTC
Created attachment 147002 [details]
A couple of "shuttered" windows
Comment 6 Duncan 2022-02-23 07:30:47 UTC
(In reply to Vlad Zahorodnii from comment #2)
> What's window shutter?

Window Shade, I believe (took me a moment to figure out too).  Retitling bug to reflect...

(Running git-master plasma/frameworks here but I don't use the shade feature often enough to notice any changes myself and I can't test ATM as at least on wayland shade seems to be a disabled option in the window menu ATM??)
Comment 7 David Redondo 2022-03-02 14:48:04 UTC
*** Bug 450777 has been marked as a duplicate of this bug. ***
Comment 8 David Edmundson 2022-03-02 14:52:13 UTC
733469d4e7648cd1df0862c8879ac364594935da is a probable candidate
Comment 9 Hans-Peter Jansen 2022-03-02 18:01:23 UTC
(In reply to David Edmundson from comment #8)
> 733469d4e7648cd1df0862c8879ac364594935da is a probable candidate

Thanks for the hint, David.

I reverted this commit in a 5.24.2 build, but an attempt to reproduce the issue in a VM failed, hence I need to wait for my primary system to be ready for some breaking changes, i.o.w. weekend... Sorry.

The correct term in KDE is shaded. To reproduce my settings: 
systemsettings -> Window Behavior -> Titlebar Actions -> Double-click: Shade
-> Apply

If two successive double clicks on the title bar restores the windows to the original size, it failed to reproduce.
For us, that suffer from this, the restore operation displays the window with the minimal vertical size.
Comment 10 Hans-Peter Jansen 2022-03-10 21:14:37 UTC
(In reply to David Edmundson from comment #8)
> 733469d4e7648cd1df0862c8879ac364594935da is a probable candidate

That nailed it, David.

I have reverted that commit (0001-Revert-Avoid-mixing-current-and-next-state.patch) on top of 5.24.2 here: https://build.opensuse.org/package/show/home:frispete:Tumbleweed/kwin5, and kwin5 behaves fine again.
Comment 11 Duncan 2022-04-19 21:24:50 UTC
(In reply to Duncan from comment #6)
> on wayland shade seems to be a disabled option in the window menu ATM??

Can anyone confirm whether shade is /supposed/ to work on wayland?

Presumably it wouldn't work on client-side-decorations, but kwin_wayland remains server-side-decorated (IIRC that was a kwin_wayland and thus wayland-protocol for kde/kwin must-have feature/option from the get-go), and I'd hope shading will at least eventually work.

One can posit that since the shade option remains disabled on wayland (at least here), the code for it may not have been written yet, but can anyone confirm that, and assuming so, is the feature planned and just not done yet, or no dev has even considered it yet, or the feature simply can't (within reasonable coding and support limits) be made to work on wayland even with kwin's server-side-decorations given current upstream wayland protocols state.  And if it's the latter, what's the chance of protocol evolution to allow it?  Anyone know?

And as this bug's presumably x11-context, does that mean I need a new feature-request bug for kwin_wayland window shading?

(FWIW I no longer even have a stand-alone X (xorg) installed, only xwayland running on top of wayland.  Tho I do have weston as a backup wayland compositor, which given that I'm running live-git-master frameworks/plasma, I have actually had to resort to on occasion.)
Comment 12 Hans-Peter Jansen 2022-05-29 13:46:11 UTC
> Can anyone confirm whether shade is /supposed/ to work on wayland?

I think, this is a valid request, and I would be interested to get an answer as well (even, if deviating from the original issue).

At the window shade front with X backend, I call say, that the revert still applies to kwin5-5.24, but not on the upcoming 5.25 (the src/abstract_client.cpp file vanished completely. I can only hope, that the shader feature is still operational.
Comment 13 Hans-Peter Jansen 2022-05-29 13:47:30 UTC
Created attachment 149316 [details]
Attach revert for reference
Comment 14 Christian (Fuchs) 2022-05-29 16:32:19 UTC
It would be really nice if we could get that revert into a release, unless that breaks other things. Because right now as is, shade is still broken in both released versions, the upcoming 5.25 release and on main.
Comment 15 Hans-Peter Jansen 2022-05-29 17:37:43 UTC
(In reply to Christian (Fuchs) from comment #14)
> It would be really nice if we could get that revert into a release, unless
> that breaks other things. Because right now as is, shade is still broken in
> both released versions, the upcoming 5.25 release and on main.

Well, the "train is departed already" ;-) for 5.24, where the choice was to either life with the bug, that was addressed with the original patch, and loose the windows shader feature, or revert. Choose your poison.

For 5.25, this issue needs to be reevaluated, since the source code module, where the path/revert was applied, does not exist anymore.
While I appreciate any clean-up of code, it should never lead to regressions (compare with uncounted LKML discussions on that topic). 

Given, this functionality is missing in Wayland on a conceptual level, this all doesn't sound fortunate for those long term KDE users (I started with KDE in the last century). The developers seem to just concentrate on Wayland, and don't care much about our X legacy and related regressions. 

For me, it started with the removal of the XRender engine, that effectively killed my preferred VNC setup (Xvnc). Luckily, I was able to replace it with a deprecated, outdated but functional setup using x11vnc. Since then, I saw regressions, where the second screen is gone, when switching it off, scrambling the whole desktop setup. Then plasmoids size management broke, the comic plasmoid is able to exhaust a cpu out of two on my poor 10 years old Lenovo X1 (1st gen), making it a heater. 

And now the window shading is about to disappear, which is a concept, that I'm using heavily since about 20 years.

I know, developers resources are scarce, but all this is kind of alarming, and the alarms are ringing for some time already.

@Vlad, a statement from you would be really nice.
Comment 16 Roberto Ragusa 2022-06-06 12:41:44 UTC
Getting this problem on fully updated Fedora 36 using X11:
plasma-desktop-5.24.5-1.fc36.x86_64
kwin-x11-5.24.5-1.fc36.x86_64

The window "Shade" functionality is using a very low window height (near zero) when doing "unshading". 
These means the feature is completely unusable at the moment.
Comment 17 Thomas Otto 2022-06-08 15:30:25 UTC
This might only affect existing (older?) configurations, when starting fresh (moving ~/.config/ ~/.kde/ ~/.local/ out of the way) this bug is no longer present.

So far I have not tried to narrow it down to a specific config file but just reverted the commit and then replaced the libkwin.so library.

When I tried Wayland I was also surprised by the lack of the shading functionality - but it is currently grayed out, not completely missing, so there is hope!
Comment 18 Hans-Peter Jansen 2022-06-19 17:33:57 UTC
Created attachment 149930 [details]
Revert forward ported to 5.25.0 makes window shutter usable again with current kwin
Comment 19 Rob 2022-06-27 16:30:17 UTC
(In reply to Hans-Peter Jansen from comment #18)
> Created attachment 149930 [details]
> Revert forward ported to 5.25.0 makes window shutter usable again with
> current kwin

For what it's worth, prior to this revert this problem can be worked-around by setting window border size to 0 or none.  With 0 or no border the window will shade/unshade properly.

I am suspicious (not certain) that this revert might have introduced a regression with shading gtk windows... https://bugs.kde.org/show_bug.cgi?id=456039
Comment 20 Bodo Eggert 2022-07-05 06:01:04 UTC
Please fix this because it prevents me from installing security updates.
Comment 21 Nate Graham 2022-07-05 16:34:53 UTC
*** Bug 456220 has been marked as a duplicate of this bug. ***
Comment 22 Steve Vialle 2022-07-15 16:02:27 UTC
Still broken as of 5.25.3.
Does anyone actually care about all these wayland/touchscreen/SNS related regressions in well-established functionality or what?
Comment 23 Bug Janitor Service 2022-07-18 08:35:08 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/kwin/-/merge_requests/2670
Comment 24 Vlad Zahorodnii 2022-07-18 11:53:24 UTC
Git commit 4790916fb12ef4a975307a31acda28ea023058fb by Vlad Zahorodnii.
Committed on 18/07/2022 at 11:39.
Pushed by vladz into branch 'master'.

x11: Fix shading with non-zero border

There was a geometry change that fixed mixing the next and current
geometries. While it did fix issues on wayland, it broke window shading
on x11 because of an obscure resize() call.

That obscure resize() had a side-effect that ensures m_clientGeometry
has the right value so the next time the window is unshaded,
implicitSize() will return a good value.

In order to make window size computation more robust, this change makes
X11Window compute the natural frame size based on cached size in
m_client, which shouldn't change when the window is shaded.

However, given how buggy window shading is and how difficult it is to
make it work right, I think that it's better to deprecate window shading
and remove it in some future release.

M  +0    -9    src/window.cpp
M  +0    -1    src/window.h
M  +11   -2    src/x11window.cpp
M  +1    -0    src/x11window.h

https://invent.kde.org/plasma/kwin/commit/4790916fb12ef4a975307a31acda28ea023058fb
Comment 25 Vlad Zahorodnii 2022-07-18 12:28:00 UTC
Git commit 1c5215009865c20b18c0f3114b167080cd70a33a by Vlad Zahorodnii.
Committed on 18/07/2022 at 11:57.
Pushed by vladz into branch 'Plasma/5.25'.

x11: Fix shading with non-zero border

There was a geometry change that fixed mixing the next and current
geometries. While it did fix issues on wayland, it broke window shading
on x11 because of an obscure resize() call.

That obscure resize() had a side-effect that ensures m_clientGeometry
has the right value so the next time the window is unshaded,
implicitSize() will return a good value.

In order to make window size computation more robust, this change makes
X11Window compute the natural frame size based on cached size in
m_client, which shouldn't change when the window is shaded.

However, given how buggy window shading is and how difficult it is to
make it work right, I think that it's better to deprecate window shading
and remove it in some future release.
(cherry picked from commit 4790916fb12ef4a975307a31acda28ea023058fb)

M  +0    -9    src/window.cpp
M  +0    -1    src/window.h
M  +11   -2    src/x11window.cpp
M  +1    -0    src/x11window.h

https://invent.kde.org/plasma/kwin/commit/1c5215009865c20b18c0f3114b167080cd70a33a
Comment 26 Rob 2022-07-18 12:37:41 UTC
> However, given how buggy window shading is and how difficult it is to
> make it work right, I think that it's better to deprecate window shading
> and remove it in some future release.

Please do not do this.
Comment 27 Christian (Fuchs) 2022-07-18 14:15:32 UTC
(In reply to Vlad Zahorodnii from comment #25)

Hi, thank you a lot for the fix, however,

> However, given how buggy window shading is and how difficult it is to
> make it work right, I think that it's better to deprecate window shading
> and remove it in some future release.

Indeed please do not do this. Window shading is a base Linux window management feature, very helpful and has been around for decades, it would be a huge loss if kwin would stop supporting it.
Comment 28 Hans-Peter Jansen 2022-07-18 16:50:35 UTC
Hi Vlad,

I hereby confirm, that your commit fixed the issue in the first tests (I built kwin as of 61b1eac5b against 5.25.3 here: https://build.opensuse.org/package/show/home:frispete:Tumbleweed:testing/kwin5)

Thanks a lot! Much appreciated.

Will update my primary system to this kwin version and give it some hard knocks!
Comment 29 matthewsha 2022-07-18 17:21:05 UTC
Please keep window shade/shutter in kwin.  It's  important for my work.
Comment 30 Roberto Ragusa 2022-07-19 07:41:00 UTC
> However, given how buggy window shading is and how difficult it is to
> make it work right, I think that it's better to deprecate window shading
> and remove it in some future release.

Please don't.
These are the things that make Linux/KDE better than the rest, do not align to mediocrity.

Removal of features is happening too much already.
Comment 31 Bodo Eggert 2022-07-19 07:58:46 UTC
> However, given how buggy window shading is and how difficult it is to
> make it work right, I think that it's better to deprecate window shading
> and remove it in some future release.
> (cherry picked from commit 4790916fb12ef4a975307a31acda28ea023058fb)

Given that there already is a fix, you needlessly break a key feature and I hate you.

Please just don't make the offending call on X11 if wayland needs it, or make a propper fix for the things that are broken on wayland instead. Or maybe remove wayland support if it is that broken.
Comment 32 Christian (Fuchs) 2022-07-19 08:02:35 UTC
Can we please refrain from personal attack and stay calm, even if we are enthusiastic about that feature, thanks.
Comment 33 Hans-Peter Jansen 2022-07-19 10:39:27 UTC
@Audience: offensive language does not help anyone here. It only leads to worsening the situation as non-technical aspects come to the fore. 

Hi Vlad,

> However, given how buggy window shading is and how difficult it is to make it work right, I think that it's better to deprecate window
> shading and remove it in some future release.

Please, don't. 

Repeating here, what I wrote in the merge request already:

There are a lot of users out there, that depend on it. Hard. Those, that I know of, used to work with KDE since the early 2000s. Well, personally, I'm using KDE as my main desktop since ~1999, doing all kinds of administration, development, and support work whole day long. So, during the last 23 years, I probably had less than 50 days not using a KDE desktop somehow (self-employed, you know..). And that legacy turned into strong habits, that most of us won't get rid of, because they use it for their own advantage.

My workflows and desktop organization is so entangled with this feature, that I cannot *even* switch to Wayland unless Wayland will provide a similar feature on its own. It's also one of those features, that sets us apart.

It might not be the nicest thing in the world implementationwise, but I'm immensely grateful, that you fixed it! 

As a reminder: as far as I know, the Wayland protocol has no such concept of shaded/shuttered windows. Therefore, preserving this feature in the longer term would, in my humble view, require that such an option be considered in the Wayland protocol itself. If anyone has more information on this topic, please let me know. Thank you!
Comment 34 Duncan 2022-07-19 13:04:46 UTC
(In reply to Hans-Peter Jansen from comment #33)
> As a reminder: as far as I know, the Wayland protocol has no such concept of
> shaded/shuttered windows. Therefore, preserving this feature in the longer
> term would, in my humble view, require that such an option be considered in
> the Wayland protocol itself. If anyone has more information on this topic,
> please let me know. Thank you!

Just thinking... What would it take?  AFAIK/IMO, wayland protocol shading support would be nice, but isn't absolutely required, certainly for apps that are prepared to deal with kwin's compositor-side-decorations, as most modern wayland apps can, nowdays.

Basically, it would require "lying" a bit to the app, telling it it's either mimimized/offscreen or a normal window, because telling it the real state, that it's shaded, isn't supported.  In the former case, kwin would have to keep track of the previous window size and position, and continue drawing the (compositor-side for kwin) titlebar, but ONLY the titlebar.  In the latter case, the app would think it was shown so would behave normally, but the kwin-compositor, always in charge of where and how the window is drawn in any case, would simply only draw the titlebar part of the (compositor-side) decoration, not the client/application window.

The latter case, telling the app its window is normalized, which in some ways it effectively would be but with the app-window portion effectively 100% transparent or covered with 100% opacity (something a wayland app doesn't normally  know in any case),  would probably avoid some problems, but would use more power for apps that basically suspend themselves when they're not doing any output drawing anyway, as is the case when they're offscreen/minimized, because they wouldn't know that and would continue drawing their window as if it was visible.

The former case, telling the app its window is minimized/offscreen, would likely be more complicated, as kwin would have to keep track of more state about the actual window that the app might not track when its minimized.   Apps that update their titlebar frequently (think a resource monitor updating information in the titlebar every second, for instance) but suspend that updating when they're minimized/offscreen would be particularly troublesome, as the titlebar would still be displayed when shaded, but the title would simply stop updating.

Either way it should be possible, even without direct protocol support.  Someone, presumably someone with the necessary skills (which I don't claim to have) suitably motivated to scratch their own itch, just has to code it up and get it accepted.   It /might/ even be doable as a kwin window management script, allowing it to be uploaded to the kde store, thus avoiding the "get it accepted" part it'd have to go through to become part of kwin proper.
Comment 35 Hans-Peter Jansen 2022-07-19 15:59:51 UTC
@Duncan: nice sum up.

Of course, we also need to take session management into account. The X11 protocol does provide the shaded flag for this task.
I even hacked a related tool to handle my firefox setup. FF failed to handle the geometry and shaded state for some time:

https://github.com/frispete/wm-win-tool
Comment 36 Duncan 2022-07-19 22:47:48 UTC
(In reply to Hans-Peter Jansen from comment #35)
> Of course, we also need to take session management into account.

Not being one that uses session management (per se, I use startup scripts and window rules to do the configuring I need) much, I hadn't thought of that.  Good call, altho I'd suggest an initial implementation wouldn't need it due to the complexity.

> The X11
> protocol does provide the shaded flag for this task.
> I even hacked a related tool to handle my firefox setup. FF failed to handle
> the geometry and shaded state for some time:
> 
> https://github.com/frispete/wm-win-tool

What about other gtk3-based apps?  Firefox only or anything gtk-based?

As for your tool, I was thinking wmctrl as well, and sure enough it's a dep.   But I'm a shellscript guy so that's what all my wmctrl/xprop/etc stuff is, not the python you used.  Well, that and kwin window rules.

There was also the khotkeys GUI app that allowed dynamic/triggered action invocation, back in the day, but last I tried it, even on X, while the configuration GUI still ran and actions could still be configured, I couldn't get them to actually work as configured.  I strongly suspect that it had lost a proper maintainer even by the time of the kde4->kde5 port, and while it was "ported" by others in terms of made to build and the GUI run, I'm not sure it ever actually worked properly on kde5, and certainly didn't seem to even on xorg last I tried it.

But I can't /imagine/ the kde wayland port being considered complete without /some/ sort of scriptable/invokable custom window management comparable to the static functionality kwin window rules provides on X and wayland, and wmctrl and friends  provide dynamically on X, so I guess eventually they'll either fix/port khotkeys, or reimpliment similar functionality from scratch.  But I actually don't expect it until after the (currently underway) qt6 port is done.   (And qt6 being the first qt to have wayland support from the get-go, I expect its wayland support to be better and more efficient, such that implementing such stuff in it may well be 2/3 the work it'd be on qt5.)

Anyway, doing wayland-shading session support without full protocol support is still possible, but I expect it'd be several times the complexity of the "simple" solution I was imagining, so wouldn't be likely for initial support at least.  But as long as apps can be forced to run in X mode via xwayland, existing solutions like your scripting solution should remain viable.  And pre-setting such vars before app startup or auto-adding commandline options as appropriate, is what wrapper scripts are for! =:^)
Comment 37 Hans-Peter Jansen 2022-07-23 14:09:18 UTC
Hi Vlad, unfortunately I have to report, that your fix doesn't fully fix this issue.

In order to reproduce, please shade some windows, and restart: kwin_x11 --replace. 

The result here is, that windows a reduced to the tiniest size I ever saw for a window (screenshot attached below). 

I'm not allowed to reopen this issue.
Comment 38 Hans-Peter Jansen 2022-07-23 14:10:52 UTC
Created attachment 150847 [details]
Shaded windows after a forceful kwin restart with kwin_x11 --replace.
Comment 39 Hans-Peter Jansen 2022-08-04 03:40:08 UTC
Why is it not possible to reopen bugs?
Do I need to open a new one?
Comment 40 David Edmundson 2022-08-04 09:58:44 UTC
>Do I need to open a new one?

Yeah.
Comment 41 Hans-Peter Jansen 2022-08-04 15:58:04 UTC
(In reply to David Edmundson from comment #40)
> >Do I need to open a new one?
> 
> Yeah.

Done: https://bugs.kde.org/show_bug.cgi?id=457487
Comment 42 Nate Graham 2022-08-11 17:54:53 UTC
*** Bug 457745 has been marked as a duplicate of this bug. ***