Jazqa / kwin-yakts

An easy tiling script for KWin
GNU General Public License v2.0
368 stars 20 forks source link

Problems moving windows between desktops and improving moving between screens #19

Closed 3ximus closed 7 years ago

3ximus commented 7 years ago

Im having problems when moving windows betweeen desktops. When I have some layout and move one of the windows to a different desktop the space for it is maintained on the original desktop and the window "accumulates movement" or something and is not placed correctly on the 2nd desktop either, as shown in the gif. Also if i were to open windows on the desktop to which i moved the window previously, the space for it is also there. So its like the window in not being deleted from both layouts on both desktops correctly...

out

Also could you add a hotkey to quickly move windows from one screen to another? I have a dual monitor setup and if I use the default Kwin keybinding "Window to next screen" it doesn't respect tilling, however dragging with the mouse does...

I'm currently using commit 3ed8215 (6 commits behind master) since for me the script doesn't work after you added support for activities.

Thanks in advance, you are doing a great job, i which i could help but im not great on javascript.

Jazqa commented 7 years ago

Ohh boy, I've already typed two really long responses but both vanished (yay, mobile).

Anyways, tl;dr:

3ximus commented 7 years ago

No hurries. I don't know if its due to activities but it seems to be from code submited on b5a48c5 I tested this on my laptop aswell and it had the same problem...

Jazqa commented 7 years ago

Definitely the activity support. It was a huge transfer from two-dimensional arrays (tiles[desktop][screen]) to three-dimensional arrays (tiles[activity][desktop][screen]). Almost three hundred lines to change so I may have missed one specific to your use-case (got multiple responses of it working). I'll definitely check it out tomorrow!

Really appreciate the use of my script as well as all the help to improve it. Sorry if the fix takes a while!

Jazqa commented 7 years ago

I'm having hard time figuring this one out. Could you try disabling the script, updating it to the newest commit and logging out? After logging back in, enable the script and see if it works. The activity support changed the whole script, so getting it to run after an update might require a KWin restart.

If you manage to get the activity commit running, test the desktop switch again and see if it works. If it doesn't, I'll try harder to reproduce it and find the bug. If you can't manage to get the activity commit running, report back and I'll keep hunting the issue.

As for the screen switch, I'll add it in ~5 days. Easy addition but need to access multi-screen setup. I'll probably need to add a script-specific shortcut because the KWin's screen switch doesn't send any signals for the script to respond to (which is why moving works, dragging a window sends a signal and I can check the screen after window movement).

3ximus commented 7 years ago

I tried logging out or even restarting but sadly it still doesn't work šŸ˜ž ... However, I found that (on the latest commit) if I load the script and am on desktop 2, as soon as i create a window it moves me to desktop 1, but no tiling occurs and the windows dont come to desktop 1 as they would on another commit. So i assume the script crashes as soon as it moves me to another desktop.

About windows leaving allocated space on the previous desktop, I found a way to trigger this behavior. It only happens when i restart the script and have windows already opened. As soon as I open another window it retiles every window I have into the first desktop... After this when I move one of the windows I had left open when I restarted the script, they leave their space on the previous layout. This only occurs on the windows i had oppened when the script starts. If I open another window and then move it all the others adjust fine. Maybe the windows that existed dont get deleted from the array?

Also, I would like to make another request if you find the time for it. When deleting windows that leave the desktop empy I noticed you decrease the desktop by 1. I understand you made this to use the desktops in order, but usually I have windows on specific desktops say 1 and 3, and if i close everything on the 3rd I'm moved to the 2nd which would have nothing. This is not a big deal at all, however if I have floating windows, or that are ignored by the tilling, they dont enter the array, I assume, and the desktop is considered empty. I end up being moved to another desktop with windows still open in the previous. The same thing happens if I float the only window on a desktop. This case is specially bad since it doesn't mean that I want to be in that desktop. I think you could either remove this feature, make it optional or at least check for floating windows still left on the desktop. I don't know if you find this feature really useful and if it justifies having the extra code but in my opinion it could be removed (since It's not usefull for me).

Sory for the long post but I hope it helps... If you can't reproduce any of the errors I can show you the output of your script or specific variables, if you tell me how šŸ˜„

Jazqa commented 7 years ago

