mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-14 02:56:36 +00:00
fix(sports): rebuild un-shared faces through the pinned layout engine
unshare_element_fonts re-instantiates a duplicate font face so two elements can be told apart by id(). It did so through bare ImageFont.truetype, which takes PIL's default layout engine rather than the one src/common/font_layout.py pins. Raqm and Basic disagree on fractional advances -- that disagreement is the reason the pin exists, having broken golden images across machines -- so a rebuilt face could measure differently from the shared face it replaced, on any host where Raqm is installed. These were the only two call sites in src/ bypassing the pin. The guard asserts that the rebuild goes through the pinned loader rather than comparing engine values: where Raqm is absent, bare truetype returns BASIC anyway, so an engine comparison passes whether or not the pin is honoured. The first draft of this test did exactly that and passed with the bug reintroduced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -448,7 +448,7 @@ def unshare_element_fonts(logger, fonts):
|
||||
path) are left shared, and their draws stay white as before.
|
||||
"""
|
||||
try:
|
||||
from PIL import ImageFont as _IF
|
||||
from src.common.font_layout import load_truetype as _load
|
||||
except ImportError: # pragma: no cover
|
||||
return fonts
|
||||
seen = {}
|
||||
@@ -463,7 +463,7 @@ def unshare_element_fonts(logger, fonts):
|
||||
if not path or not size:
|
||||
continue
|
||||
try:
|
||||
fonts[key] = _IF.truetype(path, size)
|
||||
fonts[key] = _load(path, size)
|
||||
except (OSError, ValueError, TypeError):
|
||||
logger.debug(
|
||||
"Could not un-share the %s face; it keeps the default colour", key)
|
||||
|
||||
@@ -787,7 +787,7 @@ class SportsCoreSharedMixin:
|
||||
path) are left shared, and their draws stay white as before.
|
||||
"""
|
||||
try:
|
||||
from PIL import ImageFont as _IF
|
||||
from src.common.font_layout import load_truetype as _load
|
||||
except ImportError: # pragma: no cover
|
||||
return fonts
|
||||
seen = {}
|
||||
@@ -802,7 +802,7 @@ class SportsCoreSharedMixin:
|
||||
if not path or not size:
|
||||
continue
|
||||
try:
|
||||
fonts[key] = _IF.truetype(path, size)
|
||||
fonts[key] = _load(path, size)
|
||||
except (OSError, ValueError, TypeError):
|
||||
self.logger.debug(
|
||||
"Could not un-share the %s face; it keeps the default colour", key)
|
||||
|
||||
@@ -193,3 +193,72 @@ class TestTables:
|
||||
|
||||
def test_month_and_weekday_tables_are_complete(self):
|
||||
assert len(C.MONTH_ABBR) == 12 and len(C.WEEKDAY_ABBR) == 7
|
||||
|
||||
|
||||
class TestUnshareElementFonts:
|
||||
"""Re-instantiated faces must match the ones they replace.
|
||||
|
||||
``unshare_element_fonts`` rebuilds a duplicate face purely so two elements
|
||||
can be told apart by ``id()``. That is only safe while the rebuilt face
|
||||
lays text out identically -- and PIL's two layout engines disagree on
|
||||
fractional advances, which is why ``src.common.font_layout`` exists and
|
||||
pins one. Rebuilding through bare ``ImageFont.truetype`` took the default
|
||||
engine instead, so a re-instantiated face could measure differently from
|
||||
the shared face it replaced.
|
||||
"""
|
||||
|
||||
def _face(self, size=10):
|
||||
from src.common.font_layout import load_truetype
|
||||
path = os.path.join("assets", "fonts", "PressStart2P-Regular.ttf")
|
||||
return load_truetype(path, size)
|
||||
|
||||
def test_rebuilt_face_goes_through_the_pinned_loader(self, log, monkeypatch):
|
||||
"""Asserted on the loader, not on the resulting engine value.
|
||||
|
||||
PIL only selects Raqm when it is installed; where it is not, a bare
|
||||
``ImageFont.truetype`` returns BASIC too, so comparing engine values
|
||||
passes on those machines whether or not the pin is honoured -- this
|
||||
test did exactly that before it was rewritten. Spying on the pinned
|
||||
loader fails on every machine when the pin is bypassed, which is the
|
||||
point: the cross-machine mismatch font_layout exists to prevent
|
||||
cannot be reproduced on a Raqm-less runner.
|
||||
"""
|
||||
import src.common.font_layout as fl
|
||||
|
||||
# Build the face BEFORE patching: _face() loads through the same
|
||||
# pinned loader, so patching first would let the fixture's own call
|
||||
# satisfy the assertion and the test would pass either way.
|
||||
shared = self._face()
|
||||
|
||||
calls = []
|
||||
real = fl.load_truetype
|
||||
|
||||
def spy(font, size, **kwargs):
|
||||
calls.append((font, size))
|
||||
return real(font, size, **kwargs)
|
||||
|
||||
monkeypatch.setattr(fl, "load_truetype", spy)
|
||||
fonts = {"score": shared, "time": shared}
|
||||
C.unshare_element_fonts(log, fonts)
|
||||
assert fonts["time"] is not shared, "the duplicate should have been rebuilt"
|
||||
assert calls, "the rebuild must go through the pinned loader"
|
||||
|
||||
def test_rebuilt_face_keeps_the_shared_faces_layout_engine(self, log):
|
||||
shared = self._face()
|
||||
fonts = {"score": shared, "time": shared}
|
||||
C.unshare_element_fonts(log, fonts)
|
||||
assert fonts["time"].layout_engine == shared.layout_engine
|
||||
|
||||
def test_rebuilt_face_measures_identically(self, log):
|
||||
shared = self._face()
|
||||
fonts = {"score": shared, "time": shared}
|
||||
C.unshare_element_fonts(log, fonts)
|
||||
text = "88-88"
|
||||
assert (fonts["time"].getlength(text)
|
||||
== shared.getlength(text)), "metrics must not shift"
|
||||
|
||||
def test_distinct_faces_are_left_alone(self, log):
|
||||
a, b = self._face(10), self._face(8)
|
||||
fonts = {"score": a, "time": b}
|
||||
C.unshare_element_fonts(log, fonts)
|
||||
assert fonts["score"] is a and fonts["time"] is b
|
||||
|
||||
@@ -374,3 +374,51 @@ class TestPluginDirIsToldNotDeduced:
|
||||
assert host._schema_font_size("score") == 16, (
|
||||
"without the schema this is None, which is what made a configured "
|
||||
"size look user-chosen and skipped the grid snap")
|
||||
class TestUnshareKeepsTheLayoutEngine:
|
||||
"""``_unshare_element_fonts`` rebuilds a face; the rebuild must be pinned.
|
||||
|
||||
This body did move verbatim from the plugins, but it is the one that
|
||||
rebuilds a font, and it rebuilt through bare ``ImageFont.truetype`` --
|
||||
taking PIL's default layout engine rather than the one
|
||||
``src.common.font_layout`` pins. Raqm and Basic disagree on fractional
|
||||
advances, so a re-instantiated face could measure differently from the
|
||||
shared face it replaced, on any machine where Raqm is installed. That is
|
||||
invisible on a Raqm-less runner, which is why this asserts on the loader
|
||||
rather than on the resulting engine value.
|
||||
"""
|
||||
|
||||
def _host(self):
|
||||
import logging
|
||||
return type("H", (SportsCoreSharedMixin,), {
|
||||
"logger": logging.getLogger("test_sports_shared")})()
|
||||
|
||||
def _face(self, size=10):
|
||||
from src.common.font_layout import load_truetype
|
||||
return load_truetype(
|
||||
os.path.join("assets", "fonts", "PressStart2P-Regular.ttf"), size)
|
||||
|
||||
def test_rebuild_goes_through_the_pinned_loader(self, monkeypatch):
|
||||
import src.common.font_layout as fl
|
||||
|
||||
# Built before patching: _face() uses the same loader, so patching
|
||||
# first would let the fixture's own call satisfy the assertion.
|
||||
shared = self._face()
|
||||
|
||||
calls = []
|
||||
real = fl.load_truetype
|
||||
|
||||
def spy(font, size, **kwargs):
|
||||
calls.append((font, size))
|
||||
return real(font, size, **kwargs)
|
||||
|
||||
monkeypatch.setattr(fl, "load_truetype", spy)
|
||||
fonts = {"score": shared, "time": shared}
|
||||
self._host()._unshare_element_fonts(fonts)
|
||||
assert fonts["time"] is not shared, "the duplicate should have been rebuilt"
|
||||
assert calls, "the rebuild must go through the pinned loader"
|
||||
|
||||
def test_rebuilt_face_measures_like_the_one_it_replaced(self):
|
||||
shared = self._face()
|
||||
fonts = {"score": shared, "time": shared}
|
||||
self._host()._unshare_element_fonts(fonts)
|
||||
assert fonts["time"].getlength("88-88") == shared.getlength("88-88")
|
||||
|
||||
Reference in New Issue
Block a user