Bug 265818 - Yakuake width incorrect with plasma panel on the side
Summary: Yakuake width incorrect with plasma panel on the side
Status: RESOLVED FIXED
Alias: None
Product: yakuake
Classification: Applications
Component: general (show other bugs)
Version: 2.9.8
Platform: Arch Linux Linux
: NOR normal
Target Milestone: ---
Assignee: Eike Hein
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-08 17:40 UTC by Leonidas Arvanitis
Modified: 2011-10-29 02:24 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Figure 1 - Change width: ->80->100, works fine (49.38 KB, image/png)
2011-06-25 14:29 UTC, Aleksander Mierzwicki
Details
Figure 2 - Hide & Show, works wrong (44.17 KB, image/png)
2011-06-25 14:30 UTC, Aleksander Mierzwicki
Details
Proposed fix (2.86 KB, patch)
2011-10-18 13:33 UTC, Alexandre Courbot
Details
Final fix (2.80 KB, patch)
2011-10-19 05:09 UTC, Alexandre Courbot
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Leonidas Arvanitis 2011-02-08 17:40:05 UTC
Version:           2.9.8 (using KDE 4.6.0) 
OS:                Linux

Putting plasma panels on the side of the screen don't affect the width of yakuake.

Reproducible: Always

Steps to Reproduce:
Configure yakuake's width to 100%.
Add a panel to the right side of the screen and set it to maximized and always on top.
Quit yakuake and restart it.
Press the shortcut (default F12) to display it.
Press again to hide it.
Press once more to make it appear for a second time.

Actual Results:  
The first time yakuake appears it has the expected width of "screen width - panel", but every time after that it appears 100% of the whole screen, having its rightmost part hidden behind the panel.

Expected Results:  
Every time the yakuake window appears the width should be from the left edge of the screen up to the beginning of the panel.

Changing the width to other sizes (eg. 50%) has the same effect, ie. 50% of the whole screen width.
Tried turning kwin effects off, kwin animation management off, locking/unlocking the plasma widgets but it didn't help.
Comment 1 Eike Hein 2011-02-09 23:10:18 UTC
I've tried to reproduce this, but I haven't been able to. The discrepancy you're seeing between first show and subsequent shows is particularly interesting, because identical window size calculation code runs both times, acting on working area geometry data retrieved from a KDE API.

I've only been able to check on a KDE 4.5 system so far, but will try on 4.6 later if it's different there.
Comment 2 Eike Hein 2011-02-09 23:17:53 UTC
No luck reproducing on 4.6 either, nor for another user in #yakuake. Don't see what I can do at the moment, sorry ... please reopen when you find out anything more that can be useful to reproducing.
Comment 3 Dmitry Yudakov 2011-04-26 09:48:14 UTC
I'm having the same problem, but my environment is slightly different.

I also have plasma panel, but it's on the left side. When only one display is configured, it works correctly. But when I configure another display in TwinMode yakuake works correctly only the first time it appears. In all appearances after that its right side is hidden if the width is set to 100%.

My main display's resolution is 1920x1680, the secondary is 1280x1024. Yakuake 2.9.8, KDE 4.4.5 on Debian testing.
Comment 4 Aleksander Mierzwicki 2011-06-25 14:29:16 UTC
Created attachment 61318 [details]
Figure 1 - Change width: ->80->100, works fine
Comment 5 Aleksander Mierzwicki 2011-06-25 14:30:04 UTC
Created attachment 61319 [details]
Figure 2 - Hide & Show, works wrong
Comment 6 Aleksander Mierzwicki 2011-06-25 14:31:05 UTC
The same here. Debian testing, Yakuake 2.9.8, KDE 4.6.3, Nvidia with TwinView - two screens (not tested with one monitor only).

When I change the width of yakuake for example to 80%, next to 100%, then it works fine. But after hide, show it displays wrong.

What usefull info can I provide?
Comment 7 Alexandre Courbot 2011-10-13 08:21:59 UTC
Same problem here with dual-screen configuration, intel driver, KDE 4.7.2, yakuake 2.9.8. I confirm the behavior observed by Dmitry Yudakov and Aleksander Mierzwicki. With one screen things work as expected, however in dual screen the window is correctly sized only the first time it is open. If the workAreaChanged() signal is emitted (because the panel moved for instance), then dimensions become correct again.

It seems that the value returned by getDesktopGeometry() is indeed only correct when the window is opened the first time or workAreaChanged has been emitted. If I display it:

QRect(2048,0 2448x1413)

However, in all other cases, I get:

QRect(2048,0 2560x1440)

Which correspond to the geometry of the whole screen.

Looking deeper into getDesktopGeometry(), I noticed that currentDesktop, which is set as the return value of KWindowSystem::windowInfo(NET::WMDesktop), is -1 on the correct (first) call, and 0 for all others. And if I force it to -1 all the time, I get the correct dimensions and behavior, hurray!

