khimaros / smart-auto-move

Smart Auto Move learns the size and position of your application windows and restores them to the correct place on subsequent launches. Supports Wayland.
https://extensions.gnome.org/extension/4736/smart-auto-move/
GNU General Public License v3.0
79 stars 4 forks source link

Listen for signals instead of polling #35

Closed rockerbacon closed 1 year ago

rockerbacon commented 1 year ago

I took a quick look at the code and it seems that the extension frequently polls the windows and applies the updates. I was curious if there was any specific reason for doing it like that instead of listening for the app's windows-changed signal, for example.

One difference I can notice between this extension and auto-move-windows - which does use the aforementioned signal - is that that extension moves windows very quickly, before they're visible. With smart-auto-move, the windows sometimes flicker on the currently active workspace before being moved to their proper location. I haven't modified the code to confirm whether the polling is the cause but I strongly suspect that it is.

khimaros commented 1 year ago

this was actually how smart-auto-move was originally implemented: https://github.com/khimaros/smart-auto-move/commit/00c189e053c4c6ac48a77025fcb23ca89569fb2a

however, due to a limitation with the window-changed signal handler, it is not actually possible to manipulate window properties inside of the callback, so it was necessary to workaround this by returning control to the main loop and then moving the window in another context.

from the linked changelist:

there is a fixed delay of 4000ms between when a window is created and when smart-auto-move restores its position. this is due to a limitation of the windows-changed signal. changing a window's frame rect inside of this handler does not actually move the window, so it instead needs to happen in the Mainloop. rather than using a fixed delay before restoring a window, it may be possible to poll the window's frame rect to determine if it is ready to be moved. this may be as simple as checking if the rect is currently set to all 0 values.

that said, it may be worth re-evaluating this approach, as there are some bugs and issues with the time based approach, such as https://github.com/khimaros/smart-auto-move/issues/31

khimaros commented 1 year ago

duplicate of https://github.com/khimaros/smart-auto-move/issues/28