From c693278d338eea421e502471800271bb55366730 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Thu, 17 Sep 2026 08:59:47 -0400 Subject: [PATCH] fix(display): size the preview after orientation and pixel mappers display_geometry.physical_size claimed to give DisplayManager's answer but only computed cols*chain x rows*parallel. RGBMatrix.width/height are measured after the library's pixel mappers, so a Rotate:90 / orientation 90 chain previewed 128x32 for a 32x128 panel and a U-mapper chain of four 256x32 for 128x64. Model the built-in mappers' size effect as the pinned lib/pixel-mapper.cc does (Rotate, U-mapper, V-mapper, StackToRow, Remap; Mirror and unknown names leave it alone), and move the orientation composition here so DisplayManager and the preview share it. The module docstring no longer claims the sync handshake uses it; that imports only DEFAULT_CHAIN_LENGTH. Audit finding F18. Co-Authored-By: Claude Opus 5 --- src/display_geometry.py | 150 ++++++++++++++++++++++++++++++++-- test/test_display_geometry.py | 51 ++++++++++++ 2 files changed, 193 insertions(+), 8 deletions(-) diff --git a/src/display_geometry.py b/src/display_geometry.py index b74cb361..b5655c12 100644 --- a/src/display_geometry.py +++ b/src/display_geometry.py @@ -1,17 +1,25 @@ -"""Display size from config: the one computation every caller shares. +"""Display size from config: the one computation the size readers share. ``DisplayManager`` sizes its canvas from ``display.hardware`` plus -``display.double_sided``. The web preview, the Starlark magnify default and -the multi-display sync handshake used to re-derive that size themselves, -each with its own defaults (``chain_length`` fell back to 2 in one place and -1 in three others) and none of them applying double-sided mode. They now all -call this module. +``display.double_sided``. The web preview endpoints (``/display/current``, the +SSE fallback), the Starlark magnify default and ``scripts/dev/vegas_audit.py`` +used to re-derive that size themselves, each with its own defaults +(``chain_length`` fell back to 2 in one place and 1 in others) and none of +them applying double-sided mode. They now call this module. (The multi-display +sync handshake doesn't compute a size; it shares only +``DEFAULT_CHAIN_LENGTH``.) + +The size includes what the rgbmatrix library's pixel mappers do to it -- +``orientation`` and ``pixel_mapper_config`` (``Rotate:90`` swaps the axes, +``U-mapper`` folds the chain) -- because ``RGBMatrix.width``/``height``, which +``DisplayManager`` reports on hardware, are measured after them. Kept free of hardware imports on purpose: the web interface imports it, and ``display_manager`` pulls in ``rgbmatrix``. """ import logging +import re from typing import Any, Dict, Mapping, Optional, Tuple logger = logging.getLogger(__name__) @@ -22,6 +30,10 @@ DEFAULT_COLS = 64 DEFAULT_CHAIN_LENGTH = 2 DEFAULT_PARALLEL = 1 +#: ``display.hardware.orientation`` -> degrees of the ``Rotate`` mapper +#: DisplayManager appends to ``pixel_mapper_config`` (None: no mapper). +ORIENTATION_ROTATE_DEGREES = {'normal': None, '90': 90, '180': 180, '270': 270} + def _display(config: Optional[Mapping[str, Any]]) -> Mapping[str, Any]: # A hand-edited config.json can hold anything here; treat a non-mapping @@ -35,10 +47,128 @@ def _hardware(config: Optional[Mapping[str, Any]]) -> Mapping[str, Any]: return hw if isinstance(hw, Mapping) else {} +def compose_pixel_mapper_config(hardware: Mapping[str, Any]) -> str: + """The ``pixel_mapper_config`` DisplayManager hands the library. + + ``pixel_mapper_config`` stays a free-form advanced field (e.g. "U-mapper" + for chain layouts); ``orientation`` is the user-facing mounting rotation, + appended as a trailing ``Rotate:`` mapper rather than overwriting it. + """ + base = hardware.get('pixel_mapper_config') or '' + base = base.strip() if isinstance(base, str) else '' + degrees = ORIENTATION_ROTATE_DEGREES.get(hardware.get('orientation', 'normal')) + if degrees is None: + return base + rotate = f'Rotate:{degrees}' + return f'{base};{rotate}' if base else rotate + + +def _c_div(a: int, b: int) -> int: + """C integer division (truncates toward zero).""" + q = abs(a) // abs(b) + return q if (a >= 0) == (b >= 0) else -q + + +def _c_strtol(text: str) -> Tuple[int, str]: + """strtol(text, &end, 10): the parsed value (0 if none) and the rest.""" + match = re.match(r'\s*([+-]?\d+)', text) + if not match: + return 0, text + return int(match.group(1)), text[match.end():] + + +def _remap_size(param: Optional[str], width: int, height: int, + chain: int, parallel: int) -> Optional[Tuple[int, int]]: + """RemapMapper::SetParameters and GetSizeMapping (lib/pixel-mapper.cc).""" + if not param: + return None + new_w, rest = _c_strtol(param) + if not rest.startswith(','): + return None + new_h, rest = _c_strtol(rest[1:]) + if not rest.startswith('|'): + return None + rest = rest[1:] + tiles = [] + while rest: + x, rest = _c_strtol(rest) + if not rest.startswith(','): + return None + y, rest = _c_strtol(rest[1:]) + if not rest or rest[0].lower() not in 'neswx': + return None + tiles.append((x, y, rest[0].lower())) + rest = rest[1:] + if rest.startswith('|'): + rest = rest[1:] + elif rest: + return None + if len(tiles) != chain * parallel: + return None + panel_w, panel_h = _c_div(width, chain), _c_div(height, parallel) + for x, y, kind in tiles: + if kind == 'x': + continue + # MapTile::MapToVisible of the panel's (0, 0) and far corner. + x0, y0, x1, y1 = { + 'n': (x, y, x + panel_w - 1, y + panel_h - 1), + 'w': (x, y + panel_w - 1, x + panel_h - 1, y), + 's': (x + panel_w - 1, y + panel_h - 1, x, y), + 'e': (x + panel_h - 1, y, x, y + panel_w - 1), + }[kind] + if x1 < 0 or x0 >= new_w or y1 < 0 or y0 >= new_h: + return None + return new_w, new_h + + +def apply_pixel_mappers(width: int, height: int, mapper_config: str, + chain: int, parallel: int) -> Tuple[int, int]: + """The canvas size after the library applies ``mapper_config``. + + Mirrors ``RGBMatrix::Impl::ApplyNamedPixelMappers`` and each built-in + mapper's ``SetParameters``/``GetSizeMapping`` in the pinned + ``lib/pixel-mapper.cc``: mappers apply left to right, and one the library + doesn't know or can't configure is skipped and leaves the size alone. + ``multiplexing`` isn't modelled: its mappers give back the configured + size for the panel sizes they are made for. + """ + for entry in (mapper_config or '').split(';'): + name, colon, param = entry.partition(':') + name = name.lower() + param = param if colon else None + if name == 'rotate': + if not param: + continue + angle, rest = _c_strtol(param) + if rest or angle % 90: + continue + if angle % 180: + width, height = height, width + elif name == 'u-mapper': + if chain < 2 or chain % 2 or height % parallel: + continue + width, height = _c_div(width, 64) * 32, 2 * height + elif name == 'v-mapper': + width, height = (_c_div(width * parallel, chain), + _c_div(height * chain, parallel)) + elif name == 'stacktorow': + if param and any(c not in 'ZzFf, ' for c in param): + continue + width, height = width * parallel, _c_div(height, parallel) + elif name == 'remap': + size = _remap_size(param, width, height, chain, parallel) + if size is not None: + width, height = size + # "mirror" keeps the size; the library skips names it doesn't know. + return width, height + + def physical_size(config: Optional[Mapping[str, Any]]) -> Tuple[int, int]: """Width and height of the whole panel chain, in pixels. - ``cols * chain_length`` by ``rows * parallel``. Raises ``ValueError`` or + ``cols * chain_length`` by ``rows * parallel``, then through the pixel + mappers ``orientation`` and ``pixel_mapper_config`` set up -- what + ``RGBMatrix.width``/``height`` report. Raises ``ValueError`` or ``TypeError`` on a non-numeric value, as ``DisplayManager`` does; callers decide their own fallback. @@ -54,7 +184,11 @@ def physical_size(config: Optional[Mapping[str, Any]]) -> Tuple[int, int]: parallel = int(hw.get('parallel', DEFAULT_PARALLEL)) except OverflowError as e: raise ValueError(f"display.hardware size is not finite: {e}") from e - return max(1, cols * chain_length), max(1, rows * parallel) + width, height = max(1, cols * chain_length), max(1, rows * parallel) + if chain_length >= 1 and parallel >= 1: + width, height = apply_pixel_mappers( + width, height, compose_pixel_mapper_config(hw), chain_length, parallel) + return max(1, width), max(1, height) def resolve_double_sided(physical_width: int, physical_height: int, diff --git a/test/test_display_geometry.py b/test/test_display_geometry.py index 781e2eda..30a5e35d 100644 --- a/test/test_display_geometry.py +++ b/test/test_display_geometry.py @@ -142,3 +142,54 @@ def test_preview_callers_do_not_rederive_the_size(): assert "get('chain_length', 1)" not in source, rel assert 'get("chain_length", 1)' not in source, rel assert 'cols * chain_length' not in source, rel + + +# --- Pixel mappers ---------------------------------------------------------- +# RGBMatrix.width/height are measured after the library's pixel mappers +# (lib/pixel-mapper.cc), so the preview has to apply them too. This used to +# report 128x32 for a Rotate:90 chain the panel drew at 32x128. + +@pytest.mark.parametrize('hardware,expected', [ + ({'pixel_mapper_config': 'Rotate:90'}, (32, 128)), + ({'pixel_mapper_config': 'Rotate:270'}, (32, 128)), + ({'pixel_mapper_config': 'Rotate:-90'}, (32, 128)), + ({'pixel_mapper_config': 'Rotate:180'}, (128, 32)), + ({'orientation': '90'}, (32, 128)), + ({'orientation': '270'}, (32, 128)), + ({'orientation': '180'}, (128, 32)), + # Two quarter turns cancel out. + ({'pixel_mapper_config': 'Rotate:90', 'orientation': '270'}, (128, 32)), + # U-mapper folds a chain of 4 into two rows: (256 / 64) * 32 by 2 * 32. + ({'chain_length': 4, 'pixel_mapper_config': 'U-mapper'}, (128, 64)), + ({'chain_length': 4, 'pixel_mapper_config': 'u-mapper;Rotate:90'}, (64, 128)), + # U-mapper needs an even chain of at least 2, or the library skips it. + ({'chain_length': 3, 'pixel_mapper_config': 'U-mapper'}, (192, 32)), + ({'cols': 32, 'chain_length': 4, 'pixel_mapper_config': 'V-mapper'}, (32, 128)), + ({'chain_length': 1, 'parallel': 2, 'pixel_mapper_config': 'StackToRow:Z'}, (128, 32)), + ({'pixel_mapper_config': 'Remap:64,64|0,0n|0,32n'}, (64, 64)), + # A Remap panel entirely outside the visible area: the library skips it. + ({'pixel_mapper_config': 'Remap:64,64|0,0n|0,99n'}, (128, 32)), + ({'pixel_mapper_config': 'Mirror:H'}, (128, 32)), + # Unknown or unusable mappers are skipped by the library. + ({'pixel_mapper_config': 'Bogus;Rotate:45;Rotate:ninety'}, (128, 32)), +]) +def test_pixel_mappers_change_the_size_as_the_library_does(hardware, expected): + hw = {'rows': 32, 'cols': 64, 'chain_length': 2, 'parallel': 1} + hw.update(hardware) + assert physical_size(_config(hw)) == expected + assert logical_size(_config(hw)) == expected + + +def test_double_sided_splits_the_mapped_size(): + cfg = _config({'rows': 32, 'cols': 64, 'chain_length': 2, 'parallel': 1, 'orientation': '90'}, + {'enabled': True, 'copies': 2, 'axis': 'vertical'}) + assert logical_size(cfg) == (32, 64) + + +def test_display_manager_composes_the_mapper_config_it_sizes_from(): + from src.display_geometry import compose_pixel_mapper_config + assert compose_pixel_mapper_config({}) == '' + assert compose_pixel_mapper_config({'orientation': '90'}) == 'Rotate:90' + assert compose_pixel_mapper_config( + {'pixel_mapper_config': ' U-mapper ', 'orientation': '180'}) == 'U-mapper;Rotate:180' + assert compose_pixel_mapper_config({'orientation': 'sideways'}) == ''