This is just a dirty workaround though, and I need to look a little further to provide an actual fix, but just wanted to confirm the issue and see if this information could be enough for someone else to understand where the problem is. If you have any additional information please feel free to post it as I am not very familiar with KDE's APIs.
Comment 8 Eike Hein 2011-10-13 23:54:43 UTC
Thanks for researching, Alexandre - sounds interesting ...
Comment 9 Alexandre Courbot 2011-10-18 13:33:52 UTC
Created attachment 64669 [details]
Proposed fix
Comment 10 Alexandre Courbot 2011-10-18 13:34:31 UTC
Hi Eike,

I think I got it - commit 814840672d4a5da4867381beb1b00788bf9694fa seems to be the culprit here. It makes sure that windows on the same desktop but not the same screen that have a top strut (e.g. panels) are ignored when invoking workArea. For this is creates a list of windows which struts should be ignored. In effect, every window without a top strut is put into this list - which is indeed fine for regular windows, but not for windows with a left, right or bottom strut which are put into the list even though their strut may cover the work area. 

Actually, rather than a problem with Yakuake, isn't it a more general bug with workArea()? In this case, shouldn't we address this upstream?

Anyway, proposed fix is attached - I tried to submit it to the review board but it failed with this error:

The repository path "git://anongit.kde.org/yakuake.git" is not in the
list of known repositories on the server.

If you can tell me which repository I should use, I'd be grateful as I may send a couple more patches in the future. ;)
Comment 11 Eike Hein 2011-10-18 13:42:06 UTC
Thanks, patch looks reasonable at first glance, hopefully I can give it a spin tonight.

About ReviewBoard, according to its config the repo URL it knows about is 'git://anongit.kde.org/yakuake', i.e. sans .git.
Comment 12 Alexandre Courbot 2011-10-19 00:46:52 UTC
Thanks. I have uploaded the patch on the review board:

https://git.reviewboard.kde.org/r/102914/

Please use this instead of the one I uploaded as the email address was not correct.

About the possibility to fix that directly in workArea(), what are your thoughts? Could the bug actually be there?
Comment 13 Alexandre Courbot 2011-10-19 05:09:22 UTC
Created attachment 64690 [details]
Final fix
Comment 14 Alexandre Courbot 2011-10-19 05:10:09 UTC
Since it seems you cannot retrieve patches from review board, I have attached the final version of this fix. Please use this one.
Comment 15 Eike Hein 2011-10-20 06:22:22 UTC
Hm, what do you mean with "retrieve", exactly? It's certainly possible to download a diff from ReviewBoard, and you can also update a review request with a new version of a patch.

As for the patch, I sadly haven't had a chance to test it yet. It might have to wait until the weekend, as I want to test it with multi-head as well and can't get my hands on a second monitor until then.

As for whether this should be in workArea(), yeah, I had a talk about that with the KWindowSystem maintainers back then but they weren't very enthusiastic. Since other apps (e.g. KRunner) suffer from the same types of problems and duplicating code across apps is iffy I do think it ought to be in the libs, personally.
Comment 16 Alexandre Courbot 2011-10-20 15:11:39 UTC
I meant I don't think it is possible to get a proper git patch with author and commit information from review board - please correct me if I'm wrong.

Don't hurry yourself for testing, this is not a major bug and it is applied to my tree anyway. Works like a charm so far.

Thanks!
Comment 17 Alexandre Courbot 2011-10-28 03:38:01 UTC
Eike, any news on this? I am using this patch since more than one week on two different setups (single and multi-head), and heavily tested it. I can assert it is safe.

Just a reminder so that you don't forget to merge it. ;)
Comment 18 Eike Hein 2011-10-28 16:34:25 UTC
Sorry Alexandre, I'm currently busied up between battling influenza, work stuff and readying Konversation for release :). I haven't gotten around to organizing the second monitor to test it, but I'll merge the patch for now, at least that will make it easier for others to test.
Comment 19 Eike Hein 2011-10-28 16:37:17 UTC
Git commit 81ac35a141a353641d304d2dcd4b0a8c12d76693 by Eike Hein, on behalf of Alexandre Courbot.
Committed on 19/10/2011 at 02:39.
Pushed by hein into branch 'master'.

Fix geometry in multi-screen cases

Commit 81484067 addressed and issue with multi-head setups where the
top strut of a window not visible on yakuake's display would be removed
from its work area. However, it did not consider bottom, right, and left
struts, and also prevented the left, right and bottom struts of windows
on the same display as yakuake from being removed from its work area.
This patch fixes this issue and tries to make the code more explicit.

BUG: 265818

M  +20   -4    app/mainwindow.cpp

http://commits.kde.org/yakuake/81ac35a141a353641d304d2dcd4b0a8c12d76693
Comment 20 Alexandre Courbot 2011-10-29 02:24:51 UTC
Great, thanks! Proud to see my first patch into Yakuake! ;) It is probably the tool I use the most for both work & personal stuff.