Skip to content
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

Upgrade to wxPython 4.0.0 #7077

Closed
josephsl opened this issue Apr 16, 2017 · 29 comments
Closed

Upgrade to wxPython 4.0.0 #7077

josephsl opened this issue Apr 16, 2017 · 29 comments
Labels
p3 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority
Milestone

Comments

@josephsl
Copy link
Collaborator

Hi,

Robin Dunn announced on April 15th that wxPython Phoenix will be known as wxPython 4, and an alpha was posted to PyPI. In addition to providing an easier way for us to migrate to Python 3.x, it modernizes wxPython code.

Python Package Index link: https://pypi.python.org/pypi/wxPython/4.0.0a1

Speaking of wxPython phoenix and NVDA: I worked on porting NVDA source code to Phoenix last year under the project name of "Heliopolis". Due to huge GUI improvements introduced in NVDA 2016.4, current Heliopolis code (based on 2016.2) will not work, so I'm porting from scratch.

A few things to be aware of:

  • system tray icon class now lives in wx.adv package.
  • The "Six" compatibility layer must be included.
  • wx.SpinCtrlNameStr is no longer in use, so I replaced the relevant code with a string literal.
  • wx.CenterOnScreen is unavailable.

So far:

  • Copying wxPython 4 files to misc dependencies and compiling NVDA works.
  • Initialization works.
  • Log viewer is working, although you may get an error that says, "wx.Yield called recursively".
  • You cannot press ENTER after you type code in Python Console.
  • When you exit a settings dialog and attempt to open another, NVDA will say that another settings dialog is open, likely having to do with instance tracking.

Thanks.

@josephsl
Copy link
Collaborator Author

CC @feerrenrut, @LeonarddeR, @DavyKager, @ctoth, @nishimotz, @nvdaes, @derekriemer

josephsl added a commit to josephsl/nvda that referenced this issue Apr 16, 2017
…vaccess#7077.

One of the biggest benefits of using wxPython 4 is easier route for upgrading to Python 3.x. Due to changes made in wxPython 4, NvDA source code (at least wx routines) must be modified. This provides some foundations, namely wx.adv.TaskbarIcon and replacing wx.SpinCtrlNameStr with a string literal.
josephsl added a commit to josephsl/nvda that referenced this issue Apr 16, 2017
…xtCtrl.write. re nvaccess#7077.

wx.TextCtrl.write is no more - wx.TextCtrl.WriteText should be used instead.
josephsl added a commit to josephsl/nvda that referenced this issue Apr 16, 2017
…rsor tracking. re nvaccess#7077.

If wx.TextCtrl.WriteText is used and if the cursor is located at the top of the output window for the console, newly typed text will be inserted instead of being appended, thus wx.TextCtrl.AppendText will be used.
josephsl added a commit to josephsl/nvda that referenced this issue Apr 16, 2017
…nter instead. re nvaccess#7077.

Somehow, documentation says wx.CENTRE_ON_SCREEN is included but it isn't there at runtime. Instead, wx.Center is used (does not follow variable naming convention though), so using it for now.
josephsl added a commit to josephsl/nvda that referenced this issue Apr 16, 2017
@Brian1Gaff
Copy link

Brian1Gaff commented Apr 16, 2017 via email

@josephsl
Copy link
Collaborator Author

josephsl commented Apr 16, 2017 via email

josephsl added a commit to josephsl/nvda that referenced this issue Apr 16, 2017
josephsl added a commit to josephsl/nvda that referenced this issue Apr 16, 2017
…r. re nvaccess#7077.

In wxPython 4, when instantiating wx.GridSizer, horizontal and vertical gaps in pixels must be specified.
josephsl added a commit to josephsl/nvda that referenced this issue Apr 16, 2017
…fore the dialog is destroyed. re nvaccess#7077.

