Bug 357669

Summary: shading and unshading a window makes it lose height
Product: [Plasma] kwin Reporter: Marc Haber <mh+kde-bugs>
Component: decorationsAssignee: KWin default assignee <kwin-bugs-null>
Status: RESOLVED FIXED    
Severity: normal Flags: thomas.luebking: ReviewRequest+
Priority: NOR    
Version First Reported In: 5.4.3   
Target Milestone: ---   
Platform: Debian unstable   
OS: Linux   
URL: https://git.reviewboard.kde.org/r/126671/
Latest Commit: http://commits.kde.org/kwin/1c344c16d9161a646eec076e79f87ae7397a4648 Version Fixed/Implemented In: 5.6
Sentry Crash Report:

Description Marc Haber 2016-01-07 20:05:57 UTC 
Hi,

when I shade and unshade a window, it loses a few pixels in height. This is not a duplicate of #351161 or its dupes, since it doesn't resize the window to a minimal size, it just makes them lose a few pixels in height, and it also happens for gtk applications like Firefox or virt-viewer.

This is especially annoying for virt-viewer, as this either causes the guest OS not fitting in the window any more, or a rescale of the window contents, losing text readability, or causes the guest OS to do a resize as well.

Greetings
Marc


Reproducible: Always

Steps to Reproduce:
1. Open a window
2. Shade and unshade a few times
3. see the window lose height


Actual Results:  
The window was a few pixels less high than before the shading operation.


Expected Results:  
The windows should have exactly the same size as it had before shading.
Comment 1 Thomas Lübking 2016-01-07 22:12:43 UTC 
Happens.
The cause is that the unshaded window is first restored, then shadeChanged() is emitted.
The result is that the bottom border size is reported as 0 by the deco while it should be more™ (this doesn't impact w/ NoBorders or if the deco doesn't change the bottom border for shaded windows)

We probably wont get around emitting the signal twice:

diff --git a/client.cpp b/client.cpp
index 2663b00..54f66d1 100644
--- a/client.cpp
+++ b/client.cpp
@@ -812,6 +812,7 @@ void Client::setShade(ShadeMode mode)
         }
     } else {
         shade_geometry_change = true;
+        emit shadeChanged();
         QSize s(sizeForClientSize(clientSize()));
         shade_geometry_change = false;
         plainResize(s);
Comment 2 Marc Haber 2016-01-08 07:31:06 UTC 
Gee, that was fast! Thanks, and appreciation!
Comment 3 Thomas Lübking 2016-01-08 09:56:22 UTC 
Might only fix for 5.6 though
I'm actually not too happy w/ the doubled signal, more like a "proof-of-bug".
I guess adding a function to DecoratedClientImpl to explicitly "emit decoratedClient->shadedChanged(client->isShade());" (or turn the lambda into a slot) would be the "cleaner" solution (so the signal doesn't bleed into effects or anything else)
Comment 4 Marc Haber 2016-01-10 09:38:42 UTC 
As long as there is any reaction, I am fine with that. This is by far not a show stopper bug, it's fine to have the fix in a year. Plasma 5 does have a truckload of much more annoying regressions over KDE SC 4 with no visible developer side intention to fix in foreseeable time.
Comment 5 Thomas Lübking 2016-01-18 21:51:00 UTC 
Git commit 1c344c16d9161a646eec076e79f87ae7397a4648 by Thomas Lübking.
Committed on 18/01/2016 at 21:50.
Pushed by luebking into branch 'master'.

emit shadeChanged before calculating unshaded size

Otherwise the old (shaded) border sizes will be invoked, causing
a shrinkage of the window
FIXED-IN: 5.6
REVIEW: 126671

M  +2    -0    client.cpp
M  +5    -5    decorations/decoratedclient.cpp
M  +2    -0    decorations/decoratedclient.h

http://commits.kde.org/kwin/1c344c16d9161a646eec076e79f87ae7397a4648