FIX unify NaN and alpha handling across quickflat, WebGL and RGB dataviews - #695
Open
mvdoc wants to merge 7 commits into
Open
FIX unify NaN and alpha handling across quickflat, WebGL and RGB dataviews#695mvdoc wants to merge 7 commits into
mvdoc wants to merge 7 commits into
Conversation
…#684) The dat.GUI opacity slider and the `o` shortcut both drive the `dataAlpha` uniform. The volume shaders apply it (`vColor *= dataAlpha`), but the `surface_vertex` fragment shader only declared it and never read it, so opacity was a silent no-op for Vertex, Vertex2D and VertexRGB data. - shaderlib.js: scale the per-vertex data colour by `dataAlpha` before compositing over the curvature. `vColor` is a varying, so scale into a local `dColor`. Scaling all four channels keeps the premultiplied-alpha convention of the volume path, so opacity 0 shows curvature only. - mriview_surface.js: `toggleOpacity` now remembers the last visible opacity instead of rounding it (0.7 -> 0 -> 0.7, not 0.7 -> 0 -> 1), and no longer leaks an implicit global. - test_webgl_headless.py: regression test rendering a saturated Vertex at opacity 1 -> 0 -> 1 and counting coloured pixels. Fails on main (53,760 red pixels at opacity 0) and passes with the fix. Closes #684 Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01P8oA4mUvTLd7sbs9iFaqM8
…ed, image-relative quantities Address review on #685: the 1000 / 50 / 1000 pixel-count thresholds were unexplained. The assertions now use named constants derived from the measured renders and related to the image size or to each other: - opacity 1: red-dominant pixels must cover >= 10% of the frame (measured ~27% at this view; curvature-only renders give 0%). - opacity 0: at most 1% of the opacity-1 count (measured 0). - opacity restored to 1: within 5% of the original count, which is a stronger check of the round trip than the previous "> 1000". Re-verified: fails against the pre-fix shader (53,760 red pixels remain at opacity 0), passes with the fix. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01P8oA4mUvTLd7sbs9iFaqM8
…views Rule: a NaN anywhere at a voxel/vertex (data, either 2D dimension, any RGB channel, or the alpha map) renders fully transparent in both renderers; otherwise the alpha (2D colormap, alpha= kwarg, RGB alpha channel) is honored. - Volume2D/Vertex2D: alpha= is a real attribute (not an ndarray in attrs, which crashed the WebGL viewer with a 500); multiplied into the colormap alpha in quickflat for both classes, shipped to WebGL as a normalized float brain and applied in the shaders (folded into the nanmask attribute for vertex data: 2D vertex views already use 15 of the 16 guaranteed vertex attributes and adding more made the program fail to link silently). Saved/restored in HDF. - VolumeRGB/VertexRGB: NaN mask is applied without writing into the temporary returned by .volume for masked (linear) alpha Volumes; single- frame alpha is broadcast against multi-frame NaN masks (closes #629); NaN inside the alpha map -> 0 explicitly; color_voxels no longer mutates the caller's alpha array. - quickflat: RGBA is averaged across thickness in premultiplied space (no dark halos around transparent voxels, matching the WebGL compositor); make_svg no longer indexes arr[..., 3] on 2-D scalar images. - Dataview2D.to_json keeps vmin/vmax of exactly 0. - Package deduplicates byte-identical brains (Vertex2D(x, x) / VertexRGB(r, r, r) used to be reordered twice and crash the viewer). - WebGL hover/click readout shows NaN for masked vertices instead of 0. - Tests: browser-free NaN/alpha unit tests, quickflat-vs-WebGL parity tests, and dataset-switching tests checking that NaN masks and alpha never leak between datasets (setData and addData). Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01KdjyZMFXsn6mRMJu9i8ige
… by default The WebGL multi-layer sampling summed the layer samples, so one NaN voxel at any depth made the whole fragment transparent (quickflat's nanmean=False). surface_pixel now averages only the valid (non-NaN) layer samples and is transparent only when none is valid, matching quickflat's nanmean=True. A `nanmean` toggle in the surface controls (on by default) restores the old any-NaN-is-transparent behavior, and quickflat's make_figure / make_flatmap_image / add_data default to nanmean=True so both renderers agree. NaN-free data renders identically either way. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01KdjyZMFXsn6mRMJu9i8ige
Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01KdjyZMFXsn6mRMJu9i8ige
Contributor
There was a problem hiding this comment.
Pull request overview
Unifies NaN transparency and alpha handling across quickflat, WebGL, 2D, and RGB dataviews.
Changes:
- Adds normalized alpha maps and consistent NaN masking.
- Introduces valid-layer averaging with a
nanmeancontrol. - Adds HDF persistence, packaging fixes, documentation, and regression tests.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
examples/datasets/plot_data_with_alpha.py |
Demonstrates 2D alpha maps. |
docs/dataset.rst |
Documents NaN and alpha behavior. |
cortex/webgl/resources/js/shaderlib.js |
Adds alpha sampling and valid-layer averaging. |
cortex/webgl/resources/js/mriview.js |
Reports masked vertex values as NaN. |
cortex/webgl/resources/js/mriview_surface.js |
Adds the WebGL nanmean control. |
cortex/webgl/resources/js/dataset.js |
Loads alpha data and combines vertex masks. |
cortex/webgl/data.py |
Deduplicates packaged brains. |
cortex/tests/test_webgl_switching.py |
Tests dataset-switching isolation. |
cortex/tests/test_webgl_nan_alpha_parity.py |
Tests renderer parity. |
cortex/tests/test_webgl_data.py |
Tests package deduplication. |
cortex/tests/test_nan_alpha.py |
Covers NaN and alpha behavior. |
cortex/quickflat/view.py |
Defaults quickflat to nanmean=True. |
cortex/quickflat/utils.py |
Adds premultiplied RGBA averaging. |
cortex/quickflat/composite.py |
Propagates the new default. |
cortex/dataset/views.py |
Restores 2D alpha from HDF. |
cortex/dataset/viewRGB.py |
Fixes RGB masking and broadcasting. |
cortex/dataset/view2D.py |
Adds first-class 2D alpha support. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+33
to
+36
| for brain in data.uniques(collapse=True): | ||
| if brain.name not in seen: | ||
| seen.add(brain.name) | ||
| self.uniques.append(brain) |
… package dedup - quickflat: nanmean now applies to dataviews that reach the RGBA branch. 2D/raw conversions carry their NaN mask, so nanmean=True averages the valid voxels only and nanmean=False hides any pixel touched by a NaN voxel; native RGB (NaN already alpha 0) treats fully transparent voxels as missing, like the WebGL RGB textures. - WebGL surface_pixel: NANMEAN also applies to RGB textures (fully transparent layer samples are skipped; validity from the current frame since an unbound next-frame sampler reads as opaque black). - WebGL vertex data: the nanmask combines the masks of both frames being blended (next frame only while framemix > 0), so a NaN in the next frame no longer interpolates towards a fake 0. - _mask_alpha: uint8 alpha maps are filled with byte 0 (their inferred vmin is a percentile of the bytes, 255 for a constant map). - Package: same-name brains are deduplicated only when their class, subject, transform, shape, dtype and mask match; otherwise raise a clear ValueError instead of serving one brain with another's metadata. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01KdjyZMFXsn6mRMJu9i8ige
Contributor
Author
|
Addressed the Copilot review in the latest commit:
Each has a test ( |
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
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.
Closes #629
Rule, applied identically in quickflat and the WebGL viewer: a NaN anywhere at a voxel/vertex (data, either dimension of a 2D view, any RGB channel, or the alpha map) renders fully transparent; otherwise the alpha (2D colormap,
alpha=kwarg, RGB alpha channel) is honored.Example
Fixes
Volume2D/Vertex2D(alpha=...): was an ndarray inattrs→ WebGL viewer failed to load (500); quickflat-Volume2D painted NaNs opaque, quickflat-Vertex2D ignored it. Now a proper attribute, multiplied into the colormap alpha, shipped to WebGL as a normalized brain (vertex data: folded into thenanmaskattribute — 2D vertex views already use 15/16 vertex attributes and extra ones made the program fail to link silently), and saved in HDF.VolumeRGB/VertexRGB: NaN mask was written into the temporary returned by.volumefor masked alpha Volumes (NaN voxels stayed opaque); single-frame alpha now broadcasts against multi-frame NaN masks (VertexRGB multiframe with NaN raises IndexError in alpha mask #629); NaN inside the alpha map → 0 explicitly;color_voxelsno longer mutates the caller's array.make_svgno longer indexesarr[..., 3]on 2-D images.nanmeantoggle in the surface controls (on by default); quickflat'snanmeandefaults toTruetoo, so both renderers agree. NaN-free data renders identically either way.Dataview2D.to_jsonkeepsvmin/vmaxof exactly 0.Packagededuplicates byte-identical brains (Vertex2D(x, x)crashed the viewer inreorder).NaNfor masked vertices instead of0.Tests
test_nan_alpha.py: browser-free unit tests for all of the above.test_webgl_nan_alpha_parity.py: 15 quickshow-vs-WebGL flatmap comparisons + the multi-layernanmeantoggle.test_webgl_switching.py: switching between datasets (setDataandaddData) never carries NaN masks or alpha over — checked for vertex/volume, scalar/RGB/2D.Full suite green locally in a fresh env (Python 3.12). Deliberately not touching the opacity slider (#685),
headless.py(#677) or reference images (#672).🤖 Generated with Claude Code
https://claude.ai/code/session_01KdjyZMFXsn6mRMJu9i8ige