changes magnifier types and function name for clarity (#19882)

pre - #19473
parts of #19810
Summary of the issue:

#19810 was still to big so I focused on types changes and some functions renaming to go accordingly.
All these changes are nescessary for futur magnifier implementations.
Description of user facing changes:

"default"s have been removed so it can be seen in the doc
Description of developer facing changes:

replaced MagnifierPosition with MagnifierParameters for better handling of size, place and filter of the magnifier.
added a MagnifierType to handle magnifiers different types.
Description of development approach:

This work was split out from PR #19473 to make review and integration easier.
We first added the new structure: MagnifierParameters for size/position/filter, and MagnifierType for futur mode switching.

Author: Boumtchack

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,
 	getFollowState,
 	setFollowState,
@@ -104,16 +104,16 @@ def toggleMagnifier() -> None:
 	else:
 		initialize()
 
-		filter = getDefaultFilter()
-		fullscreenMode = getDefaultFullscreenMode()
+		filter = getFilter()
+		fullscreenMode = getFullscreenMode()
 
 		ui.message(
 			pgettext(
 				"magnifier",
 				# 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,
 			),

source/_magnifier/config.py

@@ -50,72 +50,75 @@ 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]
+	closestIndex = min(
+		range(len(zoomValues)),
+		key=lambda i: abs(zoomValues[i] - zoomLevel),
+	)
+	return zoomStrings[closestIndex]
 
 
-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.
 	"""
-	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.
 	"""
-	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
 
 
 _FOLLOW_CONFIG_KEYS: dict[MagnifierFollowFocusType, str] = {
@@ -223,3 +226,21 @@ def shouldKeepMouseCentered() -> bool:
 	:return: True if mouse should be kept centered, False otherwise.
 	"""
 	return config.conf["magnifier"]["keepMouseCentered"]
+
+
+def getFullscreenMode() -> FullScreenMode:
+	"""
+	Get full-screen mode from config.
+
+	:return: The full-screen mode.
+	"""
+	return FullScreenMode(config.conf["magnifier"]["fullscreenMode"])
+
+
+def setFullscreenMode(mode: FullScreenMode) -> None:
+	"""
+	Set full-screen mode from settings.
+
+	:param mode: The full-screen mode to set.
+	"""
+	config.conf["magnifier"]["fullscreenMode"] = mode.value

source/_magnifier/fullscreenMagnifier.py

@@ -15,17 +15,26 @@
 from .magnifier import Magnifier
 from .utils.filterHandler import FilterMatrix
 from .utils.spotlightManager import SpotlightManager
-from .utils.types import Filter, Coordinates, FullScreenMode
-from .config import getDefaultFullscreenMode
+from .utils.types import (
+	Filter,
+	MagnifierType,
+	FullScreenMode,
+	Size,
+	MagnifierParameters,
+	Coordinates,
+)
+from .config import getFullscreenMode, isTrueCentered
 from .utils.errorHandling import trackNativeMagnifierErrors
 
 
 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._displaySize = Size(self._displayOrientation.width, self._displayOrientation.height)
 		self._startMagnifier()
 
 	@property
@@ -193,11 +202,11 @@ def _fullscreenMagnifier(self, coordinates: Coordinates) -> None:
 
 		:coordinates: The (x, y) coordinates to center the magnifier on
 		"""
-		left, top, visibleWidth, visibleHeight = self._getMagnifierPosition(coordinates)
+		params = self._getMagnifierParameters(coordinates)
 		magnification.MagSetFullscreenTransform(
 			self.zoomLevel,
-			left,
-			top,
+			params.coordinates.x,
+			params.coordinates.y,
 		)
 
 	def _getCoordinatesForMode(
@@ -231,11 +240,11 @@ def _keepMouseCentered(self) -> None:
 		):
 			log.debug("Mouse button pressed, skipping cursor repositioning to avoid interfering with click")
 			return
-		coords = self._getCoordinatesForMode(self._currentCoordinates)
-		left, top, visibleWidth, visibleHeight = self._getMagnifierPosition(coords)
-		centerX = left + visibleWidth // 2
-		centerY = top + visibleHeight // 2
-		self._setCursorToCenter(centerX, centerY)
+		coordinates = self._getCoordinatesForMode(self._currentCoordinates)
+		params = self._getMagnifierParameters(coordinates)
+		centerX = params.coordinates.x + params.magnifierSize.width // 2
+		centerY = params.coordinates.y + params.magnifierSize.height // 2
+		winUser.setCursorPos(centerX, centerY)
 
 	@trackNativeMagnifierErrors
 	def _setCursorToCenter(self, x: int, y: int) -> None:
@@ -259,14 +268,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
@@ -283,8 +294,8 @@ def _borderPos(
 
 		if dx != 0 or dy != 0:
 			return Coordinates(
-				self._lastScreenPosition[0] + dx,
-				self._lastScreenPosition[1] + dy,
+				self._lastScreenPosition.x + dx,
+				self._lastScreenPosition.y + dy,
 			)
 		else:
 			return self._lastScreenPosition
@@ -304,21 +315,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
 
@@ -338,3 +349,31 @@ 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
+
+		: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,
+		)

source/_magnifier/magnifier.py

@@ -19,17 +19,17 @@
 from winAPI import _displayTracking
 from winAPI._displayTracking import OrientationState, getPrimaryDisplayOrientation
 from .utils.types import (
-	MagnifierPosition,
+	MagnifierParameters,
 	MagnifierAction,
-	Coordinates,
 	MagnifierType,
 	Direction,
 	Filter,
+	Coordinates,
 )
 from .config import (
-	getDefaultZoomLevel,
-	getDefaultPanStep,
-	getDefaultFilter,
+	getZoomLevel,
+	getPanStep,
+	getFilter,
 	ZoomLevel,
 	isTrueCentered,
 	shouldKeepMouseCentered,
@@ -43,16 +43,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
 		self._consecutiveErrors: int = 0
 		# Register for display changes
@@ -343,29 +343,12 @@ 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
 
-		: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")