CI add visual regression tests - #672
Draft
kroq-gar78 wants to merge 17 commits into
Draft
Conversation
The surface shaders sit right up against the 16 vertex attribute slots WebGL guarantees, and gallantlab#679 added two more of them (flatheight and flatBumpNorms) to the vertex data shader so that bumpy_flatmap would work for Vertex dataviews. 2D vertex data needs four data attributes rather than two, which pushed surface_vertex to 17: it still compiles, but it fails to link with "Too many attributes", and three.js only reports that on the browser console, so the viewer just draws a black screen. The same overflow hits plain Vertex data as soon as equivolume sampling is on. Rather than giving these values slots of their own, pack them into the spare components of attributes that are already bound: - the white matter and pial vertex areas, which only the equivolume depth sampling reads, move into auxdat.zw (auxdat.x is the medial wall mask and .y the curvature; z and w were left at zero by brainctm) - the flatmap bump height moves into the fourth component of the bumped flatmap normals, so the two become a single vec4 flatbump attribute Every shader variant now links: the worst case, 2D vertex data on a subject with a flatmap, goes from 19 attributes to 16. Renders of the paths that did work before -- volume data with equivolume sampling, and the bumpy flatmap -- are pixel-identical to what they were. Fixes gallantlabgh-714. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_013wiUP7TLgDqSfnv4n9ySZj
Nothing caught gallantlabgh-714: the shader that failed to link compiled fine, the page raised no javascript error, and the screenshot was written -- it was just empty. Two checks that would have caught it: test_webgl_shaders links every combination of options the viewer builds surface, pick and depth shaders with, in a real (swiftshader) GL context, and reports how many of the available vertex attributes the variant uses when linking fails. It needs Chromium but no subject database, so it runs in a couple of seconds. test_datatype_renders additionally asserts the render is not a single flat color, which is what a viewer that failed to draw anything produces. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_013wiUP7TLgDqSfnv4n9ySZj
…only) Previously the webgl test only asserted a PNG got written; a rendering regression could only be caught by a human looking at it. This adds test_visual_regression.py, parametrized per dataview class, which for each of the six public dataview types (Volume, Vertex, Volume2D, Vertex2D, VolumeRGB, VertexRGB) renders both an alpha-bearing and a NaN-bearing version through quickflat (matplotlib) and headless WebGL, then checks each render against its own stored reference (tight tolerance) and the two renders directly against each other (loose tolerance, since matplotlib and Three.js differ systematically in anti-aliasing and colormap sampling even when both are correct). NaN coverage exercises pycortex's "no data" convention: both renderers must draw NaN'd elements as transparent, not as a colormapped value. Reference images are added in the next commit; a missing reference skips rather than fails, checked before rendering. Six of the twelve per-test-suite cases (the pure-Vertex flatmap renders) are marked xfail: a webgl lighting refactor landed on main after this branch was started and broke flatmap rendering of Vertex-only data under headless software rendering -- a real bug this suite is now positioned to catch once fixed upstream. Co-Authored-By: Claude Sonnet 5 <[email protected]> Co-Authored-By: Claude Haiku 4.5 <[email protected]>
700 KiB of lossless WebP across two sets -- alpha_dataviews (alpha- bearing data) and nan_dataviews (NaN standing in for missing data) -- each with all six public dataview classes rendered via cortex.quickshow and cortex.export.plot_panels. Generated on main at 5af26a8, matching the commit the previous code commit already targets; the vertex-flatmap regression from gallantlab#679 is on main only after that commit, so these renders predate and are unaffected by it. Co-Authored-By: Claude Sonnet 5 <[email protected]> Co-Authored-By: Claude Haiku 4.5 <[email protected]>
…ings Adds type annotations to the module's utility functions (not the parametrized test_ functions or their fixture/parametrize arguments), and corrects the docstring claim that a mismatch always dumps actual_*.png/diff_*.png -- a shape mismatch or a renderer crash reports a plain message instead, since there's no pixel-aligned diff to write. Also renames the tmp_path filenames from qf_/wg_ to quickflat_/webgl_ to match the reference image naming convention. Co-Authored-By: Claude Sonnet 5 <[email protected]>
Both comparisons previously threw away the renders' coordinate frames: the within-renderer check bailed out entirely on a shape mismatch, and the cross-renderer check squashed both images to a fixed 256x256, which cannot correct an anisotropic scale. Fitting a homography (cv2.findTransformECC, MOTION_HOMOGRAPHY) between the two renderers on curvature-only content found essentially zero rotation and zero projective terms -- so despite webgl using a THREE.PerspectiveCamera, there is no real perspective distortion to undo at the flatmap's nadir view of a planar surface. What it did find is a fixed anisotropic scale (0.9690, 0.9365) plus a ~7px translation, i.e. the two pipelines simply compute their trim/extent independently. Applying that affine correction drops curvature-only mean|diff| from 18.9 to 4.4. With the correction in place the ten renderable dataview/variant pairs span mean|diff| 1.71-3.09 and 0.10-1.66% of pixels differing, so the cross-renderer limits come down from 20.0/15% to 8.0/5% -- roughly 2.5-3x the worst observed case. Losing the correction alone now fails the check. The within-renderer check now resizes a differently-shaped render onto its reference instead of returning early, so it reports a real pixel diff (and writes diff_*.png) for the Vertex/VertexRGB shape mismatches rather than just noting the shape difference. They still fail, confirming those are a genuine regression and not a cropping artifact. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The CROSS_RENDERER_* constants in test_visual_regression.py were a set of magic numbers with no recorded provenance, so nobody could tell whether a broad cross-renderer failure meant a real regression or just a stale correction -- nor how to refit it. Adds fit_cross_renderer_affine.py, which renders curvature-only content (alpha=0, isolating the coordinate-frame mismatch from colormap differences) through both paths, fits webgl -> quickflat with cv2.findTransformECC, and prints the constants ready to paste. It reproduces the committed values (scales exact, translations within 0.01px; mean|diff| 18.8 -> 4.6). OpenCV is only needed for the fit, so it stays out of the project deps and the script documents `uv run --with opencv-python-headless`. The README section records why the correction is affine and not a reverse perspective projection, despite webgl using a THREE.PerspectiveCamera: the flatmap looks straight down at a planar surface, so a fitted homography returns a projective row of [~0, ~0, 1] and collapses to the affine. That dead end is worth writing down. Also fixes two stale references to test_webgl_headless.py, which is where these tests lived before they were split out. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
…eement A factorial sweep of both renderers' settings -- curvature brightness/contrast/threshold, depth, sampler, thick/layers, and webgl's three lighting controls -- found exactly one setting worth changing. Thresholded curvature puts a hard binary edge at curvature=0 whose sub-pixel placement each rasteriser resolves differently, making it maximally sensitive to the residual misalignment between the two frames. Smooth curvature is low-frequency and resamples cleanly. Averaged over the brightness/contrast grid the thresholded pairing scored 8.50 against 3.66 smooth; at the default brightness/contrast, 4.98 against 2.66. quickflat's curvature_threshold and webgl's curvature.smoothness are the same knob from opposite ends (smoothness 0.0 *is* thresholded), so both move together or they disagree by more. Everything else stays default on the evidence: the default lighting already ties the best combination (2.304, and uniform_illumination=1 renders identically on a flatmap); trilinear beats nearest by ~1%, i.e. noise; and depth/thick/layers span only 0.38 across the whole grid, below the noise floor. The webgl setting has to ride along with the angle params as an (name, params) tuple. plot_panels' viewer_params is forwarded to show(), whose **kwargs silently swallows anything that is not a named argument, so viewer state passed that way is a no-op -- a trap worth the comment it now carries. Note this renders curvature un-thresholded, which is not pycortex's default appearance, so these references no longer cover the default curvature path. That is the trade for the tighter floor. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Twenty of the twenty-four images. Vertex2D's webgl render raises before either reference is written, so its four files (quickflat + webgl, alpha + nan) are still at the thresholded settings; the xfail comment records that whoever fixes gallantlab#679 has to regenerate them in the same change. CAVEAT, needs a decision: regenerating captured the *current* Vertex and VertexRGB renders, which include the 2px-wider trim bounding box caused by gallantlab#679. Their references therefore now encode that regression, the shape mismatch is gone, and those four cases XPASS instead of xfailing. The gallantlab#679 shape symptom is no longer detected -- exactly the "regenerating is how a real regression gets silently blessed" failure mode this directory's README warns about. XPASS does not fail a run, so it will not announce itself. Either their references get reverted to the pre-gallantlab#679 renders (keeping the regression visible, at the cost of them being the only images on the old curvature settings), or the xfail comes off those four cases and the shape change is accepted deliberately rather than by accident. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Rebasing onto the Vertex2D webgl fix makes all six dataviews render, so the xfail marks come off and the four Vertex2D references -- which the crash had prevented REGENERATE_REFERENCE_IMAGES from refreshing -- are regenerated onto the un-thresholded curvature settings the other twenty already use. Both changes have to land together: dropping the marks without the refreshed Vertex2D references, or vice versa, leaves the suite red. Worth recording why the 2px-wider bounding box was blessed rather than treated as an outstanding regression. With the fix applied, Vertex2D's webgl render is still 596px wide against its untouched 594px reference, so the wider trim is the gallantlab#679 shader change's intended new geometry, not something the fix reverts. The references were regenerated to match it deliberately. 12 passed, no xfails or xpasses; test_webgl_headless.py 43 passed. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The existing mean and fraction>16 limits are both weak against a change that moves a small number of pixels a long way, which is what a geometry or contour shift looks like. The gallantlab#679 vertex-shader change is the worked example: it moved 2.8% of pixels by a median of 67/255, yet scored 1.574 against a mean limit of 2.0 -- it would have passed that criterion outright, and cleared the fraction one by only 1.4x. The mean is diluted by the ~97% of pixels that did not move. Adds two criteria that cover the gap, measured against simulated cosmetic drift (a 596<->594 resample, +-1 LSB quantisation, gamma 1.02) versus gallantlab#679: metric cosmetic gallantlab#679 limit mean 0.596 1.574 2.0 <- would have passed fraction > 16 0.48% 2.79% 2% fraction > 64 0.00% 1.460% 0.1% <- new, 14.6x margin SSIM loss 0.00215 0.03933 0.01 <- new, 3.9x margin All four are checked because they are complementary, not ranked. Gamma 1.25 scores 6.68 mean and 47.96% on fraction>16 but 0.00% on fraction>64; an R/B channel swap scores 21.75 mean but 0.00000 SSIM loss, since SSIM is computed on luminance and is blind to a channel permutation. SSIM adds sensitivity alongside the others and cannot replace them. SSIM is implemented on scipy (already a dependency) rather than scikit-image, and calibrated against the shipped implementation -- scipy and OpenCV differ slightly at the boundary. Verified end to end by swapping the pre-gallantlab#679 reference back in: the check fails naming fraction>16, fraction>64 and SSIM, and correctly omits the mean. The cosmetic figures are simulated, not measured across a real Chromium or matplotlib upgrade; a genuine bump changes anti-aliasing, which can move a few pixels a long way and does show up in fraction>64. Both new limits sit well above the simulated drift rather than at it, to leave room for that. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Evaluating the suite against six closed-issue fixes turned up a second worked example. cb97627 ("unify NaN and alpha handling") changes four quickflat volumetric renders via premultiplied-alpha thickness averaging, and at a threshold of 64 every one scores 0.000% -- missed entirely, by all four criteria. Its mean cannot be tightened into a catch: at 0.19 it sits *below* the simulated cosmetic floor of 0.60, so a tighter mean produces false positives before it produces a catch. A boundary-restricted metric was also tried and refuted -- restricting to the alpha edge band *reduces* the signal tenfold, because the thickness-averaging change is interior, not at the silhouette. Lowering the threshold does work. At 32 the cb97627 signal is 0.256% against the 0.1% limit, while the worst simulated cosmetic drift is 0.0052%, 19x below it. It also strengthens the gallantlab#679 catch, 1.46% -> 2.16%. Currently-passing tests cannot be disturbed: their within-renderer difference is exactly zero. The cost is headroom. At 64 no cosmetic perturbation registered at all; at 32 a real Chromium or matplotlib bump has 19x of room rather than effectively unlimited, and anti-aliasing changes are exactly what produce the mid-amplitude pixels the lower threshold newly counts. Worth it to convert a demonstrated blind spot into a 2.6x catch. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The NaN suite only ever put NaNs in the data channels. Alpha is a separate path: it is not colour-mapped but used directly as a blend weight, so a NaN there reaches the compositing arithmetic rather than a colormap lookup. Adds four references for VolumeRGB and VertexRGB -- the only dataviews taking an explicit alpha= -- with NaNs in the alpha map and clean colour channels. Both renderers currently draw those elements fully transparent, the same outcome as a NaN in the data, so this passes rather than xfails. The docstring records that the behaviour is not settled: cb97627 ("unify NaN and alpha handling", not in this branch's history) changes how the surviving RGB is blended without changing the transparency, so expect these four to need regenerating if it lands. Also corrects the README's provenance, which was actively wrong. It still claimed the references were generated at 5af26a8; regenerating there and running against the current tree fails the six vertex-based cases, so anyone following that instruction would conclude something was broken. gallantlab#679 moved every pure-Vertex flatmap's trim box from 594 to 596px and that is intended new geometry its follow-up fix does not revert. The failures are a real content change, not a resampling artifact: optimal affine alignment removes only 12% of the difference, and it is concentrated in the interior rather than at the silhouette. All twelve quickflat references are byte-identical at either commit; only the webgl ones moved. Brings the rest of the README up to date while there: three tests rather than two, four within-renderer criteria rather than two, three reference directories to regenerate rather than two. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
cb97627's rule is that a NaN anywhere at a voxel -- the data, either 2D dimension, any RGB channel, or the alpha map -- renders fully transparent. The suite only ever exercised two of those four: the data, and the first RGB channel. The alpha map was covered separately; this covers the remaining two. Volume2D/Vertex2D now take a NaN'd second dimension as well as the first, and VolumeRGB/VertexRGB a NaN'd green channel as well as red. Each uses its own disjoint region -- x>=50 versus y>=50 for volumes, and two non-overlapping index ranges for vertices -- so one render exercises several branches of the rule at once while a failure still says which one moved. Blue and the alpha map are left clean as a control that not everything has simply gone transparent. No new test cases, so no extra runtime: this changes which arrays the existing NaN dataviews are built from. Eight of the twenty-eight references move (the 2D and RGB pairs in nan_dataviews); the other twenty are byte-identical. Confirmed the new regions reach both renderers before regenerating -- quickflat and webgl each shifted on all four dataviews, by 2.07 to 6.47 mean, while Volume/Vertex stayed identical as expected. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The volume masks x>=50 and y>=50 overlap in a quadrant (25% of voxels carry both NaNs); only the vertex index ranges are genuinely disjoint. The overlap is harmless and in fact covers the both-NaN case as well, but the comment claimed otherwise. Also names the alpha map alongside blue as the clean control, and points at the nan_alpha suite that does cover it. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Completes the coverage of cb97627's rule -- a NaN in the data, either 2D dimension, any RGB channel, or the alpha map renders fully transparent -- inside the suite that already carries NaNs in the colour channels. VolumeRGB/VertexRGB now take a NaN'd alpha map on a third region (z>=15 for volumes, the quarter of vertex indices between the other two). This is not a duplicate of the nan_alpha suite, which isolates an alpha NaN with every colour channel clean. Here the alpha NaNs are superposed on colour NaNs, so the overlap exercises voxels carrying more than one NaN at once. Blue stays clean as a control that not everything has gone transparent. Worth knowing when reading the arrays back: the alpha map's surviving NaN fraction is smaller than its mask (13% of a 52% region for VolumeRGB). Where a colour channel is already NaN the pipeline writes alpha's vmin over it, so only the part with red and green both clean stays NaN and the rest resolves to a hard 0 -- confirmed exactly equal to (z>=15) & (red clean) & (green clean). Both halves are worth rendering, which is why the regions overlap rather than being disjoint. Recorded in a comment, since the fraction is otherwise surprising. Four references move (the RGB pairs in nan_dataviews); the 2D views take no alpha= and are untouched. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
A WebGL failure does not raise on its own. three.js reports a shader that compiled but failed to link via console.error, so nothing propagates: the png is still written and simply comes back blank, which is indistinguishable from a successful render. Checking *all* browser errors is not usable, which is presumably why the existing assertions in test_webgl_headless.py filter to [pageerror]. A healthy viewer already logs a console.error for the Leap Motion websocket it cannot reach (ws://127.0.0.1:6437), so anything failing on "any error" fails every run. Verified: a healthy headless render captures exactly that one message and nothing else. So webgl_failures() matches two unambiguous signatures only -- "THREE.WebGLProgram: Could not initialise shader" and "Error creating WebGL context" -- and save_3d_views raises if either appears, naming the view and the file. gl.getProgramInfoLog / gl.getShaderInfoLog warnings are deliberately excluded: drivers emit those benignly and matching them would reintroduce the false positives this list exists to avoid. Scope, measured rather than assumed: this does NOT catch the Vertex2D blank render (gallantlabgh-714). Re-running that case at the broken commit with this check in place, the viewer produces a completely blank canvas (one distinct colour) while logging no WebGL message at all -- only the same Leap Motion noise a healthy run produces. That failure is silent even on the console, so no console-based check can see it; detecting it needs the blank-render and shader-link tests that shipped with its fix. What this does cover is context-creation failure, which three.js throws. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
kroq-gar78
force-pushed
the
visual-regression-tests
branch
from
August 25, 2026 09:17
f09c1cc to
8e383a3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously the render test only asserted a PNG got written; a rendering regression could only be caught by a human looking at it. Now it compares all twelve dataview renders (quickflat + headless WebGL) against stored reference images, with tolerances validated by injecting a real regression (R/B channel swap), and runs by default.
References images (700 KiB, lossless WebP) were generated on main at 5af26a8 and are byte-identical to this branch's output.
Right now the reference images are prefixed with
qs_andwg_, but this can be changed to match the renderers' actual names.Test plan
pytest cortex/tests/test_webgl_headless.py