TST add visual regression tests

.gitattributes

@@ -0,0 +1 @@
+cortex/tests/reference_images/**/*.webp filter=lfs diff=lfs merge=lfs -text

.github/workflows/install_from_wheel.yml

@@ -21,6 +21,8 @@ jobs:
       max-parallel: 5
 
     steps:
+    # Don't clone with LFS because the wheel will not include the reference
+    # images, so we can save some bandwidth.
     - uses: actions/checkout@v7
     - name: Set up Python
       uses: actions/setup-python@v7

.github/workflows/publish.yml

@@ -15,6 +15,10 @@ jobs:
     - uses: actions/checkout@v7
       with:
         fetch-depth: 0  # Required for setuptools-scm to get version from tags
+        # MANIFEST.in sweeps the LFS-tracked reference images into the sdist;
+        # without this it would ship pointer stubs instead, which twine check
+        # does not catch.
+        lfs: true
 
     - name: Set up Python
       uses: actions/setup-python@v7

.github/workflows/run_tests.yml

@@ -21,7 +21,11 @@ jobs:
       max-parallel: 5
 
     steps:
+    # The visual-regression reference images are LFS-tracked; without this the
+    # checkout leaves pointer files and those tests skip.
     - uses: actions/checkout@v7
+      with:
+        lfs: true
     - name: Set up Python
       uses: actions/setup-python@v7
       with:

cortex/export/headless.py

@@ -90,7 +90,7 @@ def _wait_for_viewer_loaded(handle, timeout: float = 60.0) -> None:
     raise RuntimeError(
         f"Viewer's .loaded deferred did not resolve within {timeout:.0f}s "
         f"(last response: {last_err!r}). The CTM mesh may have failed to "
-        "download or parse, or mriview.js failed to initialise."
+        "download or parse, or mriview.js failed to initialize."
     )
 
 
@@ -99,6 +99,35 @@ def _wait_for_viewer_loaded(handle, timeout: float = 60.0) -> None:
 # --------------------------------------------------------------------------- #
 
 
