Skip to content

Window attributes not updated after window move (on windows 10) #494

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
codecat555 opened this issue Mar 4, 2022 · 18 comments
Closed

Window attributes not updated after window move (on windows 10) #494

codecat555 opened this issue Mar 4, 2022 · 18 comments

Comments

@codecat555
Copy link

Talon version v0.2.0-357-g3b3e (357)
knausj_talon @14a370f18f248ba19177a2543c49335b415a1464

I've noticed that the 'snap next' command is no longer working as expected on my windows 10 system (and 'snap last', and related commands).

This seems to be due to a failure to update the window attributes after moving the window.

For example, I used the repl to display the attributes of a notepad.exe window on my main screen (DISPLAY1). Then, I ran the 'snap next' command while the notepad window was focused. The notepad window did in fact move to the next screen (DISPLAY3), but the window attributes (including the window screen) reported by the repl were unchanged.

I recorded a video of this test, but it's too big to upload here (112 mb). I also have the talon log and I captured the event log.

@codecat555
Copy link
Author

I forgot to copy my repl output yesterday, so I repeated the test this morning -

talon.log

Talon REPL | Python 3.9.10 (tags/v3.9.10:f2f3f53, Jan 17 2022, 15:14:21) [MSC v.1929 64 bit (AMD64)] on win32)
>>> import time
>>> time.sleep(3); w=ui.active_window()
>>> w
Window(app=App(pid=26068, "Notepad"), title="Untitled - Notepad")
>>> w.id
6229688
>>> w.rect
Rect(1604, 399, 455, 365)
>>> w.screen.name
'\\\\.\\DISPLAY1'
>>> w.screen
Screen(x=0.0, y=0.0, size=(2560.0, 1440.0), mm=(600.000, 340.000))
>>>
>>>
>>>
>>> ui.screens()
[Screen(x=0.0, y=0.0, size=(2560.0, 1440.0), mm=(600.000, 340.000)), Screen(x=-2560.0, y=-166.0, size=(2560.0, 1600.0), mm=(641.000, 401.000)), Screen(x=2560.0, y=227.0, size=(1920.0, 1200.0), mm=(641.000, 401.000))]
>>>
>>> for s in ui.screens():
...   print(f'{s.name}: {str(s)}')
...
\\.\DISPLAY1: Screen(x=0.0, y=0.0, size=(2560.0, 1440.0), mm=(600.000, 340.000))
\\.\DISPLAY2: Screen(x=-2560.0, y=-166.0, size=(2560.0, 1600.0), mm=(641.000, 401.000))
\\.\DISPLAY3: Screen(x=2560.0, y=227.0, size=(1920.0, 1200.0), mm=(641.000, 401.000))
>>>
>>>
>>>
>>> time.sleep(3); ui.active_window().id; mimic('snap next')
6229688
>>>
>>>
>>>
>>> w
Window(app=App(pid=26068, "Notepad"), title="Untitled - Notepad")
>>> w.id
6229688
>>> w.rect
Rect(1604, 399, 455, 365)
>>> w.screen.name
'\\\\.\\DISPLAY1'
>>> w.screen
Screen(x=0.0, y=0.0, size=(2560.0, 1440.0), mm=(600.000, 340.000))
>>>
>>>

@codecat555
Copy link
Author

Note: in this test, the notepad.exe window started on my main display, named 'DISPLAY1'.

The 'snap next' command moved the window to the display named 'DISPLAY3', which is correct.

After, the window reference 'w' shown above did not show the updated values.

Then, I re-acquired the 'w' reference and it did show the new values -

>>>
>>> time.sleep(3); w=ui.active_window()
>>> w.screen.name
'\\\\.\\DISPLAY3'
>>> w.id
6229688
>>>                                                                  

I expected the reference to show the updated values without needing to be reset. That's the way it used to work.

@jcaw
Copy link

jcaw commented Mar 4, 2022

I've also encountered this, in the same context

@lunixbochs
Copy link

lunixbochs commented Mar 4, 2022

@codecat555 when you say "That's the way it used to work" do you have a timeframe?

git says I haven't really touched this code since December, and that was only changes to Window focus handling. I don't think I've messed with anything besides the focus code since much longer than that (maybe last June or earlier)

Oh, I just realized what you meant by reacquiring ui.active_window(). Can you show me these for your test, before setting w = ui.active_window() again?

id(w)
id(ui.active_window())
w == ui.active_window()
id(w.app)
id(ui.active_app())
id(w.app._windows[w.id])
id(ui.active_app()._windows[w.id])

@codecat555
Copy link
Author

Yes, I was working intensively on my window move/resize code for a month or two during November/December of last year. Even into January...

During that time, I frequently used the repl in this way to troubleshoot my code.

To be clear, I originally started looking into this because the snap next/last commands were behaving oddly on my windows 10 system. I wasn't interested in the behavior of references, but just trying to figure out why my windows weren't behaving.

For instance, at one point yesterday I said 'snap next' and the current window jumped to the next screen and then immediately popped back to its previous location.That's not happening anymore, but perhaps it's related.

