From b8829d7ff19821a4cc1b8c46bed182c5cccb2a42 Mon Sep 17 00:00:00 2001 From: Aditya Vaidya Date: Tue, 6 Oct 2026 16:47:45 -0700 Subject: [PATCH 1/4] TST cover voxline and slice-plane shaders in WebGL shader link tests - Link surface_pixel with voxline on, once per color type. - Link the three main variants the slice planes use. - Replace Any annotations with precise types. --- cortex/tests/test_webgl_shaders.py | 69 +++++++++++++++++++++++++----- 1 file changed, 58 insertions(+), 11 deletions(-) diff --git a/cortex/tests/test_webgl_shaders.py b/cortex/tests/test_webgl_shaders.py index 6a676d1ae..926169102 100644 --- a/cortex/tests/test_webgl_shaders.py +++ b/cortex/tests/test_webgl_shaders.py @@ -13,7 +13,7 @@ import itertools import os -from typing import Any, Callable, Iterator +from typing import TYPE_CHECKING, Callable, Iterator, TypedDict import pytest @@ -21,6 +21,11 @@ from cortex.export.headless import SWIFTSHADER_CHROMIUM_ARGS from cortex.tests.testing_utils import has_playwright +if TYPE_CHECKING: + # What pytest.param returns; pytest does not export it publicly. + from _pytest.mark.structures import ParameterSet + from playwright.sync_api import Page + pytestmark = pytest.mark.skipif( not has_playwright, reason="playwright and chromium are required" ) @@ -73,9 +78,10 @@ // only makes THREE.WebGLRenderer refresh built-in light uniforms against // the material's uniforms object -- which needs the viewer's full merged // uniform set to exist. None of these shaders reference THREE's light - // uniforms (they compute shading themselves), and the MAX_*_LIGHTS - // defines this test cares about come from the renderer's light count - // regardless of this flag, so leaving it out avoids that crash for free. + // uniforms (they compute shading themselves) except ``main``, whose + // lights_phong chunks loop over MAX_*_LIGHTS. Those defines come from the + // renderer's light count regardless of this flag (zero here, so the loops + // compile away), so leaving it out avoids that crash for free. var material = new THREE.ShaderMaterial({ vertexShader: code.vertex, fragmentShader: frag, @@ -121,6 +127,19 @@ """ + +class LinkResult(TypedDict): + """What the page's ``linkShader``/``linkRawShader`` hooks return.""" + + linked: bool + log: str + + +# Link one Shaders[name](opts) variant / raw (vertex, fragment) GLSL source. +LinkShader = Callable[[str, dict[str, object]], LinkResult] +LinkRawShader = Callable[[str, str], LinkResult] + + # The options the viewer generates surface shaders with. ``morphs`` is the # number of surfaces to mix between (anatomical, inflated and flat), ``volume`` # says the subject has a white matter surface; the rest come from the dataview @@ -129,7 +148,7 @@ halo=False, dither=False, voxline=False, sampler="nearest") -def _surface_variants() -> Iterator[Any]: +def _surface_variants() -> Iterator["ParameterSet"]: """Every (shader, opts) pair the viewer can ask for a surface shader. On top of the dataview and surface options, gh-695 added three that @@ -137,6 +156,11 @@ def _surface_variants() -> Iterator[Any]: ``dataalpha`` adds a second pair of samplers and the alpha-map arithmetic, ``nanmean`` changes how layer samples are combined, and ``layers`` decides how many of those sampling blocks are emitted. + + ``voxline`` (the ``[webgl_viewopts] voxlines`` debug grid) only appends a + fixed blend to ``surface_pixel`` that reads the cortical sheet position and + the final color -- no attributes or samplers -- so it is checked once per + color type on the largest variant instead of doubling the whole product. """ bools = (False, True) for shader, rgb, twod, hasflat, equivolume, dataalpha, nanmean, layers in itertools.product( @@ -169,9 +193,32 @@ def _surface_variants() -> Iterator[Any]: ) yield pytest.param(shader, opts, id=name) + # `voxline` + for rgb, twod in ((False, False), (False, True), (True, False)): + opts = dict(SURFACE_OPTS, rgb=rgb, twod=twod, hasflat=True, + equivolume=True, dataalpha=not rgb, nanmean=True, + layers=32, voxline=True) + name = "surface_pixel-%s%s-voxline" % ("rgb" if rgb else "cmap", + "-2d" if twod else "") + yield pytest.param("surface_pixel", opts, id=name) + + +def _main_variants() -> Iterator["ParameterSet"]: + """Every (shader, opts) pair the slice planes ask ``main`` for. + + The slice planes (sliceplane.js) are the only live users of ``main``; they + only show volume data and fix every option except ``raw`` and ``twod``. + """ + for raw, twod in ((False, False), (False, True), (True, False)): + opts = dict(sampler="nearest", raw=raw, twod=twod, voxline=False, + viewspace=True) + name = "main-%s%s" % ("rgb" if raw else "cmap", "-2d" if twod else "") + yield pytest.param("main", opts, id=name) + -def _variants() -> Iterator[Any]: +def _variants() -> Iterator["ParameterSet"]: yield from _surface_variants() + yield from _main_variants() # The shaders the picker renders with; they morph the same geometry but # carry no data. yield pytest.param("pick", dict(morphs=3, volume=1), id="pick") @@ -179,7 +226,7 @@ def _variants() -> Iterator[Any]: @pytest.fixture(scope="module") -def webgl_page(tmp_path_factory: pytest.TempPathFactory) -> Iterator[Any]: +def webgl_page(tmp_path_factory: pytest.TempPathFactory) -> Iterator["Page"]: """Load the shader-linking page into a real GL context, shared by both hooks.""" from playwright.sync_api import sync_playwright @@ -200,7 +247,7 @@ def webgl_page(tmp_path_factory: pytest.TempPathFactory) -> Iterator[Any]: @pytest.fixture(scope="module") -def link_shader(webgl_page: Any) -> Callable[[str, dict], Any]: +def link_shader(webgl_page: "Page") -> LinkShader: """Return a function linking one shader variant in a real GL context.""" return lambda shader, opts: webgl_page.evaluate( "args => window.linkShader(args[0], args[1])", [shader, opts] @@ -208,7 +255,7 @@ def link_shader(webgl_page: Any) -> Callable[[str, dict], Any]: @pytest.fixture(scope="module") -def link_raw_shader(webgl_page: Any) -> Callable[[str, str], Any]: +def link_raw_shader(webgl_page: "Page") -> LinkRawShader: """Return a function linking raw GLSL source in the same GL context.""" return lambda vertex, fragment: webgl_page.evaluate( "args => window.linkRawShader(args[0], args[1])", [vertex, fragment] @@ -217,7 +264,7 @@ def link_raw_shader(webgl_page: Any) -> Callable[[str, str], Any]: @pytest.mark.parametrize("shader,opts", list(_variants())) def test_shader_links( - shader: str, opts: dict, link_shader: Callable[[str, dict], Any] + shader: str, opts: dict[str, object], link_shader: LinkShader ) -> None: """Each shader variant has to compile *and* link. @@ -233,7 +280,7 @@ def test_shader_links( def test_shader_link_catches_compile_error( - link_raw_shader: Callable[[str, str], Any] + link_raw_shader: LinkRawShader ) -> None: """A shader that fails to *compile* must fail ``linked`` too. From c1a03cf39c237da773135326e3f1326e002b0169 Mon Sep 17 00:00:00 2001 From: Aditya Vaidya Date: Thu, 8 Oct 2026 18:22:34 -0700 Subject: [PATCH 2/4] TST link WebGL shaders with the viewer's lights and scene The test scene had no lights, so all lighting GLSL compiled away. Build it from the viewer's Axes3D instead, and assert the lighting code stays compiled. Also fail, rather than skip, when the test page throws while loading. --- cortex/tests/test_webgl_shaders.py | 101 +++++++++++++++++------------ 1 file changed, 61 insertions(+), 40 deletions(-) diff --git a/cortex/tests/test_webgl_shaders.py b/cortex/tests/test_webgl_shaders.py index 926169102..06b6c8d44 100644 --- a/cortex/tests/test_webgl_shaders.py +++ b/cortex/tests/test_webgl_shaders.py @@ -39,18 +39,40 @@ # attribute declarations) itself instead of a copy hand-kept here going stale. PAGE = """ + + + + + """ @@ -133,6 +135,7 @@ class LinkResult(TypedDict): linked: bool log: str + uniforms: list[str] # Link one Shaders[name](opts) variant / raw (vertex, fragment) GLSL source. @@ -227,7 +230,10 @@ def _variants() -> Iterator["ParameterSet"]: @pytest.fixture(scope="module") def webgl_page(tmp_path_factory: pytest.TempPathFactory) -> Iterator["Page"]: - """Load the shader-linking page into a real GL context, shared by both hooks.""" + """Load the shader-linking page into a real GL context, shared by both hooks. + + Skips without WebGL, but fails on any page script error. + """ from playwright.sync_api import sync_playwright page_path = tmp_path_factory.mktemp("shaders") / "shaders.html" @@ -237,13 +243,24 @@ def webgl_page(tmp_path_factory: pytest.TempPathFactory) -> Iterator["Page"]: browser = playwright.chromium.launch( headless=True, args=SWIFTSHADER_CHROMIUM_ARGS ) - page = browser.new_page() - page.goto("file://%s" % page_path, wait_until="load", timeout=60000) - if not page.evaluate("() => !!window.linkShader"): + try: + page = browser.new_page() + errors: list[str] = [] + page.on("pageerror", + lambda error: errors.append(error.stack or str(error))) + page.goto("file://%s" % page_path, wait_until="load", timeout=60000) + if not page.evaluate( + "() => !!document.createElement('canvas').getContext('webgl')" + ): + pytest.skip("no WebGL context available in this browser") + if errors or not page.evaluate("() => !!window.linkShader"): + pytest.fail( + "the shader-linking page failed to load:\n%s" + % "\n".join(errors or ["window.linkShader is not defined"]) + ) + yield page + finally: browser.close() - pytest.skip("no WebGL context available in this browser") - yield page - browser.close() @pytest.fixture(scope="module") @@ -277,6 +294,10 @@ def test_shader_links( assert result["linked"], "%s failed to compile or link:\n%s" % ( shader, result["log"] ) + if shader not in ("pick", "depth"): + assert "directionalLightColor[0]" in result["uniforms"], ( + "%s compiled without its lighting code" % shader + ) def test_shader_link_catches_compile_error( From 6663d34dab180e369fd70ebd5359cc843a4e79fa Mon Sep 17 00:00:00 2001 From: Aditya Vaidya Date: Thu, 8 Oct 2026 18:32:36 -0700 Subject: [PATCH 3/4] TST link raw and library shaders through one WebGL test hook The invalid-GLSL test now takes the same path as the real shaders. --- cortex/tests/test_webgl_shaders.py | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/cortex/tests/test_webgl_shaders.py b/cortex/tests/test_webgl_shaders.py index 06b6c8d44..f631a4a09 100644 --- a/cortex/tests/test_webgl_shaders.py +++ b/cortex/tests/test_webgl_shaders.py @@ -76,8 +76,8 @@ var ITEM_SIZE = {f: 1, v2: 2, v3: 3, v4: 4}; -window.linkShader = function(shadername, opts) { - var code = Shaders[shadername](opts); +// Link one {vertex, fragment, attrs} object, as Shaders[name](opts) returns. +window.linkCode = function(code) { // The pick shader returns one fragment shader per axis; any of them will // do, they all go with the vertex shader that holds the attributes. var frag = code.fragment instanceof Array ? code.fragment[0] : code.fragment; @@ -111,27 +111,12 @@ return linkResult(material); }; - -// Same path as linkShader, but from raw GLSL source instead of a Shaders[] -// lookup, so a test can hand it deliberately invalid GLSL. -window.linkRawShader = function(vertexShader, fragmentShader) { - var geometry = new THREE.BufferGeometry(); - geometry.addAttribute('position', new THREE.BufferAttribute(new Float32Array(9), 3)); - var material = new THREE.ShaderMaterial({vertexShader: vertexShader, fragmentShader: fragmentShader}); - var mesh = new THREE.Mesh(geometry, material); - mesh.frustumCulled = false; - scene.add(mesh); - renderer.render(scene, camera); - scene.remove(mesh); - - return linkResult(material); -}; """ class LinkResult(TypedDict): - """What the page's ``linkShader``/``linkRawShader`` hooks return.""" + """What the page's ``linkCode`` hook returns.""" linked: bool log: str @@ -253,10 +238,10 @@ def webgl_page(tmp_path_factory: pytest.TempPathFactory) -> Iterator["Page"]: "() => !!document.createElement('canvas').getContext('webgl')" ): pytest.skip("no WebGL context available in this browser") - if errors or not page.evaluate("() => !!window.linkShader"): + if errors or not page.evaluate("() => !!window.linkCode"): pytest.fail( "the shader-linking page failed to load:\n%s" - % "\n".join(errors or ["window.linkShader is not defined"]) + % "\n".join(errors or ["window.linkCode is not defined"]) ) yield page finally: @@ -267,7 +252,7 @@ def webgl_page(tmp_path_factory: pytest.TempPathFactory) -> Iterator["Page"]: def link_shader(webgl_page: "Page") -> LinkShader: """Return a function linking one shader variant in a real GL context.""" return lambda shader, opts: webgl_page.evaluate( - "args => window.linkShader(args[0], args[1])", [shader, opts] + "args => window.linkCode(Shaders[args[0]](args[1]))", [shader, opts] ) @@ -275,7 +260,8 @@ def link_shader(webgl_page: "Page") -> LinkShader: def link_raw_shader(webgl_page: "Page") -> LinkRawShader: """Return a function linking raw GLSL source in the same GL context.""" return lambda vertex, fragment: webgl_page.evaluate( - "args => window.linkRawShader(args[0], args[1])", [vertex, fragment] + "args => window.linkCode({vertex: args[0], fragment: args[1], attrs: {}})", + [vertex, fragment], ) From 12840c06aedff74f058de914678b33310c3cfb8d Mon Sep 17 00:00:00 2001 From: Aditya Vaidya Date: Thu, 8 Oct 2026 19:03:34 -0700 Subject: [PATCH 4/4] TST cover volume=0 shaders and late page errors in WebGL link tests - Link both surface shaders without a white matter surface (volume=0), including voxline's non-cortical-sheet branch. - Fail on page script errors thrown after load, not only during it. - Dispose each linked material and geometry, and inline linkResult. --- cortex/tests/test_webgl_shaders.py | 87 +++++++++++++++++++----------- 1 file changed, 57 insertions(+), 30 deletions(-) diff --git a/cortex/tests/test_webgl_shaders.py b/cortex/tests/test_webgl_shaders.py index f631a4a09..f314a29db 100644 --- a/cortex/tests/test_webgl_shaders.py +++ b/cortex/tests/test_webgl_shaders.py @@ -57,23 +57,6 @@ var renderer = axes.renderer, camera = axes.camera; var scene = axes.setGrid(1, 1, 0); -function linkResult(material) { - var gl = renderer.context; - var program = material.program; - var vs = program.vertexShader, fs = program.fragmentShader, gp = program.program; - var linked = !!gl.getProgramParameter(gp, gl.LINK_STATUS); - var uniforms = []; - var nuniforms = linked ? gl.getProgramParameter(gp, gl.ACTIVE_UNIFORMS) : 0; - for (var i = 0; i < nuniforms; i++) - uniforms.push(gl.getActiveUniform(gp, i).name); - return { - linked: linked, - log: [gl.getShaderInfoLog(vs), gl.getShaderInfoLog(fs), - gl.getProgramInfoLog(gp)].join("\\n"), - uniforms: uniforms, - }; -} - var ITEM_SIZE = {f: 1, v2: 2, v3: 3, v4: 4}; // Link one {vertex, fragment, attrs} object, as Shaders[name](opts) returns. @@ -109,7 +92,23 @@ renderer.render(scene, camera); scene.remove(mesh); - return linkResult(material); + var gl = renderer.context; + var program = material.program; + var vs = program.vertexShader, fs = program.fragmentShader, gp = program.program; + var linked = !!gl.getProgramParameter(gp, gl.LINK_STATUS); + var uniforms = []; + var nuniforms = linked ? gl.getProgramParameter(gp, gl.ACTIVE_UNIFORMS) : 0; + for (var i = 0; i < nuniforms; i++) + uniforms.push(gl.getActiveUniform(gp, i).name); + var result = { + linked: linked, + log: [gl.getShaderInfoLog(vs), gl.getShaderInfoLog(fs), + gl.getProgramInfoLog(gp)].join("\\n"), + uniforms: uniforms, + }; + material.dispose(); + geometry.dispose(); + return result; }; """ @@ -190,6 +189,16 @@ def _surface_variants() -> Iterator["ParameterSet"]: "-2d" if twod else "") yield pytest.param("surface_pixel", opts, id=name) + # Subjects without a white matter surface (``volume=0``) skip the cortical + # sheet code, voxline's included; check that once per shader. + for shader in ("surface_vertex", "surface_pixel"): + pixel = shader == "surface_pixel" + opts = dict(SURFACE_OPTS, rgb=False, twod=False, hasflat=True, + equivolume=True, dataalpha=pixel, nanmean=True, + layers=32 if pixel else 1, volume=0, voxline=pixel) + name = "%s-cmap-novolume%s" % (shader, "-voxline" if pixel else "") + yield pytest.param(shader, opts, id=name) + def _main_variants() -> Iterator["ParameterSet"]: """Every (shader, opts) pair the slice planes ask ``main`` for. @@ -214,7 +223,15 @@ def _variants() -> Iterator["ParameterSet"]: @pytest.fixture(scope="module") -def webgl_page(tmp_path_factory: pytest.TempPathFactory) -> Iterator["Page"]: +def page_errors() -> list[str]: + """Script errors the shader-linking page has thrown so far.""" + return [] + + +@pytest.fixture(scope="module") +def webgl_page( + tmp_path_factory: pytest.TempPathFactory, page_errors: list[str] +) -> Iterator["Page"]: """Load the shader-linking page into a real GL context, shared by both hooks. Skips without WebGL, but fails on any page script error. @@ -230,38 +247,48 @@ def webgl_page(tmp_path_factory: pytest.TempPathFactory) -> Iterator["Page"]: ) try: page = browser.new_page() - errors: list[str] = [] page.on("pageerror", - lambda error: errors.append(error.stack or str(error))) + lambda error: page_errors.append(error.stack or str(error))) page.goto("file://%s" % page_path, wait_until="load", timeout=60000) if not page.evaluate( "() => !!document.createElement('canvas').getContext('webgl')" ): pytest.skip("no WebGL context available in this browser") - if errors or not page.evaluate("() => !!window.linkCode"): + if page_errors or not page.evaluate("() => !!window.linkCode"): pytest.fail( "the shader-linking page failed to load:\n%s" - % "\n".join(errors or ["window.linkCode is not defined"]) + % "\n".join(page_errors or ["window.linkCode is not defined"]) ) yield page finally: browser.close() +def _linker( + page: "Page", page_errors: list[str], expression: str +) -> Callable[..., LinkResult]: + """Call ``expression`` in the page with the arguments, failing on page errors.""" + def link(*args: object) -> LinkResult: + result: LinkResult = page.evaluate(expression, list(args)) + if page_errors: + pytest.fail("the shader-linking page threw:\n%s" % "\n".join(page_errors)) + return result + return link + + @pytest.fixture(scope="module") -def link_shader(webgl_page: "Page") -> LinkShader: +def link_shader(webgl_page: "Page", page_errors: list[str]) -> LinkShader: """Return a function linking one shader variant in a real GL context.""" - return lambda shader, opts: webgl_page.evaluate( - "args => window.linkCode(Shaders[args[0]](args[1]))", [shader, opts] - ) + return _linker(webgl_page, page_errors, + "args => window.linkCode(Shaders[args[0]](args[1]))") @pytest.fixture(scope="module") -def link_raw_shader(webgl_page: "Page") -> LinkRawShader: +def link_raw_shader(webgl_page: "Page", page_errors: list[str]) -> LinkRawShader: """Return a function linking raw GLSL source in the same GL context.""" - return lambda vertex, fragment: webgl_page.evaluate( + return _linker( + webgl_page, page_errors, "args => window.linkCode({vertex: args[0], fragment: args[1], attrs: {}})", - [vertex, fragment], )