feat: fix skeleton UI problems

python/tests/skeleton_options_test.py

@@ -63,10 +63,17 @@ def test_skeleton_options(webdriver):
         s.layout = "3d"
         s.layers[0].skeleton_rendering.line_width3d = 100
     screenshot = webdriver.viewer.screenshot(size=[10, 10]).screenshot
-    np.testing.assert_array_equal(
-        screenshot.image_pixels,
-        np.tile(np.array([255, 0, 0, 255], dtype=np.uint8), (10, 10, 1)),
-    )
+    # In the perspective view the skeleton is now rendered with lit
+    # cylinder/sphere impostors rather than flat billboards, so the red channel
+    # is modulated by the lighting factor instead of being a uniform 255.  The
+    # shader emits only the red channel, so green/blue remain 0 and the
+    # composited alpha remains 255; some red must be present (the skeleton
+    # renders).
+    pixels = screenshot.image_pixels
+    np.testing.assert_array_equal(pixels[..., 1], 0)  # green
+    np.testing.assert_array_equal(pixels[..., 2], 0)  # blue
+    np.testing.assert_array_equal(pixels[..., 3], 255)  # alpha
+    assert pixels[..., 0].max() > 0  # red skeleton rendered
 
     with webdriver.viewer.txn() as s:
         s.layers[0].source[0].subsources["default"] = False

src/chunk_manager/backend.spec.ts

@@ -2,29 +2,14 @@ import { describe, expect, it, vi } from "vitest";
 
 import { ChunkQueueManager } from "#src/chunk_manager/backend.js";
 import { ChunkState } from "#src/chunk_manager/base.js";
