Repository navigation
NF webgl views animation gui - #752
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Anonymized exports expose original subject IDs, and flat-keyframe reframing currently updates the wrong camera target.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (3)
What changed in this PR
Moves saved-view and animation workflows into the WebGL viewer, with matching Python and browser interpolation and rendering.
Changes:
- Adds browser view management, animation editing, ZIP/MP4 rendering, and download capture.
- Adds subject-aware default camera views and quickflat-compatible framing.
- Adds shared interpolation logic and extensive tests/documentation.
| File | Description |
|---|---|
.gitignore |
Ignores uv.lock. |
docs/database.rst |
Documents views, framing, and animations. |
cortex/export/headless.py |
Captures browser downloads in headless sessions. |
cortex/export/save_views.py |
Adds default views and subject framing. |
cortex/tests/test_default_views.py |
Tests default views and framing. |
cortex/tests/test_interpolation.py |
Tests interpolation modes and channels. |
cortex/webgl/interpolation.py |
Implements Python keyframe interpolation. |
cortex/webgl/view.py |
Integrates saved views, framing, and animations. |
cortex/webgl/template.html |
Loads new browser modules. |
cortex/webgl/resources/css/mriview.css |
Styles animation and view panels. |
cortex/webgl/resources/js/axes3d.js |
Releases rendered WebGL targets. |
cortex/webgl/resources/js/interpolation.js |
Implements browser interpolation. |
cortex/webgl/resources/js/movement.js |
Separates folded and flat targets. |
cortex/webgl/resources/js/mp4mux.js |
Adds H.264 MP4 muxing. |
cortex/webgl/resources/js/mriview.js |
Adds flat framing and camera controls. |
cortex/webgl/resources/js/mriview_surface.js |
Computes flat-surface bounds. |
cortex/webgl/resources/js/viewtools.js |
Implements view and animation UI. |
cortex/webgl/resources/js/zipstore.js |
Packages PNG frames into ZIP files. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+360
to
+364
| # Views saved in the filestore, for the "camera > views" menu. Only the | ||
| # subjects this viewer displays are read. | ||
| my_viewopts["saved_views"] = _load_saved_views(subjects) | ||
| my_viewopts["quickflat_size"] = {subj: _quickflat_size(subj) | ||
| for subj in subjects} |
| // Saved views and the keyframe animation panel (resources/js/viewtools.js). | ||
| // These go in sub-folders/buttons rather than into cam_ui.add directly, so | ||
| // they do not show up in JSMixer.view_props as capturable properties. | ||
| jsplot.viewtools.installCameraUI(this, cam_ui); |
Comment on lines
+680
to
+682
| this._el("anim-width").val(this.viewer.imageWidth || 2400); | ||
| this._el("anim-height").val(this.viewer.imageHeight || 1200); | ||
| this._el("anim-flatmatch").prop("checked", false).on("change", function() { |
Contributor
|
@claude review |
|
Claude finished @mvdoc's task in 3m 34s —— View job Claude's reviewTodo list
Verified findings from the Copilot reviewI re-checked all three of Copilot's findings directly against the current code on this branch — all three are real and still present. 1. my_viewopts["saved_views"] = _load_saved_views(subjects)
my_viewopts["quickflat_size"] = {subj: _quickflat_size(subj) for subj in subjects}
2. Unconditional jsplot.viewtools.installCameraUI(this, cam_ui);This has no guard, unlike the 3. Editing width/height after ticking "match quickflat size" doesn't reframe flat keyframes (Medium) — Other observations (no new bugs found)
Testing noteI could not execute SummaryThe three Copilot-flagged issues are all confirmed real and worth fixing before merge — #1 (anonymization leak) is the most important since it defeats the stated purpose of |
Contributor
I've tested @marklescroart 's WebGL animation feature. Overall, it works very well. The animation only controls the 3D view of the brain, not the data (brain map) shown on it, so it isn't affected by whichever map is currently displayed. That made it easy to create exactly the same animation for different datasets just by switching between them, and it doesn't conflict with 4D datasets or with the frames of a timeseries. In the future, it might be nice to animate both at once (camera movement together with 4D data changing over time). I'd like to raise one issue with keyframes, specifically deleting them. Keyframes are very intuitive to use, and the design matches how keyframes work in other 3D animation tools. However, the keyframe markers (yellow dots on the timeline) don't line up exactly with the frames they represent. For example, I made keyframes at frames 0, 10, 20 and 30 and moved the slider to frame 20, but the slider and the yellow dot (for frame 20) were not in the same place. This makes keyframes hard to delete. To delete the keyframe at frame 20, I naturally tried to click its yellow dot, but clicking the dot doesn't select it. Instead, I had to click frame 20 on the timeline itself, and because of the offset it was hard to land exactly on frame 20; sometimes I clicked frame 19. I'd suggest making the markers line up exactly with the timeline, and letting users click a marker to jump to that keyframe! Thanks for this amazing feature! |
Saving views and building animations previously required driving the
viewer from an interactive python session: _capture_view, _set_view and
make_movie_views all live on the JSMixer handle. The views/ folder that
every subject has in the filestore was invisible from the browser, since
db.get_view needs a live viewer handle just to read a JSON file.
This moves the whole loop into the viewer, with python needed only to
pull results back out and to write PNGs to disk.
Saved views
show() and make_static() read the views/*.json of the subject(s) the
viewer displays -- `subjects` is the list already used to build the CTM
packs, so the filestore is never scanned wholesale -- and ship them to
the browser inside the existing viewopts dict, which needs no template
plumbing and works in static exports too. Each becomes a button under
camera > views.
save view
Captures the current view in javascript, into viewer._new_views. That
stays in the browser, separate from the views loaded at startup, until
the new JSMixer.retrieve_new_views() asks for it -- so saving a view in
the GUI never touches the filestore. The javascript capture mirrors
_capture_view, discovering properties from the live menu tree and
keeping the literal {subject} placeholder in the keys, so views
interchange with the existing python API and with saved view files.
create animation
A panel with frame/first/last/fps fields, a frame slider marked with a
yellow dot per keyframe, and add/clear keyframe, play and render.
Scrubbing, playback and rendering all run through one setFrame ->
interpolate -> apply path, reusing the viewer's own _animInterp so
camera.azimuth still takes the short way around. Values that cannot be
blended (booleans, strings, and the discrete `layers`, which recompiles
the shaders on assignment) hold their starting value, the same rule
_get_anim_seq uses.
Rendering posts frames one at a time -- chained, never parallel, since
both the webgl readback and the upload are async -- to a new /movie
endpoint. It is deliberately separate from MixerHandler.post, which pairs
uploads with filenames by queue order that a browser-driven loop cannot
keep in step.
The server binds all interfaces and serves the page unauthenticated, so
the per-session token only keeps unrelated local processes out. The real
containment is the new movie_dir argument to show(): the browser sends a
path relative to a root python chose (cwd by default) and the handler
refuses anything resolving outside it.
setup.py needs no change; its resources/js/*.js and *.css globs already
cover the new files, and htmlembed inlines them generically.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two gaps in the saved-views GUI. save_new_views() retrieve_new_views() handed back a dict and then told the caller, in its docstring, to json.dump each entry into <filestore>/<subject>/views/ by hand -- leaving them to get the path convention and the name checking right. cortex.db.save_view cannot be reused for this: it calls vw._capture_view() and stores the *current* view, not a dict it is given. The new handle method writes each view to the same location the rest of pycortex reads views from, so they appear in the camera > views menu of every viewer opened for that subject afterwards. Names come from a text field in the browser, so each is checked against the same pattern MovieHandler uses before anything is written; validating every name and destination up front also means a clash partway through cannot leave some views stored and others not. Unlike db.save_view it creates the views directory, which is otherwise a latent FileNotFoundError for a subject imported without one. A stored view is no longer "new": promoteNewView moves it out of the browser's _new_views and into the views folder, so retrieve_new_views() means exactly "not yet on disk" and a second save is a no-op. Wider view names dat.GUI puts a controller's label in a fixed width: 40% column sized for a slider in the remaining 60%, which truncated any view name longer than about fifteen characters for no reason. The views folder now tags its <li class="folder"> from javascript -- dat.GUI gives folders no distinguishing class -- so the stylesheet can widen and centre just those rows and leave fold/reset/inflate and Save image alone. Checked in a browser against the real dat.gui + menu.js: a function controller's row is already clickable in full and the div in .c is empty for these buttons, so the name can simply take the whole row. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Commit d49aaec left an unterminated conflict marker pair at lines 905-906 of cortex/tests/test_webgl_headless.py: <<<<<<< HEAD ======= There is no closing marker. The module does not parse, so pytest cannot collect any test in it -- not only the saved-views and animation tests added on this branch, but every pre-existing alpha, opacity and addData regression test in the file as well. Remove both lines. The same merge also left two groups numbered "Group 10", so renumber the second one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The animation panel interpolated linearly between the two bracketing
keyframes, so the brain changed direction abruptly at every keyframe.
Add eight per-keyframe interpolation modes, following the way Adobe
describes keyframe interpolation -- a mode names how a keyframe is
entered and how it is left, so the curve between two keyframes depends
on the pair of modes at its ends -- with cubic Hermite added.
A "smoothing" dropdown in the panel sets the mode of the keyframe under
the playhead, or the mode new keyframes will be given. Bezier is the
default: it carries velocity smoothly through the interior keyframes and
its automatic tangents go flat at local extrema, so the camera never
swings past a pose you set.
Smoothing needs the keyframes on both sides of a keyframe to compute its
tangent, so a bracketing pair is no longer enough. Each property becomes
its own one-dimensional channel -- one per component for the vectors --
spanning the whole keyframe list. Booleans, strings and the discrete
`layers` still step; camera.azimuth is unwrapped so a spin takes the
short way around.
The same eight modes are available from python, so a movie rendered with
make_movie_views matches the preview played in the browser. That means
two implementations of the same arithmetic, in cortex/webgl/
interpolation.py and resources/js/interpolation.js; they use identical
closed forms and a headless test asserts they agree numerically.
_get_anim_seq keeps its existing pairwise path verbatim for the
whole-animation easings ('linear', 'smoothstep', 'smootherstep'), which
have no per-keyframe equivalent, and only takes the new path when a mode
is named. Frame times are generated the same way in both, so frame
counts do not move.
Four deliberate deviations from the reference implementation, each
commented at the site:
* Control-point offsets are signed. The reference takes an unsigned
square root for the value offset, putting the handles of a keyframe
with a negative derivative off its own tangent line and breaking the
C1 continuity the scheme exists to provide.
* The Bezier parameter is solved for rather than assumed equal to
normalized time. The reference returns early, leaving its own
root-finding loop unreachable; both coordinates of a cubic Bezier are
cubic in the parameter, so the two agree only when the handles happen
to be evenly spaced in time.
* A Linear keyframe followed by one entered smoothly produces a Hermite
segment entered along the chord. The reference enumerates no segment
for that combination, desynchronizing its segment and end-time lists.
* A lone keyframe yields a constant interpolator instead of a segment
list that is built and then discarded.
One subtlety worth recording: half a turn is equally short either way,
and Viewer._animInterp breaks that tie by travelling against the sign of
the raw difference. A symmetric shortest-angle formula reverses one of
the two cases, which would flip a 180-degree spin that used to work, so
shortest_step reproduces the existing tie-break and is tested against it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A freshly imported subject had nothing under camera > views until someone saved something there. Offer nine standard views for every subject instead: dorsal, ventral, lateral_left and lateral_right on the fiducial surface, the same four inflated, and flat. They are assembled from the tables save_views already keeps for save_3d_views rather than spelled out again, so the camera conventions stay in one place. dorsal/ventral/lateral_left/lateral_right are the existing top/bottom/left/right angles under anatomical names, which is worth stating precisely: the viewer puts the camera at radius * (sin(alt)cos(azi+90), sin(alt)sin(azi+90), cos(alt)) with up fixed at +z, and surfaces are in surface RAS, so azimuth 90 sits left of the brain and 270 right of it, altitude 0 above and 180 below. Because lookAt resolves the degenerate straight-up and straight-down cases through the azimuth, anterior lands at the top of the image at azimuth 180 seen from above and at azimuth 0 seen from below -- which is what "frontal lobe pointed up" needs. test_default_views.py reproduces that arithmetic and asserts each name against it, so the claim is checked rather than just documented. A view stored in the subject's filestore views/ directory under one of these names replaces that default, for that subject only, leaving the other eight in place. Saving over a default now also takes effect immediately: the views menu looks up what a button applies when it is clicked instead of capturing it, so a re-saved name no longer keeps applying the old view until the page is reloaded. Without a flat surface there is no flat view to offer, and the inflated views sit at full inflation rather than half -- the same correction save_3d_views makes. The flat view uses the viewer's established flatmap preset. It cannot match quickflat.make_figure exactly, since the viewer draws the flat surface through a 45-degree perspective camera while quickflat rasterizes it orthographically. What can be matched is the pixel size: make_png writes the flatmap image itself, whose width follows from the subject's flat surface bounding box, so _quickflat_size works that out with quickflat's own arithmetic and ships it in viewopts. The animation panel shows it in the render form once an animation actually reaches the flat surface, which is the only time matching it means anything. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…angle Two things about the flat view, plus the five test failures this branch was carrying. Rendering a flat keyframe did not produce the image quickflat.make_png writes: the flatmap came out small and low in the frame. Nothing ever framed it. The camera target stayed wherever it was -- the flat surface sits some sixty units below the origin, and movement.js compensates with a hardcoded _flattarget.y of -60 that happens to land within 0.1 of S1's true centre and would be wrong for any other subject -- and nothing set the camera distance at all. quickflat has no camera: it maps the flat surface's bounding box onto the bounds of the image. The viewer can reproduce that exactly, because a plane square-on to a perspective camera projects as a uniform scaling. Surface.flatBBox works out where the flatmap actually is (the flat morph target lives in the mixSurfs1 attribute, so the geometry's own bounding boxes describe the fiducial surface and say nothing about it), and Viewer.flatFraming turns that into a target and a radius for a frame of a given shape: filling it exactly at the flatmap's own aspect ratio, which is what _quickflat_size reports, and fitting inside any other shape rather than cropping to it. Rendered at that size the frame is make_png's png, same position and same scale, to a mask overlap of 0.99. The framing is applied on request, never behind a caller's back. The views menu applies it, JSMixer.fit_flat_view applies it from python, and getImage re-frames what was framed for the image it is about to write, since that need not have the shape of the window. _set_view does not, so save_3d_views and anything else driving the viewer renders exactly what it always did -- the stored visual regression references are untouched, which is the evidence for that claim. The animation panel offers the framing behind a "match quickflat size" box, unticked, which fills in the size and frames flat keyframes for it, including any already down. The second thing is the transition into that view. The controls pin the camera square-on to a flat surface and discard whatever azimuth and altitude they are given (setAzimuth/setAltitude in movement.js), unless the surface is tilt-enabled -- but below full unfolding those same setters write the *folded* angle. So a flat view carrying azimuth 180 gave an animation something to interpolate towards on the way in: the brain spun as it flattened, fighting the blend setMix performs over the same quantity, and the folded angle it would return to was overwritten on the way out. A flat pose now carries neither property, in the default views, in angle_view_params, and in both capture paths. That leaves the interpolators needing a rule for a property only some keyframes carry. Each channel is now built from the keyframes that do carry it -- in viewtools.js, in interpolation.py, and in the legacy easings of _get_anim_seq, which raised KeyError on a missing key -- so a property every keyframe carries behaves as it always did, one carried by a single keyframe is constant, and a flat keyframe is simply not a knot on the angle's curve. Folded to flat holds the angle; folded to flat to folded sweeps smoothly across the whole span. The five failures: Two tests read handle.send()'s reply as a value rather than the one-entry-per-client list it is. _js_run unwraps it, the way _js_attrs already did for query. save_new_views refused a view name starting with an underscore, which the filestore itself accepts -- test_saved_views_are_loaded_into_the_viewer writes one straight to disk and the viewer loads it. Only a leading '.', which is what makes '..' possible, is kept out of the first position now. make_static(html_embed=False) wrote tornado's bytes to a text-mode file. Latent since 4906f5f; the two static-viewer tests added on this branch are the first callers to exercise that path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The animation panel's render button used to POST every frame to a /movie endpoint, which wrote it under show()'s movie_dir on the machine serving the viewer. That was a file-write primitive on the server -- guarded by a token and a path check, on a server that binds every interface unauthenticated -- and it put the frames on the wrong machine whenever the browser was not running where python was. Now the movie is built in the page and handed over as one download, the way the viewer's "Save image" button works: it lands wherever that browser saves downloads, and the server writes nothing. One download per frame is not an option, since browsers throttle or prompt for hundreds of them, so the render form offers two single-file formats. PNG frames (.zip): one lossless, transparent PNG per frame -- the frames to use when they must match quickflat.make_png. zipstore.js writes stored entries, since PNG is already compressed, which leaves only a CRC-32 per frame to compute; the frames stay as Blobs and the archive is a Blob of parts, so the image bytes are never copied into one buffer. Classic ZIP only, with a clear message past 65,535 frames or 4 GiB. MP4 video: H.264, encoded by the browser's own WebCodecs VideoEncoder and packed by mp4mux.js, a small muxer for one constant-frame-rate track -- ftyp, moov, then mdat, so the file can play before it has loaded. WebCodecs exists only in secure contexts (localhost, https, file://), so the option is disabled elsewhere; the encoder's size limit is checked before the first frame renders; and since H.264 has no alpha channel and needs even dimensions, frames are laid on black and an odd size gets a row or column of padding. With no server involved, rendering also works in make_static viewers. movie_dir, MovieHandler, the /movie route and viewopts.movie_post are gone; they were added on this branch and never released, so there is nothing to deprecate. Axes3D.getImage allocated a WebGLRenderTarget on every call and never freed it -- a few megabytes of GPU memory per rendered frame. It now disposes of it once the pixels are read back, which Save image and save_3d_views benefit from too; the visual regression references pass unchanged. The headless harness now captures downloads (headless_viewer's download_dir, wait_for_download), so the tests check the files a person would actually get: every CRC in the zip, the MP4's box structure and sample table, and that a <video> element plays it back at the right size and duration. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
camera.target -- the point the camera orbits and looks at, a hidden menu entry -- stood for two things. LandscapeControls keeps a folded target and a flat target, and shows their blend by how flat the surface is; a write to camera.target went to whichever matched the unfold state at the moment it was made. That was harmless for a view applied all at once, but not for an animation. Between a folded keyframe and a flat one every frame is partly unfolded, so each interpolated target was written into the folded copy: the brain slid down ahead of the flattening -- 88% of the way at frame 20 of 30 -- and after unfolding again it sat some 53 units below where it started. The same class of bug as the camera angle, fixed earlier on this branch, for position. camera.target now always means the folded target, and a new hidden camera.flat_target always means the flat one (setFoldedTarget and setFlatTarget in movement.js). Views and keyframes carry both, each is interpolated on its own and written only to its own copy, and the transition is simply the blend setMix already performs. Panning still goes through setTarget, so dragging behaves as it did. A flat view saved before camera.flat_target existed stores its flat target as camera.target, since that is where it went once the surface was flat, so a flat view carrying camera.target and no camera.flat_target is read that way -- in vt.applyView and in JSMixer._set_view alike. save_3d_views depends on that reading for the target it passes with its flatmap: regenerating every visual regression reference in place leaves all 28 byte-identical. The flat target also no longer starts at a hardcoded y = -60, a guess within 0.1 of S1's flatmap centre and wrong for any other subject. It starts at the flatmap's real centre, measured when the viewer loads. The surface is folded then, and flatBBox measures through the current transforms, so Surface.flatViewBBox poses the pivot groups the way the flat view would -- flat, pivoted 180, unshifted -- measures, and puts them back before anything is drawn. It agrees with the fitted flat view to the digits shown. The built-in flat view accordingly names no target, and setting it from python lands the flatmap centred rather than sixty units low. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The default views named camera angles and an unfold amount, but no camera radius, so clicking one kept whatever zoom the viewer happened to have -- a view could not reliably return the same scene, and so could not be relied on to render the same images twice. They also all aimed at the origin, while a brain need not be centred there: S1's is 22 mm anterior and 11 mm superior of it, which left it off-centre in every view. Every default view but flat now carries a camera target and radius. The target is the middle of the surface the view shows; the radius is the smallest distance at which every vertex of that surface falls inside 85% of a 4:3 frame, worked out through the viewer's own perspective camera (camera_basis, moved here from the tests so the fit and the anatomical name checks share one model of it). A wider window just leaves more room either side. flat keeps the framing it fits to the window itself. No constant could do this for every subject -- brains differ in size, pycortex serves macaque data as well as human, and an inflated surface is a different shape again -- so the framing is fitted to each subject's own surfaces, as the viewer lays them out. That is not the surface files as they stand: brainctm packs a subject on its pial surface, with the white matter alongside and the folded brain drawn at their midpoint, and rescales the inflated surface one hemisphere at a time into the pial bounding box. Fitting the raw inflated file left the inflated views at about half size; _viewer_points reproduces the packing, and a test checks it vertex for vertex against brainctm's own output. Reading the surfaces takes about half a second, so the fit is cached as default_view_framing.json in the subject's cache directory, and refitted when a surface file or a framing constant changes. A filestore that cannot be written only loses the caching, and surfaces that cannot be read leave the views as they were rather than breaking a viewer. default_subject_views gains an optional subject and is otherwise unchanged; save_3d_views, which builds its views from the shared tables, is untouched, and the visual regression references pass as they are. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The animation panel's keyframe dots did not sit under the slider's thumb when the playhead was on their frame. Measured in headless Chrome, with keyframes at frames 0, 10 and 30 of 30, they sat 4 px left of the thumb at the start and 3 px right of it at the end -- more than a dot's width. The dots were inset by a hardcoded 7 px, a guess at half the browser's own thumb. A thumb's centre travels from half a thumb in at the first frame to half a thumb short of the end at the last, but the native thumb differs in size and inner padding from one browser and platform to the next (about 20 px here), so no fixed inset suits every browser. The slider now draws its own 12 px thumb and track, and a single --anim-thumb custom property sizes both the thumb and the dots' inset, so the two cannot drift apart. Restyling the slider also exposed w2ui's rule for every input -- a border and padding the native slider ignored, and whose 1 px border would have moved the thumb in from the ends -- so that is undone for it. The offsets are now 0 px at every keyframe. The dots are also clickable: a click stops playback and puts the playhead on that keyframe through the same path as typing a frame number (AnimationPanel.goToFrame, which both now use), so the slider, the frame field and the smoothing dropdown all follow. Each dot takes clicks in a 12 px area around its 6 px mark, laid out in the band below the slider so it never covers it; the dots' container still passes everything else through. The tests find the thumb and the dots by colour in a screenshot of the slider, click the dots, and drag the slider over them. That needs the page itself, which sync Playwright only lets its own thread touch, so the headless harness gains _PlaywrightThread.run_on_page, which runs a function on the worker thread and returns what it returns. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…nges Three issues from review of this branch. Anonymized exports leaked the real subject IDs. make_static(anonymize=True) renames subjects S0, S1, ... in the surface files and the dataset metadata, but the saved views and the quickflat size hint added on this branch were still keyed by the real IDs -- putting them back into the page, and leaving the viewer unable to find either entry, since it looks them up under the anonymized surface names (so an anonymized export's views menu came out empty). They are now keyed by the page's own names. Doing that exposed an older inconsistency: make_static numbered the anonymized names once in the order of a set, which changes from one process to the next, and once in sorted order, so a multi-subject export could give one subject two names. It now builds one sorted mapping and uses it for the files, the metadata and the viewer options alike; a single-subject export's names are unchanged. A custom template.html written before viewtools.js existed stopped the viewer from opening at all: mriview.js called jsplot.viewtools.installCameraUI unconditionally. The call is now guarded, so such a viewer opens without the views menu and the animation panel, and rendering reports a missing zipstore.js or mp4mux.js in the panel's status line instead of throwing. With "match quickflat size" ticked, changing the render size did not re-frame the flat keyframes already laid down, so a render at a size typed after ticking the box used framing meant for the previous shape of frame. Typed sizes now re-frame them, as does setRenderSize, which sets the fields from code without firing their change events; and render() re-frames first, so a size that reached the fields any other way is still honoured. Each fix has a test that fails without it: an anonymized export whose subjects and viewer options must name the same subjects, a shadowing template with the new script tags removed, and the size set from code, typed, and changed with no event at all. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
marklescroart
force-pushed
the
webgl-views-animation-gui
branch
from
October 1, 2026 06:09
0a15e9e to
4bfe68d
Compare
Contributor
Author
|
Latest commits should rebase and address all above concerns. Waiting for tests to run on git (all tests ran locally and passed), then will squash and merge. |
Contributor
|
You'll probably need to add Also, I am going to merge the bumpy flatmap refactor now. It will introduce a small merge conflict in |
Contributor
Author
|
Errrr don't know how to add words to codespell, will look into it, but why does it hate trough? That's a real English word, legitimately used. These will take me a min to reconcile, I'm teaching today and have meetings. |
Contributor
lmao dumb bot |
Brings in #695 (unified NaN and alpha handling across quickflat, WebGL and RGB dataviews), #755 (setup-uv bump) and #720 (bumpy flatmap redesign). Merged rather than rebased: the branch has an open, reviewed pull request, and pycortex squash-merges pull requests, so a merge commit here never reaches main's history while a rebase would mean another force-push under the review. One conflict, in cortex/tests/test_webgl_headless.py, where both lines of work appended tests at the end of the file. Both are kept: main's "Group 6: Bumpy flatmap", then this branch's sections. No names collide. The rest merged cleanly as text, including the files both sides changed: mriview.js, mriview_surface.js and docs/database.rst. #720 reworks how the bumpy flatmap's relief is built and adds surface-info attributes to the surface packs, but leaves alone what this branch's flat framing and default-view framing rely on: _makeFlat, flatoff, the pivot groups, and brainctm's renormalized packing. Checked rather than assumed: the quickflat-match tests, the vertex-for-vertex check against brainctm and main's own bumpy-flatmap test all pass on the merged tree. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
"a peak and a trough" describes the test curve correctly, but codespell flags "trough" as a misspelling of "through" and fails the spelling check. Reworded to "a peak and a dip" rather than adding "trough" to the repo-wide ignore list, which would stop codespell catching the far more common case: "trough" written for "through" in prose. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Changed 'trough' to 'dip' (eff off codespell), dealt with conflicts with main, should be ready to merge once tests run. |
Contributor
|
I'll increase the test timeout to 40 mins and re-run it. (It looks like this branch adds several browser-based tests, so no surprise that we need more time.) |
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.
Saving views and building animations used to require driving the viewer from Python (
_capture_view,_set_view,make_movie_views), and a subject'sviews/folder was invisible from the browser. This branch moves that loop into the viewer. It also makes flat renders (when they are part of an animation) matchquickflat.make_pngon request.What's new for users
Views in the browser
views/folder, including inmake_staticexports.JSMixer.retrieve_new_views()returns it to Python, andsave_new_views()writes it to the filestore, where every later viewer of that subject picks it up. [NOTE, Mark rephrasing Claude's text: to permanently save new views, you have to callsave_new_views()method of the python handle to the webgl viewer. This is for safety (we can' t have web page controlling python process on host), and may be a model for saving other elements added in webgl viewers (e.g. ROIs, sulci, other labels).brainctmpacks them — pial-based, with the inflated surface rescaled into the pial box — and cached asdefault_view_framing.jsonin the subject'scache/directory.Animation panel (camera › create animation)
make_movie_viewsrenders what the panel previews. (MARK NOTE: I'm not 100% sure these are working properly)make_png.localhost,https,file://); sizes the encoder can't handle are refused up front.Flat view matches quickflat on request
quickflatframes its image: centred, and as large as fits. Rendered at the subject's quickflat size, the frame matchesmake_png's PNG (mask overlap > 0.97 in tests).JSMixer.fit_flat_view()from Python, or match quickflat size in the render form (unticked by default), which also re-frames existing flat keyframes.getImagere-frames a fitted flat view for the image size it writes.Behaviour fixes
camera.target/camera.flat_target), so an animation into or out of the flat view no longer overwrites the folded pose. The flat target now starts at the real flatmap centre instead of a hardcodedy = -60.KeyErroron a missing key.Axes3D.getImageno longer leaks a WebGL render target per call (a few MB per rendered frame).make_static(..., html_embed=False)no longer fails writing Tornado's bytes to a text-mode file (a latent bug since4906f5f).API additions
JSMixer:retrieve_new_views(),save_new_views(),fit_flat_view().cortex.export.save_views:default_subject_views(has_flatmap, subject=None),default_view_framing(subject),camera_basis(),FLAT_INERT_PROPS, and the framing constants.cortex.webgl.interpolation(Python twin ofresources/js/interpolation.js).cortex.export.headless_viewer(..., download_dir=None), plusdownloads/wait_for_download()for capturing browser downloads in tests.interpolation.js,viewtools.js,zipstore.js(stored-ZIP writer),mp4mux.js(single-track H.264 MP4 muxer). No third-party code is vendored.Compatibility
save_3d_viewsoutput is unchanged. The visual-regression suite passes against the existing references, and regenerating all 28 in place reproduced them byte for byte.camera.flat_targetexisted still load as before:camera.targeton a flat view is read as the flat target.default_subject_views()called without a subject returns what it did before.Testing
make_pngoutput, end to end through the panel's render<video>element)brainctm's own packing, cache invalidation, and a real 4:3 render's marginsNotes for reviewers
mainhas moved since this branch forked. A trial merge against the last-fetchedmainis clean (four files are touched on both sides, none conflicting), butmainhas had more commits since, so GitHub's mergeability check is the one to trust.mainincludes Fix shader problem that broke 2-D vertex data #715 ("Fix shader problem that broke 2-D vertex data"), and this branch has strict xfails for the Vertex2D shader (Vertex2D objects do not render in webviewer #714), so it's worth re-running the suite after mergingmainin.🤖 Generated with Claude Code