Release v0.3.4: colored ROI outlines + 3 audit-followup fixes

Follow-up to the v0.3.3 adversarial audit (four findings from a full-project review):

1. Colored ROI outlines. The palette color was stored/exported and shown as a
   panel swatch but the baked outline was always white, so the swatch meant
   nothing on the surface. The adapter now strokes each ROI in its color over a
   white halo (kept legible on data + anatomy). Imported colors run through a
   hex-only safeColor() before entering the SVG style attribute.
2. Bezier/vertex consistency. deriveRoiFromLasso no longer attaches a fitted
   bezier when re-derivation from it encloses zero vertices — it would leave the
   source-of-truth curve disagreeing with the fallback lasso vertices. New
   headless test covers the drop + the normal-case equality.
3. Display-mode framing. _onMix now auto-frames only while in Draw mode, so a
   user unfolding the surface in Display no longer has the camera fight their
   zoom/pan every morph frame.
4. _worldOf hardening. Apply the flatoff offset to our own vector instead of
   mutating get_position()'s returned pos (may be shared/cached in mriview).

Suite 93 -> 94 JS tests. Bundle rebuilt (94,844 B).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: jackgallant

README.md

@@ -2,7 +2,8 @@
 
 In-browser **ROI drawing + export** for [pycortex](https://github.com/gallantlab/pycortex) WebGL
 viewers. Lasso a region on the flattened cortical surface; the stroke is fitted to a smooth,
-**editable bezier** that renders as a white outline + label **baked into the surface** (so it
+**editable bezier** that renders as a colored outline (its palette color, over a white halo for
+legibility) + label **baked into the surface** (so it
 occludes and morphs correctly), and exports to a portable JSON. The bezier is stored alongside the
 vertex set, so reloaded ROIs can be re-edited by dragging their control points.
 

adapter/pycortex-adapter.js

@@ -34,9 +34,18 @@ const COLLAPSE_WINDOW_MS = 8000;  // ...and on setData within this startup windo
 const DEFAULT_THICKMIX = 0.5;     // thick-surface blend fed to get_position (constant; we don't expose it)
 const FALLBACK_TEX_W = 1024;      // overlay viewBox width fallback when the surface reports no size
 const FALLBACK_TEX_H = 768;       // overlay viewBox height fallback
-const OUTLINE_STROKE_PX = 3;      // ROI white-outline stroke width, in overlay viewBox px
+const OUTLINE_STROKE_PX = 3;      // ROI colored-outline stroke width, in overlay viewBox px
+const OUTLINE_HALO_PX = 2;        // extra width of the white halo drawn under the colored stroke
+const OUTLINE_FALLBACK_COLOR = "#ffffff"; // stroke when an ROI has no (valid) color
 const LABEL_FONT_PT = 14;         // ROI label font size, in pt
 
+// A CSS hex color (#rgb / #rrggbb / #rrggbbaa) or the fallback. ROI colors are our own palette, but an
+// imported file could carry anything, and the value goes into an SVG style attribute — restrict it to
+// a hex literal so it can't smuggle in extra style declarations.
+function safeColor(c) {
+    return (typeof c === "string" && /^#[0-9a-fA-F]{3,8}$/.test(c)) ? c : OUTLINE_FALLBACK_COLOR;
+}
+
 // Vertex count of a THREE BufferAttribute (pycortex's old three.js lacks `.count`).
 function attrCount(attr) {
     if (attr.count !== undefined && !isNaN(attr.count)) return attr.count;
@@ -174,10 +183,13 @@ export class PycortexAdapter extends ViewerAdapter {
 
     // World position of geometry-local vertex `i` at the current mix (incl. the flatoff offset
     // so it lands on the *rendered* mesh, not floating above it). Mutates+returns this._v.
+    // Applies the flatoff offset to our OWN vector (never to get_position's returned `pos`, which
+    // may be a shared/cached vector inside mriview — mutating it would corrupt host state).
     _worldOf(pd, mw, i, surfmix, foy) {
         const gp = this.mriview.get_position(pd, surfmix, this._thickmix, i).pos;
-        gp.y -= foy;
-        return this._v.copy(gp).applyMatrix4(mw);
+        this._v.copy(gp);
+        this._v.y -= foy;
+        return this._v.applyMatrix4(mw);
     }
 
     projectVertices({ subsample = 1 } = {}) {
@@ -430,9 +442,16 @@ export class PycortexAdapter extends ViewerAdapter {
         for (const roi of rois) {
             const d = this._roiSvgPath(roi, W, H);
             if (d) {
+                // White halo under a colored stroke: the halo keeps the outline legible on any
+                // background (colored data or white anatomy), while the color carries the ROI
+                // identity the panel swatch shows. Same path `d`, drawn wider + white underneath.
+                const halo = doc.createElementNS(SVGNS, "path");
+                halo.setAttribute("d", d);
+                halo.setAttribute("style", "fill:none;stroke:#ffffff;stroke-width:" + (OUTLINE_STROKE_PX + OUTLINE_HALO_PX) + ";stroke-opacity:0.9");
+                shapesEl.appendChild(halo);
                 const path = doc.createElementNS(SVGNS, "path");
                 path.setAttribute("d", d);
-                path.setAttribute("style", "fill:none;stroke:#ffffff;stroke-width:" + OUTLINE_STROKE_PX + ";stroke-opacity:1");
+                path.setAttribute("style", "fill:none;stroke:" + safeColor(roi.color) + ";stroke-width:" + OUTLINE_STROKE_PX + ";stroke-opacity:1");
                 shapesEl.appendChild(path);
             }
             const ptidx = this._labelPtidx(roi.labelVert);

adapter/viewer-adapter.js

@@ -63,11 +63,11 @@ export class ViewerAdapter {
 
     /**
      * Create/replace a named overlay layer rendered INTO the surface (so it occludes and morphs
-     * like built-in ROIs). `rois` carries, per ROI, the boundary ring + label vertex (and, when
-     * present, an editable flat-UV `bezier` the adapter renders as a smooth cubic path); the
-     * adapter converts vertices/bezier→uv→layer geometry.
+     * like built-in ROIs). `rois` carries, per ROI, the boundary ring + label vertex + display
+     * color (and, when present, an editable flat-UV `bezier` the adapter renders as a smooth cubic
+     * path); the adapter converts vertices/bezier→uv→layer geometry and strokes it in the ROI color.
      * @param {string} name
-     * @param {Array<{name, outline:[{h,g}], labelVert:{h,g}, bezier?}>} rois
+     * @param {Array<{name, color?, outline:[{h,g}], labelVert:{h,g}, bezier?}>} rois
      */
     setOverlayLayer(_name, _rois) { throw new Error("ViewerAdapter.setOverlayLayer not implemented"); }
 

draw-pipeline.js

@@ -72,12 +72,17 @@ export function deriveRoiFromLasso(adapter, pts) {
 
     const lassoRing = buildOutline(pts, sel0);                       // px-space ring of the stroke
     const ringUv = ringToUv(adapter, lassoRing);
-    const bezier = ringUv && ringUv.length >= 3 ? fitClosedBezier(ringUv) : null;
-    // membership from the bezier when we have one; otherwise keep the raw lasso selection
-    const derived = bezier ? roiFromBezier(adapter, bezier) : null;
-    const sel = (derived && derived.total) ? derived : {
+    const fitted = ringUv && ringUv.length >= 3 ? fitClosedBezier(ringUv) : null;
+    // Prefer bezier-derived membership so the stored vertices match the editable curve. But only keep
+    // the bezier if it actually encloses something: a curve that re-derives to zero vertices (a very
+    // thin/tiny ROI the smoothing shrank past every vertex) would leave the bezier — the source of
+    // truth for the drawn outline and future edits — disagreeing with the fallback lasso vertices. In
+    // that case drop it, so the ROI stays a consistent (non-editable) vertex set.
+    const derived = fitted ? roiFromBezier(adapter, fitted) : null;
+    if (derived && derived.total)
+        return { left: derived.left, right: derived.right, outline: derived.outline, labelVert: derived.labelVert, bezier: fitted, total: derived.total };
+    return {
         left: sel0.left, right: sel0.right, outline: lassoRing,
-        labelVert: pickLabelVertex(sel0), total: sel0.total,
+        labelVert: pickLabelVertex(sel0), bezier: null, total: sel0.total,
     };
-    return { left: sel.left, right: sel.right, outline: sel.outline, labelVert: sel.labelVert, bezier, total: sel.total };
 }

index.js

@@ -95,7 +95,9 @@ class ROIDrawer {
         if (this._dm.noteMix(this.adapter.isFlat()).exit) { this.setMode("display"); return; }
         this._updateDrawActive();   // lasso turns on once the flatten finishes
         if (this.editOverlay.isEditing()) this.editOverlay.reproject();  // keep knots on the surface
-        this._frame();
+        // Auto-frame only while drawing (so Draw's flatten glide stays centered). In Display the user
+        // owns the camera; re-framing on every unfold-slider morph there would fight their zoom/pan.
+        if (this.mode === "draw") this._frame();
         this._renderStatus();
     }
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "pycortex-roidraw",
-  "version": "0.3.3",
+  "version": "0.3.4",
   "private": true,
   "type": "module",
   "description": "Pycortex WebGL viewer with in-browser ROI drawing + editing + export. Modular: pure core / viewer adapter / UI.",

test/draw-pipeline.test.js

@@ -101,6 +101,24 @@ test("backfillLabel: returns null for an empty/uv-less ring", () => {
     assert.strictEqual(backfillLabel(adapter(), []), null);
 });
 
+test("deriveRoiFromLasso: a returned bezier always matches the stored vertices; else it is dropped", () => {
+    // Force the fallback branch deterministically: a surface where the lasso projection still selects
+    // vertices (projectVertices non-empty) but the uv-space membership is empty (allVertexUV empty), so
+    // roiFromBezier re-derives to zero. The pipeline must NOT keep a bezier that encloses nothing.
+    const base = adapter();
+    const starved = Object.create(base);
+    starved.allVertexUV = () => ({ left: { idx: [], uv: [] }, right: { idx: [], uv: [] } });
+    const roi = deriveRoiFromLasso(starved, lassoUvRect(base, 0.25, 0.25, 0.75, 0.75));
+    assert.ok(roi.total > 0, "the lasso still selects vertices (fallback path)");
+    assert.strictEqual(roi.bezier, null, "a bezier that encloses nothing must not be attached");
+
+    // and in the normal case the attached bezier's re-derived membership equals the stored vertices
+    const ok = deriveRoiFromLasso(adapter(), lassoUvRect(base, 0.25, 0.25, 0.75, 0.75));
+    assert.ok(ok.bezier, "normal case keeps the editable bezier");
+    assert.deepStrictEqual(new Set(roiFromBezier(adapter(), ok.bezier).left), new Set(ok.left),
+        "stored vertices equal the bezier's own membership (no disagreement)");
+});
+
 test("round-trip: a drawn lasso's bezier re-derives a non-empty membership inside the lassoed region", () => {
     const a = adapter();
     const pts = lassoUvRect(a, 0.3, 0.3, 0.7, 0.7);