Skip to content

Revert "Improve responsiveness of input and focus by pumping immediately instead of after a delay. (#14701)"#14925

Merged
michaelDCurran merged 1 commit into
masterfrom
revert_pr14701
May 15, 2023
Merged

Revert "Improve responsiveness of input and focus by pumping immediately instead of after a delay. (#14701)"#14925
michaelDCurran merged 1 commit into
masterfrom
revert_pr14701

Conversation

@michaelDCurran

Copy link
Copy Markdown
Member

This reverts pr #14701, commit cc8b5ad

Link to issue number:

Fixes #14753
May fix #14803

Summary of the issue:

After the merging of pr #14701, It was reported in #14753 that with input help mode on and moving your finger around the touch screen, many errors would be logged and eventually input help mode would be disabled.
An example rrror was:

ERROR - scriptHandler.executeScript (20:37:51.892) - MainThread (1032):
error executing script: <bound method GlobalCommands.script_touch_explore of <globalCommands.GlobalCommands object at 0x07873CF0>> with gesture 'hover'
Stack trace:
  File "nvda.pyw", line 406, in <module>
  File "core.pyc", line 794, in main
  File "wx\core.pyc", line 2237, in MainLoop
  File "core.pyc", line 761, in Notify
  File "core.pyc", line 731, in request
  File "core.pyc", line 761, in Notify
... another 100 times or so of request / notify calls... 
  File "queueHandler.pyc", line 97, in pumpAll
  File "queueHandler.pyc", line 64, in flushQueue
  File "scriptHandler.pyc", line 243, in _queueScriptCallback
  File "inputCore.pyc", line 221, in executeScript
  File "scriptHandler.pyc", line 297, in executeScript
Traceback (most recent call last):
  File "scriptHandler.pyc", line 295, in executeScript
  File "globalCommands.pyc", line 3849, in script_touch_explore
  File "screenExplorer.pyc", line 31, in moveTo
  File "NVDAObjects\__init__.pyc", line 322, in objectFromPoint
  File "NVDAObjects\__init__.pyc", line 99, in __call__
  File "NVDAObjects\IAccessible\__init__.pyc", line 482, in findOverlayClasses
  File "baseObject.pyc", line 41, in __get__
  File "NVDAObjects\IAccessible\__init__.pyc", line 908, in _get_role
  File "baseObject.pyc", line 62, in __get__
  File "baseObject.pyc", line 168, in _getPropertyViaCache
  File "NVDAObjects\IAccessible\__init__.pyc", line 920, in _get_IAccessibleStates
  File "comtypes\__init__.pyc", line 856, in __call__
  File "monkeyPatches\comtypesMonkeyPatches.pyc", line 32, in __call__
ctypes.ArgumentError: argument 1: <class 'RecursionError'>: maximum recursion depth exceeded while calling a Python object

The recursion itself was caused by the core pump's Notify method calling request directly, which itself calls Notify.
The obvious fix would be to wx.CallAfter the call to request. However, this then caused the WX event loop to fill up with responses to these call afters, and due to the way the wx event loop works (msgWaitForMultipleObjects waiting on a pending event win32 event), the event loop never ended up getting a chance to process OS and 3rd party messages. And because we monkeypatch CallAfter to PostMessage (in case we are in a win32 modal or menu and don't wake up) the win32 message queue was being filled completely thus not allowing future messages to be posted.

@jcsteh and I have considered several different ways of trying to work around this. Most of which have not been successful. There are probably more things we could try, but for now it is better to revert until we can get this right.

Description of development approach

Fully reverts pr #14701 for now.

…ely instead of after a delay. (#14701)"

This reverts commit cc8b5ad.
@michaelDCurran michaelDCurran requested a review from a team as a code owner May 14, 2023 23:32
@michaelDCurran michaelDCurran requested a review from seanbudd May 14, 2023 23:32
@jcsteh

jcsteh commented May 14, 2023

Copy link
Copy Markdown
Contributor

@michaelDCurran, when you get some time, it'd be great if you can document what you tried and how it failed so I have a chance of fixing this. We discussed this a bit on Slack, but, for example, I don't know why the while loop approach failed.

One of your concerns was potentially blocking the core for too long. I don't see how a timer would fix this. Sure, it yields to the message loop more often, but that is effectively just kicking the can down the road to the next pump, where it will process a large batch of queued scripts without yielding.

@michaelDCurran

Copy link
Copy Markdown
Member Author

@jcsteh see NV Access branch i14753. There are several various commits there, including the while loop one I think.
I think much of what we have tried would have worked, if it were not for the need to PostMessage on wx.CallAfter if we are in a modal / menu. If we could find a replacement for that, I think the rest would work.

@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 3b1ce99eb1

@michaelDCurran michaelDCurran merged commit 15b5c69 into master May 15, 2023
@michaelDCurran michaelDCurran deleted the revert_pr14701 branch May 15, 2023 04:10
@nvaccessAuto nvaccessAuto added this to the 2023.2 milestone May 15, 2023
josephsl pushed a commit to josephsl/nvda that referenced this pull request May 16, 2023
…ely instead of after a delay. (nvaccess#14701)" (nvaccess#14925)

This reverts pr nvaccess#14701, commit cc8b5ad

Link to issue number:
Fixes nvaccess#14753
May fix nvaccess#14803

Summary of the issue:
After the merging of pr nvaccess#14701, It was reported in nvaccess#14753 that with input help mode on and moving your finger around the touch screen, many errors would be logged and eventually input help mode would be disabled.

The recursion itself was caused by the core pump's Notify method calling request directly, which itself calls Notify.
The obvious fix would be to wx.CallAfter the call to request. However, this then caused the WX event loop to fill up with responses to these call afters, and due to the way the wx event loop works (msgWaitForMultipleObjects waiting on a pending event win32 event), the event loop never ended up getting a chance to process OS and 3rd party messages. And because we monkeypatch CallAfter to PostMessage (in case we are in a win32 modal or menu and don't wake up) the win32 message queue was being filled completely thus not allowing future messages to be posted.
@jcsteh and I have considered several different ways of trying to work around this. Most of which have not been successful. There are probably more things we could try, but for now it is better to revert until we can get this right.

Description of development approach
Fully reverts pr nvaccess#14701 for now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NVDA is stopping during touchscreen exploration Touchscreen support: recursion error when moving around the screen in Windows 11 22H2

5 participants