+#: How often the worker thread calls into Playwright to dispatch queued browser
+#: events; ``browser_errors`` is current to within this interval.
+EVENT_POLL_INTERVAL = 0.25
+
+#: Browser messages that mean WebGL itself failed, as opposed to unrelated
+#: javascript a page may log. Deliberately narrow: a healthy viewer already logs
+#: a console.error for the Leap Motion websocket it cannot reach
+#: (ws://127.0.0.1:6437), so asserting on any error at all fails every run.
+#:
+#: A link failure arrives as a console.error rather than an exception, so
+#: nothing raises and the render comes back blank -- how Vertex2D broke
+#: (gh-714). three.js emits it alongside "gl.VALIDATE_STATUS false" and
+#: "gl.getError() 0"; do not add those. Nothing here calls gl.validateProgram(),
+#: so VALIDATE_STATUS is false for want of a run, and getError() 0 is the
+#: absence of an error. Driver shader-info warnings are excluded likewise.
+WEBGL_FAILURE_PATTERNS = (
+    "THREE.WebGLProgram: Could not initialise shader",
+    "Error creating WebGL context",
+)
+
+
+def filter_webgl_failures(browser_errors: list[str]) -> list[str]:
+    """Return only those browser messages that indicate WebGL itself failed."""
+    return [
+        e for e in browser_errors
+        if any(pattern in e for pattern in WEBGL_FAILURE_PATTERNS)
+    ]
+
+
 class _PlaywrightThread:
     """Manages the Playwright lifecycle on a private daemon thread.
 
@@ -230,7 +259,27 @@ def _worker(self) -> None:
             return
 
         # Keep the thread (and therefore Playwright) alive until shutdown.
-        self._shutdown_event.wait()
+        #
+        # Playwright's sync API dispatches queued events only while something is
+        # calling into it, so parking here for the viewer's lifetime would leave
+        # console messages undelivered until _cleanup(). Poll instead: the cheap
+        # round-trip is what makes Playwright dispatch them.
+        while not self._shutdown_event.wait(EVENT_POLL_INTERVAL):
+            try:
+                self._page.evaluate("0")
+            except Exception:
+                # Stop polling, but do not tear the viewer down underneath the
+                # caller: park until shutdown, as this thread did before
+                # polling existed. Falling through to _cleanup() here would
+                # close the browser mid-session, surfacing much later as the
+                # next getImage timing out.
+                logger.warning(
+                    "event polling stopped; browser_errors will no longer "
+                    "update for this viewer",
+                    exc_info=True,
+                )
+                self._shutdown_event.wait()
+                break
         self._cleanup()
 
     # -- Playwright event handlers (called on the worker thread) ---------- #
@@ -407,7 +456,7 @@ def _await_client() -> None:
         # any point during the session (each call returns a fresh snapshot).
         handle._pw_thread = pw_thread
 
-        # Block until the WebGL viewer has finished initialising (CTM mesh
+        # Block until the WebGL viewer has finished initializing (CTM mesh
         # download + parse + first setData). Replaces ad-hoc time.sleep(10)
         # calls in tests and callers, and shortens the wait when the
         # browser is faster than the worst-case timeout.

cortex/export/save_views.py

@@ -196,6 +196,20 @@ def save_3d_views(
                 )
             time.sleep(1)
 
+            if headless:
+                # Only check for WebGL failures in headless mode, since we don't
+                # capture console output in the interactive mode.
+                pw_thread = handle._pw_thread  # `handle` is a `JSMixer`
+                from cortex.export.headless import filter_webgl_failures
+
+                failures = filter_webgl_failures(pw_thread.browser_errors)
+                if failures:
+                    raise RuntimeError(
+                        f"WebGL failed while rendering {view_name!r}/{surface!r}; "
+                        f"{file_name!r} is likely blank.\n  "
+                        + "\n  ".join(sorted(set(failures)))
+                    )
+
             # Trim transparent edges
             if trim:
                 try:

cortex/tests/conftest.py

@@ -0,0 +1,49 @@
+"""Pin ``cortex.db`` to the filestore bundled with pycortex.
+
+The filestore is a configured path (``basic.filestore`` in ``options.cfg``),
+so on a machine with a real filestore the suite would otherwise run against
+whatever subjects that machine happens to have. Every test here uses the demo
+subject ``S1``, and the reference renders in ``reference_images/`` are pixel
+comparisons against the bundled one; a lab filestore with its own ``S1`` would
+fail them for reasons that have nothing to do with the code under test, and
+would collect flatmap caches along the way.
+"""
+import os
+import sys
+
+import cortex
+from cortex import database, options
+
+
+def _bundled_filestore():
+    """The demo filestore shipped alongside the installed ``cortex`` package."""
+    pkgdir = os.path.dirname(os.path.abspath(cortex.__file__))
+    candidates = [
+        # Source checkout or editable install: filestore/ sits beside cortex/.
+        os.path.join(pkgdir, os.pardir, "filestore", "db"),
+        # Installed: setup.py copies filestore/ to <install_base>/share/pycortex.
+        os.path.join(sys.prefix, "share", "pycortex", "db"),
+    ]
+    for path in candidates:
+        path = os.path.realpath(path)
+        if os.path.isdir(path):
+            return path
+    raise RuntimeError(
+        "could not locate the filestore bundled with pycortex; looked in "
+        + ", ".join(candidates)
+    )
+
+
+FILESTORE = _bundled_filestore()
+
+options.config.set("basic", "filestore", FILESTORE)
+database.default_filestore = FILESTORE
+# The `filestore=default_filestore` defaults throughout database.py were bound
+# at import, so the singleton has to be repointed by hand. Everything reached
+# through it (SubjectDB and below) is passed `self.filestore` explicitly.
+cortex.db.filestore = FILESTORE
+cortex.db.reload_subjects()
+
+
+def pytest_report_header(config):
+    return f"pycortex filestore: {FILESTORE}"

cortex/tests/reference_images/README.md

@@ -0,0 +1,107 @@
+# Reference images
+
+Stored renders that `cortex/tests/test_visual_regression.py` asserts against.
+
+## Contents
+
+| directory | images | contents |
+| --- | --- | --- |
+| `alpha_dataviews/` | 10 | five of the six public dataview classes (`Volume`, `Vertex`, `Volume2D`, `VolumeRGB`, `VertexRGB`), both renderers |
+| `nan_dataviews/` | 10 | the same five, with NaNs over roughly half the primary data channel |
+| `nan_alpha_dataviews/` | 4 | `VolumeRGB`/`VertexRGB` only, with the NaNs in the `alpha=` map |
+| `nonflat_views/` | 4 | `Volume`/`Vertex` on the inflated and fiducial surfaces at `lateral_pivot`, webgl only |
+
+Filenames are `quickflat_<Class>` and `webgl_<Class>`, except `nonflat_views/`,
+which uses `webgl_<surface>_<angle>_<Class>`.
+
+`Vertex2D` is the sixth class and has no images: its webgl flatmap renders
+blank (gh-714) and `save_3d_views` raises, so it cannot be tested through the
+webgl path at all. The two `Vertex2D` tests are **xfailed** on that
+`RuntimeError`, strictly — if the render ever succeeds the XPASS says so rather
+than passing silently.
+
+## Render settings
+
+The three flatmap directories render `quickflat_*` with
+`cortex.quickflat.make_png` and `webgl_*` with `save_3d_views`, both with
+curvature **un-thresholded** (`curvature_threshold=False` and
+`surface.{subject}.curvature.smoothness=1.0`). (This is to avoid failures from
+differences in the renderers' anti-aliasing implementations.)
+Everything else is at its default.
+
+`nonflat_views/` keeps pycortex's default thresholded curvature, unlike the
+flatmap groups.
+
+The exact keyword arguments are in `_render_and_check_dataview` and
+`_render_and_check_webgl_only`; change either and the references must be
+regenerated.
+
+## Checks
+
+The three flatmap tests check each render twice: against its own stored
+reference at a tight tolerance (`MAX_MEAN_ABS_DIFF`, `MAX_FRACTION_DIFFERING`,
+`MAX_FRACTION_GROSSLY_DIFFERING`, `MAX_SSIM_LOSS`, all four of which must pass),
+and against the other renderer's render of the same dataview at a loose one
+(`CROSS_MAX_MEAN_ABS_DIFF`, `CROSS_MAX_FRACTION_DIFFERING`), with no stored
+fixture. `test_visual_comparison_nonflat_views` runs the reference check only.
+
+Both renderers write their flatmap content-tight and transparent outside it, so
+the cross-renderer check only resizes webgl to quickflat's size before diffing.
+RGB under fully transparent pixels is normalized first: it is undefined there,
+and matplotlib leaves white where the browser leaves black.
+
+## Provenance
+
+Generated on `main` (`3779f7ca`), from the demo subject `S1` in the filestore
+bundled with pycortex, which is pinned by `cortex/tests/conftest.py`.
+
+| | |
+| --- | --- |
+| chromium | 151.0.7922.34 (headless shell, SwiftShader software rendering) |
+| playwright | 1.62.0 (fixes the chromium build above) |
+| matplotlib | 3.10.9 |
+
+Both are pinned in the `test` dependency group, and re-pinning is part of
+regenerating. playwright fixes the chromium build, which determines the 16 webgl
+references; matplotlib rasterizes the 12 quickflat ones.
+
+Update matplotlib beyond 3.10.9 once Python 3.10 is dropped.
+
+## Format
+
+Lossless WebP (`method=6`, `quality=100`, `exact=True`): bit-exact after decode,
+and 59% the size of optimized PNG (1229 KiB versus 2061 KiB for the set of 28).
+
+## Storage
+
+Tracked with **git LFS**. If yours are 130-byte text files rather than images,
+the clone has not fetched them:
+
+```
+git lfs install && git lfs pull
+```
+
+The tests skip on that, and on the images being absent altogether, rather than
+failing.
+
+## Distribution
+
+Kept out of the wheel (`exclude_package_data` in `setup.py`) and kept in the
+source tarball (`MANIFEST.in`'s `recursive-include cortex *`), so a run against
+an installed wheel degrades gracefully.
+
+## Regenerating
+
+The renders are deterministic: repeated runs on one machine produce
+bit-identical output, including the WebGL ones under software rendering. They
+are coupled to the Chromium and matplotlib builds above, so an upgrade can shift
+anti-aliasing and rasterization; the tolerances absorb small shifts. If a
+failure exceeds them, inspect the `diff_*.png` files it writes, confirm the
+change is cosmetic, then:
+
+```
+REGENERATE_REFERENCE_IMAGES=1 pytest cortex/tests/test_visual_regression.py
+```
+
+That rewrites all four directories in one run. Review the resulting diff before
+committing.