First of all, thank you for the thoroughly explained report! Truly appreciate it. I've mostly created the script for personal use, which is why it has some annoying features (like the automatic desktop switch). I really appreciate all the feedback that helps me adjust the script for a wider audience.

However, I found that (on the latest commit) if I load the script and am on desktop 2, as soon as i create a window it moves me to desktop 1, but no tiling occurs and the windows dont come to desktop 1 as they would on another commit. So i assume the script crashes as soon as it moves me to another desktop.

I'll try to change the script to start on the current desktop. Should be fairly easy.

About windows leaving allocated space on the previous desktop, I found a way to trigger this behavior. It only happens when i restart the script and have windows already opened. As soon as I open another window it retiles every window I have into the first desktop... After this when I move one of the windows I had left open when I restarted the script, they leave their space on the previous layout. This only occurs on the windows i had oppened when the script starts. If I open another window and then move it all the others adjust fine. Maybe the windows that existed dont get deleted from the array?

I am aware of this bug and I've tried to fix it earlier. It's a Qtscript/KWin API thing. I'm following the documentation but I can't properly disconnect the changeDesktop function from the desktopChanged signal. The rest of the functions can be properly disconnected. As a result, changing the desktops of clients that have existed earlier is buggy, like you mentioned.

Also, I would like to make another request if you find the time for it. When deleting windows that leave the desktop empy I noticed you decrease the desktop by 1. I understand you made this to use the desktops in order, but usually I have windows on specific desktops say 1 and 3, and if i close everything on the 3rd I'm moved to the 2nd which would have nothing. This is not a big deal at all, however if I have floating windows, or that are ignored by the tilling, they dont enter the array, I assume, and the desktop is considered empty. I end up being moved to another desktop with windows still open in the previous. The same thing happens if I float the only window on a desktop. This case is specially bad since it doesn't mean that I want to be in that desktop. I think you could either remove this feature, make it optional or at least check for floating windows still left on the desktop. I don't know if you find this feature really useful and if it justifies having the extra code but in my opinion it could be removed (since It's not usefull for me).

A truly wonderful piece of feedback. Thank you. I personally use the feature (saves me clicks), but it's not supposed to happen when manually floating clients, I've just been careless. Having the feature prefer desktops with active clients makes more sense than decreasing the current desktop by one. This is common sense, can't believe I haven't thought of this. I'll improve the feature so it prefers desktops that have the most windows open. I'll also remove the feature when floating clients and add the whole feature as an option, in case you'd rather not use it.

Sory for the long post but I hope it helps... If you can't reproduce any of the errors I can show you the output of your script or specific variables, if you tell me how šŸ˜„

The debugging tool is a bit iffy, now that you've narrowed the cause to the desktop switch (starting the script on desktop 2), I should be able to reproduce it and fix it myself. Anyways, if you feel like going the extra mile, you need to disable the script, copy the whole main.js into KWin scripting console (opened by typing wm console into KRunner [F2]) and run the script from there. You'll have to keep the console open and it'll log all the events. Once the script crashes, copying last 10-20 lines will help me locate the cause. Running the script from the console won't read any of the options and starts bugging out quite easily.

I'll make these issues a priority and start working on them as soon as I get my hands on a computer. Unfortunately the development is extremely slow this week, but I'll catch up next week.

Jazqa commented 7 years ago

Working on these issues inside a different "beta" branch as I don't have an access to a dual-screen setup and I don't want to mess master up. Currently the script can be initiated from any desktop (doesn't throw you back to the first one) and after closing the last window of a desktop, the script finds the busiest desktop and switches to that one. Floating a window doesn't change the desktop automatically anymore.

3ximus commented 7 years ago

Thanks! I checked and as you said it starts in the current desktop, but it still crashes like i described before unfortunately... I haven't tried to debug the problem yet, but as soon as I do I'll try and work it out with you.

Jazqa commented 7 years ago

Are you using programs with borders hidden via KWin's "Window Rules"? Yesterday I found out that such windows don't have activities-property like all other windows, thus crashing the script. I've quickly fixed the issue by using the current activity instead of the window's activity when dealing with such windows.

If this was the case. Updating the script to master should fix most, if not all, of the bugs you're experiencing. I'll also add the shortcut for screen-switching later today. (Edit: Committed the shortcuts for moving windows between screens)

3ximus commented 7 years ago

Yep, I was using no borders with kwin rules in all windows... It's working now!! Thank you and keep it up!!