Magnifier Readme for future developers by Boumtchack · Pull Request #20880 · nvaccess/nvda

Boumtchack · 2026-09-22T10:22:39Z

Link to issue number:

Summary of the issue:

Synthetisation of the work I've done in the past regarding NVDA Magnifier, this documentation is for anyone who would want to work on it in the future.

It regroups some of the processes we've been through and why.

Description of user facing changes:

Description of developer facing changes:

Helper for Magnifier developement

Description of development approach:

Testing strategy:

Known issues with pull request:

Code Review Checklist:

Documentation:
- Change log entry
- User Documentation
- Developer / Technical Documentation
- Context sensitive help for GUI changes
Testing:
- Unit tests
- System (end to end) tests
- Manual testing
UX of all users considered:
- Speech
- Braille
- Low Vision
- Different web browsers
- Localization in other languages / culture than English
API is compatible with existing add-ons.
Security precautions taken.

Copilot

Copilot review overview

🟢 Approval recommended

The only noted issue is a minor documentation clarification.

Review effort: Lite
Findings: 1

Open (1)

Correct inaccurate one-test-file-per-source-file documentation · New

What changed in this PR

Adds developer handover documentation for NVDA’s Magnifier module.

Changes:

Documents architecture, runtime behavior, API pitfalls, and conventions.
Records limitations, technical debt, and future work.
Provides testing and maintenance guidance.

File	Description
`source/_magnifier/README.md`	Adds comprehensive Magnifier development documentation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Clarified unit test organization and details in README. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

CyrilleB79 · 2026-09-22T14:50:44Z

+
+5. **`MagSetInputTransform` requires UIAccess**, so an installed copy of NVDA. On a portable copy or when running from source, the call fails with `WinError 5` (access denied). The code checks `systemUtils.hasUiAccess()` up front, then on `WinError 5` sets `_inputTransformSupported` to `False` and continues without it. Consequence: **touch cannot be properly tested from source**. You need an installed build.
+
+6. **Native errors arrive as `OSError`**, thanks to the bindings' `errcheck`. COM/UIA errors coming from focus tracking arrive as `COMError` (for example `RPC_E_DISCONNECTED` in recent Notepad). The loop catches both.


What are "native errors"?
More generally, what is called "native" in this documentation? Seems to be the Windows Magnification API, but a confirmation would be welcome.

yeah exactly, I will clarify that

CyrilleB79

Thanks for this documentation.

Some small remarks / improvements which do not prevent at all this PR to be merged (even as is).

CyrilleB79 · 2026-09-22T14:52:17Z

+| **Blocking touch input while the magnifier runs** (flag in `touchHandler`, alert sound, warning dialog) | Degraded the feature and coupled a core NVDA module to the magnifier. Replaced by `MagSetInputTransform`, the primitive Windows provides for exactly this case. **Lesson: before adding a mechanism to NVDA's core, exhaust the Magnification API.** |
+| **Repeating 8 ms timer with `timeBeginPeriod(1)`** | `timeBeginPeriod` changes the clock resolution for the whole system and costs power. A repeating timer does not re-arm cleanly after an error. The smoothness problem was solved by the mouse hook. |
+| **Calling the Magnification API from the mouse hook** | Slows down the mouse for the whole system (section 3.4). |
+| **API initialized once per session** | Conflicted with Screen Curtain. Replaced by the dummy cycle (section 5, pitfall 2). |


Why couldn't the same API be used by both screen curtain and magnifier? That would avoid the screen "light flash" (no color filter) between inverted color filter and black screen of screen curtain. E.g.:

magnifier color active with color inversion.

Command to enable screen curtain

disable magnification (zzom)

screen curtain changes color filter to black screen

Command to disable screen curtain

change color filter to color inversion

restore zoom level

CyrilleB79 · 2026-09-22T14:53:28Z

+
+### 4.2 Configuration
+
+* **Never read `config.conf["magnifier"]` outside `config.py`.** Add a getter/setter in `config.py`, then use it everywhere (commands, panel, classes).


Why? Is it specific to magnifier? In other part of NVDA, we just use config.conf directly.

CyrilleB79 · 2026-09-22T14:54:36Z

+1. `_managePanning()`: if the user panned manually and the focus moves, manual panning mode ends.
+2. If not in manual panning mode, `currentCoordinates` receives the position computed by the `FocusManager`.
+3. `_doUpdate()` (implemented by the subclass) applies the transform.
+4. On `OSError` or `COMError`: consecutive error counter. At 3, `_attemptRecovery()`.


Why specifically these errors are filtered?

CyrilleB79 · 2026-09-22T14:58:56Z

+This document is for whoever takes over development of NVDA's built-in magnifier.
+It explains how the module is built, why it is built that way, which pitfalls have already been hit, and what is left to do.
+
+The starting point is the magnifier as shipped in NVDA 2026.2.


