diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index 71eaaa3cc..2ebfe2ead 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -60,6 +60,8 @@ jobs: restore-keys: | ${{ runner.os }}-playwright- + # Firefox is left out on purpose, to keep CI runs short: its tests + # skip here and are run locally instead (see "Tests" in AGENTS.md). - name: Install Playwright Chromium run: playwright install --with-deps --only-shell chromium diff --git a/AGENTS.md b/AGENTS.md index 4fdf2e06a..d2861f55a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,6 +13,7 @@ The package has two Cython extensions (`cortex/formats.pyx` for surface mesh I/O ```bash uv sync --all-extras # headless extra (playwright) + dev group (test/type-checking deps) by default; builds the Cython extensions uv run playwright install --only-shell chromium # browser for headless WebGL tests +uv run playwright install firefox # optional: also run the Firefox tests (needs Xvfb, see Tests) ``` Versioning is by `setuptools-scm` from git tags (written to `cortex/_version.py`); never edit version files by hand. A new git worktree gets its own `.venv` — rerun `uv sync` there to (re)build the compiled extensions. @@ -28,9 +29,10 @@ uv run pytest -p no:cacheprovider --no-cov -x cortex/tests/test_formats.py # qu - `pytest.ini` sets a 240 s per-test timeout (via `pytest-timeout`) because headless browser sessions can hang; override per-test with `@pytest.mark.timeout(N)`. - Tests self-skip based on available tools: Inkscape (quickflat/dataset overlay tests), Playwright Chromium (`cortex/tests/testing_utils.py:has_playwright` — WebGL headless tests), and FreeSurfer's `mri_surf2surf`. +- Firefox tests (every test ID containing `firefox`; `pytest -k firefox` runs just those) self-skip unless Playwright's own Firefox build is installed (`testing_utils.py:has_playwright_firefox`). Headless Firefox needs an X display for WebGL, so they also need `Xvfb` (`apt-get install xvfb`) unless `DISPLAY` is set. These tests catch driver differences Chromium hides. For example, Mesa (Firefox) counts declared-but-unused vertex attributes against the 16-attribute limit, which ANGLE (Chromium) does not. Run them after touching `shaderlib.js` or the attributes `dataset.js` dispatches. - `pytest.ini` deselects `-m slow` by default. The one slow test, `cortex/tests/test_autoflatten.py:test_autoflatten_subject_end_to_end`, really runs autoflatten (15-30 min); it needs FreeSurfer sourced with `fsaverage` in `$SUBJECTS_DIR`, autoflatten installed, and `$PYCORTEX_TEST_FS_SUBJECT` naming a recon-all'd subject — the bundled S1 is a pycortex filestore entry, not a FreeSurfer subject directory, so it cannot be flattened. Run it with `pytest -m slow`. - Nearly all tests use the stub subject `S1` bundled in `filestore/db/S1` (transform `fullhead`, 304380 vertices, volume shape `(31, 100, 100)`). -- CI (`.github/workflows/run_tests.yml`) runs `pytest --cov=./` on a matrix of Python versions, with Inkscape and Playwright Chromium installed. The only enforced lint is codespell (config in `pyproject.toml`). mypy is configured in `pyproject.toml` and installed with the dev group but not run in CI — new Python code should still carry type annotations. +- CI (`.github/workflows/run_tests.yml`) runs `pytest --cov=./` on a matrix of Python versions, with Inkscape and Playwright Chromium installed. It deliberately does not install Firefox, to keep CI runs short, so the Firefox tests only run locally. The only enforced lint is codespell (config in `pyproject.toml`). mypy is configured in `pyproject.toml` and installed with the dev group but not run in CI — new Python code should still carry type annotations. ### Docs @@ -67,7 +69,7 @@ Flow: dataviews → JSON + PNG mosaics (`data.py:Package`) + compressed CTM surf - The bundled Three.js is **r69** — very old; the shader pipeline depends on its conventions, do not casually upgrade. - All GLSL lives in `resources/js/shaderlib.js` as arrays of string lines assembled per-configuration — there are no `.glsl` files. - CTM packs reorder vertices: `cortex.utils.get_ctmmap` / `get_ctm2webgl_map` translate between CTM/WebGL ordering and the original surface ordering. Indexing viewer data with original vertex indices without remapping is a classic bug. -- `cortex/export/headless.py` (`headless_viewer`) runs the viewer in headless Chromium via Playwright for screenshots/tests. +- `cortex/export/headless.py` (`headless_viewer`) runs the viewer in headless Chromium (default) or Firefox (`browser="firefox"`) via Playwright for screenshots/tests. - Some JS resources use CRLF line endings (e.g. `dataset.js`); keep the existing endings when editing. - `setup.py` explicitly enumerates `cortex.webgl` `package_data` patterns — new resource subdirectories must be added there or they won't ship in wheels. diff --git a/cortex/export/headless.py b/cortex/export/headless.py index 3b1ccc563..bbf195f88 100644 --- a/cortex/export/headless.py +++ b/cortex/export/headless.py @@ -1,6 +1,6 @@ """headless.py - context manager that runs the pycortex WebGL viewer inside a -headless Chromium browser via Playwright, allowing ``save_3d_views`` to -produce screenshots without any manual browser interaction. +headless Chromium or Firefox browser via Playwright, allowing ``save_3d_views`` +to produce screenshots without any manual browser interaction. The approach: 1. Start the Tornado WebGL server with ``open_browser=False`` so that it does @@ -34,8 +34,16 @@ Requirements ------------ +Playwright, plus the one browser you render with; you do not need both:: + pip install playwright - playwright install chromium + playwright install chromium # for browser="chromium" (the default) + playwright install firefox # only for browser="firefox" + +``browser="firefox"`` additionally requires the ``Xvfb`` binary to be on +``PATH`` (e.g. ``apt-get install xvfb``) unless a ``DISPLAY`` is already set, +since Firefox (unlike Chromium's bundled SwiftShader) needs a real or virtual +X server to create a software WebGL context. """ import concurrent.futures @@ -43,11 +51,14 @@ import logging import os import queue +import select import shutil +import subprocess import tempfile import threading import time -from typing import Any, Callable, Mapping, Optional, TypeVar +from collections.abc import Generator +from typing import Any, Callable, Literal, Mapping, Optional, TypeVar, get_args T = TypeVar("T") @@ -65,6 +76,102 @@ "--disable-dev-shm-usage", ] +BrowserName = Literal["chromium", "firefox"] + + +@contextlib.contextmanager +def _xvfb_display() -> Generator[str]: + """Run a private Xvfb server for the duration of the context. + + Yields its ``DISPLAY`` string (e.g. ``":99"``). Xvfb picks a free display + number itself and reports it via ``-displayfd`` once it accepts + connections. + + Raises + ------ + RuntimeError + If the ``Xvfb`` binary is not installed, or it fails to start. + """ + read_fd, write_fd = os.pipe() + try: + try: + proc = subprocess.Popen( + ["Xvfb", "-displayfd", str(write_fd), + "-screen", "0", "1280x1024x24", "-nolisten", "tcp"], + pass_fds=(write_fd,), + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + finally: + os.close(write_fd) + except FileNotFoundError as exc: + os.close(read_fd) + raise RuntimeError( + "Firefox headless WebGL rendering requires Xvfb, which was not " + "found on PATH. Install it with e.g. `apt-get install xvfb`, or " + "set the DISPLAY environment variable to an existing X server." + ) from exc + + try: + with os.fdopen(read_fd) as displayfd: + if not select.select([displayfd], [], [], 10.0)[0]: + raise RuntimeError("Timed out waiting for Xvfb to start.") + display_num = displayfd.readline().strip() + if not display_num: + raise RuntimeError("Xvfb exited unexpectedly during startup.") + yield f":{display_num}" + finally: + try: + proc.terminate() + proc.wait(timeout=10) + except Exception: + logger.warning("Failed to terminate Xvfb process", exc_info=True) + + +@contextlib.contextmanager +def webgl_browser( + playwright: Any, browser: BrowserName = "chromium" +) -> Generator[Any]: + """Launch a headless Playwright browser with software WebGL enabled. + + Yields the browser, and closes it (and the private Xvfb server started for + Firefox when no ``DISPLAY`` is set) on exit. + + Raises + ------ + ValueError + If ``browser`` is not one of the supported browsers. + """ + with contextlib.ExitStack() as stack: + if browser == "firefox": + # Unlike Chromium (which bundles SwiftShader and can create a + # software WebGL context with no display server at all), + # Firefox creates its GL context via GLX and needs a real (or + # virtual) X display to do so, even when launched "headless". + # If the caller hasn't already provided one, start a private + # Xvfb so WebGL still works with no GPU and no visible display. + launch_env = None + if not os.environ.get("DISPLAY"): + display = stack.enter_context(_xvfb_display()) + launch_env = {**os.environ, "DISPLAY": display} + instance = playwright.firefox.launch( + headless=True, + env=launch_env, + # Bypasses Firefox's GPU driver blocklist, which could refuse WebGL when + # DISPLAY is backed by a blocklisted driver (not applicable for Xvfb). + firefox_user_prefs={"webgl.force-enabled": True}, + ) + elif browser == "chromium": + instance = playwright.chromium.launch( + headless=True, args=SWIFTSHADER_CHROMIUM_ARGS + ) + else: + raise ValueError( + f"Unknown browser {browser!r}; expected one of {get_args(BrowserName)}" + ) + stack.callback(instance.close) + yield instance + def _wait_for_viewer_loaded(handle, timeout: float = 60.0) -> None: """Block until ``window.viewer.loaded`` resolves in the browser. @@ -182,7 +289,7 @@ def __init__(self, download_dir: Optional[str] = None) -> None: self._thread: Optional[threading.Thread] = None # Set during worker startup; only touched by the worker thread. self._playwright: Any = None - self._browser: Any = None + self._browser_stack = contextlib.ExitStack() self._page: Any = None # Browser-side errors collected by Playwright event listeners. # Written by the worker thread, read by the main thread via @@ -192,11 +299,19 @@ def __init__(self, download_dir: Optional[str] = None) -> None: # -- public API -------------------------------------------------------- # - def start(self, url: str, *, timeout: float = 60.0) -> None: - """Launch the worker thread, open Chromium, and navigate to *url*. + def start( + self, url: str, *, timeout: float = 60.0, browser: BrowserName = "chromium" + ) -> None: + """Launch the worker thread, open the browser, and navigate to *url*. Blocks until the page has finished loading (or raises on failure). + Parameters + ---------- + browser : str + Which Playwright browser type to launch: ``"chromium"`` (default) + or ``"firefox"``. + Raises ------ RuntimeError @@ -206,6 +321,7 @@ def start(self, url: str, *, timeout: float = 60.0) -> None: """ self._url = url self._nav_timeout = timeout + self._browser_type = browser self._thread = threading.Thread( target=self._worker, name="PlaywrightThread", daemon=True ) @@ -296,7 +412,7 @@ def _worker(self) -> None: except ImportError as exc: self._error = ImportError( "playwright is required for headless rendering. Install it with:\n" - " pip install playwright && playwright install chromium" + f" pip install playwright && playwright install {self._browser_type}" ) self._error.__cause__ = exc self._ready_future.set_result(None) # unblock caller @@ -304,10 +420,10 @@ def _worker(self) -> None: try: self._playwright = sync_playwright().start() - self._browser = self._playwright.chromium.launch( - headless=True, args=SWIFTSHADER_CHROMIUM_ARGS + browser = self._browser_stack.enter_context( + webgl_browser(self._playwright, self._browser_type) ) - self._page = self._browser.new_page() + self._page = browser.new_page() # Register listeners *before* navigation so we capture # errors that fire during page load (e.g. WebGL failures). @@ -410,7 +526,7 @@ def _cleanup(self) -> None: individually guarded so a failure in one does not prevent the rest.""" for obj, method in [ (self._page, "close"), - (self._browser, "close"), + (self._browser_stack, "close"), (self._playwright, "stop"), ]: if obj is not None: @@ -437,9 +553,10 @@ def headless_viewer( *, timeout: float = 60.0, download_dir: Optional[str] = None, + browser: BrowserName = "chromium", ): """Context manager that yields a connected ``JSMixer`` handle rendered in a - headless Chromium browser. + headless browser. Parameters ---------- @@ -458,6 +575,11 @@ def headless_viewer( ``handle._pw_thread.downloads``; ``handle._pw_thread.wait_for_download()`` waits for one. Default None, meaning a temporary directory removed when the viewer closes. + browser : str + Which Playwright browser to use: ``"chromium"`` (default) or + ``"firefox"``. The ``"firefox"`` browser requires the ``Xvfb`` + binary to be installed unless ``DISPLAY`` is already set (see + module docstring). Yields ------ @@ -543,7 +665,7 @@ def _await_client() -> None: # Launch the browser and navigate. python_interface.js runs on # load and sends "connect" over WebSocket, which unblocks # server.get_client(). - pw_thread.start(url, timeout=timeout) + pw_thread.start(url, timeout=timeout, browser=browser) # Wait (bounded) for the getter to return; the timeout guard # surfaces hung state clearly instead of blocking indefinitely. diff --git a/cortex/export/panels.py b/cortex/export/panels.py index 8335a33fd..96dba8c47 100644 --- a/cortex/export/panels.py +++ b/cortex/export/panels.py @@ -17,6 +17,7 @@ from matplotlib.figure import Figure from ..dataset import Dataview +from .headless import BrowserName from .save_views import save_3d_views, ViewParams from ._default_params import ( params_inflatedless_lateral_medial_ventral, @@ -52,6 +53,7 @@ def plot_panels( interpolation: str = "nearest", layers: int = 1, headless: bool = False, + browser: BrowserName = "chromium", ) -> Figure: """Plot on the same figure a number of views, as defined by a list of panel specifications. @@ -102,13 +104,22 @@ def plot_panels( plotting the data. (Default: 1). headless: bool - If True, render using a headless Chromium browser via Playwright instead + If True, render using a headless browser via Playwright instead of requiring the user to manually open a browser window. This allows the function to run fully autonomously without any user interaction. Requires ``playwright`` to be installed (``pip install playwright``) and - Chromium to be available (``playwright install chromium``). - Software WebGL (SwiftShader) is used, so no GPU or display server is - needed. (Default: False) + the chosen ``browser`` to be installed: ``playwright install + chromium`` for the default, or ``playwright install firefox`` for + ``browser="firefox"`` (only that one is needed). + Software WebGL rendering is used, so no GPU is needed, and with the + default Chromium no display server either (``browser="firefox"`` + needs one, see below). (Default: False) + + browser: str + Which Playwright browser to use when ``headless=True``: ``"chromium"`` + (default) or ``"firefox"``. Ignored when ``headless=False``. + ``"firefox"`` additionally requires ``Xvfb`` to be installed unless + ``DISPLAY`` is already set (see ``cortex.export.headless``). Returns ------- @@ -150,6 +161,7 @@ def plot_panels( interpolation=interpolation, layers=layers, headless=headless, + browser=browser, ) fig = plt.figure(figsize=figsize) diff --git a/cortex/export/save_views.py b/cortex/export/save_views.py index e250b3cfb..1464958b6 100644 --- a/cortex/export/save_views.py +++ b/cortex/export/save_views.py @@ -11,6 +11,7 @@ import cortex from ..dataset import Dataview +from .headless import BrowserName file_pattern = "{base}_{view}_{surface}.png" @@ -46,12 +47,13 @@ def save_3d_views( trim: bool = True, sleep: float = 10, headless: bool = False, + browser: BrowserName = "chromium", ) -> list[str]: """Saves 3D views of `volume` under multiple specifications. By default (``headless=False``), a webgl viewer is launched and a display - server is required. With ``headless=True``, a headless Chromium browser - is used instead, so no display server or GPU is needed. + server is required. With ``headless=True``, a headless browser (Chromium + or Firefox) is used instead, so no display server or GPU is needed. Parameters ---------- @@ -96,13 +98,22 @@ def save_3d_views( Time in seconds, to let the viewer open. headless: bool - If True, render using a headless Chromium browser via Playwright instead + If True, render using a headless browser via Playwright instead of requiring the user to manually open a browser window. This allows the function to run fully autonomously without any user interaction. Requires ``playwright`` to be installed (``pip install playwright``) and - Chromium to be available (``playwright install chromium``). - Software WebGL (SwiftShader) is used, so no GPU or display server is - needed. (Default: False) + the chosen ``browser`` to be installed: ``playwright install + chromium`` for the default, or ``playwright install firefox`` for + ``browser="firefox"`` (only that one is needed). + Software WebGL rendering is used, so no GPU is needed, and with the + default Chromium no display server either (``browser="firefox"`` + needs one, see below). (Default: False) + + browser: str + Which Playwright browser to use when ``headless=True``: ``"chromium"`` + (default) or ``"firefox"``. Ignored when ``headless=False``. + ``"firefox"`` additionally requires ``Xvfb`` to be installed unless + ``DISPLAY`` is already set (see ``cortex.export.headless``). Returns ------- @@ -117,7 +128,7 @@ def save_3d_views( if headless: from cortex.export.headless import headless_viewer as _headless_viewer - cm = _headless_viewer(volume, viewer_params) + cm = _headless_viewer(volume, viewer_params, browser=browser) else: cm = contextlib.nullcontext(cortex.webshow(volume, **viewer_params)) diff --git a/cortex/tests/test_export.py b/cortex/tests/test_export.py index e8bd3e32c..af3211bcc 100644 --- a/cortex/tests/test_export.py +++ b/cortex/tests/test_export.py @@ -13,7 +13,7 @@ import cortex -from .testing_utils import has_playwright +from .testing_utils import BROWSERS, has_playwright pytestmark = pytest.mark.skipif( not has_playwright, @@ -24,7 +24,8 @@ subj, xfmname, volshape = "S1", "fullhead", (31, 100, 100) -def test_save_3d_views_headless(): +@pytest.mark.parametrize("browser", BROWSERS) +def test_save_3d_views_headless(browser): """save_3d_views with headless=True should produce an image file.""" vol = cortex.Volume(np.random.randn(*volshape), subj, xfmname) @@ -41,6 +42,7 @@ def test_save_3d_views_headless(): # _set_view can succeed; sleep=10 (the default) is safe. sleep=10, headless=True, + browser=browser, ) assert len(file_names) == 1 diff --git a/cortex/tests/test_headless.py b/cortex/tests/test_headless.py index 5b136ce9d..beb9e82e4 100644 --- a/cortex/tests/test_headless.py +++ b/cortex/tests/test_headless.py @@ -6,17 +6,27 @@ pip install playwright playwright install chromium + +Firefox-specific tests additionally require ``playwright install firefox`` +and are skipped otherwise. """ +import contextlib import os +import subprocess import tempfile import numpy as np import pytest import cortex +from cortex.export import headless from cortex.export.headless import _PlaywrightThread -from .testing_utils import has_playwright, wait_for_file +from .testing_utils import ( + BROWSERS, + has_playwright, + wait_for_file, +) pytestmark = pytest.mark.skipif( not has_playwright, @@ -27,17 +37,45 @@ subj, xfmname, volshape = "S1", "fullhead", (31, 100, 100) -def test_headless_viewer_opens_and_closes(): - """The headless viewer context manager should yield a working handle and - tear down cleanly.""" - vol = cortex.Volume(np.random.randn(*volshape), subj, xfmname) +@pytest.mark.parametrize("browser", BROWSERS) +def test_headless_viewer_opens_and_closes(browser, monkeypatch): + """The headless viewer should open in each browser and shut it down on exit. - with cortex.export.headless_viewer(vol, viewer_params={}) as handle: - # The handle should have a .server attribute (the WebApp) - assert hasattr(handle, "server") - # The server should be serving on some port - assert handle.server.port > 0 + Entering the context already proves the browser loaded the viewer: + ``headless_viewer`` raises unless the page connects and finishes loading. + On exit, the browser and any private Xvfb server (Firefox without a + ``DISPLAY``) must be gone rather than left running after every render. + """ + browsers = [] + real_webgl_browser = headless.webgl_browser + + @contextlib.contextmanager + def recording_webgl_browser(*args, **kwargs): + with real_webgl_browser(*args, **kwargs) as instance: + browsers.append(instance) + yield instance + xvfbs = [] + real_popen = subprocess.Popen + + def recording_popen(args, *rest, **kwargs): + proc = real_popen(args, *rest, **kwargs) + if args[0] == "Xvfb": + xvfbs.append(proc) + return proc + + monkeypatch.setattr(headless, "webgl_browser", recording_webgl_browser) + monkeypatch.setattr(subprocess, "Popen", recording_popen) + + vol = cortex.Volume(np.random.randn(*volshape), subj, xfmname) + with cortex.export.headless_viewer(vol, viewer_params={}, browser=browser): + pass + + assert len(browsers) == 1 + assert not browsers[0].is_connected(), "browser still running after exit" + assert all(proc.poll() is not None for proc in xvfbs), ( + "Xvfb still running after exit" + ) def test_browser_errors_collected(): diff --git a/cortex/tests/test_webgl_headless.py b/cortex/tests/test_webgl_headless.py index 7adba20d0..f391af34a 100644 --- a/cortex/tests/test_webgl_headless.py +++ b/cortex/tests/test_webgl_headless.py @@ -1,6 +1,7 @@ """Tests for headless WebGL rendering across data types, angles, surfaces, and panels. -These tests exercise the WebGL viewer through the headless Chromium browser, +These tests exercise the WebGL viewer through the headless Chromium browser +(the data type tests also through Firefox, when it is installed), verifying that all supported data types, camera angles, surface morphing states, and predefined panel layouts render correctly without a display server. @@ -25,7 +26,13 @@ default_view_params, unfold_view_params, ) -from cortex.tests.testing_utils import count_red_pixels, has_playwright, wait_for_file +from cortex.tests.testing_utils import ( + BROWSERS, + FIREFOX_MISSING, + count_red_pixels, + has_playwright, + wait_for_file, +) from cortex.webgl.serve import JSON, JSProxy pytestmark = pytest.mark.skipif( @@ -44,6 +51,9 @@ } +DTYPE_NAMES = ["Volume", "Vertex", "VolumeRGB", "VertexRGB", "Volume2D", "Vertex2D"] + + def make_dataview(dtype_name): """Return a Dataview instance for the given type name.""" np.random.seed(0) @@ -118,30 +128,56 @@ def _assert_not_blank(path): # --------------------------------------------------------------------------- -@pytest.mark.parametrize( - "dtype_name", - [ - "Volume", - "Vertex", - "VolumeRGB", - "VertexRGB", - "Volume2D", - "Vertex2D", - ], -) -def test_datatype_renders(dtype_name, tmp_path): - """Each data type should render in the headless viewer without errors.""" +@pytest.mark.parametrize("browser", BROWSERS) +@pytest.mark.parametrize("dtype_name", DTYPE_NAMES) +def test_datatype_renders(dtype_name, browser, tmp_path): + """Each data type should render in the headless viewer without errors. + + Run in Firefox too: its Mesa rasterizer counts declared-but-unused vertex + attributes against the limit of 16, which Chromium's ANGLE does not, so a + shader that only fits after pruning fails to link there and renders blank. + """ vol = make_dataview(dtype_name) - with cortex.export.headless_viewer(vol, viewer_params={}) as handle: + with cortex.export.headless_viewer( + vol, viewer_params={}, browser=browser + ) as handle: outfile = str(tmp_path / "test.png") handle.getImage(outfile, (512, 384)) wait_for_file(outfile) - assert os.path.isfile(outfile) - assert os.path.getsize(outfile) > 0 + # Browser errors first, since one may explain a blank file. + _assert_no_browser_failures(handle) + _assert_not_blank(outfile) - # Browser errors first, since one may explain a blank file. - _assert_no_browser_failures(handle) - _assert_not_blank(outfile) + +@FIREFOX_MISSING +@pytest.mark.parametrize("dtype_name", DTYPE_NAMES) +def test_chromium_matches_firefox(dtype_name, tmp_path): + """Each data type should look the same in both browsers. + + Software WebGL rasterizers differ slightly in anti-aliasing and float + precision, so compare with a tolerance rather than byte for byte. A render + that is blank in both browsers matches; ``test_datatype_renders`` is what + catches that. + """ + from PIL import Image + + images = {} + for browser in ("chromium", "firefox"): + outfile = str(tmp_path / f"{browser}.png") + with cortex.export.headless_viewer( + make_dataview(dtype_name), viewer_params={}, browser=browser + ) as handle: + handle.getImage(outfile, (512, 384)) + wait_for_file(outfile) + with Image.open(outfile) as img: + images[browser] = np.asarray(img.convert("RGB"), dtype=np.float64) + + assert images["chromium"].shape == images["firefox"].shape + mean_abs_diff = np.abs(images["chromium"] - images["firefox"]).mean() + assert mean_abs_diff < 5.0, ( + f"{dtype_name} differs between Chromium and Firefox (mean abs diff " + f"= {mean_abs_diff:.2f} out of 255)." + ) # --------------------------------------------------------------------------- diff --git a/cortex/tests/test_webgl_shaders.py b/cortex/tests/test_webgl_shaders.py index 6a676d1ae..8dc5ec205 100644 --- a/cortex/tests/test_webgl_shaders.py +++ b/cortex/tests/test_webgl_shaders.py @@ -5,10 +5,16 @@ shows up in the viewer as an unexplained black screen. This is how ``Vertex2D`` broke (gh-714). -Linking a shader needs no subject database or render, just Chromium, so it +Linking a shader needs no subject database or render, just a browser, so it costs milliseconds instead of seconds per case -- cheap enough to cover every combination of options here, including the equivolume shaders that no render test enables. + +Every variant is linked in both Chromium and Firefox, because they count +attributes differently: Chromium's ANGLE drops declared-but-unused attributes +before enforcing the limit, Firefox's Mesa counts every declaration. A shader +that only fits after that pruning passes in Chromium and renders blank in +Firefox. """ import itertools @@ -18,8 +24,8 @@ import pytest import cortex.webgl -from cortex.export.headless import SWIFTSHADER_CHROMIUM_ARGS -from cortex.tests.testing_utils import has_playwright +from cortex.export.headless import webgl_browser +from cortex.tests.testing_utils import BROWSERS, has_playwright pytestmark = pytest.mark.skipif( not has_playwright, reason="playwright and chromium are required" @@ -178,25 +184,24 @@ def _variants() -> Iterator[Any]: yield pytest.param("depth", dict(morphs=3, volume=1), id="depth") -@pytest.fixture(scope="module") -def webgl_page(tmp_path_factory: pytest.TempPathFactory) -> Iterator[Any]: +@pytest.fixture(scope="module", params=BROWSERS) +def webgl_page( + request: pytest.FixtureRequest, tmp_path_factory: pytest.TempPathFactory +) -> Iterator[Any]: """Load the shader-linking page into a real GL context, shared by both hooks.""" from playwright.sync_api import sync_playwright page_path = tmp_path_factory.mktemp("shaders") / "shaders.html" page_path.write_text(PAGE.replace("__JSDIR__", JS_PATH)) - with sync_playwright() as playwright: - browser = playwright.chromium.launch( - headless=True, args=SWIFTSHADER_CHROMIUM_ARGS - ) + with sync_playwright() as playwright, webgl_browser( + playwright, request.param + ) as browser: page = browser.new_page() page.goto("file://%s" % page_path, wait_until="load", timeout=60000) if not page.evaluate("() => !!window.linkShader"): - browser.close() pytest.skip("no WebGL context available in this browser") yield page - browser.close() @pytest.fixture(scope="module") diff --git a/cortex/tests/testing_utils.py b/cortex/tests/testing_utils.py index 8bfcfeddf..eb5e64d0e 100644 --- a/cortex/tests/testing_utils.py +++ b/cortex/tests/testing_utils.py @@ -1,8 +1,10 @@ -from typing import Any +from typing import Any, get_args import numpy as np import numpy.typing as npt +import pytest +from cortex.export.headless import BrowserName from cortex.export.save_views import ViewParams # Skip any test that relies on playwright if it's not available. @@ -19,6 +21,35 @@ except Exception: has_playwright = False +# Separately check for a working Playwright Firefox install, since it may not +# be present even when Chromium is (e.g. `playwright install chromium` only). +try: + from playwright.sync_api import sync_playwright + + _pw = sync_playwright().start() + try: + _b = _pw.firefox.launch(headless=True) + _b.close() + finally: + _pw.stop() + has_playwright_firefox = True +except Exception: + has_playwright_firefox = False + +# CI installs only Chromium, so the Firefox tests skip there; say how to run them. +FIREFOX_MISSING = pytest.mark.skipif( + not has_playwright_firefox, + reason="Playwright Firefox not available; install it with " + "`playwright install firefox`; it also needs Xvfb unless DISPLAY is set", +) + +# Every browser headless_viewer supports; parametrize browser tests over these +# so the Firefox cases skip when it is not installed. +BROWSERS = [ + pytest.param(name, marks=FIREFOX_MISSING if name == "firefox" else ()) + for name in get_args(BrowserName) +] + def wait_for_file(path, timeout=30): """Poll until `path` exists and has nonzero size; raise after `timeout`. diff --git a/cortex/webgl/resources/js/dataset.js b/cortex/webgl/resources/js/dataset.js index dfed6bde8..0f907695a 100644 --- a/cortex/webgl/resources/js/dataset.js +++ b/cortex/webgl/resources/js/dataset.js @@ -305,8 +305,13 @@ var dataset = (function(module) { return; } this.uniforms.framemix.value = frame - fframe; - for (var i = 0; i < this.data.length; i++) { - this.data[i].set(this.uniforms, i, fframe, this._dispatch); + + if (this.vertex && this.data.length > 1 && !this.data[0].raw) { + this._setPacked2D(fframe); + } else { + for (var i = 0; i < this.data.length; i++) { + this.data[i].set(this.uniforms, i, fframe, this._dispatch); + } } // Optional alpha map (volumes): bind the alpha textures for this // frame and the next. A single-frame alpha is reused for every frame @@ -326,13 +331,15 @@ var dataset = (function(module) { this.uniforms.dataalpha.value[1] = (a1 !== undefined) ? a1 : a0; } } + // Vertex data: build the single shared "nanmask" attribute, which the // shader multiplies into the color. It is 0 wherever any dim (or the // alpha map) is NaN, otherwise the frame-mixed alpha value (1 when // there is no alpha map). Folding alpha into this attribute instead - // of adding attributes matters: a 2D vertex view already uses 15 of - // the 16 guaranteed vertex attributes, and exceeding the limit makes - // the program fail to link silently. + // of adding attributes matters: some drivers count every declared + // attribute against the 16 guaranteed ones (see surface_vertex in + // shaderlib.js), and exceeding the limit makes the program fail to + // link silently. if (this.vertex && !this.data[0].raw && this.data[0].nanmasks.length > 0) { // The shader mixes frame fframe with the next one (framemix), and // NaN was replaced by 0 in the buffers, so both adjacent frames @@ -379,6 +386,36 @@ var dataset = (function(module) { this._dispatch({type:"attribute", name:"nanmask", value:combined}); } } + // 2D vertex data: the shader reads both dims from the .x/.y of two vec2 + // attributes (see surface_vertex in shaderlib.js), so interleave dim 0 + // with dim 1 for this frame and the next. setFrame runs on every animation + // frame during playback, so the packed attributes of the current frame + // pair are kept: an unchanged frame costs nothing and a one-frame step + // reuses the previous "next" frame. (Kept here, not on the VertexData, + // because brains are shared between dataviews.) + module.DataView.prototype._setPacked2D = function(fframe) { + var d0 = this.data[0], d1 = this.data[1]; + var f0 = d0.framePair(fframe), f1 = d1.framePair(fframe); + var prev = this._packed || {}, cache = {}; + [0, 1].forEach(function(which) { + var key = f0[which] + "," + f1[which]; + if (!(key in cache)) + cache[key] = prev[key] || [0, 1].map(function(side) { + var a = d0.verts[f0[which]][side].array; + var b = d1.verts[f1[which]][side].array; + var out = new Float32Array(a.length * 2); + for (var i = 0; i < a.length; i++) { + out[2 * i] = a[i]; + out[2 * i + 1] = b[i]; + } + var attr = new THREE.BufferAttribute(out, 2); + attr.needsUpdate = true; + return attr; + }); + this._dispatch({type:"attribute", name:"data"+which, value:cache[key]}); + }, this); + this._packed = cache; + } // A streamed movie frame just arrived. If it is the frame on screen (or // its blend partner), re-apply the textures: a seek to a not-yet-loaded // frame otherwise stays blank until the next setFrame (e.g. playback). @@ -648,15 +685,19 @@ var dataset = (function(module) { module.VertexData.prototype.setFilter = function(interp) { //nothing to do... } + // Indices into this.verts of frame fframe and the one it blends into. + // The next frame may not have streamed in yet: blend with the current + // one rather than wrapping (by the loaded count) round to frame 0. + module.VertexData.prototype.framePair = function(fframe) { + var cur = fframe.mod(this.frames), next = (fframe+1).mod(this.frames); + return [cur, this.verts[next] !== undefined ? next : cur]; + } + // Single-dim vertex data only; 2D data is interleaved by + // DataView._setPacked2D. module.VertexData.prototype.set = function(uniforms, dim, fframe, dispatch) { - var name = dim == 0 ? "data0":"data2"; - // The next frame may not have streamed in yet: blend with the current - // one rather than wrapping (by the loaded count) round to frame 0. - var next = this.verts[(fframe+1).mod(this.frames)]; - if (next === undefined) - next = this.verts[fframe]; - dispatch({type:"attribute", name:"data"+(2*dim), value:this.verts[fframe]}); - dispatch({type:"attribute", name:"data"+(2*dim+1), value:next}); + var f = this.framePair(fframe); + dispatch({type:"attribute", name:"data0", value:this.verts[f[0]]}); + dispatch({type:"attribute", name:"data1", value:this.verts[f[1]]}); // The combined nanmask is dispatched by DataView.setFrame after // every dim's data has been set, so we don't dispatch it here. } diff --git a/cortex/webgl/resources/js/mriview_surface.js b/cortex/webgl/resources/js/mriview_surface.js index ffb86e85d..d18999441 100644 --- a/cortex/webgl/resources/js/mriview_surface.js +++ b/cortex/webgl/resources/js/mriview_surface.js @@ -283,8 +283,6 @@ var mriview = (function(module) { //Queue blank data attributes for vertexdata hemi.addAttribute("data0", new THREE.BufferAttribute(new Float32Array(), 1)); hemi.addAttribute("data1", new THREE.BufferAttribute(new Float32Array(), 1)); - hemi.addAttribute("data2", new THREE.BufferAttribute(new Float32Array(), 1)); - hemi.addAttribute("data3", new THREE.BufferAttribute(new Float32Array(), 1)); // Colormapped vertex data only: a soft 0-1 opacity (0 = NaN, // else the alpha map), filled by DataView.setFrame (dataset.js). hemi.addAttribute("nanmask", new THREE.BufferAttribute(new Float32Array(), 1)); diff --git a/cortex/webgl/resources/js/shaderlib.js b/cortex/webgl/resources/js/shaderlib.js index 2cf3af0ae..7decb5836 100644 --- a/cortex/webgl/resources/js/shaderlib.js +++ b/cortex/webgl/resources/js/shaderlib.js @@ -833,10 +833,20 @@ var Shaderlib = (function() { "attribute vec4 data0;", "attribute vec4 data1;", "#else", + // 2D vertex data packs both dimensions into the .x/.y of two vec2 + // attributes (one per interpolated frame) rather than using four + // separate float attributes. Drivers that do not eliminate unused + // attribute declarations before enforcing GL_MAX_VERTEX_ATTRIBS + // (e.g. Mesa/llvmpipe, used by headless Firefox) count every declared + // attribute, and four floats push this shader past the 16-attribute + // limit. dataset.js interleaves the two dimensions to match. + "#ifdef TWOD", + "attribute vec2 data0;", + "attribute vec2 data1;", + "#else", "attribute float data0;", "attribute float data1;", - "attribute float data2;", - "attribute float data3;", + "#endif", // Soft 0-1 opacity for colormapped vertex data, built by // DataView.setFrame (dataset.js) from each dim's nanmasks and the // Vertex2D alpha map: 0 = NaN, else alpha (1 without an alpha map). @@ -850,7 +860,7 @@ var Shaderlib = (function() { utils.flatbump_attr, // "attribute float dropout;", - + "varying vec3 vViewPosition;", "varying vec3 vNormal;", "varying vec2 vUv;", @@ -871,11 +881,12 @@ var Shaderlib = (function() { "vColor = mix(data0, data1, framemix);", "#else", "vec2 cuv;", + "#ifdef TWOD", + "cuv.x = (mix(data0.x, data1.x, framemix) - vmin[0]) / (vmax[0] - vmin[0]);", + "cuv.y = (mix(data0.y, data1.y, framemix) - vmin[1]) / (vmax[1] - vmin[1]);", + "#else", // "vValue.x = (mix(data0, data1, framemix) - vmin[0]) / (vmax[0] - vmin[0]);", "cuv.x = (mix(data0, data1, framemix) - vmin[0]) / (vmax[0] - vmin[0]);", - "#ifdef TWOD", - // "vValue.y = (mix(data2, data3, framemix) - vmin[1]) / (vmax[1] - vmin[1]);", - "cuv.y = (mix(data2, data3, framemix) - vmin[1]) / (vmax[1] - vmin[1]);", "#endif", "vColor = texture2D(colormap, cuv);", // Data mask: WebGL drivers sanitize NaN in vertex attributes, @@ -1022,8 +1033,15 @@ var Shaderlib = (function() { attributes.flatbump = { type: 'v4', value:null }; } - for (var i = 0; i < 4; i++) - attributes['data'+i] = {type:opts.rgb ? 'v4':'f', value:null}; + // Declare only the data attributes the shader actually reads: two + // vec4s for RGB, two vec2s for 2D (both dims interleaved), else two + // floats. See the attribute declarations above for why unused + // declarations are not harmless on all drivers. + for (var i = 0; i < 2; i++) + attributes['data'+i] = { + type: opts.rgb ? 'v4' : (opts.twod ? 'v2' : 'f'), + value: null, + }; if (!opts.rgb) attributes['nanmask'] = {type:'f', value:null};