@lunixbochs
Copy link

lunixbochs commented Mar 4, 2022

I just added some values to grab in my last comment, they may help diagnose

@codecat555
Copy link
Author

codecat555 commented Mar 4, 2022

In your example, should this be assignment instead?

w == ui.active_window()

@codecat555
Copy link
Author

codecat555 commented Mar 4, 2022

>>> time.sleep(3); ui.active_window().id; mimic('snap next')
6229688
>>>
>>>
>>> id(w)
76808096
>>> time.sleep(3); id(w); id(ui.active_window()); w == ui.active_window(); w = ui.active_window()
76808096
76808096
True
>>> time.sleep(3); id(w.app); id(ui.active_app()); id(w.app._windows[w.id]); id(ui.active_app()._windows[w.id])
85748416
85748416
76808096
76808096
>>>

@codecat555
Copy link
Author

codecat555 commented Mar 4, 2022

I see in the output above that it looks like the window id changed from 6229688 to 76808096.

That seems odd, so I repeated the test. Interestingly, this time 'snap next' moved my window from DISPLAY1 to DISPLAY2 rather than to DISPLAY3.

Here's the output -

>>> time.sleep(3); w=ui.active_window(); id(ui.active_window());
76808096
>>> time.sleep(3); id(w.app); id(ui.active_app()); id(w.app._windows[w.id]); id(ui.active_app()._windows[w.id])
85748416
85748416
76808096
76808096
>>> time.sleep(3); ui.active_window().id; mimic('snap next')
6229688
>>> time.sleep(3); id(w); id(ui.active_window()); w == ui.active_window(); w = ui.active_window()
76808096
76808096
True
>>> time.sleep(3); id(w.app); id(ui.active_app()); id(w.app._windows[w.id]); id(ui.active_app()._windows[w.id])
85748416
85748416
76808096
76808096
>>>

@lunixbochs
Copy link

lunixbochs commented Mar 4, 2022

id(w) is the address of the object, that's different from w.id which is the window handle
I don't see anything in this output that suggests ui.active_window() would have fixed w.rect

Can you switch to a big print in this style so it's easier to read? And try to show the w.rect != ui.active_window().rect issue at the same time you print all the IDs.

print(f"{w.id=} {id(w.app)=}")

@jcaw
Copy link

jcaw commented Mar 4, 2022

I remember talking about an issue similar to this on Slack a long while ago - I think it was when I was first working on the knausj window snapping implementation.

When windows are manipulated programmatically with talon's API (on Windows), it seems the ui.Window.hidden attribute will sometimes start returning True, even if the window is still visible. I have a note in my window snapping code talking about this dated 2020/10/05, so that may be when the slack conversations happened.

I remember there were issues interfacing with the Windows API and you eventually triaged them as something to tackle another time. I don't have any other notes there, so I'm not sure if other attributes came up as failing to update in the discussion.

@lunixbochs
Copy link

hidden comes from win32gui.IsWindowVisible, which might just be wrong sometimes. Maybe I could fetch that one on demand instead of caching it.

@lunixbochs
Copy link

lunixbochs commented Mar 4, 2022

@codecat555

Then, I re-acquired the 'w' reference and it did show the new values -

In this example, you sleep(3) and presumably change window focus before grabbing ui.active_window() again, so the output doesn't rule out the sleep or window focus as the reason for the attribute change. I think you should set up one big print() like the template I showed you and run the entire print for each test, printing the attributes and IDs of both ui.active_window() and your saved w object. Maybe version your w, so you have w1, w2, w3 based on when you fetched the ui.active_window(). And you can check w1 is w2 to see if they're the same object or not.

@jcaw
Copy link

jcaw commented Mar 4, 2022

hidden comes from win32gui.IsWindowVisible, which might just be wrong sometimes. Maybe I could fetch that one on demand instead of caching it.

Alternatively, a bool param for force refresh?

@codecat555
Copy link
Author

@lunixbochs

The problem magically fixed itself while I was working on reproducing the problem with the alternate formatting you requested. I didn't make any changes to my system or talon config, I didn't restart talon or the repl or use a different target window. All I did was switch between various windows and edit the test code. Very frustrating.

Here's the output, just to document the new test procedure -

>>> time.sleep(3); w=ui.active_window();
>>> time.sleep(3); print(f"{w.id=}\n{id(w)=}\n{id(ui.active_window())=}\n{id(w.app)=}\n{w == ui.active_window()=}\n{w.rect == ui.active_window().rect=}\n{w.screen.name=}")
w.id=6229688
id(w)=76808096
id(ui.active_window())=76808096
id(w.app)=85748416
w == ui.active_window()=True
w.rect == ui.active_window().rect=True
w.screen.name='\\\\.\\DISPLAY1'
>>>
>>>
>>> time.sleep(3); mimic('snap next')
>>>
>>>
>>> time.sleep(3); print(f"{w.id=}\n{id(w)=}\n{id(ui.active_window())=}\n{id(w.app)=}\n{w == ui.active_window()=}\n{w.rect == ui.active_window().rect=}\n{w.screen.name=}")
w.id=6229688
id(w)=76808096
id(ui.active_window())=76808096
id(w.app)=85748416
w == ui.active_window()=True
w.rect == ui.active_window().rect=True
w.screen.name='\\\\.\\DISPLAY3'
>>>

@codecat555
Copy link
Author

BTW, here's some more background information which may be helpful.

When things were acting weird, 'snap next' and 'snap last' we're only working across two of my three screens. Normally, one can say 'snap next' or 'snap last' repeatedly and cycle through all of one's screens. That just stops working sometimes - the second 'snap next' or 'snap last' command just does nothing. That was true for me earlier, but now it's all working fine again. I've seen this problem more than a few times.

Perhaps I should also mention that I actually have four screens on my system, since it's a laptop. I almost always use it with the lid closed and connected to a docking station. One of the external displays is plugged directly into the HDMI port on the laptop and the other two are connected to the docking station (one HDMI and one DisplayPort).

I wouldn't expect these low level details to make any difference to Talon, since the Windows layer seems to have no problems with this setup. Indeed, ui.screens() correctly shows just the 3 external displays. I do see problems if I leave talon running and disconnect from the docking station and then reconnect later. A reboot clears that up.

@codecat555
Copy link
Author

codecat555 commented Mar 23, 2022

Talon version: v0.2.0-376-gd29b (376)

Update - I just hit another instance of bizarre 'snap next' behavior. It's a problem that recurs with some regularity, if intermittently. In fact, it's the problem that originally caused me to start investigating things and eventually leading to this bug.

I had a firefox window on my main (middle) screen, sized to about 50% of the screen and roughly centered. I said 'snap next' and instead of jumping to the next screen, the window jumped to the upper left corner of the same screen and shrunk down to a fraction of its original size.

Since this is a recurring problem, I had added some debug code to log some additional information. This is what it printed, note the bizarre window coordinates (-31993, -32000) (also, the width and height are not correct):

2022-03-23 13:10:24    IO 'snap next'
2022-03-23 13:10:24    IO [audio]=1260.000ms [compile]=0.018ms [emit]=46.815ms [decode]=4.769ms [total]=51.602ms
2022-03-23 13:10:24    IO _move_to_screen: BEFORE - src_screen.name='\\\\.\\DISPLAY1', dest_screen.name='\\\\.\\DISPLAY3', Rect(-31993, -32000, 146, 31)
2022-03-23 13:10:24    IO _move_to_screen: AFTER - \\.\DISPLAY1, Rect(-31993, -32000, 146, 31)

Here's what the repl shows for my screens -

>>> print('\n'.join([f'{s.name}: {s}' for s in ui.screens()]))
[\\.\DISPLAY1](file:///DISPLAY1): Screen(x=0.0, y=0.0, size=(2560.0, 1440.0), mm=(600.000, 340.000))
[\\.\DISPLAY2](file:///DISPLAY2): Screen(x=-2560.0, y=-166.0, size=(2560.0, 1600.0), mm=(641.000, 401.000))
[\\.\DISPLAY3](file:///DISPLAY3): Screen(x=2560.0, y=227.0, size=(1920.0, 1200.0), mm=(641.000, 401.000))
>>>

This was shortly after resuming from hibernate, and I wonder if that might have something to do with talon's confusion.

For clarity, here is the diff showing my debug changes relative to the stock knausj_talon code/window_snap.py @9c622539cd0d0af35a8d87cc159b8792ceb45754 -

--- code/window_snap.py.master  2022-03-23 13:36:59.038693200 -0700
+++ code/window_snap.py.wip.py  2022-03-23 14:24:07.254022700 -0700
@@ -66,6 +66,8 @@
     else:
         dest_screen = actions.user.screens_get_by_number(screen_number)

+    print(f'_move_to_screen: BEFORE - {src_screen.name=}, {dest_screen.name=}, {window.rect}')
+
     if src_screen == dest_screen:
         return

@@ -118,6 +120,18 @@
         height = window.rect.height * proportional_height
     _set_window_pos(window, x=x, y=y, width=width, height=height)

+    print(f'_move_to_screen: AFTER - {window.screen.name}, {window.rect}')
+    if src_screen.name != dest_screen.name and window.screen.name == src_screen.name:
+        import win32gui
+        win32gui.MoveWindow(
+            window.id,
+            round(x),
+            round(y),
+            round(width),
+            round(height),
+            False
+        )
+        print(f'_move_to_screen: win32gui result - {window.screen.name}, {window.rect}')

 def _snap_window_helper(window, pos):
     screen = window.screen.visible_rect

@lunixbochs
Copy link

lunixbochs commented Oct 25, 2022

I believe this is fixed on Talon 0.3.1-49-g8e47. If you encounter it on that version or later, before restarting Talon, please open the repl and run

debug.print_threads()

and post the output here, as well as your Talon version. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants