Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 79 additions & 18 deletions source/_magnifier/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,55 +9,116 @@
"""

from typing import TYPE_CHECKING
from .fullscreenMagnifier import FullScreenMagnifier
from .config import getMagnifierType
from .utils.types import MagnifierType

if TYPE_CHECKING:
from .magnifier import Magnifier

_magnifier: "Magnifier | None" = None


def initialize():
def createMagnifier(magnifierType: MagnifierType) -> "Magnifier":
"""
Initialize the magnifier module
For now, only the full-screen magnifier is supported
Create a magnifier instance based on the specified type.

:param magnifierType: The type of magnifier to create
:return: The created magnifier instance
:raises ValueError: If the magnifier type is not supported
"""

match magnifierType:
case MagnifierType.FULLSCREEN:
from .fullscreenMagnifier import FullScreenMagnifier

return FullScreenMagnifier()

case MagnifierType.FIXED:
from .placeholderMagnifier import PlaceholderMagnifier

return PlaceholderMagnifier()

case MagnifierType.DOCKED:
from .placeholderMagnifier import PlaceholderMagnifier

return PlaceholderMagnifier()

case MagnifierType.LENS:
from .placeholderMagnifier import PlaceholderMagnifier

return PlaceholderMagnifier()
Comment on lines +39 to +49
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

createMagnifier returns PlaceholderMagnifier() for FIXED/DOCKED/LENS, but PlaceholderMagnifier currently reports its type as MagnifierType.PLACEHOLDER. This breaks type cycling (the current type no longer matches the configured/selected type) and causes user announcements/config (setMagnifierType) to diverge from the actual instance state. Consider passing the requested magnifierType into PlaceholderMagnifier (and setting self._magnifierType accordingly) so the instance reflects the selected type.

Suggested change
return PlaceholderMagnifier()
case MagnifierType.DOCKED:
from .placeholderMagnifier import PlaceholderMagnifier
return PlaceholderMagnifier()
case MagnifierType.LENS:
from .placeholderMagnifier import PlaceholderMagnifier
return PlaceholderMagnifier()
magnifier = PlaceholderMagnifier()
magnifier._magnifierType = magnifierType
return magnifier
case MagnifierType.DOCKED:
from .placeholderMagnifier import PlaceholderMagnifier
magnifier = PlaceholderMagnifier()
magnifier._magnifierType = magnifierType
return magnifier
case MagnifierType.LENS:
from .placeholderMagnifier import PlaceholderMagnifier
magnifier = PlaceholderMagnifier()
magnifier._magnifierType = magnifierType
return magnifier

Copilot uses AI. Check for mistakes.

case _:
raise ValueError(f"Unsupported magnifier type: {magnifierType}")


def _setMagnifierType(magnifierType: MagnifierType) -> None:
"""
Set the magnifier type, stopping the current one if active and creating a new instance.

:param magnifierType: The type of magnifier to set
"""
global _magnifier

# Stop current magnifier if active
if _magnifier and _magnifier._isActive:
_magnifier._stopMagnifier()

magnifier = FullScreenMagnifier()
setMagnifier(magnifier)
# Create and set new magnifier instance
_magnifier = createMagnifier(magnifierType)


def initialize() -> None:
"""
Initialize the magnifier module with the default magnifier type from config.
"""
magnifierType = getMagnifierType()
_setMagnifierType(magnifierType)
_magnifier._startMagnifier()


def isActive() -> bool:
"""
Check if magnifier is currently active for settings
Check if magnifier is currently active.

:return: True if magnifier is active, False otherwise
"""
global _magnifier
return _magnifier and _magnifier._isActive
return _magnifier is not None and _magnifier._isActive


def getMagnifier() -> "Magnifier | None":
def changeMagnifierType(magnifierType: MagnifierType) -> None:
"""
Get current magnifier
Change the magnifier type at runtime.
Stops the current magnifier and starts a new one of the specified type.

:param magnifierType: The new magnifier type to use
:raises RuntimeError: If no magnifier is currently active
"""
global _magnifier
return _magnifier
if not _magnifier or not _magnifier._isActive:
raise RuntimeError("Cannot change magnifier type: magnifier is not active")

_setMagnifierType(magnifierType)
_magnifier._startMagnifier()

def setMagnifier(magnifier: "Magnifier") -> None:

def getMagnifier() -> "Magnifier | None":
"""
Set magnifier instance
Get the current magnifier instance.

:param magnifier: The magnifier instance to set
:return: The current magnifier instance, or None if not initialized
"""
global _magnifier
_magnifier = magnifier
return _magnifier


def terminate():
def terminate() -> None:
"""
Called when NVDA shuts down
Terminate the magnifier module.
Called when NVDA shuts down.
"""
global _magnifier
if _magnifier and _magnifier._isActive:
_magnifier._stopMagnifier()
_magnifier = None
_magnifier = None
56 changes: 47 additions & 9 deletions source/_magnifier/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@

from typing import Literal
import ui
from . import getMagnifier, initialize, terminate
from . import getMagnifier, initialize, terminate, changeMagnifierType
from .config import (
getDefaultZoomLevelString,
getDefaultFilter,
getDefaultFullscreenMode,
getMagnifierType,
setMagnifierType,
ZoomLevel,
)
from .magnifier import Magnifier
Expand Down Expand Up @@ -101,19 +103,31 @@ def toggleMagnifier() -> None:
initialize()

filter = getDefaultFilter()
fullscreenMode = getDefaultFullscreenMode()

ui.message(
pgettext(
magnifierType = getMagnifierType()
zoomLevel = getDefaultZoomLevelString()
if magnifierType == MagnifierType.FULLSCREEN:
fullscreenMode = getDefaultFullscreenMode()
msg = pgettext(
"magnifier",
# Translators: Message announced when starting the NVDA magnifier.
"Starting magnifier with {zoomLevel} zoom level, {filter} filter, and {fullscreenMode} full-screen mode",
"Starting {magnifierType} magnifier with {zoomLevel} zoom level, {filter} filter, and {fullscreenMode} full-screen mode",
).format(
zoomLevel=getDefaultZoomLevelString(),
magnifierType=magnifierType.displayString,
zoomLevel=zoomLevel,
filter=filter.displayString,
fullscreenMode=fullscreenMode.displayString,
),
)
)
else:
msg = pgettext(
"magnifier",
# Translators: Message announced when starting the NVDA magnifier.
"Starting {magnifierType} magnifier with {zoomLevel} zoom level and {filter} filter",
).format(
magnifierType=magnifierType.displayString,
zoomLevel=zoomLevel,
filter=filter.displayString,
)
ui.message(msg)


def zoom(direction: Direction) -> None:
Expand Down Expand Up @@ -175,6 +189,30 @@ def toggleFilter() -> None:
)


def cycleMagnifierType() -> None:
"""Cycle through magnifier types (full-screen, fixed, docked (to do), lens (to do))"""
magnifier: Magnifier = getMagnifier()
if magnifierIsActiveVerify(
magnifier,
MagnifierAction.CHANGE_MAGNIFIER_TYPE,
):
types = list(MagnifierType)
currentType = magnifier._magnifierType
idx = types.index(currentType)
newType = types[(idx + 1) % len(types)]
log.debug(f"Changing magnifier type from {currentType} to {newType}")
changeMagnifierType(newType)
setMagnifierType(newType)
magnifier = getMagnifier()
Comment on lines +192 to +206
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

New behavior (cycling magnifier types and persisting magnifierType in config) isn’t covered by unit tests. Adding tests that (1) cycle from FULLSCREEN through the supported types, (2) verify the created instance type matches the selected/configured type, and (3) verify commands like toggleFilter/pan don’t crash for placeholder modes would prevent regressions while the real implementations are still pending.

Copilot uses AI. Check for mistakes.
ui.message(
pgettext(
"magnifier",
# Translators: Message announced when changing the magnifier type with {type} being the new magnifier type.
"Magnifier type changed to {type}",
).format(type=magnifier._magnifierType.displayString),
Comment on lines +199 to +212
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

cycleMagnifierType iterates list(MagnifierType), which currently includes PLACEHOLDER. This can select a type that createMagnifier doesn’t support, raising at runtime. Also, when FIXED/DOCKED/LENS are implemented via PlaceholderMagnifier with _magnifierType == PLACEHOLDER, cycling will not behave as expected because currentType won’t equal the configured mode. Use an explicit ordered list of user-selectable types (e.g. FULLSCREEN/FIXED/DOCKED/LENS) and ensure the active magnifier instance reports the selected type.

Suggested change
types = list(MagnifierType)
currentType = magnifier._magnifierType
idx = types.index(currentType)
newType = types[(idx + 1) % len(types)]
log.debug(f"Changing magnifier type from {currentType} to {newType}")
changeMagnifierType(newType)
setMagnifierType(newType)
magnifier = getMagnifier()
ui.message(
pgettext(
"magnifier",
# Translators: Message announced when changing the magnifier type with {type} being the new magnifier type.
"Magnifier type changed to {type}",
).format(type=magnifier._magnifierType.displayString),
selectableTypes = [
MagnifierType.FULLSCREEN,
MagnifierType.FIXED,
MagnifierType.DOCKED,
MagnifierType.LENS,
]
currentType = (
magnifier._magnifierType
if magnifier._magnifierType in selectableTypes
else getMagnifierType()
)
if currentType not in selectableTypes:
currentType = selectableTypes[0]
idx = selectableTypes.index(currentType)
newType = selectableTypes[(idx + 1) % len(selectableTypes)]
log.debug(f"Changing magnifier type from {currentType} to {newType}")
changeMagnifierType(newType)
setMagnifierType(newType)
ui.message(
pgettext(
"magnifier",
# Translators: Message announced when changing the magnifier type with {type} being the new magnifier type.
"Magnifier type changed to {type}",
).format(type=newType.displayString),

Copilot uses AI. Check for mistakes.
)

Comment on lines +192 to +214
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

After switching to a non-fullscreen magnifier type, other commands (e.g. toggleFilter) still assume magnifier.filterType exists. PlaceholderMagnifier doesn’t implement filterType, so toggling filters will raise AttributeError once users cycle into FIXED/DOCKED/LENS. Either move a filterType property to the base Magnifier class (using _filterType), or implement it on PlaceholderMagnifier (and other future types).

Copilot uses AI. Check for mistakes.

def toggleFullscreenMode() -> None:
"""Cycle through full-screen focus modes (center, border, relative)"""
magnifier: Magnifier = getMagnifier()
Expand Down
20 changes: 19 additions & 1 deletion source/_magnifier/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"""