Due to changes to how dialog sizes are constructed, keyword arguments cannot be used.
Also, if Window.Destroy is called, somehow text controls and what not are gone, so save speech viewer position before destroying the window.
josephsl added a commit to josephsl/nvda that referenced this issue Apr 16, 2017
… sure to check if the treeview is alive. re nvaccess#7077.

Somehow, when Cancel button is licked, treeview is removed first, yet item selection event is run. Make sure to catch this.
@nishimotz
Copy link
Contributor

I have tried scons source and source\nvda.pyw, then got an error as follows:

INFO - __main__ (19:33:43):
Starting NVDA
INFO - core.main (19:33:44):
Config dir: C:\work\nvda\wxPy4\source\userConfig
INFO - config.ConfigManager._loadConfig (19:33:44):
Loading config: .\userConfig\nvda.ini
INFO - core.main (19:33:45):
NVDA version source-wxPy4-8f21e03
INFO - core.main (19:33:45):
Using Windows version 6.2.9200 workstation
INFO - core.main (19:33:45):
Using Python version 2.7.13 (v2.7.13:a06454b1afa1, Dec 17 2016, 20:42:59) [MSC v.1500 32 bit (Intel)]
INFO - core.main (19:33:45):
Using comtypes version 0.6.2
CRITICAL - __main__ (19:33:46):
core failure
Traceback (most recent call last):
  File "nvda.pyw", line 195, in <module>
    core.main()
  File "core.py", line 200, in main
    import appModuleHandler
  File "appModuleHandler.py", line 27, in <module>
    import NVDAHelper
  File "NVDAHelper.py", line 12, in <module>
    import eventHandler
  File "eventHandler.py", line 9, in <module>
    import api
  File "api.py", line 11, in <module>
    import review
  File "review.py", line 9, in <module>
    from NVDAObjects import NVDAObject
  File "NVDAObjects\__init__.py", line 24, in <module>
    import treeInterceptorHandler
  File "treeInterceptorHandler.py", line 13, in <module>
    import braille
  File "braille.py", line 16, in <module>
    import keyboardHandler
  File "keyboardHandler.py", line 20, in <module>
    import ui
  File "ui.py", line 18, in <module>
    import gui
  File "gui\__init__.py", line 16, in <module>
    import wx.adv
ImportError: No module named adv

@josephsl
Copy link
Collaborator Author

josephsl commented Apr 16, 2017 via email

@nishimotz
Copy link
Contributor

Thank you for suggestion.
After doing pip install wxPython==4.0.0a1, I have removed miscDeps/python/wx from my working files.
Then I could run your branch.

@josephsl
Copy link
Collaborator Author

Hi,

Front-end (GUI) is working as advertised. The only anoyance is wx.Yield issue. For now, if you'd like to give it a go, please test it from source (I'll generate a try build that I'll host for a while).

For add-ons: anything that deals with wx.PyDeadObjectError must be rewritten to catch RuntimeError. So far, Remote add-on version 2 fails to load due to missing function in wxPython 4.

I'll hold off on sending a pull request until wxPython 4 enters beta testing phase. Thanks.

@josephsl
Copy link
Collaborator Author

Hi,

Follow-up: there might be code changes during the alpha and beta cycle, so don't assume that my readme commit is the final one. I'll refresh the test builds once more wxPython 4 alphas and betas are posted.

Thanks.

@derekriemer
Copy link
Collaborator

derekriemer commented Apr 17, 2017 via email

@jcsteh
Copy link
Contributor

jcsteh commented Apr 17, 2017 via email

@josephsl
Copy link
Collaborator Author

josephsl commented Apr 17, 2017 via email

josephsl added a commit to josephsl/nvda that referenced this issue Apr 25, 2017
…t issues when installing more than one add-on remotely. re nvaccess#7077.

Here 'remotely' refers to letting people install add-ons via Windows Explorer and other means. Without nullifying instance flag when the add-ons manager closes, instance flag will be kept, which causes wxPython to think (and correctly) that the dialog is still active, resulting in runtime error on add-on list control being thrown.
@jcsteh jcsteh added the p3 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority label Apr 25, 2017
@jcsteh
Copy link
Contributor

jcsteh commented Apr 25, 2017

P2 because being able to move to Python 3 is important for many reasons and upgrading wxPython is required for that. Also, we want to discover any major issues now so we can hopefully get them addressed before wxPython 4 goes final.

@josephsl, how is this work going? Are there any major broken bits you've discovered since your original comment?

@jcsteh
Copy link
Contributor

jcsteh commented Apr 25, 2017

Log viewer is working, although you may get an error that says, "wx.Yield called recursively".

When do you see this error in Log Viewer? Is there something specific you do to reproduce this?

You cannot press ENTER after you type code in Python Console.

What happens if you do?

When you exit a settings dialog and attempt to open another, NVDA will say that another settings dialog is open, likely having to do with instance tracking.

That suggests __del__ isn't being called, which means something is holding a reference to a destroyed SettingsDialog or there is a reference cycle which can't be garbage collected. Does it behave if you run gc.collect()? If not, is there anything in gc.garbage?

@ctoth
Copy link
Contributor

ctoth commented Apr 26, 2017 via email

@jcsteh
Copy link
Contributor

jcsteh commented Apr 26, 2017

Agree we eventually need to do both of those things, but wxPython has priority because it's actively being worked on now and because we have a chance to get any major issues addressed early. We don't have the cycles to deal with other Python 3 stuff at the moment. To answer your question, there's no meta issue for Py3 as far as I know.

@josephsl
Copy link
Collaborator Author

josephsl commented Apr 26, 2017 via email

josephsl added a commit to josephsl/nvda that referenced this issue Apr 27, 2017
…cess#7077.

Just like add-ons manager, if the instance flag for config profiles dialog isn't nullified, wxWidgets will think that the dialog is active when it is gone. Thus nullify it in two places: when the actual dialog closes and when a new profile is activated right away (for the latter, it is parent.Destroy).
@josephsl
Copy link
Collaborator Author

Hi,

Adding #7105 for reference purposes, as this issue must be done before moving onto that issue fully. Thanks.