+import { getChunkKey } from "#src/sliceview/base.js";
 
 describe("ChunkQueueManager targeted source invalidation", () => {
-  it("invalidates only chunks whose keys match requested cell prefixes", () => {
-    const matchingWorkerChunk = {
-      key: "13,9,5:0",
-      state: ChunkState.SYSTEM_MEMORY_WORKER,
-      freeSystemMemory: vi.fn(),
-    };
-    const matchingSystemChunk = {
-      key: "13,9,5:1",
-      state: ChunkState.SYSTEM_MEMORY,
-      freeSystemMemory: vi.fn(),
-    };
-    const adjacentCellChunk = {
-      key: "13,9,50:0",
-      state: ChunkState.SYSTEM_MEMORY,
-      freeSystemMemory: vi.fn(),
-    };
-    const otherCellChunk = {
-      key: "13,9,6:0",
-      state: ChunkState.SYSTEM_MEMORY,
-      freeSystemMemory: vi.fn(),
-    };
+  function makeChunk(key: string, state: ChunkState) {
+    return { key, state, freeSystemMemory: vi.fn() };
+  }
+
+  function makeQueueManager() {
     const rpc = { invoke: vi.fn() };
     const queueManager = Object.assign(
       Object.create(ChunkQueueManager.prototype),
@@ -38,39 +23,84 @@ describe("ChunkQueueManager targeted source invalidation", () => {
         ),
       },
     );
+    return { rpc, queueManager };
+  }
+
+  it("requeues only the identified chunks", () => {
+    const workerChunk = makeChunk(
+      getChunkKey([13, 9, 5]),
+      ChunkState.SYSTEM_MEMORY_WORKER,
+    );
+    const systemChunk = makeChunk(
+      getChunkKey([13, 9, 6]),
+      ChunkState.SYSTEM_MEMORY,
+    );
+    // Would have been caught by a bare `startsWith` test against key "13,9,5".
+    const untouchedChunk = makeChunk(
+      getChunkKey([13, 9, 50]),
+      ChunkState.SYSTEM_MEMORY,
+    );
+    const { rpc, queueManager } = makeQueueManager();
     const source = {
       rpcId: 7,
       chunks: new Map([
-        [matchingWorkerChunk.key, matchingWorkerChunk],
-        [matchingSystemChunk.key, matchingSystemChunk],
-        [adjacentCellChunk.key, adjacentCellChunk],
-        [otherCellChunk.key, otherCellChunk],
+        [workerChunk.key, workerChunk],
+        [systemChunk.key, systemChunk],
+        [untouchedChunk.key, untouchedChunk],
       ]),
     };
 
-    queueManager.invalidateSourceCacheKeyPrefixes(source, ["13,9,5:"]);
+    queueManager.invalidateSourceCacheKeys(source, [
+      workerChunk.key,
+      systemChunk.key,
+    ]);
 
-    expect(matchingWorkerChunk.freeSystemMemory).toHaveBeenCalledTimes(1);
+    expect(workerChunk.freeSystemMemory).toHaveBeenCalledTimes(1);
     expect(queueManager.updateChunkState).toHaveBeenCalledWith(
-      matchingWorkerChunk,
+      workerChunk,
       ChunkState.QUEUED,
     );
     expect(queueManager.updateChunkState).toHaveBeenCalledWith(
-      matchingSystemChunk,
-      ChunkState.QUEUED,
-    );
-    expect(queueManager.updateChunkState).not.toHaveBeenCalledWith(
-      adjacentCellChunk,
+      systemChunk,
       ChunkState.QUEUED,
     );
     expect(queueManager.updateChunkState).not.toHaveBeenCalledWith(
-      otherCellChunk,
+      untouchedChunk,
       ChunkState.QUEUED,
     );
     expect(rpc.invoke).toHaveBeenCalledWith("Chunk.update", {
       source: 7,
-      keyPrefixes: ["13,9,5:"],
+      keys: [workerChunk.key, systemChunk.key],
     });
+    // Marking chunks QUEUED only takes effect once the queue is processed.
     expect(queueManager.scheduleUpdate).toHaveBeenCalledTimes(1);
   });
+
+  it("tells the frontend only about the keys it actually invalidated", () => {
+    const chunk = makeChunk(getChunkKey([13, 9, 5]), ChunkState.SYSTEM_MEMORY);
+    const { rpc, queueManager } = makeQueueManager();
+    const source = { rpcId: 7, chunks: new Map([[chunk.key, chunk]]) };
+
+    queueManager.invalidateSourceCacheKeys(source, [
+      chunk.key,
+      getChunkKey([99, 99, 99]),
+    ]);
+
+    expect(rpc.invoke).toHaveBeenCalledWith("Chunk.update", {
+      source: 7,
+      keys: [chunk.key],
+    });
+  });
+
+  it("does not notify the frontend when no key matched", () => {
+    const chunk = makeChunk(getChunkKey([13, 9, 6]), ChunkState.SYSTEM_MEMORY);
+    const { rpc, queueManager } = makeQueueManager();
+    const source = { rpcId: 7, chunks: new Map([[chunk.key, chunk]]) };
+
+    queueManager.invalidateSourceCacheKeys(source, [getChunkKey([13, 9, 5])]);
+
+    expect(queueManager.updateChunkState).not.toHaveBeenCalled();
+    expect(rpc.invoke).not.toHaveBeenCalled();
+    expect(queueManager.scheduleUpdate).not.toHaveBeenCalled();
+  });
 });

src/chunk_manager/backend.ts

@@ -23,7 +23,7 @@ import {
   CHUNK_LAYER_STATISTICS_RPC_ID,
   CHUNK_MANAGER_RPC_ID,
   CHUNK_QUEUE_MANAGER_RPC_ID,
-  CHUNK_SOURCE_INVALIDATE_KEY_PREFIXES_RPC_ID,
+  CHUNK_SOURCE_INVALIDATE_KEYS_RPC_ID,
   CHUNK_SOURCE_INVALIDATE_RPC_ID,
   ChunkDownloadStatistics,
   ChunkMemoryStatistics,
@@ -61,10 +61,6 @@ import {
 
 const DEBUG_CHUNK_UPDATES = false;
 
-function keyMatchesAnyPrefix(key: string, keyPrefixes: readonly string[]) {
-  return keyPrefixes.some((keyPrefix) => key.startsWith(keyPrefix));
-}
-
 export interface ChunkStateListener {
   (chunk: Chunk, oldState: ChunkState): void;
 }
@@ -1132,16 +1128,15 @@ export class ChunkQueueManager extends SharedObjectCounterpart {
     this.scheduleUpdate();
   }
 
-  invalidateSourceCacheKeyPrefixes(
-    source: ChunkSource,
-    keyPrefixes: readonly string[],
-  ) {
-    let invalidated = false;
-    for (const chunk of source.chunks.values()) {
-      const key = chunk.key;
-      if (key === null || !keyMatchesAnyPrefix(key, keyPrefixes)) {
-        continue;
-      }
+  /**
+   * Like {@link invalidateSourceCache}, but limited to the chunks named by `keys`.  Keys are the
+   * `Chunk.key` values the source assigns; keys naming no cached chunk are ignored.
+   */
+  invalidateSourceCacheKeys(source: ChunkSource, keys: readonly string[]) {
+    const invalidatedKeys: string[] = [];
+    for (const key of keys) {
+      const chunk = source.chunks.get(key);
+      if (chunk === undefined) continue;
       switch (chunk.state) {
         case ChunkState.DOWNLOADING:
           cancelChunkDownload(chunk);
@@ -1152,14 +1147,14 @@ export class ChunkQueueManager extends SharedObjectCounterpart {
       }
       // Note: After calling this, chunk may no longer be valid.
       this.updateChunkState(chunk, ChunkState.QUEUED);
-      invalidated = true;
+      invalidatedKeys.push(key);
     }
-    if (!invalidated) {
+    if (invalidatedKeys.length === 0) {
       return;
     }
     this.rpc!.invoke("Chunk.update", {
       source: source.rpcId,
-      keyPrefixes: [...keyPrefixes],
+      keys: invalidatedKeys,
     });
     this.scheduleUpdate();
   }
@@ -1415,18 +1410,9 @@ registerRPC(CHUNK_SOURCE_INVALIDATE_RPC_ID, function (x) {
   source.chunkManager.queueManager.invalidateSourceCache(source);
 });
 
-registerRPC(CHUNK_SOURCE_INVALIDATE_KEY_PREFIXES_RPC_ID, function (x) {
+registerRPC(CHUNK_SOURCE_INVALIDATE_KEYS_RPC_ID, function (x) {
   const source = <ChunkSource>this.get(x.id);
-  const keyPrefixes = Array.isArray(x.keyPrefixes)
-    ? x.keyPrefixes.filter(
-        (keyPrefix: unknown): keyPrefix is string =>
-          typeof keyPrefix === "string" && keyPrefix.length !== 0,
-      )
-    : [];
-  source.chunkManager.queueManager.invalidateSourceCacheKeyPrefixes(
-    source,
-    keyPrefixes,
-  );
+  source.chunkManager.queueManager.invalidateSourceCacheKeys(source, x.keys);
 });
 
 registerPromiseRPC(

src/chunk_manager/base.ts

@@ -100,8 +100,8 @@ export const PREFETCH_PRIORITY_MULTIPLIER = 1e13;
 export const CHUNK_QUEUE_MANAGER_RPC_ID = "ChunkQueueManager";
 export const CHUNK_MANAGER_RPC_ID = "ChunkManager";
 export const CHUNK_SOURCE_INVALIDATE_RPC_ID = "ChunkSource.invalidate";
-export const CHUNK_SOURCE_INVALIDATE_KEY_PREFIXES_RPC_ID =
-  "ChunkSource.invalidateKeyPrefixes";
+export const CHUNK_SOURCE_INVALIDATE_KEYS_RPC_ID =
+  "ChunkSource.invalidateKeys";
 
 export const REQUEST_CHUNK_STATISTICS_RPC_ID =
   "ChunkQueueManager.requestChunkStatistics";

src/chunk_manager/frontend.ts

@@ -22,7 +22,7 @@ import {
   CHUNK_LAYER_STATISTICS_RPC_ID,
   CHUNK_MANAGER_RPC_ID,
   CHUNK_QUEUE_MANAGER_RPC_ID,
-  CHUNK_SOURCE_INVALIDATE_KEY_PREFIXES_RPC_ID,
+  CHUNK_SOURCE_INVALIDATE_KEYS_RPC_ID,
   CHUNK_SOURCE_INVALIDATE_RPC_ID,
   ChunkState,
   REQUEST_CHUNK_STATISTICS_RPC_ID,
@@ -46,10 +46,6 @@ import {
 
 const DEBUG_CHUNK_UPDATES = false;
 
-function keyMatchesAnyPrefix(key: string, keyPrefixes: readonly string[]) {
-  return keyPrefixes.some((keyPrefix) => key.startsWith(keyPrefix));
-}
-
 export class Chunk {
   state = ChunkState.SYSTEM_MEMORY;
   constructor(public source: ChunkSource) {}
@@ -247,15 +243,13 @@ export class ChunkQueueManager extends SharedObject {
     }
     if (update.promise !== undefined) {
       this.handleFetch_(source, update);
-    } else if (update.keyPrefixes !== undefined) {
-      const keyPrefixes = update.keyPrefixes as string[];
-      const chunkKeysToDelete = [...source.chunks.keys()].filter((chunkKey) =>
-        keyMatchesAnyPrefix(chunkKey, keyPrefixes),
-      );
-      for (const chunkKey of chunkKeysToDelete) {
-        source.deleteChunk(chunkKey);
+    } else if (update.keys !== undefined) {
+      for (const chunkKey of update.keys as string[]) {
+        if (source.chunks.has(chunkKey)) {
+          source.deleteChunk(chunkKey);
+          visibleChunksChanged = true;
+        }
       }
-      visibleChunksChanged = chunkKeysToDelete.length !== 0;
     } else if (update.id === undefined) {
       // Invalidate source.
       for (const chunkKey of source.chunks.keys()) {
@@ -496,19 +490,17 @@ export class ChunkSource extends SharedObject {
   }
 
   /**
-   * Invalidates cached chunks whose backend keys match any of the specified prefixes.
-   * Operates asynchronously.
+   * Invalidates the cached chunks named by `keys`, leaving the rest of the cache intact.  Keys are
+   * the `Chunk.key` values the source assigns.  Operates asynchronously.
    */
-  invalidateCacheKeyPrefixes(keyPrefixes: Iterable<string>): void {
-    const normalizedKeyPrefixes = [...new Set(keyPrefixes)].filter(
-      (keyPrefix) => keyPrefix.length !== 0,
-    );
-    if (normalizedKeyPrefixes.length === 0) {
+  invalidateCacheKeys(keys: Iterable<string>): void {
+    const uniqueKeys = [...new Set(keys)];
+    if (uniqueKeys.length === 0) {
       return;
     }
-    this.rpc!.invoke(CHUNK_SOURCE_INVALIDATE_KEY_PREFIXES_RPC_ID, {
+    this.rpc!.invoke(CHUNK_SOURCE_INVALIDATE_KEYS_RPC_ID, {
       id: this.rpcId,
-      keyPrefixes: normalizedKeyPrefixes,
+      keys: uniqueKeys,
     });
   }
 

src/datasource/catmaid/api.ts

@@ -1709,7 +1709,31 @@ export class CatmaidClient implements CatmaidSpatialSkeletonEditApi {
         "CATMAID skeleton/reroot did not return the requested new root.",
       );
     }
-    return {};
+    // The `nocheck` path is used for optimistic compensation, where no cached revision tokens are
+    // being reconciled, so CATMAID is not asked for an edition time either.
+    if (options.nocheck === true) {
+      return {};
+    }
+    // Rerooting rewrites the parent links along the path from the old root to the new one, bumping
+    // the edition time of every node on that path. CATMAID reports a single edition time for the
+    // operation, which applies to all of them; without it the cached revision tokens for those nodes
+    // would go stale and the next edit would be rejected as out of date.
+    const revisionToken = normalizeCatmaidRevisionToken(response?.edition_time);
+    if (revisionToken === undefined) {
+      throw new Error(
+        "CATMAID skeleton/reroot did not return the new root edition_time.",
+      );
+    }
+    const sourceState = makeCatmaidNodeSourceState(revisionToken)!;
+    const nodeSourceStateUpdates = (editContext?.nodes ?? []).map(
+      ({ nodeId: affectedNodeId }) => ({
+        nodeId: affectedNodeId,
+        sourceState,
+      }),
+    );
+    return nodeSourceStateUpdates.length === 0
+      ? {}
+      : { nodeSourceStateUpdates };
   }
 
   async deleteNode(

src/help/input_event_bindings.ts

@@ -255,7 +255,7 @@ export class InputEventBindingHelpDialog extends SidePanel {
           layerBindings = [];
           layerToolBindingsMap.set(tool.context, layerBindings);
         }
-        layerBindings.push([`shift+key${key.toLowerCase()}`, tool.description]);
+        layerBindings.push([`key${key.toLowerCase()}`, tool.description]);
       }
     }
     const layerToolBindings = Array.from(layerToolBindingsMap.entries());

src/layer/index.ts

@@ -1156,6 +1156,10 @@ export class MouseSelectionState implements PickState {
   position: Float32Array = kEmptyFloat32Vec;
   unsnappedPosition: Float32Array = kEmptyFloat32Vec;
   active = false;
+  // When true, the global picking-indicator ring is hidden even though the mouse
+  // state is active. Set during a skeleton node move, where the on-screen node is
+  // driven by the drag preview rather than by picking.
+  pickingIndicatorSuppressed = false;
   displayDimensions: DisplayDimensions | undefined = undefined;
   pickedRenderLayer: RenderLayer | null = null;
   pickedValue = 0n;