Repository navigation
Conversation
|
@claude review |
|
Claude finished @mvdoc's task in 1m 52s —— View job Review of #739 — the WebGL aligner
This is a substantial, well-documented replacement of the mayavi aligner. The docstrings are thorough and the test suite ( 1. 2. 3. 4. Unguarded Everything else — the coordinate-frame math ( No code changes were made — this was a review-only request, per the instructions ("review" → provide feedback, don't implement). |
|
This now adds a history panel so you can undo to any edit in the edit history |
|
@claude review |
|
Claude finished @candytaco's task in 1m 58s —— View job Review of #739 — the WebGL aligner (update)
I re-checked the four issues Copilot and my earlier review flagged (
Test coverage for all of this in New finding, in code not covered by the earlier reviews (the slice-view/crosshair/picking commits added since): Crosshair can desync from the slice-plane images if a pick happens before "ortho views" is ever turned on —
if (this._sliceviews) {
var slices = {x: voxel.x, y: voxel.y, z: voxel.z};
for (var name in this.sliceplanes) { ... .update(slices[name]); }
}But This isn't exercised by the new tests: Minor / non-blocking:
Everything else — |
Port of the mayavi aligner to the WebGL viewer: the functional reference volume stays on its own voxel grid, the pial and white matter surfaces are moved into its space by rotations and translations only, and they are cut off at the displayed slices so that their outline shows on each slice. - cortex/webgl/aligner.py: tornado server (reference mosaic, CTM pack, page, save endpoint), the world frame (voxel grid in mm permuted to RAS) and the JSAligner handle with tagged calls and frame waits - resources/js/aligner.js, aligner.html, resources/css/aligner.css: coronal, axial, sagittal and 3D views, cursor and slice navigation, mesh translation and rotation by mouse and keyboard with undo, colormap with range, brightness, contrast, gamma and flip, mesh color and opacity, a mode painting the volume onto the surface, saving - shaderlib.js: aligner_volume and aligner_mesh shader builders - menu.js: color picker controls - cortex.align.webgl_manual, [webgl_aligner] config defaults, docs and tests Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
The browser aligner refused to open a transform that had cached masks. It now opens, and deletes those masks when it saves: they were cut out of the reference volume through the alignment being replaced, nothing else invalidates them, and db.save_xfm refuses to write over a transform that still has them. - cortex/webgl/aligner.py: cached_masks and clear_masks, called from the save handler before db.save_xfm; the save response names what it deleted, and the page config lists the masks - the page warns, on opening a transform that has masks, that saving deletes them and that data masked with them has to be masked again - view_only stays an explicit choice rather than something masks force Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
The browser aligner replaces it, and it had not worked since mayavi stopped running on anything newer than Ubuntu 14.04. - drop cortex/mayavi_aligner.py, cortex.align.mayavi_manual and the [mayavi_aligner] config section - drop the mayavi sections of docs/align.rst and their screenshots, and point docs/transforms.rst at the browser aligner - the mayavi imports elsewhere (segmentation, surface display in freesurfer and database) are untouched, as are the mypy overrides they need Also make the aligner tests redirect the transform's mask paths into a directory of their own, so they neither depend on nor delete the masks another test file leaves in the bundled filestore. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
- WASD moves the mesh in the view under the mouse, like the arrow keys, in both cases with shift for the fine step - the colormap dropdown draws a strip of each colormap beside its name, through select2, the library the viewer's colormap picker uses; a css block in the shared template lets the aligner's stylesheet load last, since the viewer hides the select2 control it now needs - a transform field above the save button names what the alignment is saved under, so it can be saved as a new transform, leaving the one it was loaded from and its masks alone; the name is checked before it reaches the filesystem - an asterisk on the save button and in the title marks an alignment that differs from the one last saved - cortex.align.webgl_manual spells out every option rather than taking **kwargs menu.js keeps the controller of a button action, so its label can change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
A `layout` control chooses where the surface is shown. "4 panels" is the layout as it was. "panels + surface" keeps the three slices showing their planes and outlines and turns the corner the 3D view occupies into the viewer, so the mesh can be nudged in the slices while the data on the surface follows. "surface" gives that viewer the whole window, framed on the surface the way the viewer opens on one. Both carry the reference data through the alignment as it stands, saved or not, and redraw as the mesh moves, so an alignment can be judged from the pattern the data makes on the cortex before it is committed. Also corrects the documented mouse buttons of the 3D view: its controls are the viewer's, which pan on the middle button and zoom on the right. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
- replace the `view` and `layout` dropdowns with one `display` control: 3 ortho + 3D slices, 3 ortho + 3D brain, or the data on the surface - draw the data view's surfaces in the anatomy's own frame with the morph targets of the CTM pack, so they inflate and flatten there while the alignment only moves where the volume is sampled - add `unfold` and `pivot`, and cull the medial wall while the flatmap shows - take the keyboard back from the panel when the mouse enters a view, and start with a view under the keys - keep the open colormap list within the window, which was scrolling the page sideways when it reached past the right edge Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
- WebApp binds the loopback interface unless a caller asks otherwise; the viewer opens up to every interface only when domain_name names one - the aligner's save endpoint takes only posts carrying the token of the page it served, so nothing else reaching the port can write a transform - JSAligner.save waits for the save to land and returns the server's message, raising when it was refused - get_client hands the handle its server, which the documented open_browser=False flow needs for the tagged-reply loop - guard a zero vmin/vmax span in the aligner shader - drop get_local_client, which called an attribute WebApp does not have - add an edit history to the panel: a row per edit since load, with how far it moved and turned the surfaces; clicking one puts that alignment back - confirm closing the page while the alignment differs from the saved one Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
bind_local_sockets binds one socket per address the names in serve.LOCAL resolve to, all on one port: localhost, 127.0.0.1 and whatever this computer's hostname stands for. A name that does not resolve is passed over, and one that resolves to an address another name already covers is bound once, so a machine whose hostname is its loopback address still gets a server. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
- the link the servers print names this computer, which is what a port
forward from another machine is set up under; a machine whose name leads
nowhere falls back to localhost
- the history reads newest first
- a translation says where it took the surfaces along the anatomical axes
("2.00 mm left, 3.00 mm anterior"), a rotation how far and which way
round it turned them in the plane it was made in ("0.20 CW in coronal")
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
854a0fa to
430769b
Compare
Every handler, the websocket and the static files included, is wrapped in serve.TokenGuard: the address the server prints carries the token, the page it opens is handed it back as a per-port cookie, and a request with neither gets a 403. Build addresses with WebApp.url, which carries it; pass token="" to answer anything that reaches the port. The aligner's save token is now posted as save_token, so that it and the session token cannot stand in for each other. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
The picker sizes its parts by their content and lays them out to add up to its own width. The panel sits inside a w2ui one, whose reset counts border and padding within every width instead, which left the hue bar too wide to fit beside the saturation square and dropped it below the picker. The picker shows only while the pointer is over it, so the bar could not be reached: the picker closed on the way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
The corner of the four panels and the data view now draw the same surface, in the anatomy's own frame, so unfold, pivot and depth carry from one to the other and act on both. r, i and f jump to the folded surface, the inflated one and the flatmap, as they do in the viewer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
sliceplanes > ortho_views (or the v key) splits the canvas between the three slice planes, each drawn straight down its own axis by a camera of its own, and the 3D view, which keeps the last quarter, the camera the controls move and the surface as it has always been drawn. The keys that step through the slices act on the views they are showing. Data with no volume to slice, such as data on the vertices, leaves the surface in view instead, seen down each of the three axes. A view in jsplot.Axes3D can now carry a camera, a prepare callback and a surface index of its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
A pick in the 3D view takes the three slice views to the slices that point is on and draws a crosshair there, so the four views agree on where it is. The crosshair lies in the plane of the view that draws it and goes on top of the slice in a pass of its own, since the slice is transparent and is drawn over everything opaque. It is not the picker's own marker, which rides with the surface as it unfolds; the 3D view keeps that one. The picker draws the 3D view over the whole canvas, so a pick in the corner the split gives it is mapped there first, and a pick in a slice view picks nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
- turn the scissor test off once the split has been drawn, so that the picker's own passes cover the canvas instead of the last view of the loop, which is what kept a click on the brain in the corner from picking it - leave the crosshair where it is when a click picks nothing, rather than taking it away - read a click in a slice view against the frame that view is drawn in, and take the other two views to the point it lands on Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
- draw the crosshair from a scene of its own, laid over each of the four views once the rest of it is in, so that a point picked in a slice view is marked in the 3D one as well - give the key its own entry: menu.js calls a descriptor's action with no arguments, which threw on the pair that names a method - name the control "ortho views", and draw it as a switch - tell dat.GUI what a control was set to from outside it, so that the click after a key press is not spent asking for the value it already has Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
The picker marks the vertex on the surface, in the 3D view alone; the crosshair marks the point in the volume, in all four views. Both were up at once in the split, so the picker's marker stands down there and comes back when the 3D view has the canvas to itself. Its own passes now put back whatever visibility the markers had rather than turning them on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
A point picked with the 3D view to itself set the crosshair and left the planes alone, so the views opened with the crosshair over slices it is not on. The slices are now taken to it as they open, which is the only moment they can disagree: once open, a pick moves them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
A view takes anything it is not named as an attr, and the page is handed its attrs as JSON, which has no way to write an array: a view carrying one, as a 2D view does when it is given an alpha to override its colormap with, served a 500 and drew nothing. Arrays are now left out of what the page is sent, with a warning naming them, and numpy's own scalars go in as the python ones rather than being left out with them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
- a switch in the camera menu swaps the camera the controls move for one with no vanishing point, aimed by the same azimuth, altitude and radius; the radius sets the frustum there, so it goes on being the zoom, and the brain is the same size on screen across the switch - the lights are children of the camera and the renderer lights a scene with the lights it finds in it, so the camera moves into the scene with them - an isometric button, shown while that camera is the one looking, turns the view down the diagonal of the three axes Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
The page is laid out from the edges of the window, with the ends of the colorbar off it on purpose, so a page that scrolls shows a band of nothing and takes the brain out from under the pointer. The body already said so; the page itself now says it too, which holds where the body's own overflow is overridden by a stylesheet loaded after this one or by the browser. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
A "low poly" slider in the surface menu, up to 32 mm, draws the surface from faces cut over groups of that many mm. Its vertices are vertices of the surface itself, so only the triangles and the normals change and everything held per vertex, the data among it, stays where it is. - polyutils.decimate_faces groups the vertices of a surface by a grid, a group being as much of one box as is joined across the mesh, and writes the faces over the vertex nearest the middle of each group. A label groups may not cross keeps the two sides of a cut apart, which is how the medial wall is handled - utils.get_lod cuts them from a subject's ctm pack, in its numbering, and keeps them beside it; the server cuts a width the first time the viewer asks for it, and make_static writes the widths in utils.LOD_CELLS, which it names in the pack's json for a viewer with no server to cut more - the viewer swaps the faces it draws and works the normals out again from them, keeping the ones the surface came with to go back to at 0 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
- a "shading" control under "low poly": the surface is drawn as it was, or with each face taking the color read where its first corner is, lit from the face's own normal (flat), at its corners (gouraud) or at every pixel of it (phong) - one color to a face asks for a vertex per corner, so the drawn faces are written out that way and carry the position the face reads the volume at; it is done below a limit of faces, and for data in a volume, since data on the vertices is itself held a vertex at a time. Past that the lighting still changes and the color goes on being interpolated, which at that many faces to a pixel is what it looks like anyway - an "outline" toggle draws the surface again through a shader of its own, inside out and grown by a width of the screen, so a line is left around it and around the folds that turn away Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
A view of such data takes its color from what each vertex carries rather than from where the vertex reads a volume, and the shading left it alone: its shader had none of the modes, and the surface was never written out with a vertex per corner for it, so flat looked like smooth. Each frame of it now goes to the surface rather than to the geometry it is held in, and is written out for the faces as it arrives, and its shader lights a face from its own normal or at its corners as the other one does. Drawn flat at 8 mm, a hemisphere is solid patches: 46% of the pixels beside each other within the surface are the same color, against 3% smooth, which is what the test holds it to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
- `Save image.Antialias` renders the image 2, 3 or 4 samples across a pixel and averages each block back down to one, which is the only antialiasing a webgl 1 render target can be given - a sample's color is weighted by its alpha, so the edge of the surface keeps the color of the surface and fades into the transparent background rather than into black - the supersampled buffer is handed back as soon as it is read Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
430769b to
d077c10
Compare
Every answer from a page now carries the id of the request it belongs to, so a reply the aligner reads straight off the server's queue, after `send` has stopped waiting for it, arrives wrapped rather than bare. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
517d53f to
8189d19
Compare
- `polyutils.vertex_occlusion` bakes it a vertex at a time, following rays over the hemisphere of the normal through a voxel grid of the surface - `utils.get_occlusion` works it out for each surface the viewer mixes between (the folded surface, the pack's names, zeros for the flat one), in the pack's numbering, and caches a byte per vertex beside the pack; the server builds it on a thread the first time it is asked for, and make_static writes it with the pack - the values ride in spare components of attributes already there (the w of wm and of each mixSurfs, auxdat.z without a white matter surface), since the shaders are at the 16 attribute slots WebGL guarantees, and are mixed with the same weights as the positions, so a sulcus darkens as it folds and opens as it inflates Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
The server listens for the machine's name as well as for localhost, so that one address reaches it from this computer and from another alike; a machine whose name leads nowhere gets a localhost link. The browser the viewer opens itself is still sent to localhost. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
- the folded surface's occlusion is worked out on the pial and on the white matter surface, and the shader mixes the two by the depth the sheet is drawn at, as it mixes the position; the two values ride as two bytes in the w of the white matter position - a file written with the earlier layout is built again when asked for - `occlusion strength` scales how much of the light is taken out, from none at 0 to twice what was worked out at 2 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
Two tests fetched the viewer by a hand-built localhost address and were turned away with a 403 once every handler demanded the token. They take the address from the server now, and `WebApp.url` adds the token after a query the page already carries rather than with a second question mark. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
- `outline corners` set to round draws a dot of the line's width at each vertex of the faces being drawn that faces away from the eye, as the line is drawn from the faces that do, so the surface covers the dots except where they round off a corner of the silhouette - the dots are a point cloud over those vertices, made again when detail or flattening changes them, and kept out of the depth image the labels are hidden by - the line is grown by as many pixels up as across, from a viewport uniform `Surface.prerender` keeps current, which is also what sizes the dots; a saved image is drawn through `drawView`, so that runs for it too Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115CeZYMxr1wvRSL2V8meKZ
I had claude re-implement the mayavi aligner in webgl.
cortex.align.webgl_maual(<surface>, <transform>)gives a webgl viewer that can edit transforms.Updated to show the data also on the surface in real-time as you move the transform.
Controls:
WASD or RMB to translate, QE to rotate, hold shift to move in 1/10th size increments. I've specifically removed the scaling/stretching options.
Aligner_.TZfs.aligner-test.-.Vivaldi.2026-09-19.13-28-07.webm
Edit histories are tracked and can be undone to any edit
Aligner_ TZfs aligner-test - Vivaldi 2026-09-19 21-51-39.webm
(I am very proud of the PR title, which, unlike the code, came solely from my brain)