What does it mean?

If you mean that this documentation describes the magnifier as shipped in 2026.2, it is not 100%: there are two exceptions which are part of 2026.3:

| source/contentRecog/recogUi.py| Uses_magnifier.isActive() to pick Windows Graphics Capture instead of GDI during OCR, so recognition sees the real screen rather than magnified or filtered pixels. |
Seems to me that Windows Graphic Capture is only used in 2026.3.

* Full-screen features: zoom from 100% to 5000% in steps of 50, three color filters, two tracking modes (center, relative), tracking of four sources (mouse, system focus, review cursor, navigator object), manual panning, animated screen overview ("spotlight"), moving the mouse to the center of the view, automatic error recovery, coexistence with Screen Curtain, touch support.

"moving the mouse to the center of the view" has only been introduced in 2026.3.

CyrilleB79 · 2026-09-22T15:06:34Z

Also one question for NV Access reviewers: this documentation uses quite often **bold formatting**, which is not rendered by default for NVDA speech or braille users. Is it necessary to keep this formatting? It may remain useful though for sighted contributors, and it's more likely that a magnifier developer be sighted, or for users who have configured formatting attribute reporting.

CyrilleB79 · 2026-09-23T05:55:17Z

One more question: Would it be worth to add the references of PRs that have been reverted? (e.g. true center)
Or even some more PRs, e.g. trials which have not been successful

into magnifierReadme

first shot of readme

b9abd82

Boumtchack requested a review from a team as a code owner September 22, 2026 10:22

Boumtchack requested review from seanbudd and a lite review from Copilot September 22, 2026 10:22

Copilot started reviewing on behalf of Boumtchack September 22, 2026 10:23 View session

Copilot AI reviewed Sep 22, 2026

View reviewed changes

Comment thread source/_magnifier/README.md Outdated

Update README with unit test structure and details

7402c69

Clarified unit test organization and details in README. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

CyrilleB79 reviewed Sep 22, 2026

View reviewed changes

CyrilleB79 approved these changes Sep 22, 2026

View reviewed changes

Boumtchack added 10 commits September 23, 2026 12:05

native explanation

85c451c

api and screen curtain

dedf41f

explain why magnifier config access goes through config.py

c401a15

explain errors filter

684bddd

limit documentation tu 2026.2

897f962

remove bold and replace with clear explanations

1bb9e59

Merge branch 'magnifierReadme' of https://github.com/France-Travail/nvda

7dc9634

into magnifierReadme

adding a pr index for reference

142bfcc

small fix

0bfb101

replaced diagram with readable table for accessibility

0dbee78

Navigation Menu

Uh oh!

Magnifier Readme for future developers - #20880

Magnifier Readme for future developers#20880
Boumtchack wants to merge 12 commits into
nvaccess:masterfrom
France-Travail:magnifierReadme

Boumtchack commented Sep 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

CyrilleB79 Sep 22, 2026

Uh oh!

Boumtchack Sep 23, 2026

Uh oh!

CyrilleB79 left a comment

Uh oh!

CyrilleB79 Sep 22, 2026

Uh oh!

CyrilleB79 Sep 22, 2026

Uh oh!

CyrilleB79 Sep 22, 2026

Uh oh!

CyrilleB79 Sep 22, 2026

Uh oh!

CyrilleB79 commented Sep 22, 2026

Uh oh!

CyrilleB79 commented Sep 23, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		5. `MagSetInputTransform` requires UIAccess, so an installed copy of NVDA. On a portable copy or when running from source, the call fails with `WinError 5` (access denied). The code checks `systemUtils.hasUiAccess()` up front, then on `WinError 5` sets `_inputTransformSupported` to `False` and continues without it. Consequence: touch cannot be properly tested from source. You need an installed build.

		6. Native errors arrive as `OSError`, thanks to the bindings' `errcheck`. COM/UIA errors coming from focus tracking arrive as `COMError` (for example `RPC_E_DISCONNECTED` in recent Notepad). The loop catches both.


		### 4.2 Configuration

		* Never read `config.conf["magnifier"]` outside `config.py`. Add a getter/setter in `config.py`, then use it everywhere (commands, panel, classes).

Uh oh!

Conversation

Boumtchack commented Sep 22, 2026

Link to issue number:

Summary of the issue:

Description of user facing changes:

Description of developer facing changes:

Description of development approach:

Testing strategy:

Known issues with pull request:

Code Review Checklist:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

Uh oh!

CyrilleB79 Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

Boumtchack Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

CyrilleB79 left a comment

Choose a reason for hiding this comment

Uh oh!

CyrilleB79 Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

CyrilleB79 Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

CyrilleB79 Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

CyrilleB79 Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

CyrilleB79 commented Sep 22, 2026

Uh oh!

CyrilleB79 commented Sep 23, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants