Conversation
The viewer could fade the data layer over the curvature but never the cortex itself, so anything drawn inside the brain was invisible. A new surfaceAlpha uniform is multiplied into the fragment alpha of both surface shaders, driven by a surface_opacity slider in the surface controls and defaulting to 1.0 from [webgl_viewopts] in defaults.cfg. The opaque path is unchanged: the material only switches to transparent with depth writes off while the value is below 1, and that toggle fires on the state flip rather than every frame, so an opaque render goes through exactly the code it did before. Because view_props enumerates the surface controls dynamically, the slider is reachable from Python through _set_view and save_3d_views without further plumbing. Closes #353. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 20, 2026
mvdoc
added this pull request to stack #744
September 20, 2026 01:08
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The default-value test depends on the user’s local configuration and is therefore not hermetic.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
What changed in this PR
Adds configurable cortical-surface opacity to the WebGL viewer while preserving opaque rendering by default.
Changes:
- Adds shader opacity handling and dynamic transparency/depth-write state.
- Exposes and documents the
surface_opacitycontrol. - Adds default-value and headless rendering tests.
| File | Description |
|---|---|
docs/userguide/webgl.rst |
Documents surface opacity usage. |
cortex/webgl/resources/js/shaderlib.js |
Applies opacity in surface shaders. |
cortex/webgl/resources/js/mriview_surface.js |
Adds the control and rendering-state changes. |
cortex/tests/test_surface_opacity.py |
Tests defaults and translucent rendering. |
cortex/defaults.cfg |
Sets the default opacity to 1.0. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
`getTexture` reads the drawing buffer back with `gl.readPixels` into an
`ImageData`. The buffer holds premultiplied colors; `ImageData` is straight
alpha. While every fragment the viewer drew was opaque the two agreed, so
the mismatch never showed. With `surface_opacity` below 1 it does: a
fragment written as (alpha*color, alpha) is exported as though its color
were already that dark, and then faded a second time by whatever composites
the PNG. A 0.25 render came out a flat washed-out silhouette with the
curvature shading crushed out of it, rather than a faded brain. Undo the
premultiplication after readback; fully opaque and fully empty pixels --
every pixel of an opaque render, since the render target has no
multisampling -- are left untouched.
The test could not see this: "translucent pixels moved toward the
background color" is satisfied by the double-darkening too. It now checks
what the feature actually promises -- the slider value reaches the fragment
alpha over the silhouette, and the color there is still the one the opaque
render gives.
Also:
- Drop `material.needsUpdate` from the prerender loop. In three r69
`transparent` and `depthWrite` are render state, read per draw call and
when objects are sorted into the opaque/transparent lists, not part of
the compiled program, so the flags can just be assigned. Setting
`needsUpdate` forced a full shader recompile on every crossing of 1.0.
- Fall back to opaque on a `surface_opacity` that does not parse. A NaN
uniform makes the whole cortex invisible with nothing to point at why.
- Add `surface.{subject}.surface_opacity` to `ViewParams`, which the docs
and the test both pass through `_set_view`.
- `cortex.export` is not in the API reference, so the `:func:` xref for
`save_3d_views` had no target; make it a literal. Note in the docs that a
translucent render exports its opacity as alpha.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ny4AHfNPZT4ma3GeV7BUDk
`cortex.options.config` has already overlaid the user's `options.cfg`, so the check was reading the effective setting rather than the shipped one. A contributor who uses the override the docs now advertise -- say `surface_opacity = 0.5` -- failed the test. Reproduced, then confirmed the parser reads `cortex/defaults.cfg` alone: it passes with that override in place and still fails if the shipped default changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ny4AHfNPZT4ma3GeV7BUDk
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.
Stack
Merge bottom-up; each PR is based on the one above it.
The viewer could fade the data layer over the curvature but never the cortex
itself, so anything drawn inside the brain was invisible. This adds a
surfaceAlphauniform multiplied into the fragment alpha of both surfaceshaders, driven by a
surface_opacityslider in the surface controls anddefaulting to
1.0from[webgl_viewopts]indefaults.cfg.The material switches to transparent with depth writes off while the value is
below 1. Both flags are plain render state in the bundled three r69 — read per
draw call, and again when objects are sorted into the opaque and transparent
lists, rather than compiled into the program — so they are assigned every frame
instead of guarded, and an opaque render reaches exactly the GL calls it always
did. The visual-regression suite is unaffected.
Because
view_propsenumerates the surface controls dynamically, the slider isreachable from Python through
_set_viewandsave_3d_viewswith no furtherplumbing.
The export path
Turning the surface translucent surfaced a latent bug one layer down.
getTexturereads the drawing buffer back withgl.readPixelsinto anImageData: the buffer holds premultiplied colors,ImageDatais straightalpha. Everything the viewer drew used to be opaque, so the two agreed and the
mismatch never showed. Below 1 it does — a fragment written as
(alpha*color, alpha)was exported as though its color were already that dark,then faded a second time by whatever composited the PNG, so a 0.25 render came
out a flat washed-out silhouette with the curvature shading crushed out of it
rather than a faded brain.
The premultiplication is now undone after readback. Fully opaque and fully
empty pixels are skipped, and since the render target has no multisampling that
is every pixel of an opaque render, so existing output is bit-identical.
Closes #353.
Testing
cortex/tests/test_surface_opacity.py: the shipped default is 1.0, and atranslucent render puts the slider value in the fragment alpha across the brain
silhouette while the colour there stays the one the opaque render gives. That
second half is what pins the straight-alpha export — an earlier version of this
test only checked that translucent pixels moved toward the background colour,
which the premultiplied output satisfied just as well. The headless leg needs
Playwright and skips without it.
test_visual_regression.py,test_webgl_headless.pyandtest_export.pypassin full locally against the stored references.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Ny4AHfNPZT4ma3GeV7BUDk