Skip to content

Conversation

@Boumtchack
Copy link

Link to issue number:

Closes #12539

Summary of the issue:

This pull request introduces a new NVDA magnifier feature, including docked and lens magnifier modes, color filter support, and a set of global commands for controlling magnification. The changes add new classes for handling magnifier windows, provide optimized color filtering, and implement keyboard shortcuts for toggling magnification, zooming, cycling modes, and color filters.

Description of user facing changes:

Implemented new script commands in globalCommands.py for starting/stopping the magnifier, zooming in/out, cycling color filters, toggling fullscreen mode, cycling magnifier types, and spotlighting the magnifier window, each with descriptive messages and gestures.

Description of developer facing changes:

Description of development approach:

Creating 3 types of magnifier that will be called based on prefered choice of the user. each one got his class and will share settings through parents class

Testing strategy:

unit test

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.

@Boumtchack Boumtchack mentioned this pull request Nov 17, 2025
8 tasks
@seanbudd seanbudd changed the title Nvda magnifier implementation Magnifier implementation Nov 18, 2025
@Boumtchack Boumtchack force-pushed the NvdaMagnifierImplementation branch from f5a86b5 to 7b3ee4f Compare November 24, 2025 10:10
Copy link
Contributor

@CyrilleB79 CyrilleB79 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two missing commas.
Though unfortunately, I do not know where to see all required changes. I seem to remember that it was possible but do not rmember how.

@Boumtchack
Copy link
Author

Boumtchack commented Dec 16, 2025

It might explain 2 errors but I have 6 so I don't know, I'll try to push these 2 and check again the others

@Boumtchack
Copy link
Author

So I had to comment a line in the pre-commit-config.yaml to make it work locally.

markdownlint-cli2 was failling and as it was not what github wanted me to change, I just simply commented the lines.

I'll revert it back

@Boumtchack Boumtchack marked this pull request as ready for review December 16, 2025 15:24
@Boumtchack
Copy link
Author

Boumtchack commented Dec 16, 2025

as said previously, my computer may not be the best machine to run all test, I marked it as ready to review to see if Github test are working.

I'll put it as a draft after the results.

@seanbudd
Copy link
Member

Thanks @CyrilleB79 and @Boumtchack for the fixups, I can see tests are passing now which is good. I think the overall design here is very solid, there's just minor fix-ups and documentation left. Note I haven't reviewed the full code in detail yet, particularly the tests.

@seanbudd seanbudd marked this pull request as draft December 17, 2025 04:26
@Boumtchack
Copy link
Author

Boumtchack commented Dec 17, 2025

Here's what I still have to work on:

  • Spotlight Manager separated from Fullscreen module
  • Display change handling
  • Magnifier Action thing
  • check tests again?

I'll work on that tomorrow

@CyrilleB79
Copy link
Contributor

I have not done an in-depth review, but here are points that I have noted:

Screen curtain conflict

If NVDA magnifier is applying color filters, this conflicts with NVDA's Screen Curtain, since they both use the color transformation matrix. Trying to enable or disable in any order creates unexpected issues. Thus:

  • either, best solution, you integrate both so that they do not conflict with each other; Screen Curtain transformation needs to have priority on Magnifier; and Magnifier transformation/filtering should be restored if Magnifier is still enabled when exiting Magnifier.
  • or, easier, you just forbid to start Magnifier when Screen Curtain is enabled and the other way around

Documentation

A section in the User Guide as well as a Change log item ("New features" section) are required. The description in the User Guide should help understand if the feature is implemented as expected.

Logging

You are using log.info all around your code; we usually use log.debug in all these cases. log.info is rather used at startup, at exit or to log version number of all NVDA components.


Again, thanks for this work. I had not yet tested all the features, but the spotlight seems a very nice one (visual double check needs to be done by someone else than I though).

@Boumtchack
Copy link
Author

I'll try to see what I can do for the screen curtain, but if is there a reason someone that uses screen curtain would use the magnifier and the other way around? I'm not sure both are used by the same users.

for the log.info, I was going to remove them anyway when everything was clean, it's just helpfull for me to see where I'm at.

I'm kinda lost on where and what I should do for Documentation, is there a way you could give me a placeholder on the files I should change?

@CyrilleB79
Copy link
Contributor

CyrilleB79 commented Dec 18, 2025

I'll try to see what I can do for the screen curtain, but if is there a reason someone that uses screen curtain would use the magnifier and the other way around? I'm not sure both are used by the same users.

I am such a user. Today, I use NVDA speech feedback as main information channel, in conjunction with Windows Magnifier (with color filter inverted colors) as additional information channel. In case I work with sensitive information, I activate Screen Curtain and use only speech channel. Today, I need to exit Magnifier or to remove color filter before enabling Screen Curtain, and to reopen Magnifier or to restore color filter after I have finished with Screen Curtain. Being able to enable and disable screen curtain keeping the same Magnifier configuration would be a plus.

Regarding the other way around, i.e. enabling / disabling Magnifier while Screen curtain is active, I have no real use cases in mind. But I can imagine these ones:

  • As a visually impaired user who suffer from the excessive brightness of a white background, I'd like to have the Magnifier enabled with its color filters as soon as I exit screen curtain.
  • Or: While Screen Curtain is enabled, I disable Screen curtain by mistake and want to re-enable it (or the other way around)

for the log.info, I was going to remove them anyway when everything was clean, it's just helpfull for me to see where I'm at.

OK. Nice.

I'm kinda lost on where and what I should do for Documentation, is there a way you could give me a placeholder on the files I should change?

The Change log is at user_docs/en/changes.md. You need to add a new item in section "2026.2", sub-section "New Features".


The User Guide is at user_docs/en/userGuide.md:

First, in section "### NVDA Settings", there is a sub-section describing each panel. You need to add a "#### Magnifier" section, and describe all its options. Take example on other panels.

Also, it seems quite likely that describing the panel's options is not enough. Since the Magnifier will be a full NVDA specific feature (not just a little feature), it needs to be described in its own section. I think that you need to write a "## Magnifier" section, probably just after the "## Vision" paragraph, to describe what the Magnifier feature in NVDA is and what it allows to do. For example:

  • describe all the Magnifier shortcuts
  • explain what are color filters (and maybe what they are for)
  • what the spotlight is and in which situation it may be useful
  • etc.

Only English documentation needs to be updated; the documentation in other languages will then be updated by translators from the English one.

@Boumtchack
Copy link
Author

Thanks for the feedback!

@Boumtchack
Copy link
Author

i have a hard time making a way to switch between magnifier and the Screen Curtain without errors, so I kept it it simple for now with just preventing one to launch if the other is enabled, I will still try to find a way to make it work as Cyrille said it would be a good point to be abble to toggle them.
Maybe it requires more deep changess that I'm not familiar with, as I try to change the least code outside of my scope (_magnifier module)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. merge-early Merge Early in a developer cycle

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow Windows Magnifier to follow NVDA virtual cursor

3 participants