From 9f0ec3345f08142fdf78ac56cf4e5c62b661fbd2 Mon Sep 17 00:00:00 2001 From: Antoine HAFFREINGUE Date: Tue, 31 Mar 2026 16:07:15 +0200 Subject: [PATCH 1/4] changes types and function name for clarity --- source/_magnifier/commands.py | 12 +-- source/_magnifier/config.py | 80 ++++++++-------- source/_magnifier/fullscreenMagnifier.py | 94 ++++++++++++++----- source/_magnifier/magnifier.py | 40 +++----- source/_magnifier/utils/types.py | 53 ++++++++--- source/config/configSpec.py | 9 +- source/gui/settingsDialogs.py | 70 +++++++------- .../test_fullscreenMagnifier.py | 24 +++-- tests/unit/test_magnifier/test_magnifier.py | 50 +--------- 9 files changed, 221 insertions(+), 211 deletions(-) diff --git a/source/_magnifier/commands.py b/source/_magnifier/commands.py index 7c75e2ec42c..a4766a1c2bb 100644 --- a/source/_magnifier/commands.py +++ b/source/_magnifier/commands.py @@ -12,9 +12,9 @@ import ui from . import getMagnifier, initialize, terminate from .config import ( - getDefaultZoomLevelString, - getDefaultFilter, - getDefaultFullscreenMode, + getZoomLevelString, + getFilter, + getFullscreenMode, ZoomLevel, ) from .magnifier import Magnifier @@ -100,8 +100,8 @@ def toggleMagnifier() -> None: else: initialize() - filter = getDefaultFilter() - fullscreenMode = getDefaultFullscreenMode() + filter = getFilter() + fullscreenMode = getFullscreenMode() ui.message( pgettext( @@ -109,7 +109,7 @@ def toggleMagnifier() -> None: # Translators: Message announced when starting the NVDA magnifier. "Starting magnifier with {zoomLevel} zoom level, {filter} filter, and {fullscreenMode} full-screen mode", ).format( - zoomLevel=getDefaultZoomLevelString(), + zoomLevel=getZoomLevelString(), filter=filter.displayString, fullscreenMode=fullscreenMode.displayString, ), diff --git a/source/_magnifier/config.py b/source/_magnifier/config.py index 7fc38ba6ab5..073f23f8d44 100644 --- a/source/_magnifier/config.py +++ b/source/_magnifier/config.py @@ -49,111 +49,111 @@ def zoom_strings(cls) -> list[str]: ] -def getDefaultZoomLevel() -> float: +def getZoomLevel() -> float: """ - Get default zoom level from config. + Get zoom level from config. - :return: The default zoom level. + :return: The zoom level. """ - zoomLevel = config.conf["magnifier"]["defaultZoomLevel"] + zoomLevel = config.conf["magnifier"]["zoomLevel"] return zoomLevel -def getDefaultZoomLevelString() -> str: +def getZoomLevelString() -> str: """ - Get default zoom level as a formatted string. + Get zoom level as a formatted string. :return: Formatted zoom level string. """ - zoomLevel = getDefaultZoomLevel() + zoomLevel = getZoomLevel() zoomValues = ZoomLevel.zoom_range() zoomStrings = ZoomLevel.zoom_strings() zoomIndex = zoomValues.index(zoomLevel) return zoomStrings[zoomIndex] -def setDefaultZoomLevel(zoomLevel: float) -> None: +def setZoomLevel(zoomLevel: float) -> None: """ - Set default zoom level from settings. + Set zoom level from settings. :param zoomLevel: The zoom level to set. """ if "magnifier" not in config.conf: config.conf["magnifier"] = {} - config.conf["magnifier"]["defaultZoomLevel"] = zoomLevel + config.conf["magnifier"]["zoomLevel"] = zoomLevel -def getDefaultPanStep() -> int: +def getPanStep() -> int: """ - Get default pan value from config. + Get pan value from config. - :return: The default pan value. + :return: The pan value. """ - return config.conf["magnifier"]["defaultPanStep"] + return config.conf["magnifier"]["panStep"] -def setDefaultPanStep(panStep: int) -> None: +def setPanStep(panStep: int) -> None: """ - Set default pan value from settings. + Set pan value from settings. :param panStep: The pan value to set. """ if "magnifier" not in config.conf: config.conf["magnifier"] = {} - config.conf["magnifier"]["defaultPanStep"] = panStep + config.conf["magnifier"]["panStep"] = panStep -def getDefaultFilter() -> Filter: +def getFilter() -> Filter: """ - Get default filter from config. + Get filter from config. - :return: The default filter. + :return: The filter. """ - return Filter(config.conf["magnifier"]["defaultFilter"]) + return Filter(config.conf["magnifier"]["filter"]) -def setDefaultFilter(filter: Filter) -> None: +def setFilter(filter: Filter) -> None: """ - Set default filter from settings. + Set filter from settings. :param filter: The filter to set. """ - config.conf["magnifier"]["defaultFilter"] = filter.value + config.conf["magnifier"]["filter"] = filter.value -def getDefaultFullscreenMode() -> FullScreenMode: +def isTrueCentered() -> bool: """ - Get default full-screen mode from config. + Check if true centered mode is enabled in config. - :return: The default full-screen mode. + :return: True if true centered mode is enabled, False otherwise. """ - return FullScreenMode(config.conf["magnifier"]["defaultFullscreenMode"]) + return config.conf["magnifier"]["isTrueCentered"] -def setDefaultFullscreenMode(mode: FullScreenMode) -> None: +def shouldKeepMouseCentered() -> bool: """ - Set default full-screen mode from settings. + Check if mouse pointer should be kept centered in magnifier view. - :param mode: The full-screen mode to set. + :return: True if mouse should be kept centered, False otherwise. """ - config.conf["magnifier"]["defaultFullscreenMode"] = mode.value + return config.conf["magnifier"]["keepMouseCentered"] -def isTrueCentered() -> bool: +def getFullscreenMode() -> FullScreenMode: """ - Check if true centered mode is enabled in config. + Get full-screen mode from config. - :return: True if true centered mode is enabled, False otherwise. + :return: The full-screen mode. """ - return config.conf["magnifier"]["isTrueCentered"] + return FullScreenMode(config.conf["magnifier"]["fullscreenMode"]) -def shouldKeepMouseCentered() -> bool: +def setFullscreenMode(mode: FullScreenMode) -> None: """ - Check if mouse pointer should be kept centered in magnifier view. + Set full-screen mode from settings. - :return: True if mouse should be kept centered, False otherwise. + :param mode: The full-screen mode to set. """ - return config.conf["magnifier"]["keepMouseCentered"] + config.conf["magnifier"]["fullscreenMode"] = mode.value diff --git a/source/_magnifier/fullscreenMagnifier.py b/source/_magnifier/fullscreenMagnifier.py index 317f2bfd8f2..9ab8f122f4d 100644 --- a/source/_magnifier/fullscreenMagnifier.py +++ b/source/_magnifier/fullscreenMagnifier.py @@ -14,17 +14,26 @@ from .magnifier import Magnifier from .utils.filterHandler import FilterMatrix from .utils.spotlightManager import SpotlightManager -from .utils.types import Filter, Coordinates, FullScreenMode, FocusType -from .config import getDefaultFullscreenMode, shouldKeepMouseCentered +from .utils.types import ( + Filter, + Coordinates, + MagnifierType, + FullScreenMode, + FocusType, + Size, + MagnifierParameters, +) +from .config import getFullscreenMode, shouldKeepMouseCentered, isTrueCentered class FullScreenMagnifier(Magnifier): def __init__(self): super().__init__() - self._fullscreenMode = getDefaultFullscreenMode() + self._magnifierType = MagnifierType.FULLSCREEN + self._fullscreenMode = getFullscreenMode() self._currentCoordinates = Coordinates(0, 0) self._spotlightManager = SpotlightManager(self) - self._startMagnifier() + self._displaySize = Size(self._displayOrientation.width, self._displayOrientation.height) @property def filterType(self) -> Filter: @@ -107,6 +116,9 @@ def _applyFilter(self) -> None: """ Apply the current color filter to the full-screen magnifier """ + if not self._isActive: + return + try: match self.filterType: case Filter.NORMAL: @@ -127,12 +139,15 @@ def _fullscreenMagnifier(self, coordinates: Coordinates) -> None: :coordinates: The (x, y) coordinates to center the magnifier on """ - left, top, visibleWidth, visibleHeight = self._getMagnifierPosition(coordinates) + if not self._isActive: + return + + params = self._getMagnifierParameters(coordinates) try: result = magnification.MagSetFullscreenTransform( self.zoomLevel, - left, - top, + params.coordinates.x, + params.coordinates.y, ) if not result: log.debug("Failed to set full-screen transform") @@ -172,11 +187,9 @@ def moveMouseToScreen(self) -> None: log.debug("Mouse button pressed, skipping cursor repositioning to avoid interfering with click") return - left, top, visibleWidth, visibleHeight = self._getMagnifierPosition( - self._currentCoordinates, - ) - centerX = int(left + (visibleWidth / 2)) - centerY = int(top + (visibleHeight / 2)) + params = self._getMagnifierParameters(self._currentCoordinates) + centerX = int(params.coordinates.x + (params.magnifierSize.width / 2)) + centerY = int(params.coordinates.y + (params.magnifierSize.height / 2)) winUser.setCursorPos(centerX, centerY) def _borderPos( @@ -192,14 +205,16 @@ def _borderPos( :return: The adjusted position (x, y) of the focus point """ focusX, focusY = coordinates - lastLeft, lastTop, visibleWidth, visibleHeight = self._getMagnifierPosition( - self._lastScreenPosition, - ) + params = self._getMagnifierParameters(self._lastScreenPosition) + magnifierWidth = params.magnifierSize.width + magnifierHeight = params.magnifierSize.height + lastLeft = params.coordinates.x + lastTop = params.coordinates.y minX = lastLeft + self._MARGIN_BORDER - maxX = lastLeft + visibleWidth - self._MARGIN_BORDER + maxX = lastLeft + magnifierWidth - self._MARGIN_BORDER minY = lastTop + self._MARGIN_BORDER - maxY = lastTop + visibleHeight - self._MARGIN_BORDER + maxY = lastTop + magnifierHeight - self._MARGIN_BORDER dx = 0 dy = 0 @@ -237,21 +252,21 @@ def _relativePos( zoom = self.zoomLevel mouseX, mouseY = coordinates - visibleWidth = self._displayOrientation.width / zoom - visibleHeight = self._displayOrientation.height / zoom + magnifierWidth = self._displayOrientation.width / zoom + magnifierHeight = self._displayOrientation.height / zoom margin = int(zoom * 10) # Calculate left/top maintaining mouse relative position - left = mouseX - (mouseX / self._displayOrientation.width) * (visibleWidth - margin) - top = mouseY - (mouseY / self._displayOrientation.height) * (visibleHeight - margin) + left = mouseX - (mouseX / self._displayOrientation.width) * (magnifierWidth - margin) + top = mouseY - (mouseY / self._displayOrientation.height) * (magnifierHeight - margin) # Clamp to screen boundaries - left = max(0, min(left, self._displayOrientation.width - visibleWidth)) - top = max(0, min(top, self._displayOrientation.height - visibleHeight)) + left = max(0, min(left, self._displayOrientation.width - magnifierWidth)) + top = max(0, min(top, self._displayOrientation.height - magnifierHeight)) # Return center of zoom window - centerX = int(left + visibleWidth / 2) - centerY = int(top + visibleHeight / 2) + centerX = int(left + magnifierWidth / 2) + centerY = int(top + magnifierHeight / 2) self._lastScreenPosition = Coordinates(centerX, centerY) return self._lastScreenPosition @@ -271,3 +286,32 @@ def _stopSpotlight(self) -> None: """ self._spotlightManager._spotlightIsActive = False self._startTimer(self._updateMagnifier) + + def _getMagnifierParameters(self, coordinates: Coordinates) -> MagnifierParameters: + """ + Compute the top-left corner of the magnifier window centered on (x, y) + + :param coordinates: The (x, y) coordinates to center the magnifier on + :param displaySize: The size of the display area (width, height) - used to calculate capture size + + :return: The size, position and filter of the magnifier window + """ + x, y = coordinates + # Calculate the size of the capture area at the current zoom level + magnifierWidth = self._displayOrientation.width / self.zoomLevel + magnifierHeight = self._displayOrientation.height / self.zoomLevel + + # Compute the top-left corner so that (x, y) is at the center + left = int(x - (magnifierWidth / 2)) + top = int(y - (magnifierHeight / 2)) + + # Clamp to screen boundaries only if not in true center mode + if not isTrueCentered(): + left = max(0, min(left, int(self._displayOrientation.width - magnifierWidth))) + top = max(0, min(top, int(self._displayOrientation.height - magnifierHeight))) + + return MagnifierParameters( + Size(int(magnifierWidth), int(magnifierHeight)), + Coordinates(left, top), + self._filterType, + ) diff --git a/source/_magnifier/magnifier.py b/source/_magnifier/magnifier.py index 4ee41440cab..59143b8d7de 100644 --- a/source/_magnifier/magnifier.py +++ b/source/_magnifier/magnifier.py @@ -18,7 +18,7 @@ from winAPI import _displayTracking from winAPI._displayTracking import OrientationState, getPrimaryDisplayOrientation from .utils.types import ( - MagnifierPosition, + MagnifierParameters, MagnifierAction, Coordinates, MagnifierType, @@ -26,9 +26,9 @@ Filter, ) from .config import ( - getDefaultZoomLevel, - getDefaultPanStep, - getDefaultFilter, + getZoomLevel, + getPanStep, + getFilter, ZoomLevel, isTrueCentered, ) @@ -41,16 +41,16 @@ class Magnifier: def __init__(self): self._displayOrientation = getPrimaryDisplayOrientation() - self._magnifierType: MagnifierType = MagnifierType.FULLSCREEN + self._magnifierType: MagnifierType self._isActive: bool = False - self._zoomLevel: float = getDefaultZoomLevel() - self._panStep: int = getDefaultPanStep() + self._zoomLevel: float = getZoomLevel() + self._panStep: int = getPanStep() self._timer: None | wx.Timer = None self._focusManager = FocusManager() self._lastScreenPosition = Coordinates(0, 0) self._currentCoordinates = Coordinates(0, 0) self._lastFocusCoordinates = Coordinates(0, 0) - self._filterType: Filter = getDefaultFilter() + self._filterType: Filter = getFilter() self._isManualPanning: bool = False # Register for display changes _displayTracking.displayChanged.register(self._onDisplayChanged) @@ -277,29 +277,13 @@ def _stopTimer(self) -> None: else: log.debug("no timer to stop") - def _getMagnifierPosition( - self, - coordinates: Coordinates, - ) -> MagnifierPosition: + def _getMagnifierParameters(self, coordinates: Coordinates) -> MagnifierParameters: """ Compute the top-left corner of the magnifier window centered on (x, y) :param coordinates: The (x, y) coordinates to center the magnifier on + :param displaySize: The size of the display area (width, height) - used to calculate capture size - :return: The position and size of the magnifier window + :return: The size, position and filter of the magnifier window """ - x, y = coordinates - # Calculate the size of the capture area at the current zoom level - visibleWidth = self._displayOrientation.width / self.zoomLevel - visibleHeight = self._displayOrientation.height / self.zoomLevel - - # Compute the top-left corner so that (x, y) is at the center - left = int(x - (visibleWidth / 2)) - top = int(y - (visibleHeight / 2)) - - # Clamp to screen boundaries only if not in true center mode - if not isTrueCentered(): - left = max(0, min(left, int(self._displayOrientation.width - visibleWidth))) - top = max(0, min(top, int(self._displayOrientation.height - visibleHeight))) - - return MagnifierPosition(left, top, int(visibleWidth), int(visibleHeight)) + raise NotImplementedError("Subclasses must implement this method") diff --git a/source/_magnifier/utils/types.py b/source/_magnifier/utils/types.py index cc18b9f70b6..8c62ed3e40f 100644 --- a/source/_magnifier/utils/types.py +++ b/source/_magnifier/utils/types.py @@ -12,14 +12,6 @@ from utils.displayString import DisplayStringStrEnum, DisplayStringEnum -class MagnifierParams(NamedTuple): - """Named tuple representing magnifier parameters for initialization""" - - zoomLevel: float - filter: str - fullscreenMode: str - - class Direction(Enum): """Direction for zoom operations""" @@ -27,6 +19,20 @@ class Direction(Enum): OUT = False +class Coordinates(NamedTuple): + """Named tuple representing x and y coordinates""" + + x: int + y: int + + +class Size(NamedTuple): + """Named tuple representing width and height""" + + width: int + height: int + + class MagnifierAction(DisplayStringEnum): """Actions that can be performed with the magnifier""" @@ -80,6 +86,7 @@ class MagnifierType(DisplayStringStrEnum): """Type of magnifier""" FULLSCREEN = "fullscreen" + FIXED = "fixed" DOCKED = "docked" LENS = "lens" @@ -88,6 +95,8 @@ 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. @@ -95,6 +104,23 @@ def _displayStringLabels(self) -> dict["MagnifierType", str]: } +class Filter(DisplayStringStrEnum): + NORMAL = "normal" + GRAYSCALE = "grayscale" + INVERTED = "inverted" + + @property + def _displayStringLabels(self) -> dict["Filter", str]: + return { + # Translators: Magnifier color filter - no filter applied. + self.NORMAL: pgettext("magnifier", "Normal"), + # Translators: Magnifier color filter - grayscale/black and white. + self.GRAYSCALE: pgettext("magnifier", "Grayscale"), + # Translators: Magnifier color filter - inverted colors. + self.INVERTED: pgettext("magnifier", "Inverted"), + } + + class FocusType(Enum): """Type of focus being tracked by the magnifier""" @@ -103,13 +129,12 @@ class FocusType(Enum): NAVIGATOR = auto() -class MagnifierPosition(NamedTuple): - """Named tuple representing the position and size of the magnifier window""" +class MagnifierParameters(NamedTuple): + """Named tuple representing the size and position of the magnifier""" - left: int - top: int - visibleWidth: int - visibleHeight: int + magnifierSize: Size + coordinates: Coordinates + filter: Filter class Coordinates(NamedTuple): diff --git a/source/config/configSpec.py b/source/config/configSpec.py index 1cc61e0c6a1..8025898ca76 100644 --- a/source/config/configSpec.py +++ b/source/config/configSpec.py @@ -114,13 +114,12 @@ # Magnifier settings [magnifier] - defaultZoomLevel = float(min=1.0, max=10.0, default=2.0) - defaultPanStep = integer(min=1, max=100, default=10) - defaultFullscreenMode = string(default="center") + zoomLevel = float(min=1.0, max=10.0, default=2.0) + panStep = integer(min=1, max=100, default=10) isTrueCentered = boolean(default=False) - defaultFilter = string(default="normal") + filter = string(default="normal") + fullscreenMode = string(default="center") keepMouseCentered = boolean(default=false) - saveShortcutChanges = boolean(default=false) # Presentation settings [presentation] diff --git a/source/gui/settingsDialogs.py b/source/gui/settingsDialogs.py index a78b64fa446..32c946dd7c6 100644 --- a/source/gui/settingsDialogs.py +++ b/source/gui/settingsDialogs.py @@ -6002,38 +6002,38 @@ def makeSettings( # ZOOM SETTINGS # Translators: The label for a setting in magnifier settings to select the default zoom level. - defaultZoomLabelText = _("Default &zoom level:") + zoomLabelText = _(" &zoom level:") zoomValues = magnifierConfig.ZoomLevel.zoom_range() zoomChoices = magnifierConfig.ZoomLevel.zoom_strings() - self.defaultZoomList = sHelper.addLabeledControl( - defaultZoomLabelText, + self.zoomList = sHelper.addLabeledControl( + zoomLabelText, wx.Choice, choices=zoomChoices, ) self.bindHelpEvent( - "MagnifierDefaultZoom", - self.defaultZoomList, + "MagnifierZoom", + self.zoomList, ) - # Set default value from config - defaultZoom = magnifierConfig.getDefaultZoomLevel() - zoomIndex = bisect.bisect_left(zoomValues, defaultZoom) + # Set value from config + zoomLevel = magnifierConfig.getZoomLevel() + zoomIndex = bisect.bisect_left(zoomValues, zoomLevel) # Find the closest value if zoomIndex == 0: closestIndex = 0 elif zoomIndex >= len(zoomValues): closestIndex = len(zoomValues) - 1 else: - closestIndex = min(zoomIndex - 1, zoomIndex, key=lambda i: abs(zoomValues[i] - defaultZoom)) - self.defaultZoomList.SetSelection(closestIndex) + closestIndex = min(zoomIndex - 1, zoomIndex, key=lambda i: abs(zoomValues[i] - zoomLevel)) + self.zoomList.SetSelection(closestIndex) # PAN SETTINGS # Translators: The label for a setting in magnifier settings to select the pan step size (in percentage). panStepSizeLabelText = _("&Panning step size (%):") - self.defaultPanSpinCtrl = sHelper.addLabeledControl( + self.panSpinCtrl = sHelper.addLabeledControl( panStepSizeLabelText, wx.SpinCtrl, min=1, @@ -6041,40 +6041,40 @@ def makeSettings( ) self.bindHelpEvent( "magnifierPanStep", - self.defaultPanSpinCtrl, + self.panSpinCtrl, ) - # Set default value from config - defaultPan = magnifierConfig.getDefaultPanStep() - self.defaultPanSpinCtrl.SetValue(defaultPan) + # Set value from config + panStep = magnifierConfig.getPanStep() + self.panSpinCtrl.SetValue(panStep) # FILTER SETTINGS # Translators: The label for a setting in magnifier settings to select the default filter - defaultFilterLabelText = _("Default &filter:") + filterLabelText = _("&filter:") filterChoices = [f.displayString for f in Filter] - self.defaultFilterList = sHelper.addLabeledControl( - defaultFilterLabelText, + self.filterList = sHelper.addLabeledControl( + filterLabelText, wx.Choice, choices=filterChoices, ) - self.bindHelpEvent("MagnifierDefaultFilter", self.defaultFilterList) + self.bindHelpEvent("MagnifierFilter", self.filterList) - # Set default value from config - defaultFilter = magnifierConfig.getDefaultFilter() - self.defaultFilterList.SetSelection(list(Filter).index(defaultFilter)) + # Set value from config + filterValue = magnifierConfig.getFilter() + self.filterList.SetSelection(list(Filter).index(filterValue)) # FULLSCREEN MODE SETTINGS - # Translators: The label for a setting in magnifier settings to select the default full-screen mode - defaultFullscreenModeLabelText = _("Default &fullscreen mode:") + # Translators: The label for a setting in magnifier settings to select the full-screen mode + fullscreenModeLabelText = _("&fullscreen mode:") fullscreenModeChoices = [mode.displayString for mode in FullScreenMode] if FullScreenMode else [] - self.defaultFullscreenModeList = sHelper.addLabeledControl( - defaultFullscreenModeLabelText, + self.fullscreenModeList = sHelper.addLabeledControl( + fullscreenModeLabelText, wx.Choice, choices=fullscreenModeChoices, ) self.bindHelpEvent( - "MagnifierDefaultFullscreenFocusMode", - self.defaultFullscreenModeList, + "MagnifierFullscreenFocusMode", + self.fullscreenModeList, ) # TRUE CENTER @@ -6103,16 +6103,16 @@ def makeSettings( def onSave(self): """Save the current selections to config.""" - selectedZoom = self.defaultZoomList.GetSelection() - magnifierConfig.setDefaultZoomLevel(magnifierConfig.ZoomLevel.zoom_range()[selectedZoom]) + selectedZoom = self.zoomList.GetSelection() + magnifierConfig.setZoomLevel(magnifierConfig.ZoomLevel.zoom_range()[selectedZoom]) - magnifierConfig.setDefaultPanStep(self.defaultPanSpinCtrl.GetValue()) + magnifierConfig.setPanStep(self.panSpinCtrl.GetValue()) - selectedFilterIdx = self.defaultFilterList.GetSelection() - magnifierConfig.setDefaultFilter(list(Filter)[selectedFilterIdx]) + selectedFilterIdx = self.filterList.GetSelection() + magnifierConfig.setFilter(list(Filter)[selectedFilterIdx]) - selectedModeIdx = self.defaultFullscreenModeList.GetSelection() - magnifierConfig.setDefaultFullscreenMode(list(FullScreenMode)[selectedModeIdx]) + selectedModeIdx = self.fullscreenModeList.GetSelection() + magnifierConfig.setFullscreenMode(list(FullScreenMode)[selectedModeIdx]) config.conf["magnifier"]["isTrueCentered"] = self.trueCenterCheckBox.GetValue() config.conf["magnifier"]["keepMouseCentered"] = self.keepMouseCenteredCheckBox.GetValue() diff --git a/tests/unit/test_magnifier/test_fullscreenMagnifier.py b/tests/unit/test_magnifier/test_fullscreenMagnifier.py index 56b23d64f9c..3f9ac2f06f0 100644 --- a/tests/unit/test_magnifier/test_fullscreenMagnifier.py +++ b/tests/unit/test_magnifier/test_fullscreenMagnifier.py @@ -10,12 +10,13 @@ from _magnifier.magnifier import Magnifier -class TestMagnifierEndToEnd(_TestMagnifier): - """End-to-end test suite for Magnifier functionality.""" +class TestFullscreenMagnifierEndToEnd(_TestMagnifier): + """End-to-end test suite for fullscreen magnifier functionality.""" def testMagnifierCreation(self): """Test creating a magnifier.""" magnifier = FullScreenMagnifier() + magnifier._startMagnifier() self.assertEqual(magnifier.zoomLevel, 2.0) self.assertEqual(magnifier.filterType, Filter.NORMAL) @@ -28,6 +29,7 @@ def testMagnifierCreation(self): def testMagnifierZoom(self): """Test zoom functionality.""" magnifier = FullScreenMagnifier() + magnifier._startMagnifier() # Set initial zoom to 1.0 for predictable testing magnifier.zoomLevel = 1.0 @@ -47,6 +49,7 @@ def testMagnifierZoom(self): def testMagnifierCoordinates(self): """Test coordinate handling.""" magnifier = FullScreenMagnifier() + magnifier._startMagnifier() # Test setting coordinates magnifier._currentCoordinates = (100, 200) @@ -62,6 +65,7 @@ def testMagnifierCoordinates(self): def testMagnifierUpdate(self): """Test magnifier update cycle.""" magnifier = FullScreenMagnifier() + magnifier._startMagnifier() # Mock the update methods magnifier._getCoordinatesForMode = MagicMock(return_value=(150, 250)) @@ -84,6 +88,7 @@ def testMagnifierUpdate(self): def testMagnifierStop(self): """Test stopping the magnifier.""" magnifier = FullScreenMagnifier() + magnifier._startMagnifier() # Mock the timer magnifier._stopTimer = MagicMock() @@ -103,20 +108,20 @@ def testMagnifierPositionCalculation(self): magnifier = FullScreenMagnifier() # Test position calculation - left, top, width, height = magnifier._getMagnifierPosition((500, 400)) + params = magnifier._getMagnifierParameters((500, 400)) # Basic checks - self.assertIsInstance(left, int) - self.assertIsInstance(top, int) - self.assertIsInstance(width, int) - self.assertIsInstance(height, int) + self.assertIsInstance(params.coordinates.x, int) + self.assertIsInstance(params.coordinates.y, int) + self.assertIsInstance(params.magnifierSize.width, int) + self.assertIsInstance(params.magnifierSize.height, int) # Width and height should be screen size divided by zoom expectedWidth = int(magnifier._displayOrientation.width / 2.0) expectedHeight = int(magnifier._displayOrientation.height / 2.0) - self.assertEqual(width, expectedWidth) - self.assertEqual(height, expectedHeight) + self.assertEqual(params.magnifierSize.width, expectedWidth) + self.assertEqual(params.magnifierSize.height, expectedHeight) # Cleanup magnifier._stopMagnifier() @@ -189,6 +194,7 @@ def testMagnifierSimpleLifecycle(self): """Test simple magnifier lifecycle.""" # Create magnifier magnifier = FullScreenMagnifier() + magnifier._startMagnifier() self.assertTrue(magnifier._isActive) self.assertEqual(magnifier.zoomLevel, 2.0) diff --git a/tests/unit/test_magnifier/test_magnifier.py b/tests/unit/test_magnifier/test_magnifier.py index 1e864689255..925351f1e41 100644 --- a/tests/unit/test_magnifier/test_magnifier.py +++ b/tests/unit/test_magnifier/test_magnifier.py @@ -3,7 +3,7 @@ # 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 -from _magnifier.magnifier import Magnifier, MagnifierType +from _magnifier.magnifier import Magnifier from _magnifier.utils.types import Filter, Direction, Coordinates, MagnifierAction import unittest from winAPI._displayTracking import getPrimaryDisplayOrientation @@ -63,7 +63,6 @@ def testMagnifierCreation(self): """Can we create a magnifier with valid parameters?""" self.assertEqual(self.magnifier.zoomLevel, 2.0) self.assertEqual(self.magnifier._filterType, Filter.NORMAL) - self.assertEqual(self.magnifier._magnifierType, MagnifierType.FULLSCREEN) self.assertFalse(self.magnifier._isActive) self.assertIsNotNone(self.magnifier._focusManager) @@ -364,50 +363,3 @@ def testStopTimer(self): # Test stopping when no timer exists (should not raise error) self.magnifier._stopTimer() self.assertIsNone(self.magnifier._timer) - - def testMagnifierPosition(self): - """Computing magnifier position and size.""" - x, y = int(self.screenWidth / 2), int(self.screenHeight / 2) - left, top, width, height = self.magnifier._getMagnifierPosition((x, y)) - - expected_width = int(self.screenWidth / self.magnifier.zoomLevel) - expected_height = int(self.screenHeight / self.magnifier.zoomLevel) - expected_left = int(x - (expected_width / 2)) - expected_top = int(y - (expected_height / 2)) - - self.assertEqual(left, expected_left) - self.assertEqual(top, expected_top) - self.assertEqual(width, expected_width) - self.assertEqual(height, expected_height) - - # Test left clamping - left, top, width, height = self.magnifier._getMagnifierPosition((100, 540)) - self.assertGreaterEqual(left, 0) - - # Test right clamping - left, top, width, height = self.magnifier._getMagnifierPosition((1800, 540)) - self.assertLessEqual(left + width, self.screenWidth) - - # Test different zoom level - self.magnifier.zoomLevel = 4.0 - left, top, width, height = self.magnifier._getMagnifierPosition((960, 540)) - expected_width = int(self.screenWidth / self.magnifier.zoomLevel) - expected_height = int(self.screenHeight / self.magnifier.zoomLevel) - self.assertEqual(width, expected_width) - self.assertEqual(height, expected_height) - - def testMagnifierPositionTrueCentered(self): - """Test magnifier position calculation with true centered mode.""" - x, y = int(self.screenWidth / 2), int(self.screenHeight / 2) - with patch("source._magnifier.magnifier.isTrueCentered", return_value=True): - left, top, width, height = self.magnifier._getMagnifierPosition((x, y)) - - expected_width = int(self.screenWidth / self.magnifier.zoomLevel) - expected_height = int(self.screenHeight / self.magnifier.zoomLevel) - expected_left = int(x - (expected_width / 2)) - expected_top = int(y - (expected_height / 2)) - - self.assertEqual(left, expected_left) - self.assertEqual(top, expected_top) - self.assertEqual(width, expected_width) - self.assertEqual(height, expected_height) From 15b72a443c180bba56c0f7578980d09035367443 Mon Sep 17 00:00:00 2001 From: Antoine HAFFREINGUE Date: Wed, 1 Apr 2026 16:18:59 +0200 Subject: [PATCH 2/4] copilot review --- source/_magnifier/config.py | 13 +++++++++++-- source/_magnifier/fullscreenMagnifier.py | 2 +- source/_magnifier/magnifier.py | 1 - source/gui/settingsDialogs.py | 8 ++++---- user_docs/en/userGuide.md | 10 +++++----- 5 files changed, 21 insertions(+), 13 deletions(-) diff --git a/source/_magnifier/config.py b/source/_magnifier/config.py index 073f23f8d44..17d86d358bd 100644 --- a/source/_magnifier/config.py +++ b/source/_magnifier/config.py @@ -68,8 +68,17 @@ def getZoomLevelString() -> str: zoomLevel = getZoomLevel() zoomValues = ZoomLevel.zoom_range() zoomStrings = ZoomLevel.zoom_strings() - zoomIndex = zoomValues.index(zoomLevel) - return zoomStrings[zoomIndex] + if not zoomValues: + # Fallback: format the current zoom level directly if no predefined values are available. + return ZoomLevel.ZOOM_MESSAGE.format( + zoomLevel=f"{zoomLevel:.1f}", + ) + # Find the index of the zoom value closest to the configured zoom level. + closestIndex = min( + range(len(zoomValues)), + key=lambda i: abs(zoomValues[i] - zoomLevel), + ) + return zoomStrings[closestIndex] def setZoomLevel(zoomLevel: float) -> None: diff --git a/source/_magnifier/fullscreenMagnifier.py b/source/_magnifier/fullscreenMagnifier.py index 9ab8f122f4d..4721dcdfb5a 100644 --- a/source/_magnifier/fullscreenMagnifier.py +++ b/source/_magnifier/fullscreenMagnifier.py @@ -34,6 +34,7 @@ def __init__(self): self._currentCoordinates = Coordinates(0, 0) self._spotlightManager = SpotlightManager(self) self._displaySize = Size(self._displayOrientation.width, self._displayOrientation.height) + self._startMagnifier() @property def filterType(self) -> Filter: @@ -292,7 +293,6 @@ def _getMagnifierParameters(self, coordinates: Coordinates) -> MagnifierParamete Compute the top-left corner of the magnifier window centered on (x, y) :param coordinates: The (x, y) coordinates to center the magnifier on - :param displaySize: The size of the display area (width, height) - used to calculate capture size :return: The size, position and filter of the magnifier window """ diff --git a/source/_magnifier/magnifier.py b/source/_magnifier/magnifier.py index 59143b8d7de..bf7c6c54818 100644 --- a/source/_magnifier/magnifier.py +++ b/source/_magnifier/magnifier.py @@ -282,7 +282,6 @@ def _getMagnifierParameters(self, coordinates: Coordinates) -> MagnifierParamete Compute the top-left corner of the magnifier window centered on (x, y) :param coordinates: The (x, y) coordinates to center the magnifier on - :param displaySize: The size of the display area (width, height) - used to calculate capture size :return: The size, position and filter of the magnifier window """ diff --git a/source/gui/settingsDialogs.py b/source/gui/settingsDialogs.py index 32c946dd7c6..457553bf432 100644 --- a/source/gui/settingsDialogs.py +++ b/source/gui/settingsDialogs.py @@ -6001,8 +6001,8 @@ def makeSettings( ) # ZOOM SETTINGS - # Translators: The label for a setting in magnifier settings to select the default zoom level. - zoomLabelText = _(" &zoom level:") + # Translators: The label for a setting in magnifier settings to select the zoom level. + zoomLabelText = _("&Zoom level:") zoomValues = magnifierConfig.ZoomLevel.zoom_range() zoomChoices = magnifierConfig.ZoomLevel.zoom_strings() @@ -6088,8 +6088,8 @@ def makeSettings( self.trueCenterCheckBox.SetValue(magnifierConfig.isTrueCentered()) # Set default value from config - defaultFullscreenMode = magnifierConfig.getDefaultFullscreenMode() - self.defaultFullscreenModeList.SetSelection(list(FullScreenMode).index(defaultFullscreenMode)) + defaultFullscreenMode = magnifierConfig.getFullscreenMode() + self.fullscreenModeList.SetSelection(list(FullScreenMode).index(defaultFullscreenMode)) # KEEP MOUSE CENTERED # Translators: The label for a checkbox to keep the mouse pointer centered in the magnifier view diff --git a/user_docs/en/userGuide.md b/user_docs/en/userGuide.md index 940c267d572..efe9307cceb 100644 --- a/user_docs/en/userGuide.md +++ b/user_docs/en/userGuide.md @@ -2856,7 +2856,7 @@ Key: `NVDA+control+w` The Magnifier category in the NVDA Settings dialog allows you to configure the default behavior of NVDA's built-in [Magnifier](#Magnifier) feature. This settings category contains the following options: -##### Default zoom level {#MagnifierDefaultZoom} +##### Zoom level {#MagnifierZoom} This slider allows you to set the default zoom level when the magnifier is first enabled. The zoom level can range from 1.0 (no magnification) to 10.0 (maximum magnification). @@ -2869,9 +2869,9 @@ You can always adjust the zoom level on the fly using the zoom in (`NVDA+shift+e |Options |1.0 to 10.0| |Default |2.0| -##### Default color filter {#MagnifierDefaultFilter} +##### Filter {#MagnifierFilter} -This combo box allows you to select the default color filter to apply when the magnifier is first enabled. +This combo box allows you to select the filter to apply when the magnifier is first enabled. You can cycle through the color filters by pressing `NVDA+shift+i`. The available options are: @@ -2887,9 +2887,9 @@ The available options are: | Grayscale | Converts all colors to shades of gray, which can help reduce eye strain and improve contrast. | | Inverted | Inverts all colors on the screen, which can be helpful for users who prefer light text on dark backgrounds or have photophobia. | -##### Default focus mode {#MagnifierDefaultFullscreenFocusMode} +##### Focus mode {#MagnifierFullscreenFocusMode} -This combo box allows you to select the default focus tracking mode when the magnifier is first enabled. +This combo box allows you to select the focus tracking mode when the magnifier is first enabled. To cycle through the focus tracking modes, please assign a custom gesture using the [Input Gestures dialog](#InputGestures). The available options are: From e8c2999357cbe4a923f954c1030cef394cc8d1a4 Mon Sep 17 00:00:00 2001 From: Antoine HAFFREINGUE Date: Wed, 1 Apr 2026 16:58:07 +0200 Subject: [PATCH 3/4] changed Coordinates to locationHelper.POINT --- source/_magnifier/fullscreenMagnifier.py | 33 +++--- source/_magnifier/magnifier.py | 12 +- source/_magnifier/utils/focusManager.py | 43 +++---- source/_magnifier/utils/types.py | 10 +- .../unit/test_magnifier/test_focusManager.py | 107 +++++++++--------- tests/unit/test_magnifier/test_magnifier.py | 37 +++--- 6 files changed, 122 insertions(+), 120 deletions(-) diff --git a/source/_magnifier/fullscreenMagnifier.py b/source/_magnifier/fullscreenMagnifier.py index f59b1823cc6..ac9da7e1182 100644 --- a/source/_magnifier/fullscreenMagnifier.py +++ b/source/_magnifier/fullscreenMagnifier.py @@ -16,14 +16,13 @@ from .utils.spotlightManager import SpotlightManager from .utils.types import ( Filter, - Coordinates, MagnifierType, FullScreenMode, - FocusType, Size, MagnifierParameters, ) -from .config import getFullscreenMode, shouldKeepMouseCentered, isTrueCentered +from .config import getFullscreenMode, isTrueCentered +import locationHelper class FullScreenMagnifier(Magnifier): @@ -31,7 +30,7 @@ def __init__(self): super().__init__() self._magnifierType = MagnifierType.FULLSCREEN self._fullscreenMode = getFullscreenMode() - self._currentCoordinates = Coordinates(0, 0) + self._currentCoordinates = locationHelper.Point(0, 0) self._spotlightManager = SpotlightManager(self) self._displaySize = Size(self._displayOrientation.width, self._displayOrientation.height) self._startMagnifier() @@ -131,7 +130,7 @@ def _applyFilter(self) -> None: except Exception as e: log.error(f"Failed to apply filter: {e}") - def _fullscreenMagnifier(self, coordinates: Coordinates) -> None: + def _fullscreenMagnifier(self, coordinates: locationHelper.Point) -> None: """ Apply full-screen magnification at given Coordinates @@ -154,8 +153,8 @@ def _fullscreenMagnifier(self, coordinates: Coordinates) -> None: def _getCoordinatesForMode( self, - coordinates: Coordinates, - ) -> Coordinates: + coordinates: locationHelper.Point, + ) -> locationHelper.Point: """ Get Coordinates adjusted for the current full-screen mode @@ -191,8 +190,8 @@ def _keepMouseCentered(self) -> None: def _borderPos( self, - coordinates: Coordinates, - ) -> Coordinates: + coordinates: locationHelper.Point, + ) -> locationHelper.Point: """ Check if focus is near magnifier border and adjust position accordingly Returns adjusted position to keep focus within margin limits @@ -227,17 +226,17 @@ def _borderPos( dy = focusY - maxY if dx != 0 or dy != 0: - return Coordinates( - self._lastScreenPosition[0] + dx, - self._lastScreenPosition[1] + dy, + return locationHelper.Point( + self._lastScreenPosition.x + dx, + self._lastScreenPosition.y + dy, ) else: return self._lastScreenPosition def _relativePos( self, - coordinates: Coordinates, - ) -> Coordinates: + coordinates: locationHelper.Point, + ) -> locationHelper.Point: """ Calculate magnifier center maintaining mouse relative position Handles screen edges to prevent going off-screen @@ -264,7 +263,7 @@ def _relativePos( # Return center of zoom window centerX = int(left + magnifierWidth / 2) centerY = int(top + magnifierHeight / 2) - self._lastScreenPosition = Coordinates(centerX, centerY) + self._lastScreenPosition = locationHelper.Point(centerX, centerY) return self._lastScreenPosition def _startSpotlight(self) -> None: @@ -284,7 +283,7 @@ def _stopSpotlight(self) -> None: self._spotlightManager._spotlightIsActive = False self._startTimer(self._updateMagnifier) - def _getMagnifierParameters(self, coordinates: Coordinates) -> MagnifierParameters: + def _getMagnifierParameters(self, coordinates: locationHelper.Point) -> MagnifierParameters: """ Compute the top-left corner of the magnifier window centered on (x, y) @@ -308,6 +307,6 @@ def _getMagnifierParameters(self, coordinates: Coordinates) -> MagnifierParamete return MagnifierParameters( Size(int(magnifierWidth), int(magnifierHeight)), - Coordinates(left, top), + locationHelper.Point(left, top), self._filterType, ) diff --git a/source/_magnifier/magnifier.py b/source/_magnifier/magnifier.py index 023e0003b2e..9adf7401a1c 100644 --- a/source/_magnifier/magnifier.py +++ b/source/_magnifier/magnifier.py @@ -20,7 +20,6 @@ from .utils.types import ( MagnifierParameters, MagnifierAction, - Coordinates, MagnifierType, Direction, Filter, @@ -34,6 +33,7 @@ shouldKeepMouseCentered, ) from .utils.focusManager import FocusManager +import locationHelper class Magnifier: @@ -48,9 +48,9 @@ def __init__(self): self._panStep: int = getPanStep() self._timer: None | wx.Timer = None self._focusManager = FocusManager() - self._lastScreenPosition = Coordinates(0, 0) - self._currentCoordinates = Coordinates(0, 0) - self._lastFocusCoordinates = Coordinates(0, 0) + self._lastScreenPosition = locationHelper.Point(0, 0) + self._currentCoordinates = locationHelper.Point(0, 0) + self._lastFocusCoordinates = locationHelper.Point(0, 0) self._filterType: Filter = getFilter() self._isManualPanning: bool = False # Register for display changes @@ -253,7 +253,7 @@ def _pan(self, action: MagnifierAction) -> bool: log.error(f"Unknown pan action: {action}") self._isManualPanning = True - self._currentCoordinates = Coordinates(x, y) + self._currentCoordinates = locationHelper.Point(x, y) self._doUpdate() return (x, y) != (originalX, originalY) @@ -298,7 +298,7 @@ def _stopTimer(self) -> None: else: log.debug("no timer to stop") - def _getMagnifierParameters(self, coordinates: Coordinates) -> MagnifierParameters: + def _getMagnifierParameters(self, coordinates: locationHelper.Point) -> MagnifierParameters: """ Compute the top-left corner of the magnifier window centered on (x, y) diff --git a/source/_magnifier/utils/focusManager.py b/source/_magnifier/utils/focusManager.py index 20c04b00f6f..dc2f2c02e6b 100644 --- a/source/_magnifier/utils/focusManager.py +++ b/source/_magnifier/utils/focusManager.py @@ -11,7 +11,8 @@ import api import winUser import mouseHandler -from .types import Coordinates, FocusType +from .types import FocusType +import locationHelper class FocusManager: @@ -23,13 +24,13 @@ class FocusManager: def __init__(self): """Initialize the focus manager.""" self._lastFocusedObject: FocusType | None = None - self._lastMousePosition = Coordinates(0, 0) - self._lastSystemFocusPosition = Coordinates(0, 0) - self._lastNavigatorObjectPosition = Coordinates(0, 0) - self._lastValidSystemFocusPosition = Coordinates(0, 0) - self._lastValidNavigatorObjectPosition = Coordinates(0, 0) + self._lastMousePosition = locationHelper.Point(0, 0) + self._lastSystemFocusPosition = locationHelper.Point(0, 0) + self._lastNavigatorObjectPosition = locationHelper.Point(0, 0) + self._lastValidSystemFocusPosition = locationHelper.Point(0, 0) + self._lastValidNavigatorObjectPosition = locationHelper.Point(0, 0) - def getCurrentFocusCoordinates(self) -> Coordinates: + def getCurrentFocusCoordinates(self) -> locationHelper.Point: """ Get the current focus coordinates based on priority. Priority: Mouse > Navigator Object > System Focus @@ -99,16 +100,16 @@ def getCurrentFocusCoordinates(self) -> Coordinates: # Default to mouse if no previous focus return mousePosition - def _getMousePosition(self) -> Coordinates: + def _getMousePosition(self) -> locationHelper.Point: """ Get the current mouse position. :return: The (x, y) coordinates of the mouse """ mousePos = winUser.getCursorPos() - return Coordinates(mousePos[0], mousePos[1]) + return locationHelper.Point(mousePos[0], mousePos[1]) - def _getSystemFocusPosition(self) -> Coordinates: + def _getSystemFocusPosition(self) -> locationHelper.Point: """ Get the current system focus position (focus object + browse mode cursor). This includes both the system focus and the browse mode cursor if active. @@ -119,9 +120,9 @@ def _getSystemFocusPosition(self) -> Coordinates: # Get caret position (works for both browse mode and regular focus) caretPosition = api.getCaretPosition() point = caretPosition.pointAtStart - coords = Coordinates(point.x, point.y) + coords = locationHelper.Point(point.x, point.y) # Store as last valid position if not (0, 0) - if coords != Coordinates(0, 0): + if coords != locationHelper.Point(0, 0): self._lastValidSystemFocusPosition = coords return coords except (NotImplementedError, LookupError, AttributeError, RuntimeError): @@ -132,8 +133,8 @@ def _getSystemFocusPosition(self) -> Coordinates: left, top, width, height = focusObj.location x = left + (width // 2) y = top + (height // 2) - coords = Coordinates(x, y) - if coords != Coordinates(0, 0): + coords = locationHelper.Point(x, y) + if coords != locationHelper.Point(0, 0): self._lastValidSystemFocusPosition = coords return coords except Exception: @@ -142,7 +143,7 @@ def _getSystemFocusPosition(self) -> Coordinates: pass return self._lastValidSystemFocusPosition - def _getReviewPosition(self) -> Coordinates | None: + def _getReviewPosition(self) -> locationHelper.Point | None: """ Get the current review position (review cursor). @@ -152,13 +153,13 @@ def _getReviewPosition(self) -> Coordinates | None: if reviewPosition: try: point = reviewPosition.pointAtStart - return Coordinates(point.x, point.y) + return locationHelper.Point(point.x, point.y) except (NotImplementedError, LookupError, AttributeError): # Review position may not support pointAtStart pass return None - def _getNavigatorObjectLocation(self) -> Coordinates | None: + def _getNavigatorObjectLocation(self) -> locationHelper.Point | None: """ Get the navigator object location from its bounding rectangle. @@ -170,13 +171,13 @@ def _getNavigatorObjectLocation(self) -> Coordinates | None: left, top, width, height = navigatorObject.location x = left + (width // 2) y = top + (height // 2) - return Coordinates(x, y) + return locationHelper.Point(x, y) except Exception: # Navigator object may not have a valid location pass return None - def _getNavigatorObjectPosition(self) -> Coordinates: + def _getNavigatorObjectPosition(self) -> locationHelper.Point: """ Get the navigator object position (NumPad navigation). Tries review position first, then navigator object location. @@ -185,13 +186,13 @@ def _getNavigatorObjectPosition(self) -> Coordinates: """ # Try review position first position = self._getReviewPosition() - if position and position != Coordinates(0, 0): + if position and position != locationHelper.Point(0, 0): self._lastValidNavigatorObjectPosition = position return position # Fallback: use navigator object location position = self._getNavigatorObjectLocation() - if position and position != Coordinates(0, 0): + if position and position != locationHelper.Point(0, 0): self._lastValidNavigatorObjectPosition = position return position diff --git a/source/_magnifier/utils/types.py b/source/_magnifier/utils/types.py index 8c62ed3e40f..c69ae24a553 100644 --- a/source/_magnifier/utils/types.py +++ b/source/_magnifier/utils/types.py @@ -9,6 +9,7 @@ from enum import Enum, auto from typing import NamedTuple +import locationHelper from utils.displayString import DisplayStringStrEnum, DisplayStringEnum @@ -19,13 +20,6 @@ class Direction(Enum): OUT = False -class Coordinates(NamedTuple): - """Named tuple representing x and y coordinates""" - - x: int - y: int - - class Size(NamedTuple): """Named tuple representing width and height""" @@ -133,7 +127,7 @@ class MagnifierParameters(NamedTuple): """Named tuple representing the size and position of the magnifier""" magnifierSize: Size - coordinates: Coordinates + coordinates: locationHelper.Point filter: Filter diff --git a/tests/unit/test_magnifier/test_focusManager.py b/tests/unit/test_magnifier/test_focusManager.py index 24ca10e77d9..905c8dc6ec0 100644 --- a/tests/unit/test_magnifier/test_focusManager.py +++ b/tests/unit/test_magnifier/test_focusManager.py @@ -5,22 +5,23 @@ from dataclasses import dataclass from _magnifier.utils.focusManager import FocusManager -from _magnifier.utils.types import Coordinates, FocusType +from _magnifier.utils.types import FocusType import unittest from unittest.mock import MagicMock, Mock, patch import mouseHandler import winUser +import locationHelper @dataclass(frozen=True) class FocusTestParam: """Parameters for focus coordinate testing.""" - navigatorObjectPos: Coordinates - systemFocusPos: Coordinates + navigatorObjectPos: locationHelper.Point + systemFocusPos: locationHelper.Point mousePos: tuple leftPressed: bool - expectedCoords: Coordinates + expectedCoords: locationHelper.Point expectedFocus: FocusType description: str = "" lastFocusedObject: FocusType | None = None @@ -36,9 +37,9 @@ def setUp(self): def testFocusManagerCreation(self): """Can we create a FocusManager with initialized values?""" self.assertIsNone(self.focusManager._lastFocusedObject) - self.assertEqual(self.focusManager._lastSystemFocusPosition, Coordinates(0, 0)) - self.assertEqual(self.focusManager._lastNavigatorObjectPosition, Coordinates(0, 0)) - self.assertEqual(self.focusManager._lastMousePosition, Coordinates(0, 0)) + self.assertEqual(self.focusManager._lastSystemFocusPosition, locationHelper.Point(0, 0)) + self.assertEqual(self.focusManager._lastNavigatorObjectPosition, locationHelper.Point(0, 0)) + self.assertEqual(self.focusManager._lastMousePosition, locationHelper.Point(0, 0)) def testGetNavigatorObjectPosition(self): """Getting navigator object position with different API responses.""" @@ -50,7 +51,7 @@ def testGetNavigatorObjectPosition(self): mock_review.return_value.pointAtStart = mock_point coords = self.focusManager._getNavigatorObjectPosition() - self.assertEqual(coords, Coordinates(300, 400)) + self.assertEqual(coords, locationHelper.Point(300, 400)) # Case 2: Review position fails, navigator object works with patch("_magnifier.utils.focusManager.api.getReviewPosition", return_value=None): @@ -59,7 +60,7 @@ def testGetNavigatorObjectPosition(self): coords = self.focusManager._getNavigatorObjectPosition() # Center: (100 + 200//2, 150 + 300//2) = (200, 300) - self.assertEqual(coords, Coordinates(200, 300)) + self.assertEqual(coords, locationHelper.Point(200, 300)) # Case 3: Everything fails - should return last valid position from Case 2 with patch("_magnifier.utils.focusManager.api.getReviewPosition", return_value=None): @@ -68,7 +69,7 @@ def testGetNavigatorObjectPosition(self): coords = self.focusManager._getNavigatorObjectPosition() # Should return last valid position (200, 300) - self.assertEqual(coords, Coordinates(200, 300)) + self.assertEqual(coords, locationHelper.Point(200, 300)) def testGetSystemFocusPosition(self): """Getting system focus position with different API responses.""" @@ -80,7 +81,7 @@ def testGetSystemFocusPosition(self): mock_caret.return_value.pointAtStart = mock_point coords = self.focusManager._getSystemFocusPosition() - self.assertEqual(coords, Coordinates(500, 600)) + self.assertEqual(coords, locationHelper.Point(500, 600)) # Case 2: Caret fails, focus object works with patch("_magnifier.utils.focusManager.api.getCaretPosition", side_effect=RuntimeError): @@ -89,104 +90,104 @@ def testGetSystemFocusPosition(self): coords = self.focusManager._getSystemFocusPosition() # Center: (200 + 100//2, 300 + 80//2) = (250, 340) - self.assertEqual(coords, Coordinates(250, 340)) + self.assertEqual(coords, locationHelper.Point(250, 340)) # Case 3: Everything fails - should return last valid position from Case 2 with patch("_magnifier.utils.focusManager.api.getCaretPosition", side_effect=RuntimeError): with patch("_magnifier.utils.focusManager.api.getFocusObject", return_value=None): coords = self.focusManager._getSystemFocusPosition() # Should return last valid position (250, 340) - self.assertEqual(coords, Coordinates(250, 340)) + self.assertEqual(coords, locationHelper.Point(250, 340)) def testGetMousePosition(self): """Getting mouse position.""" with patch("_magnifier.utils.focusManager.winUser.getCursorPos", return_value=(123, 456)): coords = self.focusManager._getMousePosition() - self.assertEqual(coords, Coordinates(123, 456)) + self.assertEqual(coords, locationHelper.Point(123, 456)) def testGetCurrentFocusCoordinates(self): """All priority scenarios for focus coordinates.""" subTestParams = [ FocusTestParam( - navigatorObjectPos=Coordinates(0, 0), - systemFocusPos=Coordinates(0, 0), - mousePos=(0, 0), + navigatorObjectPos=locationHelper.Point(0, 0), + systemFocusPos=locationHelper.Point(0, 0), + mousePos=locationHelper.Point(0, 0), leftPressed=True, - expectedCoords=Coordinates(0, 0), + expectedCoords=locationHelper.Point(0, 0), expectedFocus=FocusType.MOUSE, description="Left click is pressed should return mouse position", ), FocusTestParam( - navigatorObjectPos=Coordinates(0, 0), - systemFocusPos=Coordinates(0, 0), - mousePos=(10, 10), + navigatorObjectPos=locationHelper.Point(0, 0), + systemFocusPos=locationHelper.Point(0, 0), + mousePos=locationHelper.Point(10, 10), leftPressed=False, - expectedCoords=Coordinates(10, 10), + expectedCoords=locationHelper.Point(10, 10), expectedFocus=FocusType.MOUSE, description="Mouse moving (not dragging)", ), FocusTestParam( - navigatorObjectPos=Coordinates(0, 0), - systemFocusPos=Coordinates(15, 15), - mousePos=(0, 0), + navigatorObjectPos=locationHelper.Point(0, 0), + systemFocusPos=locationHelper.Point(15, 15), + mousePos=locationHelper.Point(0, 0), leftPressed=False, - expectedCoords=Coordinates(15, 15), + expectedCoords=locationHelper.Point(15, 15), expectedFocus=FocusType.SYSTEM_FOCUS, description="System focus changed alone (navigator did not move)", ), FocusTestParam( - navigatorObjectPos=Coordinates(20, 20), - systemFocusPos=Coordinates(0, 0), - mousePos=(0, 0), + navigatorObjectPos=locationHelper.Point(20, 20), + systemFocusPos=locationHelper.Point(0, 0), + mousePos=locationHelper.Point(0, 0), leftPressed=False, - expectedCoords=Coordinates(20, 20), + expectedCoords=locationHelper.Point(20, 20), expectedFocus=FocusType.NAVIGATOR, description="Navigator object changed (NumPad navigation)", ), FocusTestParam( - navigatorObjectPos=Coordinates(30, 30), - systemFocusPos=Coordinates(15, 15), - mousePos=(0, 0), + navigatorObjectPos=locationHelper.Point(30, 30), + systemFocusPos=locationHelper.Point(15, 15), + mousePos=locationHelper.Point(0, 0), leftPressed=False, - expectedCoords=Coordinates(30, 30), + expectedCoords=locationHelper.Point(30, 30), expectedFocus=FocusType.NAVIGATOR, description="Both system focus and navigator changed (table cell navigation): navigator wins", ), FocusTestParam( - navigatorObjectPos=Coordinates(0, 0), - systemFocusPos=Coordinates(0, 0), - mousePos=(0, 0), + navigatorObjectPos=locationHelper.Point(0, 0), + systemFocusPos=locationHelper.Point(0, 0), + mousePos=locationHelper.Point(0, 0), leftPressed=False, - expectedCoords=Coordinates(0, 0), + expectedCoords=locationHelper.Point(0, 0), expectedFocus=FocusType.MOUSE, description="Nothing changed, last was Mouse", lastFocusedObject=FocusType.MOUSE, ), FocusTestParam( - navigatorObjectPos=Coordinates(0, 0), - systemFocusPos=Coordinates(0, 0), - mousePos=(0, 0), + navigatorObjectPos=locationHelper.Point(0, 0), + systemFocusPos=locationHelper.Point(0, 0), + mousePos=locationHelper.Point(0, 0), leftPressed=False, - expectedCoords=Coordinates(0, 0), + expectedCoords=locationHelper.Point(0, 0), expectedFocus=FocusType.NAVIGATOR, description="Nothing changed, last was NAVIGATOR", lastFocusedObject=FocusType.NAVIGATOR, ), FocusTestParam( - navigatorObjectPos=Coordinates(10, 10), - systemFocusPos=Coordinates(0, 0), - mousePos=(20, 20), + navigatorObjectPos=locationHelper.Point(10, 10), + systemFocusPos=locationHelper.Point(0, 0), + mousePos=locationHelper.Point(20, 20), leftPressed=False, - expectedCoords=Coordinates(20, 20), + expectedCoords=locationHelper.Point(20, 20), expectedFocus=FocusType.MOUSE, description="Both mouse and navigator object moved (mouse has priority)", ), FocusTestParam( - navigatorObjectPos=Coordinates(10, 10), - systemFocusPos=Coordinates(15, 15), - mousePos=(20, 20), + navigatorObjectPos=locationHelper.Point(10, 10), + systemFocusPos=locationHelper.Point(15, 15), + mousePos=locationHelper.Point(20, 20), leftPressed=True, - expectedCoords=Coordinates(20, 20), + expectedCoords=locationHelper.Point(20, 20), expectedFocus=FocusType.MOUSE, description="All three moved while dragging (mouse drag has highest priority)", ), @@ -195,9 +196,9 @@ def testGetCurrentFocusCoordinates(self): for param in subTestParams: with self.subTest(description=param.description): # Reset focus manager state - self.focusManager._lastNavigatorObjectPosition = Coordinates(0, 0) - self.focusManager._lastSystemFocusPosition = Coordinates(0, 0) - self.focusManager._lastMousePosition = Coordinates(0, 0) + self.focusManager._lastNavigatorObjectPosition = locationHelper.Point(0, 0) + self.focusManager._lastSystemFocusPosition = locationHelper.Point(0, 0) + self.focusManager._lastMousePosition = locationHelper.Point(0, 0) # Set lastFocusedObject if specified if param.lastFocusedObject is not None: diff --git a/tests/unit/test_magnifier/test_magnifier.py b/tests/unit/test_magnifier/test_magnifier.py index d0583b43452..a6b847cb68e 100644 --- a/tests/unit/test_magnifier/test_magnifier.py +++ b/tests/unit/test_magnifier/test_magnifier.py @@ -4,11 +4,12 @@ # For full terms and any additional permissions, see the NVDA license file: https://github.com/nvaccess/nvda/blob/master/copying.txt from _magnifier.magnifier import Magnifier -from _magnifier.utils.types import Filter, Direction, Coordinates, MagnifierAction +from _magnifier.utils.types import Filter, Direction, MagnifierAction import unittest from winAPI._displayTracking import getPrimaryDisplayOrientation from unittest.mock import MagicMock, patch import wx +import locationHelper class _TestMagnifier(unittest.TestCase): @@ -22,9 +23,14 @@ def setUpClass(cls): def setUp(self): """Setup before each test - mock magnification API to prevent actual screen magnification.""" - # Mock the Windows Magnification API to prevent affecting the user's screen - self.mag_patcher = patch("winBindings.magnification") - self.mock_mag = self.mag_patcher.start() + # Patch both the source module and the imported alias used by FullScreenMagnifier. + # This avoids leaking real Magnification API state across unrelated test suites. + self._magPatchers = [ + patch("winBindings.magnification"), + patch("_magnifier.fullscreenMagnifier.magnification"), + ] + self.mock_mag = self._magPatchers[1].start() + self._magPatchers[0].start() # Configure mocked API methods to return success self.mock_mag.MagInitialize.return_value = True @@ -34,7 +40,8 @@ def setUp(self): def tearDown(self): """Cleanup after each test.""" - self.mag_patcher.stop() + for magPatcher in reversed(self._magPatchers): + magPatcher.stop() class TestMagnifier(_TestMagnifier): @@ -92,7 +99,7 @@ def testZoomLevelProperty(self): def testStartMagnifier(self): """Activating the magnifier.""" self.magnifier._focusManager.getCurrentFocusCoordinates = MagicMock( - return_value=Coordinates(100, 200), + return_value=locationHelper.Point(100, 200), ) # Test starting from inactive state @@ -100,7 +107,7 @@ def testStartMagnifier(self): self.magnifier._startMagnifier() self.assertTrue(self.magnifier._isActive) - self.assertEqual(self.magnifier._currentCoordinates, Coordinates(100, 200)) + self.assertEqual(self.magnifier._currentCoordinates, locationHelper.Point(100, 200)) self.magnifier._focusManager.getCurrentFocusCoordinates.assert_called_once() # Test starting when already active (should not call getCurrentFocusCoordinates again) @@ -113,7 +120,7 @@ def testStartMagnifier(self): def testUpdateMagnifier(self): """Updating the magnifier's properties.""" self.magnifier._focusManager.getCurrentFocusCoordinates = MagicMock( - return_value=Coordinates(100, 200), + return_value=locationHelper.Point(100, 200), ) self.magnifier._doUpdate = MagicMock() self.magnifier._startTimer = MagicMock() @@ -137,7 +144,7 @@ def testUpdateMagnifier(self): self.magnifier._startTimer.assert_called_once_with( self.magnifier._updateMagnifier, ) - self.assertEqual(self.magnifier._currentCoordinates, Coordinates(100, 200)) + self.assertEqual(self.magnifier._currentCoordinates, locationHelper.Point(100, 200)) def testDoUpdate(self): """DoUpdate function raises NotImplementedError.""" @@ -189,7 +196,7 @@ def _setupPanTest(self): self.magnifier._panStep = 10 # 10% of screen width centerX = self.screenWidth // 2 centerY = self.screenHeight // 2 - self.magnifier._currentCoordinates = Coordinates(centerX, centerY) + self.magnifier._currentCoordinates = locationHelper.Point(centerX, centerY) expectedPanPixels = int( (self.screenWidth / self.magnifier.zoomLevel) * 10 / 100, ) @@ -225,12 +232,12 @@ def _testSimplePan( # Test reaching edge - movement succeeds on first contact (position changes to edge) if axis == "x": - self.magnifier._currentCoordinates = Coordinates( + self.magnifier._currentCoordinates = locationHelper.Point( edgeValue - direction * expectedPanPixels, centerY, ) else: - self.magnifier._currentCoordinates = Coordinates( + self.magnifier._currentCoordinates = locationHelper.Point( centerX, edgeValue - direction * expectedPanPixels, ) @@ -342,8 +349,8 @@ def testPanToBottomEdge(self): def testManagePanning(self): """Manual panning ends when focus coordinates change, and _lastFocusCoordinates is always kept up to date.""" - focusA = Coordinates(100, 200) - focusB = Coordinates(300, 400) + focusA = locationHelper.Point(100, 200) + focusB = locationHelper.Point(300, 400) self.magnifier._focusManager.getCurrentFocusCoordinates = MagicMock(return_value=focusA) @@ -367,7 +374,7 @@ def testManagePanning(self): def testKeepMouseCentered(self): """Base _keepMouseCentered moves cursor to _currentCoordinates.""" - self.magnifier._currentCoordinates = Coordinates(640, 360) + self.magnifier._currentCoordinates = locationHelper.Point(640, 360) with patch("_magnifier.magnifier.winUser.setCursorPos") as mockSetCursor: self.magnifier._keepMouseCentered() mockSetCursor.assert_called_once_with(640, 360) From 22d7f03dde7470de41e8f0c6c31bda38b9720e1d Mon Sep 17 00:00:00 2001 From: Antoine HAFFREINGUE Date: Tue, 7 Apr 2026 09:35:56 +0200 Subject: [PATCH 4/4] revert to coordinates --- source/_magnifier/fullscreenMagnifier.py | 26 ++--- source/_magnifier/magnifier.py | 12 +- source/_magnifier/utils/focusManager.py | 43 ++++--- source/_magnifier/utils/types.py | 10 +- .../unit/test_magnifier/test_focusManager.py | 107 +++++++++--------- tests/unit/test_magnifier/test_magnifier.py | 23 ++-- 6 files changed, 112 insertions(+), 109 deletions(-) diff --git a/source/_magnifier/fullscreenMagnifier.py b/source/_magnifier/fullscreenMagnifier.py index ac9da7e1182..f84b0302c47 100644 --- a/source/_magnifier/fullscreenMagnifier.py +++ b/source/_magnifier/fullscreenMagnifier.py @@ -20,9 +20,9 @@ FullScreenMode, Size, MagnifierParameters, + Coordinates, ) from .config import getFullscreenMode, isTrueCentered -import locationHelper class FullScreenMagnifier(Magnifier): @@ -30,7 +30,7 @@ def __init__(self): super().__init__() self._magnifierType = MagnifierType.FULLSCREEN self._fullscreenMode = getFullscreenMode() - self._currentCoordinates = locationHelper.Point(0, 0) + self._currentCoordinates = Coordinates(0, 0) self._spotlightManager = SpotlightManager(self) self._displaySize = Size(self._displayOrientation.width, self._displayOrientation.height) self._startMagnifier() @@ -130,7 +130,7 @@ def _applyFilter(self) -> None: except Exception as e: log.error(f"Failed to apply filter: {e}") - def _fullscreenMagnifier(self, coordinates: locationHelper.Point) -> None: + def _fullscreenMagnifier(self, coordinates: Coordinates) -> None: """ Apply full-screen magnification at given Coordinates @@ -153,8 +153,8 @@ def _fullscreenMagnifier(self, coordinates: locationHelper.Point) -> None: def _getCoordinatesForMode( self, - coordinates: locationHelper.Point, - ) -> locationHelper.Point: + coordinates: Coordinates, + ) -> Coordinates: """ Get Coordinates adjusted for the current full-screen mode @@ -190,8 +190,8 @@ def _keepMouseCentered(self) -> None: def _borderPos( self, - coordinates: locationHelper.Point, - ) -> locationHelper.Point: + coordinates: Coordinates, + ) -> Coordinates: """ Check if focus is near magnifier border and adjust position accordingly Returns adjusted position to keep focus within margin limits @@ -226,7 +226,7 @@ def _borderPos( dy = focusY - maxY if dx != 0 or dy != 0: - return locationHelper.Point( + return Coordinates( self._lastScreenPosition.x + dx, self._lastScreenPosition.y + dy, ) @@ -235,8 +235,8 @@ def _borderPos( def _relativePos( self, - coordinates: locationHelper.Point, - ) -> locationHelper.Point: + coordinates: Coordinates, + ) -> Coordinates: """ Calculate magnifier center maintaining mouse relative position Handles screen edges to prevent going off-screen @@ -263,7 +263,7 @@ def _relativePos( # Return center of zoom window centerX = int(left + magnifierWidth / 2) centerY = int(top + magnifierHeight / 2) - self._lastScreenPosition = locationHelper.Point(centerX, centerY) + self._lastScreenPosition = Coordinates(centerX, centerY) return self._lastScreenPosition def _startSpotlight(self) -> None: @@ -283,7 +283,7 @@ def _stopSpotlight(self) -> None: self._spotlightManager._spotlightIsActive = False self._startTimer(self._updateMagnifier) - def _getMagnifierParameters(self, coordinates: locationHelper.Point) -> MagnifierParameters: + def _getMagnifierParameters(self, coordinates: Coordinates) -> MagnifierParameters: """ Compute the top-left corner of the magnifier window centered on (x, y) @@ -307,6 +307,6 @@ def _getMagnifierParameters(self, coordinates: locationHelper.Point) -> Magnifie return MagnifierParameters( Size(int(magnifierWidth), int(magnifierHeight)), - locationHelper.Point(left, top), + Coordinates(left, top), self._filterType, ) diff --git a/source/_magnifier/magnifier.py b/source/_magnifier/magnifier.py index 9adf7401a1c..19f26e42e82 100644 --- a/source/_magnifier/magnifier.py +++ b/source/_magnifier/magnifier.py @@ -23,6 +23,7 @@ MagnifierType, Direction, Filter, + Coordinates, ) from .config import ( getZoomLevel, @@ -33,7 +34,6 @@ shouldKeepMouseCentered, ) from .utils.focusManager import FocusManager -import locationHelper class Magnifier: @@ -48,9 +48,9 @@ def __init__(self): self._panStep: int = getPanStep() self._timer: None | wx.Timer = None self._focusManager = FocusManager() - self._lastScreenPosition = locationHelper.Point(0, 0) - self._currentCoordinates = locationHelper.Point(0, 0) - self._lastFocusCoordinates = locationHelper.Point(0, 0) + self._lastScreenPosition = Coordinates(0, 0) + self._currentCoordinates = Coordinates(0, 0) + self._lastFocusCoordinates = Coordinates(0, 0) self._filterType: Filter = getFilter() self._isManualPanning: bool = False # Register for display changes @@ -253,7 +253,7 @@ def _pan(self, action: MagnifierAction) -> bool: log.error(f"Unknown pan action: {action}") self._isManualPanning = True - self._currentCoordinates = locationHelper.Point(x, y) + self._currentCoordinates = Coordinates(x, y) self._doUpdate() return (x, y) != (originalX, originalY) @@ -298,7 +298,7 @@ def _stopTimer(self) -> None: else: log.debug("no timer to stop") - def _getMagnifierParameters(self, coordinates: locationHelper.Point) -> MagnifierParameters: + def _getMagnifierParameters(self, coordinates: Coordinates) -> MagnifierParameters: """ Compute the top-left corner of the magnifier window centered on (x, y) diff --git a/source/_magnifier/utils/focusManager.py b/source/_magnifier/utils/focusManager.py index dc2f2c02e6b..a4fe0c80a2c 100644 --- a/source/_magnifier/utils/focusManager.py +++ b/source/_magnifier/utils/focusManager.py @@ -11,8 +11,7 @@ import api import winUser import mouseHandler -from .types import FocusType -import locationHelper +from .types import FocusType, Coordinates class FocusManager: @@ -24,13 +23,13 @@ class FocusManager: def __init__(self): """Initialize the focus manager.""" self._lastFocusedObject: FocusType | None = None - self._lastMousePosition = locationHelper.Point(0, 0) - self._lastSystemFocusPosition = locationHelper.Point(0, 0) - self._lastNavigatorObjectPosition = locationHelper.Point(0, 0) - self._lastValidSystemFocusPosition = locationHelper.Point(0, 0) - self._lastValidNavigatorObjectPosition = locationHelper.Point(0, 0) + self._lastMousePosition = Coordinates(0, 0) + self._lastSystemFocusPosition = Coordinates(0, 0) + self._lastNavigatorObjectPosition = Coordinates(0, 0) + self._lastValidSystemFocusPosition = Coordinates(0, 0) + self._lastValidNavigatorObjectPosition = Coordinates(0, 0) - def getCurrentFocusCoordinates(self) -> locationHelper.Point: + def getCurrentFocusCoordinates(self) -> Coordinates: """ Get the current focus coordinates based on priority. Priority: Mouse > Navigator Object > System Focus @@ -100,16 +99,16 @@ def getCurrentFocusCoordinates(self) -> locationHelper.Point: # Default to mouse if no previous focus return mousePosition - def _getMousePosition(self) -> locationHelper.Point: + def _getMousePosition(self) -> Coordinates: """ Get the current mouse position. :return: The (x, y) coordinates of the mouse """ mousePos = winUser.getCursorPos() - return locationHelper.Point(mousePos[0], mousePos[1]) + return Coordinates(mousePos[0], mousePos[1]) - def _getSystemFocusPosition(self) -> locationHelper.Point: + def _getSystemFocusPosition(self) -> Coordinates: """ Get the current system focus position (focus object + browse mode cursor). This includes both the system focus and the browse mode cursor if active. @@ -120,9 +119,9 @@ def _getSystemFocusPosition(self) -> locationHelper.Point: # Get caret position (works for both browse mode and regular focus) caretPosition = api.getCaretPosition() point = caretPosition.pointAtStart - coords = locationHelper.Point(point.x, point.y) + coords = Coordinates(point.x, point.y) # Store as last valid position if not (0, 0) - if coords != locationHelper.Point(0, 0): + if coords != Coordinates(0, 0): self._lastValidSystemFocusPosition = coords return coords except (NotImplementedError, LookupError, AttributeError, RuntimeError): @@ -133,8 +132,8 @@ def _getSystemFocusPosition(self) -> locationHelper.Point: left, top, width, height = focusObj.location x = left + (width // 2) y = top + (height // 2) - coords = locationHelper.Point(x, y) - if coords != locationHelper.Point(0, 0): + coords = Coordinates(x, y) + if coords != Coordinates(0, 0): self._lastValidSystemFocusPosition = coords return coords except Exception: @@ -143,7 +142,7 @@ def _getSystemFocusPosition(self) -> locationHelper.Point: pass return self._lastValidSystemFocusPosition - def _getReviewPosition(self) -> locationHelper.Point | None: + def _getReviewPosition(self) -> Coordinates | None: """ Get the current review position (review cursor). @@ -153,13 +152,13 @@ def _getReviewPosition(self) -> locationHelper.Point | None: if reviewPosition: try: point = reviewPosition.pointAtStart - return locationHelper.Point(point.x, point.y) + return Coordinates(point.x, point.y) except (NotImplementedError, LookupError, AttributeError): # Review position may not support pointAtStart pass return None - def _getNavigatorObjectLocation(self) -> locationHelper.Point | None: + def _getNavigatorObjectLocation(self) -> Coordinates | None: """ Get the navigator object location from its bounding rectangle. @@ -171,13 +170,13 @@ def _getNavigatorObjectLocation(self) -> locationHelper.Point | None: left, top, width, height = navigatorObject.location x = left + (width // 2) y = top + (height // 2) - return locationHelper.Point(x, y) + return Coordinates(x, y) except Exception: # Navigator object may not have a valid location pass return None - def _getNavigatorObjectPosition(self) -> locationHelper.Point: + def _getNavigatorObjectPosition(self) -> Coordinates: """ Get the navigator object position (NumPad navigation). Tries review position first, then navigator object location. @@ -186,13 +185,13 @@ def _getNavigatorObjectPosition(self) -> locationHelper.Point: """ # Try review position first position = self._getReviewPosition() - if position and position != locationHelper.Point(0, 0): + if position and position != Coordinates(0, 0): self._lastValidNavigatorObjectPosition = position return position # Fallback: use navigator object location position = self._getNavigatorObjectLocation() - if position and position != locationHelper.Point(0, 0): + if position and position != Coordinates(0, 0): self._lastValidNavigatorObjectPosition = position return position diff --git a/source/_magnifier/utils/types.py b/source/_magnifier/utils/types.py index c69ae24a553..3d4a1add0c6 100644 --- a/source/_magnifier/utils/types.py +++ b/source/_magnifier/utils/types.py @@ -9,10 +9,16 @@ from enum import Enum, auto from typing import NamedTuple -import locationHelper from utils.displayString import DisplayStringStrEnum, DisplayStringEnum +class Coordinates(NamedTuple): + """Named tuple representing x and y coordinates""" + + x: int + y: int + + class Direction(Enum): """Direction for zoom operations""" @@ -127,7 +133,7 @@ class MagnifierParameters(NamedTuple): """Named tuple representing the size and position of the magnifier""" magnifierSize: Size - coordinates: locationHelper.Point + coordinates: Coordinates filter: Filter diff --git a/tests/unit/test_magnifier/test_focusManager.py b/tests/unit/test_magnifier/test_focusManager.py index 905c8dc6ec0..b1300b465ed 100644 --- a/tests/unit/test_magnifier/test_focusManager.py +++ b/tests/unit/test_magnifier/test_focusManager.py @@ -5,23 +5,22 @@ from dataclasses import dataclass from _magnifier.utils.focusManager import FocusManager -from _magnifier.utils.types import FocusType +from _magnifier.utils.types import FocusType, Coordinates import unittest from unittest.mock import MagicMock, Mock, patch import mouseHandler import winUser -import locationHelper @dataclass(frozen=True) class FocusTestParam: """Parameters for focus coordinate testing.""" - navigatorObjectPos: locationHelper.Point - systemFocusPos: locationHelper.Point + navigatorObjectPos: Coordinates + systemFocusPos: Coordinates mousePos: tuple leftPressed: bool - expectedCoords: locationHelper.Point + expectedCoords: Coordinates expectedFocus: FocusType description: str = "" lastFocusedObject: FocusType | None = None @@ -37,9 +36,9 @@ def setUp(self): def testFocusManagerCreation(self): """Can we create a FocusManager with initialized values?""" self.assertIsNone(self.focusManager._lastFocusedObject) - self.assertEqual(self.focusManager._lastSystemFocusPosition, locationHelper.Point(0, 0)) - self.assertEqual(self.focusManager._lastNavigatorObjectPosition, locationHelper.Point(0, 0)) - self.assertEqual(self.focusManager._lastMousePosition, locationHelper.Point(0, 0)) + self.assertEqual(self.focusManager._lastSystemFocusPosition, Coordinates(0, 0)) + self.assertEqual(self.focusManager._lastNavigatorObjectPosition, Coordinates(0, 0)) + self.assertEqual(self.focusManager._lastMousePosition, Coordinates(0, 0)) def testGetNavigatorObjectPosition(self): """Getting navigator object position with different API responses.""" @@ -51,7 +50,7 @@ def testGetNavigatorObjectPosition(self): mock_review.return_value.pointAtStart = mock_point coords = self.focusManager._getNavigatorObjectPosition() - self.assertEqual(coords, locationHelper.Point(300, 400)) + self.assertEqual(coords, Coordinates(300, 400)) # Case 2: Review position fails, navigator object works with patch("_magnifier.utils.focusManager.api.getReviewPosition", return_value=None): @@ -60,7 +59,7 @@ def testGetNavigatorObjectPosition(self): coords = self.focusManager._getNavigatorObjectPosition() # Center: (100 + 200//2, 150 + 300//2) = (200, 300) - self.assertEqual(coords, locationHelper.Point(200, 300)) + self.assertEqual(coords, Coordinates(200, 300)) # Case 3: Everything fails - should return last valid position from Case 2 with patch("_magnifier.utils.focusManager.api.getReviewPosition", return_value=None): @@ -69,7 +68,7 @@ def testGetNavigatorObjectPosition(self): coords = self.focusManager._getNavigatorObjectPosition() # Should return last valid position (200, 300) - self.assertEqual(coords, locationHelper.Point(200, 300)) + self.assertEqual(coords, Coordinates(200, 300)) def testGetSystemFocusPosition(self): """Getting system focus position with different API responses.""" @@ -81,7 +80,7 @@ def testGetSystemFocusPosition(self): mock_caret.return_value.pointAtStart = mock_point coords = self.focusManager._getSystemFocusPosition() - self.assertEqual(coords, locationHelper.Point(500, 600)) + self.assertEqual(coords, Coordinates(500, 600)) # Case 2: Caret fails, focus object works with patch("_magnifier.utils.focusManager.api.getCaretPosition", side_effect=RuntimeError): @@ -90,104 +89,104 @@ def testGetSystemFocusPosition(self): coords = self.focusManager._getSystemFocusPosition() # Center: (200 + 100//2, 300 + 80//2) = (250, 340) - self.assertEqual(coords, locationHelper.Point(250, 340)) + self.assertEqual(coords, Coordinates(250, 340)) # Case 3: Everything fails - should return last valid position from Case 2 with patch("_magnifier.utils.focusManager.api.getCaretPosition", side_effect=RuntimeError): with patch("_magnifier.utils.focusManager.api.getFocusObject", return_value=None): coords = self.focusManager._getSystemFocusPosition() # Should return last valid position (250, 340) - self.assertEqual(coords, locationHelper.Point(250, 340)) + self.assertEqual(coords, Coordinates(250, 340)) def testGetMousePosition(self): """Getting mouse position.""" with patch("_magnifier.utils.focusManager.winUser.getCursorPos", return_value=(123, 456)): coords = self.focusManager._getMousePosition() - self.assertEqual(coords, locationHelper.Point(123, 456)) + self.assertEqual(coords, Coordinates(123, 456)) def testGetCurrentFocusCoordinates(self): """All priority scenarios for focus coordinates.""" subTestParams = [ FocusTestParam( - navigatorObjectPos=locationHelper.Point(0, 0), - systemFocusPos=locationHelper.Point(0, 0), - mousePos=locationHelper.Point(0, 0), + navigatorObjectPos=Coordinates(0, 0), + systemFocusPos=Coordinates(0, 0), + mousePos=Coordinates(0, 0), leftPressed=True, - expectedCoords=locationHelper.Point(0, 0), + expectedCoords=Coordinates(0, 0), expectedFocus=FocusType.MOUSE, description="Left click is pressed should return mouse position", ), FocusTestParam( - navigatorObjectPos=locationHelper.Point(0, 0), - systemFocusPos=locationHelper.Point(0, 0), - mousePos=locationHelper.Point(10, 10), + navigatorObjectPos=Coordinates(0, 0), + systemFocusPos=Coordinates(0, 0), + mousePos=Coordinates(10, 10), leftPressed=False, - expectedCoords=locationHelper.Point(10, 10), + expectedCoords=Coordinates(10, 10), expectedFocus=FocusType.MOUSE, description="Mouse moving (not dragging)", ), FocusTestParam( - navigatorObjectPos=locationHelper.Point(0, 0), - systemFocusPos=locationHelper.Point(15, 15), - mousePos=locationHelper.Point(0, 0), + navigatorObjectPos=Coordinates(0, 0), + systemFocusPos=Coordinates(15, 15), + mousePos=Coordinates(0, 0), leftPressed=False, - expectedCoords=locationHelper.Point(15, 15), + expectedCoords=Coordinates(15, 15), expectedFocus=FocusType.SYSTEM_FOCUS, description="System focus changed alone (navigator did not move)", ), FocusTestParam( - navigatorObjectPos=locationHelper.Point(20, 20), - systemFocusPos=locationHelper.Point(0, 0), - mousePos=locationHelper.Point(0, 0), + navigatorObjectPos=Coordinates(20, 20), + systemFocusPos=Coordinates(0, 0), + mousePos=Coordinates(0, 0), leftPressed=False, - expectedCoords=locationHelper.Point(20, 20), + expectedCoords=Coordinates(20, 20), expectedFocus=FocusType.NAVIGATOR, description="Navigator object changed (NumPad navigation)", ), FocusTestParam( - navigatorObjectPos=locationHelper.Point(30, 30), - systemFocusPos=locationHelper.Point(15, 15), - mousePos=locationHelper.Point(0, 0), + navigatorObjectPos=Coordinates(30, 30), + systemFocusPos=Coordinates(15, 15), + mousePos=Coordinates(0, 0), leftPressed=False, - expectedCoords=locationHelper.Point(30, 30), + expectedCoords=Coordinates(30, 30), expectedFocus=FocusType.NAVIGATOR, description="Both system focus and navigator changed (table cell navigation): navigator wins", ), FocusTestParam( - navigatorObjectPos=locationHelper.Point(0, 0), - systemFocusPos=locationHelper.Point(0, 0), - mousePos=locationHelper.Point(0, 0), + navigatorObjectPos=Coordinates(0, 0), + systemFocusPos=Coordinates(0, 0), + mousePos=Coordinates(0, 0), leftPressed=False, - expectedCoords=locationHelper.Point(0, 0), + expectedCoords=Coordinates(0, 0), expectedFocus=FocusType.MOUSE, description="Nothing changed, last was Mouse", lastFocusedObject=FocusType.MOUSE, ), FocusTestParam( - navigatorObjectPos=locationHelper.Point(0, 0), - systemFocusPos=locationHelper.Point(0, 0), - mousePos=locationHelper.Point(0, 0), + navigatorObjectPos=Coordinates(0, 0), + systemFocusPos=Coordinates(0, 0), + mousePos=Coordinates(0, 0), leftPressed=False, - expectedCoords=locationHelper.Point(0, 0), + expectedCoords=Coordinates(0, 0), expectedFocus=FocusType.NAVIGATOR, description="Nothing changed, last was NAVIGATOR", lastFocusedObject=FocusType.NAVIGATOR, ), FocusTestParam( - navigatorObjectPos=locationHelper.Point(10, 10), - systemFocusPos=locationHelper.Point(0, 0), - mousePos=locationHelper.Point(20, 20), + navigatorObjectPos=Coordinates(10, 10), + systemFocusPos=Coordinates(0, 0), + mousePos=Coordinates(20, 20), leftPressed=False, - expectedCoords=locationHelper.Point(20, 20), + expectedCoords=Coordinates(20, 20), expectedFocus=FocusType.MOUSE, description="Both mouse and navigator object moved (mouse has priority)", ), FocusTestParam( - navigatorObjectPos=locationHelper.Point(10, 10), - systemFocusPos=locationHelper.Point(15, 15), - mousePos=locationHelper.Point(20, 20), + navigatorObjectPos=Coordinates(10, 10), + systemFocusPos=Coordinates(15, 15), + mousePos=Coordinates(20, 20), leftPressed=True, - expectedCoords=locationHelper.Point(20, 20), + expectedCoords=Coordinates(20, 20), expectedFocus=FocusType.MOUSE, description="All three moved while dragging (mouse drag has highest priority)", ), @@ -196,9 +195,9 @@ def testGetCurrentFocusCoordinates(self): for param in subTestParams: with self.subTest(description=param.description): # Reset focus manager state - self.focusManager._lastNavigatorObjectPosition = locationHelper.Point(0, 0) - self.focusManager._lastSystemFocusPosition = locationHelper.Point(0, 0) - self.focusManager._lastMousePosition = locationHelper.Point(0, 0) + self.focusManager._lastNavigatorObjectPosition = Coordinates(0, 0) + self.focusManager._lastSystemFocusPosition = Coordinates(0, 0) + self.focusManager._lastMousePosition = Coordinates(0, 0) # Set lastFocusedObject if specified if param.lastFocusedObject is not None: diff --git a/tests/unit/test_magnifier/test_magnifier.py b/tests/unit/test_magnifier/test_magnifier.py index a6b847cb68e..cd85c61f240 100644 --- a/tests/unit/test_magnifier/test_magnifier.py +++ b/tests/unit/test_magnifier/test_magnifier.py @@ -4,12 +4,11 @@ # For full terms and any additional permissions, see the NVDA license file: https://github.com/nvaccess/nvda/blob/master/copying.txt from _magnifier.magnifier import Magnifier -from _magnifier.utils.types import Filter, Direction, MagnifierAction +from _magnifier.utils.types import Filter, Direction, MagnifierAction, Coordinates import unittest from winAPI._displayTracking import getPrimaryDisplayOrientation from unittest.mock import MagicMock, patch import wx -import locationHelper class _TestMagnifier(unittest.TestCase): @@ -99,7 +98,7 @@ def testZoomLevelProperty(self): def testStartMagnifier(self): """Activating the magnifier.""" self.magnifier._focusManager.getCurrentFocusCoordinates = MagicMock( - return_value=locationHelper.Point(100, 200), + return_value=Coordinates(100, 200), ) # Test starting from inactive state @@ -107,7 +106,7 @@ def testStartMagnifier(self): self.magnifier._startMagnifier() self.assertTrue(self.magnifier._isActive) - self.assertEqual(self.magnifier._currentCoordinates, locationHelper.Point(100, 200)) + self.assertEqual(self.magnifier._currentCoordinates, Coordinates(100, 200)) self.magnifier._focusManager.getCurrentFocusCoordinates.assert_called_once() # Test starting when already active (should not call getCurrentFocusCoordinates again) @@ -120,7 +119,7 @@ def testStartMagnifier(self): def testUpdateMagnifier(self): """Updating the magnifier's properties.""" self.magnifier._focusManager.getCurrentFocusCoordinates = MagicMock( - return_value=locationHelper.Point(100, 200), + return_value=Coordinates(100, 200), ) self.magnifier._doUpdate = MagicMock() self.magnifier._startTimer = MagicMock() @@ -144,7 +143,7 @@ def testUpdateMagnifier(self): self.magnifier._startTimer.assert_called_once_with( self.magnifier._updateMagnifier, ) - self.assertEqual(self.magnifier._currentCoordinates, locationHelper.Point(100, 200)) + self.assertEqual(self.magnifier._currentCoordinates, Coordinates(100, 200)) def testDoUpdate(self): """DoUpdate function raises NotImplementedError.""" @@ -196,7 +195,7 @@ def _setupPanTest(self): self.magnifier._panStep = 10 # 10% of screen width centerX = self.screenWidth // 2 centerY = self.screenHeight // 2 - self.magnifier._currentCoordinates = locationHelper.Point(centerX, centerY) + self.magnifier._currentCoordinates = Coordinates(centerX, centerY) expectedPanPixels = int( (self.screenWidth / self.magnifier.zoomLevel) * 10 / 100, ) @@ -232,12 +231,12 @@ def _testSimplePan( # Test reaching edge - movement succeeds on first contact (position changes to edge) if axis == "x": - self.magnifier._currentCoordinates = locationHelper.Point( + self.magnifier._currentCoordinates = Coordinates( edgeValue - direction * expectedPanPixels, centerY, ) else: - self.magnifier._currentCoordinates = locationHelper.Point( + self.magnifier._currentCoordinates = Coordinates( centerX, edgeValue - direction * expectedPanPixels, ) @@ -349,8 +348,8 @@ def testPanToBottomEdge(self): def testManagePanning(self): """Manual panning ends when focus coordinates change, and _lastFocusCoordinates is always kept up to date.""" - focusA = locationHelper.Point(100, 200) - focusB = locationHelper.Point(300, 400) + focusA = Coordinates(100, 200) + focusB = Coordinates(300, 400) self.magnifier._focusManager.getCurrentFocusCoordinates = MagicMock(return_value=focusA) @@ -374,7 +373,7 @@ def testManagePanning(self): def testKeepMouseCentered(self): """Base _keepMouseCentered moves cursor to _currentCoordinates.""" - self.magnifier._currentCoordinates = locationHelper.Point(640, 360) + self.magnifier._currentCoordinates = Coordinates(640, 360) with patch("_magnifier.magnifier.winUser.setCursorPos") as mockSetCursor: self.magnifier._keepMouseCentered() mockSetCursor.assert_called_once_with(640, 360)