ENH: ensure window focus for the first win.flip() with pyglet backend on Windows OS

[mh105]

Alright, round two of mouse visibility PRs (first one was #6465). Hopefully this time the annoying mouse is gone for good.

Previously there was a buggy draggable() event that caused visible mouse despite correct settings of showMouse and fullScreen. #6465 fixed it, and the mouse visibility is behaving correctly on macOS since then. Yesterday I started testing on Windows laptops and was horrified by the visible mouse again on standalone PsychoPy via builder.

After some digging, I've fixed the issues around this. Here's a breakdown of the commits, which also improve how mouseVisible and fullscr attributes are stored in window.Window().

1. Avoid using __dict__[ATTRIBUTE_NAME] along with @attributeSetter def ATTRIBUTE_NAME(self). This setup has a dangerous wobbly attribute type for ATTRIBUTE_NAME. Before self.ATTRIBUTE_NAME = xxx is called, querying self. ATTRIBUTE_NAME returns an attributeSetter method; while after the attribute is set, it returns the actual attribute. This makes type setting and downstream usage of self.ATTRIBUTE_NAME unreliable since the attribute is not guaranteed to be set before called. It is better to use @property + @ATTRIBUTE_NAME.setter def ATTRIBUTE_NAME(self) to provide the same API without the problematic mutable type behavior. b6025f7 and 4a99206 implement this for mouseVisible and fullscr. In addition, since Window() always has a backend that has a mouseVisible attribute already, it's better to have Window.mouseVisible return self.backend.mouseVisible directly instead of keeping another attribute variable around, which may go out of sync with the backend mouseVisible if something goes wrong and makes it difficult to debug.
2. Use allowGUI param to Window constructor. There appears to be some adhoc code added to attempt to set mouse visibility when generating scripts from builder. Since the allowGUI parameter is available and accepted to window.Window(), we should use that instead of the extra manual win.mouseVisible = allowGUI. This keeps the win.allowGUI relevant and avoids confusing syntax. This is done by d052c89.
3. Fix a root cause of losing focus that led to the visible mouse on Windows OS with pyglet backend. I'm guessing that win.mouseVisible = allowGUI was added because people found that passing allowGUI=False into window.Window() didn't hide the mouse. But clearly manually setting win.mouseVisible = allowGUI doesn't work either unless it is called every frame on Windows OS using the pyglet backend with Win32Window window type.

The core of this problem is a bit complicated: After instantiating the backend and the Win32Window handle, the _hwnd has lost its foreground status by the time it gets to the very first win.flip() call that does not have anything in ._to_draw(). This is due to the additional process started during starting ioHub server. This causes the PsychoPy window to temporarily obtain focus via setCurrent() during win.flip() and immediately lose the focus after, regardless of whether win is on focus before this win.flip() call. Therefore, now the PsychoPy screen has lost focus despite still being visible.

Due to the way pyglet Win32Window updates _mouse_platform_visible using a sequence of or statements:

platform_visible = (self._mouse_visible and
                    not self._exclusive_mouse and
                    (not self._mouse_cursor.gl_drawable or self._mouse_cursor.hw_drawable)) or \
                   (not self._mouse_in_window or
                    not self._has_focus)

, the mouse now stays visible, since the window no longer has focus.

UPDATE: this problem only affects very few people. If your mouse is not visible when experiment starts, you are probably fine! This is actually a very vulnerable behavior beyond the annoying visible mouse, unless users have always clicked mouse at least once on the PsychoPy screen to force grab focus and bring it to foreground from the OS level before doing anything else. Since I'm running without a mouse, this causes the focus to stay on the builder view. And sadly I'm using the spacebar to advance through PsychoPy screens, but now the keypress is double registered to the builder view, which maps to "Run Experiment" - so as I'm running a PsychoPy task, it silently spawned another experiment behind the screen... Similarly spacebar and any other keypress could cause unwanted consequences in this situation due to the messed up focus, including clearing the Runner output panel, etc., etc.

There are many different ways to fix this issue, but the cleanest solution only requires one extra line of code before the win.flip() and is provided in b295a89.

4. a860c69 and c1992f5 provide harmless code styling fixes following PEP8.

---

@peircej @TEParsons I think this is a critical PR that should be pushed out soonish, since it can impact people using Standalone PsychoPy via builder on Windows OS, unless they have developed the "good" habit of clicking the PsychoPy window every time they start an experiment. I unfortunately found that the lost focus issue could sometimes result in missing the first keyboard press on each new screen, and only the second keypress gets registered (with the first keypress gaining focus temporarily but losing it immediately after). This issue will impact logged behavioral outputs, so it'll be good to fix.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 49.62%. Comparing base (713ea1f) to head (c1992f5).
Report is 14 commits behind head on release.

Additional details and impacted files

@@           Coverage Diff            @@
##           release    #6874   +/-   ##
========================================
  Coverage    49.62%   49.62%           
========================================
  Files          332      332           
  Lines        61201    61208    +7     
========================================
+ Hits         30373    30377    +4     
- Misses       30828    30831    +3     

Components	Coverage Δ
app	∅ <ø> (∅)
boilerplate	∅ <ø> (∅)
library	∅ <ø> (∅)
vm-safe library	∅ <ø> (∅)

[mh105]

Lol just did more testing and figured out what's actually going on...

Good news: the loss of focus and visible mouse do not occur naturally to any of the released Standalone PsychoPy distributions, including 2024.2.2. Everyone's behavioral response data are safe, yay!

Bad news (for me): the reason why I was seeing the visible mouse after io.launchHubServer() is because I installed the Standalone PsychoPy but swapped out the psychopy package using a symlink to point to a Github repo so that I can more easily test changes / debug. It turned out that despite setting identical security/permission on the GitHub repo and running as administrator, the process started over a symlink seems to confuse Windows OS about whether the same window is still on foreground, hence the loss-of-focus problem after the first win.flip(). There is probably only a handful of people doing this hacky swap of psychopy package, so no one else should encounter this issue of lost focus on Windows OS with pyglet backend.

Oh well, at least I figured out the solution. If the psychopy team don't mind, I'd appreciate you merging the line

"win.winHandle.activate()  # set window to foreground to prevent losing focus\n"

into the next release. Calling the extra activate() won't be harmful in any way, and it will ensure that the PsychoPy window is indeed on the foreground by the time the first win.flip() is called; so just an extra safety measure :)

[TEParsons]

These changes all look good, to give some context on attributeSetter vs property/setter: A few contributors (@isolver and @lindeloev I think) did some timing tests and found that property/setter pairs introduced some degree of overhead compared to simple attributes, so developed a property/setter-like solution which didn't introduce this overhead, which was attributeSetter. The amount of overhead is only really important when you're doing stuff frame-by-frame, which I can't imagine anyone is doing to toggle fullscreen or mouse visibility, so switching out for property/setter pairs should be perfectly fine. It may even be that Python itself has improved such that there's no longer any overhead to property/setter pairs, as this was some time ago.

You're right about the downside of attributeSetter being that values are sometimes returned as a method handle, it's a tradeoff we make for speed. I've tried to create workarounds for this in the past but with limited success. One option I've been meaning to try out is to use the __set_name__ method in combination with inspect to essentially say "when an attributeSetter is assigned to an attribute name, if the method has a default value, set the corresponding value in self.__dict__ to that value", but I haven't had time to give it a proper go.

[mh105]

@TEParsons thanks for the contextual info! Yeah if it's used frame-by-frame then we won't be worried about the attribute not being set before called... so it's probably better to use the more traditional @Property syntax on these experimental settings :) Thanks for merging quickly!

[lindeloev]

Sounds like a good solution! Back in the days, I timed getting self.my_property to take 204 ns on average while getting self.my_var took 44 ns on average. So either way it's not the end of the world re timing.

Here's the StackOverflow question where the attributeSetter solution came from. It includes some alternative solutions too: https://stackoverflow.com/questions/17576009/python-class-property-use-setter-but-evade-getter

The primary motivation was actually to be able to set attributes like stim.size = 20 and stim.size += 1 while logging them too. Previous to that, you would do stim.setSize(20) or stim.setSize(1, "+"). Timing was secondary.

[mh105]

Thanks for that additional insight @lindeloev !! I agree that it sounds like PsychoPy should continue to use the __dict__[my_var] approach in most situations with run-time dependent attributes, which is both faster and allows for the stim.my_var += 1 syntax.

But for experiment settings like win.fullscr and win.mouseVisible that could potentially be not yet set prior to getting called, we should revert back to the @property + @my_property.Setter route. I probably won't go through the objects (including window.Window()) for now to reflect this change - we might just update them on the go, if we encounter future issues of calling an attribute returning an attributeSetter method during maintenance/debugging.