Enable experimental 64-bit support #18207
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s based on whether or not the target archetecture is the NvDA core archetecture or not, rathr than hardcoding x86 or x86_64 etc.
…ns explicit ctypes declarations for all nvdaHelperLocal functions.
… that it can be byrefed later.
…PARAM as this gives us the greatest flexibility of accepting various argument types while still ensuring the right size.
…t. The only thing that depends upon this is some old code to fetch the label of an embedded object in windows Live Mail / Outlook Express. Probbly not relevant anymore.
…t object rather than an int.
…hEdit controls, by moving this code into C++ where the OLE definitions are still available in 64 bit. Specifically: * nvdaHelper\local\oleUtils.cpp, getOleClipboardText: it now takes an IUnknown instead of an IDataObject, and does the QueryInterface to IDataObject itself. This remains fully compatible, as IDataObject can always be treated as an IUnknown. * nvdaHelper\local\oleUtils.cpp, add a getOleUserType function, which takes an IUnknown, queryInterface to IOLEObject and calls getUserType on it, writing the resutling string to its out argument. * Windows Edit NVDAObject's _getEmbeddedObjectLabel method: If the label can not be gotten by querying to IAccessible, or using displayModel, then try GetOleClipboardText, and if that fails, then getOleUserType. This reintroduces the original logic, but now in c++ functions, removing the need to import the oleTypes module, which could not be maintaind for 64 bit. Testing: The Jarte application was used. * A file was copied from Explorer, creating an embedded object with a user type of "package". * An Excel spreadsheet was inserted as an object. Insert -> Object..., create new Excel 97 - 2003 worksheet. Filled in a few cells. getOleclipboardText fetched the contents to be reported. Please enter the commit message for your changes. Lines starting
See test results for failed build of commit 175d82b51c |
See test results for failed build of commit 278d93cf94 |
…nk_Notify, by correctly defining CoTaskMemFree in a new winBindings.ole32 module.
See test results for failed build of commit 2237f40a7a |
michaelDCurran
pushed a commit
that referenced
this pull request
Jul 16, 2025
### Link to issue number: Fixup for #18207 ### Summary of the issue: HID enumeration does not work on the experimental X64 branch. I actually found two issues with the current code. ### Description of user facing changes: None ### Description of developer facing changes: None ### Description of development approach: 1. it turned out that wrong packing was used. The correct packing is 1 on X86 and 8 on X64. This can be easily found in `SetupAPI.h`, the header that defines `SP_DEVICE_INTERFACE_DETAIL_DATA_W`. I validated with a small c program that `sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_W))` returns 8 on X64 builds and 6 on x86 builds. I also validated this with ctypes on X86 and X64 runs of NVDA. 2. The current code allocates a buffer that is twice too big for the data. This is also easy to explain. The struct contains a DWORD cbSize field as well as an array of wchars. The array of wchars should be the required size as fetched by the first call of `SetupDiGetDeviceInterfaceDetail`, mines the cbSize field. In the old situation however, there wasn't accounted for the fact that WCHAR is already two bytes. Therefore we now divide through the size of WCHAR. Old situation: ```python dwNeeded = DWORD(200) class SP_DEVICE_INTERFACE_DETAIL_DATA_W(ctypes.Structure): _fields_ = ( ("cbSize", DWORD), ("DevicePath", WCHAR * (dwNeeded.value - ctypes.sizeof(DWORD))), ) >>> sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_W) 396 ``` New situation ```python dwNeeded = DWORD(200) class SP_DEVICE_INTERFACE_DETAIL_DATA_W(ctypes.Structure): _fields_ = ( ("cbSize", DWORD), ("DevicePath", WCHAR * ((dwNeeded.value - ctypes.sizeof(DWORD)) // ctypes.sizeof(WCHAR))), ) >>> sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_W) 200 ``` ### Testing strategy: Ensured that hid devices could be enumerated on both x86 master based builds as well as experimental 64 bit runs of NVDA. ### Known issues with pull request: None ### Code Review Checklist: <!-- This checklist is a reminder of things commonly forgotten in a new PR. Authors, please do a self-review of this pull-request. Check items to confirm you have thought about the relevance of the item. Where items are missing (eg unit / system tests), please explain in the PR. To check an item `- [ ]` becomes `- [x]`, note spacing. You can also check the checkboxes after the PR is created. A detailed explanation of this checklist is available here: https://github.com/nvaccess/nvda/blob/master/projectDocs/dev/githubPullRequestTemplateExplanationAndExamples.md#code-review-checklist --> - [x] Documentation: - Change log entry - User Documentation - Developer / Technical Documentation - Context sensitive help for GUI changes - [x] Testing: - Unit tests - System (end to end) tests - Manual testing - [x] UX of all users considered: - Speech - Braille - Low Vision - Different web browsers - Localization in other languages / culture than English - [x] API is compatible with existing add-ons. - [x] Security precautions taken. <!-- Please keep the following --> @coderabbitai summary --------- Co-authored-by: Sascha Cowley <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Copilot <[email protected]>
michaelDCurran
pushed a commit
that referenced
this pull request
Jul 16, 2025
### Link to issue number: Fixup for #18207 that can be applied separately. ### Summary of the issue: In `appModuleHandler`, there's a mistake in the `processEntry32W` definition. The `th32DefaultHeapID` field is declared as DWORD, but it should be PULONG. See `https://learn.microsoft.com/en-us/windows/win32/api/tlhelp32/ns-tlhelp32-processentry32w` ### Description of user facing changes: None ### Description of developer facing changes: None ### Description of development approach: When merged into #18207, app module loading will work. ### Testing strategy: Tested on both the x64 and the x86 branches of NVDA. ### Known issues with pull request: None ### Code Review Checklist: <!-- This checklist is a reminder of things commonly forgotten in a new PR. Authors, please do a self-review of this pull-request. Check items to confirm you have thought about the relevance of the item. Where items are missing (eg unit / system tests), please explain in the PR. To check an item `- [ ]` becomes `- [x]`, note spacing. You can also check the checkboxes after the PR is created. A detailed explanation of this checklist is available here: https://github.com/nvaccess/nvda/blob/master/projectDocs/dev/githubPullRequestTemplateExplanationAndExamples.md#code-review-checklist --> - [x] Documentation: - Change log entry - User Documentation - Developer / Technical Documentation - Context sensitive help for GUI changes - [x] Testing: - Unit tests - System (end to end) tests - Manual testing - [x] UX of all users considered: - Speech - Braille - Low Vision - Different web browsers - Localization in other languages / culture than English - [x] API is compatible with existing add-ons. - [x] Security precautions taken. <!-- Please keep the following --> @coderabbitai summary --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
See test results for failed build of commit f7a6fece37 |
…his allows 64 bit binary builds of NVDA to run. Note though these builds will currently still be completely unware of any previously installed 32 bit copy.
See test results for failed build of commit 17fc1ff634 |
…bit NVDA. Specifically, add logic to each of the register32BitServer, register64bitServer, apply32BitRegistryPatch and apply64BitRegistryPatch to use the correct paths for finding 32 and 64 bit system tools, depending if NVDA is 32 or 64 bit itself. The logic ant paths have been now hardcoded into these functions rather than using the module-level constants. This is easier to read and reason about per function.
…Access Bridge to access Java applications. Tested NVDA built as 64 bit interacting with PyCharm (a popular Java application).
See test results for failed build of commit 05e1de9c21 |
…a portable copy / updating an installed copy.
specific changes:
* nvdaHelper and setup.py: binary copies of NVDA now store libs in lib/{version}/{architecture} rather than lib/{architecture}/{version}. This allows again for correct removal of lib/* where * does not equal current version..
* installer.removeOldLibFiles: ensure that subdirectories of the current versioned lib dir are also skipped from removal, not just the versioned lib dir itself. Previously this dir only contained files, not directories.
Testing:
Built NvDA as 64 bit. Ran the launcher. Created a portable copy. Ensure the lib dir contained all needed libs and that the portable copy runs. Placed some random directories / files within the lib dir. Created another portable copy over the top, and ensured that those random extra directories / files wer removed, but the current version stayed.
…ote, it is still currently unaware of any previous 32 bit build, thus it does not yet clean up any old installation or correctly keep settings. Specific changes: * winbindings.kernel32: add definition for LoadLibraryEx * logHandler.RemoteHandler: fix loading of nvdaHelperRemote.dll to use the correct path. This was previously stopping nvda_slave from executing at all when trying to install NVDA. * Fix loading of nvdaHelperRemote.dll to use the correct path. We do repeat loading of nvdaHelperRemote in a few places. It may be good in future to abstract this a little better, but for now this gets things working.
5 tasks
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link to issue number:
Summary of the issue:
NVDA needs to move to being a 64-bit application.
Description of user facing changes
None.
Description of development approach
lib.Testing strategy:
Run from source and use NVDA.
Known issues with pull request:
Various, including:
NVDA+control+f1doesn't work)Code Review Checklist:
@coderabbitai summary