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 <noreply@anthropic.com>
This commit is contained in:
Chuck
2026-09-17 08:59:47 -04:00
co-authored by Claude Opus 5
parent 7ae614aa35
commit c693278d33
2 changed files with 193 additions and 8 deletions
+142 -8
View File
@@ -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:<deg>`` 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,
+51
View File
@@ -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'}) == ''