tcorreabr / Parachute

Look at your windows and desktops from above.
GNU General Public License v3.0
354 stars 32 forks source link

Windows become invisible to Parachute when virtual desktop bar dynamically removes empty desktops #12

Open AndydeCleyre opened 4 years ago

AndydeCleyre commented 4 years ago

Arch Linux kwin 5.18.3 kwin-scripts-parachute 0.5 plasma5-applets-virtual-desktop-bar-git r422.25ec1c0

Videos speak louder than text: https://streamable.com/voaam

Possibly highly relevant: https://github.com/wsdfhjxc/virtual-desktop-bar#compatibility-with-kwin-tiling-scripts

tcorreabr commented 4 years ago

As I don't have virtual-desktop-bar installed at the moment, I don't know if this is an issue here or there. For now, I'll mark it as a bug.

AndydeCleyre commented 4 years ago

Please have a look at https://github.com/wsdfhjxc/virtual-desktop-bar/blob/master/KWIN.md .

I think the issue is actually not about adding new desktops, but around the fact that VDB tries to delete any "extra" empty desktops (leaving exactly 1 empty one at all times), and in doing so does some re-arranging of desktops and windows. I'll try to better name this issue.

Inviting @wsdfhjxc for comment

wsdfhjxc commented 4 years ago

As the linked document states, using VDB with KWin scripts might cause such issues. It's a consequence of running different programs revolving around the same thing (virtual desktop management) at the same time. I'm not sure about the technical details in this particular case.

If this is a matter of your script not handling KWin signals regarding external changes, e.g. adding or removing desktops, then it's a thing to look into. But if your script can already handle these, then maybe VDB does things too fast, so there is a race condition when KWin signals are passed to the script. That's why I tried to create a custom API for communicating with KWin scripts, which is described in the linked document, but that's a pathetic, unreliable, half-assed workaround.

My advice is to not waste your time on trying to integrate interfering solutions, and instead, just do your own thing at developing Parachute. It looks fantastic, and I see you have cool ideas about improving the script further. It has great potential, and I'm going to give it a try in the future.

tcorreabr commented 4 years ago

The management of how many desktops there are and which windows belong to which desktop is all done using a data model provided by Kwin himself. Parachute is just a frontend for this model. So I thought this error was strange at first sight, unless there is code to be fixed on these models.

I really like the idea of moving virtual desktops or removing particular ones. It would be awesome if Parachute supported it. But don't you think that these features should be provided by Kwin/Plasma API? So that we can obtain them without installing any specific script or applet. You have probably gone through this question, but I ask to know if you propose this to the Kde devs.

Congrats on your applet, it looks fantastic too.

wsdfhjxc commented 4 years ago

The management of how many desktops there are and which windows belong to which desktop is all done using a data model provided by Kwin himself. Parachute is just a frontend for this model. So I thought this error was strange at first sight, unless there is code to be fixed on these models.

There are probably some redundant method calls when VDB is running in the automatic mode, for example removing one desktop and instantly creating another one. It might be unnoticeable for the user, but KWin signals are still emitted for these events. I think that changes made to the model might not be correctly propagated, because the event system isn't designed for such an unusual API usage. So, in the end, it looks like it's my fault. Not that I expected anything else ;)

I really like the idea of moving virtual desktops or removing particular ones. It would be awesome if Parachute supported it. But don't you think that these features should be provided by Kwin/Plasma API? So that we can obtain them without installing any specific script or applet. You have probably gone through this question, but I ask to know if you propose this to the Kde devs.

Yes, these features should be supported natively. And actually, virtual desktop management in Plasma has been improved in the last year or so, and it seems it's now possible to remove desktops other than the last one, either via Virtual Desktops KCM, or with a D-Bus method call.

However, I don't think the KWin's API for scripts has been updated to reflect this change. In addition, that D-Bus method requires a desktop ID (not a desktop number) as an argument. I'm not sure, is it doable to obtain such an ID within a KWin script? I guess not. Because of that, even though it's possible to call D-Bus methods within KWin scripts, that particular method can't be used correctly.

As for moving virtual desktops, I didn't find any abstracted method that does it. While it's possible to rearrange desktops by dragging them in the Desktop Grid effect, it seems to be implemented internally in its code, and it also involves massively moving individual windows between desktops.

That's all assuming I'm not mistaken, maybe I overlooked something, I don't know. Also, I didn't try to fill feature requests or contribute to KWin. I think its developers are focused on providing better Wayland support, instead of implementing new features.

tcorreabr commented 4 years ago

There are probably some redundant method calls when VDB is running in the automatic mode, for example removing one desktop and instantly creating another one. It might be unnoticeable for the user, but KWin signals are still emitted for these events. I think that changes made to the model might not be correctly propagated, because the event system isn't designed for such an unusual API usage. So, in the end, it looks like it's my fault. Not that I expected anything else ;)

Now I understand the problem better. Thanks for the explanation.

As for moving virtual desktops, I didn't find any abstracted method that does it. While it's possible to rearrange desktops by dragging them in the Desktop Grid effect, it seems to be implemented internally in its code, and it also involves massively moving individual windows between desktops.

Well, it is not ideal but in the end if there is no other way I could implement this solution internally as well. For now, I'll remove the bug label and leave this issue open.

tcorreabr commented 4 years ago

I'm closing this one just for organization. But feel free to add any relevant information.

AndydeCleyre commented 4 years ago

Do you plan on adding dynamic desktop behavior to parachute itself? As in always ensuring exactly one empty desktop exists?

tcorreabr commented 4 years ago

So far I don't know. Because I'm not sure what kind of conflicts this could cause when Parachute and some other dynamic desktop script are installed together.

AndydeCleyre commented 4 years ago

Yeah, I was thinking that including the functionality would be a way to avoid those conflicts, so that people could either remove those other scripts or disable those features of other addons.

tcorreabr commented 4 years ago

Sorry man. The discussion of whether this functionality will be implemented internally or not can continue here. My mistake.