import config
from .utils.types import Filter, FullScreenMode
from .utils.types import Filter, FullScreenMode, MagnifierType


class ZoomLevel:
Expand Down Expand Up @@ -123,6 +123,24 @@ def setDefaultFilter(filter: Filter) -> None:
config.conf["magnifier"]["defaultFilter"] = filter.value


def getMagnifierType() -> MagnifierType:
"""
Get magnifier type from config.

:return: The magnifier type.
"""
return MagnifierType(config.conf["magnifier"]["magnifierType"])


def setMagnifierType(magnifierType: MagnifierType) -> None:
"""
Set magnifier type from settings.

:param magnifierType: The magnifier type to set.
"""
config.conf["magnifier"]["magnifierType"] = magnifierType.value


def getDefaultFullscreenMode() -> FullScreenMode:
"""
Get default full-screen mode from config.
Expand Down
4 changes: 2 additions & 2 deletions source/_magnifier/fullscreenMagnifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,17 @@
from .magnifier import Magnifier
from .utils.filterHandler import FilterMatrix
from .utils.spotlightManager import SpotlightManager
from .utils.types import Filter, Coordinates, FullScreenMode
from .utils.types import Filter, Coordinates, FullScreenMode, MagnifierType
from .config import getDefaultFullscreenMode


class FullScreenMagnifier(Magnifier):
def __init__(self):
super().__init__()
self._magnifierType = MagnifierType.FULLSCREEN
self._fullscreenMode = getDefaultFullscreenMode()
self._currentCoordinates = Coordinates(0, 0)
self._spotlightManager = SpotlightManager(self)
self._startMagnifier()

@property
def filterType(self) -> Filter:
Expand Down
2 changes: 1 addition & 1 deletion source/_magnifier/magnifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class Magnifier:

def __init__(self):
self._displayOrientation = getPrimaryDisplayOrientation()
self._magnifierType: MagnifierType = MagnifierType.FULLSCREEN
self._magnifierType: MagnifierType
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

self._magnifierType: MagnifierType is only an annotation and does not assign a value, so instances of Magnifier won’t have a _magnifierType attribute until a subclass sets it. This is easy to trip into AttributeError (e.g. if any code inspects _magnifierType before subclass initialization completes). Consider initializing it to a safe default (e.g. MagnifierType.FULLSCREEN) or to None and handling that explicitly.

Suggested change
self._magnifierType: MagnifierType
self._magnifierType: MagnifierType | None = None

Copilot uses AI. Check for mistakes.
self._isActive: bool = False
self._zoomLevel: float = getDefaultZoomLevel()
self._panStep: int = getDefaultPanStep()
Expand Down
26 changes: 26 additions & 0 deletions source/_magnifier/placeholderMagnifier.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# A part of NonVisual Desktop Access (NVDA)
# Copyright (C) 2025 NV Access Limited, Antoine Haffreingue
# This file may be used under the terms of the GNU General Public License, version 2 or later, as modified by the NVDA license.
# For full terms and any additional permissions, see the NVDA license file: https://github.com/nvaccess/nvda/blob/master/copying.txt
"""
Placeholder magnifier module.
"""

from .magnifier import Magnifier
from .utils.types import Coordinates, MagnifierType


class PlaceholderMagnifier(Magnifier):
def __init__(self):
super().__init__()
self._magnifierType = MagnifierType.PLACEHOLDER
self._currentCoordinates = Coordinates(0, 0)

def _startMagnifier(self) -> None:
super()._startMagnifier()

def _stopMagnifier(self) -> None:
super()._stopMagnifier()

def _doUpdate(self):
super()._doUpdate()
Comment on lines +25 to +26
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

PlaceholderMagnifier._doUpdate calls super()._doUpdate(), which always raises NotImplementedError. Any path that triggers _doUpdate (e.g. Magnifier._pan calls _doUpdate) will crash when a placeholder magnifier is active. Implement _doUpdate as a safe no-op (or minimal placeholder behavior) instead of delegating to the abstract base method.

Suggested change
def _doUpdate(self):
super()._doUpdate()
def _doUpdate(self) -> None:
# Placeholder magnifier has no backing implementation to refresh.
# Intentionally do nothing instead of delegating to the abstract
# base implementation, which raises NotImplementedError.
return

Copilot uses AI. Check for mistakes.
9 changes: 9 additions & 0 deletions source/_magnifier/utils/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class MagnifierAction(DisplayStringEnum):
PAN_TOP_EDGE = auto()
PAN_BOTTOM_EDGE = auto()
TOGGLE_FILTER = auto()
CHANGE_MAGNIFIER_TYPE = auto()
CHANGE_FULLSCREEN_MODE = auto()
START_SPOTLIGHT = auto()

Expand Down Expand Up @@ -69,6 +70,8 @@ def _displayStringLabels(self) -> dict["MagnifierAction", str]:
self.PAN_BOTTOM_EDGE: pgettext("magnifier action", "pan to bottom edge"),
# Translators: Action description for toggling color filters.
self.TOGGLE_FILTER: pgettext("magnifier action", "toggle filters"),
# Translators: Action description for changing magnifier type.
self.CHANGE_MAGNIFIER_TYPE: pgettext("magnifier action", "change magnifier type"),
# Translators: Action description for changing full-screen mode.
self.CHANGE_FULLSCREEN_MODE: pgettext("magnifier action", "change full-screen mode"),
# Translators: Action description for starting spotlight mode.
Expand All @@ -80,18 +83,24 @@ class MagnifierType(DisplayStringStrEnum):
"""Type of magnifier"""

FULLSCREEN = "fullscreen"
FIXED = "fixed"
DOCKED = "docked"
LENS = "lens"
PLACEHOLDER = "placeholder"

@property
def _displayStringLabels(self) -> dict["MagnifierType", str]:
return {
# Translators: Magnifier type - full-screen mode.
self.FULLSCREEN: pgettext("magnifier", "Fullscreen"),
# Translators: Magnifier type - fixed mode.
self.FIXED: pgettext("magnifier", "Fixed"),
# Translators: Magnifier type - docked mode.
self.DOCKED: pgettext("magnifier", "Docked"),
# Translators: Magnifier type - lens mode.
self.LENS: pgettext("magnifier", "Lens"),
# Translators: Magnifier type - placeholder for unsupported types.
self.PLACEHOLDER: pgettext("magnifier", "Placeholder"),
}


Expand Down
1 change: 1 addition & 0 deletions source/config/configSpec.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@
[magnifier]
defaultZoomLevel = float(min=1.0, max=10.0, default=2.0)
defaultPanStep = integer(min=1, max=100, default=10)
magnifierType = string(default="fullscreen")
defaultFullscreenMode = string(default="center")
isTrueCentered = boolean(default=False)
defaultFilter = string(default="normal")
Expand Down
14 changes: 14 additions & 0 deletions source/globalCommands.py
Original file line number Diff line number Diff line change
Expand Up @@ -5159,6 +5159,20 @@ def script_toggleFilter(
) -> None:
_magnifier.commands.toggleFilter()

@script(
description=_(
# Translators: Describes a command.
"Cycle through Magnifier type",
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

The new script description string has grammar/capitalization issues ("Cycle through Magnifier type"). For consistency with other command descriptions in this section, consider something like "Cycle through magnifier types" (lowercase magnifier, plural types).

Suggested change
"Cycle through Magnifier type",
"Cycle through magnifier types",

Copilot uses AI. Check for mistakes.
),
category=SCRCAT_VISION,
gesture="kb:nvda+shift+t",
)
def script_cycleMagnifierType(
self,
gesture: inputCore.InputGesture,
) -> None:
_magnifier.commands.cycleMagnifierType()

@script(
description=_(
# Translators: Describes a command.
Expand Down
Loading
Loading