josephsl added a commit to josephsl/nvda that referenced this issue Sep 19, 2017
Before we can move to Python 3 (nvaccess#7105), two important dependencies must be satisfied: wxPython 4 (nvaccess#7077) and SCons (nvaccess#7520). As of September 18, 2017, both were satisfied, although wxPython 4 is still in beta testing phase.
For now, upgrade SCons to 3.0.0, which supports Python 2.7, 3.5, and 3.6. Key changes include changing print statements to print function and no more Environment.gettextMoFile attribute unless it has changed somehow. Upgrading to SCons 3 is one of the prerequisites to transitioning to Python 3.x.
josephsl added a commit to josephsl/nvda that referenced this issue Oct 29, 2017
Due to request from a tester, wxPython 4 developer has decided to restore wx.TextCtrl.write. This means the AppendText workaround in place is no longer needed.
josephsl added a commit to josephsl/nvda that referenced this issue Oct 30, 2017
…WXK_pAGEUP and wx.WXK_PAGEDOWN, respectively. re nvaccess#7077.

wx.WXK_PRIOR and wx.WXK_NEXT are no more, replaced by direct definition of page up and page down keys. This fixes the problem where sliders couldn't be changed in Voice Settings dialog.
@josephsl
Copy link
Collaborator Author

josephsl commented Feb 1, 2018

Hi,

wxPy 4 is officially out, so I'd like to request the PR associated with this issue to be reviewed. Thanks.

@jcsteh
Copy link
Contributor

jcsteh commented Feb 1, 2018

Assuming it hasn't been fixed already, a solution needs to be found to the wx timer re-entry problem before we can consider upgrading. NVDA relies on timers not being re-entrant (which is true in wxPython 3), but in wxPy 4, they are re-entrant. As discussed in this wxPython mailing list thread, this will not be changed; we were relying on undefined behaviour, probably even a bug in wxPython 3.

Anyway, aside from the Yield assertion, this could also cause problems in dialog modal loops. We rely on this for the core pump (search for the word "re-entrant" in core.py), but also for updateCheck's progress dialog; see 6d31167.

The simplest way to fix this is probably to subclass the wx timer to prevent re-entry, then use that subclass wherever we depend on this. This is all in the same thread, so that shouldn't be too hard: just set a boolean while the callback is running and clear it when it returns. If the boolean is set, don't run the callback, since the timer is already running the callback.

@jcsteh
Copy link
Contributor

jcsteh commented Feb 1, 2018

@josephsl commented on Apr 27, 2017, 5:44 PM GMT+10:

Testing shows that wx.YieldIfNeeded at least prevents assertions from being raised (does not show up on log).

That makes sense, but it's ultimately just hiding the underlying problem. I would recommend reverting that change; the assertion is indicative of a real problem here. See my previous comment for details on the correct way to fix this.

@josephsl
Copy link
Collaborator Author

josephsl commented Feb 2, 2018

Hi,

After taking a look at the thread and searching, I came to the conclusion that I may not be the right person for GUI back-end, as I really don't have a clue as to how to proceed with this. Also, due to other commitments (and also taking care of health and school), I'd like to invite the community to help out here ASAP (as in within the next few days), as we need to get this going very soon, otherwise we'll lose opportunity to implement #7105 and beyond.

Also, I'd like to request a special permission from NV Access to pause all our work while incubating wxPython 4 (a separate issue).

Thanks.

@LeonarddeR
Copy link
Collaborator

@josephsl commented on 2 Feb 2018, 02:37 CET:

Also, I'd like to request a special permission from NV Access to pause all our work while incubating wxPython 4 (a separate issue).

It is not entirely clear to me what you're requesting here. What work should be paused?

@josephsl
Copy link
Collaborator Author

josephsl commented Feb 4, 2018 via email

josephsl added a commit to josephsl/nvda that referenced this issue Jun 5, 2018
…gone. Re nvaccess#7077.

Previously, when closing settings dialog, instances would be cleared. It turns out that might not be the case with wxPython 4, so manually clear instances set if the only thing remaining is the SettingsDialog object.
josephsl added a commit to josephsl/nvda that referenced this issue Jun 7, 2018
josephsl added a commit to josephsl/nvda that referenced this issue Jun 7, 2018
…vaccess#7077.

Somehow, using wx.ID_ANY for speech dictionary buttons causes the buttons to not work. This is because the button is added directly to the helper sizer. Thus use NewID instead.
josephsl added a commit to josephsl/nvda that referenced this issue Jun 11, 2018
…RITE_PROMPT. Re nvaccess#7077.

Yet another change of attribute names: wx.SAVE is now wx.FD_SAVE, wx.OVERWRITE_PROMPT is fwx.FD_OVERWRITE_PROMPT. This resolves an issue where Log Viewer does not let users save logs (issue nvaccess#8385).
josephsl added a commit to josephsl/nvda that referenced this issue Jun 17, 2018
josephsl added a commit to josephsl/nvda that referenced this issue Jul 18, 2018
…ion for this checkbox just like others. re nvaccess#7077.

Somehow, for touch interaction/touch typing mode checkbox, wx.NewId function was called, whereas other checkboxes do not do this. Thus make it consistent with other fellows by not calling this function in this particular checkbox.
josephsl added a commit to josephsl/nvda that referenced this issue Jul 18, 2018
… panels. Re nvaccess#7077.

Reported by several people and subsequently confirmed: in wxPython 4.0.0 to 4.0.2, there exists a wrap-around bug in wx.NewId function that prevents programs calling this function from working normally after the ID's are exhausted. This is now fixed in wxPython 4.0.3 via wx.NewIdRef function. Thus use this function to keep an eye on panel ID's, used for announcing new categories as they become visible in multi-category NVDA Settings screen.
josephsl added a commit to josephsl/nvda that referenced this issue Jul 18, 2018
@nvaccessAuto nvaccessAuto added this to the 2018.3 milestone Jul 18, 2018
michaelDCurran pushed a commit that referenced this issue Jul 18, 2018
* wxPython4: initial foundations with wx.adv.TaskbarIcon and others. re #7077.

One of the biggest benefits of using wxPython 4 is easier route for upgrading to Python 3.x. Due to changes made in wxPython 4, NvDA source code (at least wx routines) must be modified. This provides some foundations, namely wx.adv.TaskbarIcon and replacing wx.SpinCtrlNameStr with a string literal.

* wxPython 4/Python Console: use wx.TextCtrl.WriteText instead of wx.TextCtrl.write. re #7077.

wx.TextCtrl.write is no more - wx.TextCtrl.WriteText should be used instead.

* wxPython 4/Python Console: use AppendText instead of WriteText for cursor tracking. re #7077.

If wx.TextCtrl.WriteText is used and if the cursor is located at the top of the output window for the console, newly typed text will be inserted instead of being appended, thus wx.TextCtrl.AppendText will be used.

* wxPython 4/various dialogs: wx.CENTER_ON_SCREEN is no more, use wx.Center instead. re #7077.

Somehow, documentation says wx.CENTRE_ON_SCREEN is included but it isn't there at runtime. Instead, wx.Center is used (does not follow variable naming convention though), so using it for now.

* wxPython 4/Welcome dialog: wx.NORMAL cannot be used as a font family anymore, replaced with wx.FONTFAMILY_DEFAULT. re #7077

* wxPython 4/cursor manager/Find dialog: use sizer.Add instead of sizer.AddSizer. re #7077.

* wxPython 4/Launcher: spell out GridSizer gaps, prevents overload error. re #7077.

In wxPython 4, when instantiating wx.GridSizer, horizontal and vertical gaps in pixels must be specified.

* wxPython 4/speech viewer: few tweaks to constructor, save position before the dialog is destroyed. re #7077.

Due to changes to how dialog sizes are constructed, keyword arguments cannot be used.
Also, if Window.Destroy is called, somehow text controls and what not are gone, so save speech viewer position before destroying the window.

* wxPython 4/Input gestures dialog: when Cancel button is clicked, make sure to check if the treeview is alive. re #7077.

Somehow, when Cancel button is licked, treeview is removed first, yet item selection event is run. Make sure to catch this.

* Readme: mention new wxPython and Six compatibility layer requirements. re #7077

* wx.Menu: AppendMenu is deprecated, use Append instead. re #7077

* Add-ons Manager: make sure to nullify dialog instance flag and prevent issues when installing more than one add-on remotely. re #7077.

Here 'remotely' refers to letting people install add-ons via Windows Explorer and other means. Without nullifying instance flag when the add-ons manager closes, instance flag will be kept, which causes wxPython to think (and correctly) that the dialog is still active, resulting in runtime error on add-on list control being thrown.

* gui/Config profiles dialog/wxPython 4: nullify instance flag. re #7077.

Just like add-ons manager, if the instance flag for config profiles dialog isn't nullified, wxWidgets will think that the dialog is active when it is gone. Thus nullify it in two places: when the actual dialog closes and when a new profile is activated right away (for the latter, it is parent.Destroy).

* Python console: revert to wx.TextCtrl.write function. re #7077.

Due to request from a tester, wxPython 4 developer has decided to restore wx.TextCtrl.write. This means the AppendText workaround in place is no longer needed.

* Settings/voice sliders: replace wx.WXK_PRIOR and wx.WXK_NEXT with wx.WXK_pAGEUP and wx.WXK_PAGEDOWN, respectively. re #7077.

wx.WXK_PRIOR and wx.WXK_NEXT are no more, replaced by direct definition of page up and page down keys. This fixes the problem where sliders couldn't be changed in Voice Settings dialog.

* core.CorePump:  block this timer from firing recursively. This could never happen before wxPython 4, but now it is possible.

* miscDeps now contains wxPython 4.

* Core: update copyright years, corrected issue number

* Comment spelling fix

* wxPython 4.0.1 in miscDeps

* gui: add a NonRe-entrant timer to replace use of wx.Timer and wx.PyTimer.

* Replace usage of wx.PyTimer with gui.NonReentrantTimer for all core code in NVDA. This does not change any braille display drivers.

* Clarify comment

* Clarify docstring

* Spelling

* Readme: mention wxPython 4.0.1

* wxPython 4: use wx.ID_ANY instead of NewID function. Re #8121.

Becasue NewID may raise assertion error, use wx.ID_ANY instead.

* GUI/SettingsDialogs: remove extra space at ends of lines

* Settings screen: wx.Center again.

* add a new submodule for wxPython dep

* Use `CenterOnScreen()` rather than `Center`

Use the explicit function rather than `Center()` with the CENTER_ON_SCREEN constant.
Also supply CENTER_ON_SCREEN for addons which may still rely on it.

* Define both missing constants

Despite being referred to in the documentation for wxPython,
CENTER_ON_SCREEN and CENTRE_ON_SCREEN constants are missing.
Here we redefine them to stop errors in addons. Addons should migrate to
using the explicit functions CenterOnParent or CenterOnScreen.

* Update copyright years and headers.

* Settings dialog: clear instances set when all settings instances are gone. Re #7077.

Previously, when closing settings dialog, instances would be cleared. It turns out that might not be the case with wxPython 4, so manually clear instances set if the only thing remaining is the SettingsDialog object.

* wxPython 4: revert instances checking in SettingsDialog thanks to mitigation about circular references. Re #7077.

* Dictionary dilaog: use wx.NewId for add, edit, and remove buttons. Re #7077.

Somehow, using wx.ID_ANY for speech dictionary buttons causes the buttons to not work. This is because the button is added directly to the helper sizer. Thus use NewID instead.

* Get rid of unnecessary ID usage

* Log viewer: wx.SAVE -> wx.FD_SAVE, wx.OVERWRITE_PROMPT -> wx.FD_OVERWRITE_PROMPT. Re #7077.

Yet another change of attribute names: wx.SAVE is now wx.FD_SAVE, wx.OVERWRITE_PROMPT is fwx.FD_OVERWRITE_PROMPT. This resolves an issue where Log Viewer does not let users save logs (issue #8385).

* Log viewer: add copyright headers.

* wxPython4/settings dialogs: deleteWindows -> delete_windows in sizer.Clear method. Re #7077

* What's new: add wxPython 4.0.1 entry in changes for developers section. Re #7077

* Upgrade to wxPython 4.0.3

* Touch intewraction/touch typing mode checkbox: no more wx.NewId function for this checkbox just like others. re #7077.

Somehow, for touch interaction/touch typing mode checkbox, wx.NewId function was called, whereas other checkboxes do not do this. Thus make it consistent with other fellows by not calling this function in this particular checkbox.

* Settings panels: use wx.NewIdRef function to retrieve global ID's for panels. Re #7077.

Reported by several people and subsequently confirmed: in wxPython 4.0.0 to 4.0.2, there exists a wrap-around bug in wx.NewId function that prevents programs calling this function from working normally after the ID's are exhausted. This is now fixed in wxPython 4.0.3 via wx.NewIdRef function. Thus use this function to keep an eye on panel ID's, used for announcing new categories as they become visible in multi-category NVDA Settings screen.

* Readme: mention Python 2.7.15 and wxPython 4.0.3 dependencies.

* What's new: wxPython 4.0.1 -> 4.0.3. Re #7077.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p3 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority
Projects
None yet
Development

No branches or pull requests

8 participants