Make the data path synchronous: sync layer lifecycle, sync ChunkManager, source eagerly holds its loader (no open())

Author: aganders3

src/core/layer.ts

@@ -61,13 +61,11 @@ export abstract class Layer {
 
   public onEvent(_: EventContext): void {}
 
-  // TODO: Consider making this an abstract method once chunk manager
-  // integration is finalized. Most layers will likely need access to the chunk
-  // manager, but for now, we allow optional overrides to avoid requiring
-  // placeholder implementations.
-  public async onAttached(_context: IdetikContext) {}
+  // Synchronous attach/detach. The source must be opened before a layer is attached
+  // (see ChunkManager.viewFor) so creating the layer's view needs no async step.
+  public attach(_context: IdetikContext): void {}
 
-  public onDetached(_context: IdetikContext): void {}
+  public detach(): void {}
 
   public get objects() {
     return this.objects_;

src/core/viewport.ts

@@ -29,7 +29,7 @@ export class Viewport {
   public readonly events: EventDispatcher;
   public cameraControls?: CameraControls;
 
-  // Carried only to relay to `layer.onAttached` / `layer.onDetached`.
+  // Carried only to relay to `layer.attach` / `layer.detach`.
   // To be removed when the chunk-infrastructure refactor folds chunk management
   // into the source and the attach lifecycle goes away.
   private readonly context_: IdetikContext;
@@ -72,7 +72,7 @@ export class Viewport {
 
   public addLayer(layer: Layer): void {
     this.layers_.push(layer);
-    layer.onAttached(this.context_);
+    layer.attach(this.context_);
   }
 
   public removeLayer(layer: Layer): void {
@@ -81,12 +81,12 @@ export class Viewport {
       throw new Error(`Layer to remove not found: ${layer}`);
     }
     this.layers_.splice(index, 1);
-    layer.onDetached(this.context_);
+    layer.detach();
   }
 
   public removeAllLayers(): void {
     for (const layer of this.layers_) {
-      layer.onDetached(this.context_);
+      layer.detach();
     }
     this.layers_ = [];
   }

src/data/chunk.ts

@@ -99,7 +99,8 @@ export type SliceCoordinates = {
 };
 
 export type ChunkSource = {
-  open(): Promise<ChunkLoader>;
+  /** The source's loader. Sources are opened by their factory, so this is synchronous. */
+  getLoader(): ChunkLoader;
 };
 
 export type ChunkLoader = {

src/data/chunk_manager.ts

@@ -9,11 +9,9 @@ export type QueueStats = {
 import { ChunkStore } from "./chunk_store";
 import { ChunkStoreView } from "./chunk_store_view";
 import { ImageSourcePolicy } from "../core/image_source_policy";
-import { Logger } from "../utilities/logger";
 
 export class ChunkManager {
   private readonly stores_ = new Map<ChunkSource, ChunkStore>();
-  private readonly pendingStores_ = new Map<ChunkSource, Promise<ChunkStore>>();
   private readonly queue_ = new ChunkQueue();
 
   public get queueStats(): QueueStats {
@@ -27,47 +25,20 @@ export class ChunkManager {
     return chunkMemoryStats();
   }
 
-  public async addView(
+  /**
+   * Creates a view for a source. The source is opened by its factory
+   * (`OmeZarrImageSource.fromHttp(...)` / `fromFileSystem(...)`), so this is synchronous.
+   */
+  public viewFor(
     source: ChunkSource,
     policy: ImageSourcePolicy
-  ): Promise<ChunkStoreView> {
-    const store = await this.getOrCreateStore(source);
-    const view = store.createView(policy);
-    return view;
-  }
-
-  private async getOrCreateStore(source: ChunkSource): Promise<ChunkStore> {
-    const existing = this.stores_.get(source);
-    if (existing) {
-      return existing;
-    }
-
-    const pending = this.pendingStores_.get(source);
-    if (pending) {
-      return pending;
-    }
-
-    const initializeStore = async () => {
-      const loader = await source.open();
-      return new ChunkStore(loader);
-    };
-
-    const newPending = initializeStore();
-    this.pendingStores_.set(source, newPending);
-
-    try {
-      const store = await newPending;
+  ): ChunkStoreView {
+    let store = this.stores_.get(source);
+    if (!store) {
+      store = new ChunkStore(source.getLoader());
       this.stores_.set(source, store);
-      return store;
-    } catch (error) {
-      Logger.error(
-        "ChunkManager",
-        `Failed to open chunk source: ${String(error)}`
-      );
-      throw error;
-    } finally {
-      this.pendingStores_.delete(source);
     }
+    return store.createView(policy);
   }
 
   public update() {

src/data/ome_zarr/image_source.ts

@@ -16,6 +16,7 @@ import { SourceDimensionMap } from "../chunk";
 type OmeZarrImageSourceProps = {
   location: Location<Readable>;
   version?: OmeZarrVersion;
+  loader: OmeZarrImageLoader;
 };
 
 type HttpOmeZarrImageSourceProps = {
@@ -34,18 +35,28 @@ export class OmeZarrImageSource {
   readonly location: Location<Readable>;
   readonly version?: OmeZarrVersion;
 
-  private loader_?: OmeZarrImageLoader;
+  private readonly loader_: OmeZarrImageLoader;
 
+  /**
+   * Construct only via a static factory (`fromHttp`/`fromFileSystem`). A factory must
+   * fully open the data up front — e.g. via {@link openLoader}, which resolves to a
+   * ready loader or throws — and pass that loader in. The constructor therefore always
+   * receives an opened loader, so a source instance is never in a half-open state
+   * (`getLoader`/`getDimensions` cannot fail). To add a new factory, open the loader
+   * first, then `new OmeZarrImageSource({ location, version, loader })`.
+   */
   private constructor(props: OmeZarrImageSourceProps) {
     this.location = props.location;
     this.version = props.version;
+    this.loader_ = props.loader;
   }
 
-  public async open(): Promise<OmeZarrImageLoader> {
-    if (this.loader_) return this.loader_;
-
-    let zarrVersion = omeZarrToZarrVersion(this.version);
-    const root = await openGroup(this.location, zarrVersion);
+  private static async openLoader(
+    location: Location<Readable>,
+    version?: OmeZarrVersion
+  ): Promise<OmeZarrImageLoader> {
+    let zarrVersion = omeZarrToZarrVersion(version);
+    const root = await openGroup(location, zarrVersion);
     const adaptedOmeImage = parseOmeZarrImage(root.attrs);
     const images = adaptedOmeImage.multiscales;
     if (images.length !== 1) {
@@ -61,7 +72,7 @@ export class OmeZarrImageSource {
       zarrVersion = omeZarrToZarrVersion(adaptedOmeImage.originalVersion);
     }
     const arrayParams = metadata.datasets.map((d) =>
-      createZarrArrayParams(this.location, d.path, zarrVersion)
+      createZarrArrayParams(location, d.path, zarrVersion)
     );
     const arrays = await Promise.all(
       arrayParams.map((params) => openArrayFromParams(params))
@@ -74,24 +85,22 @@ export class OmeZarrImageSource {
         `Mismatch between number of axes (${axes.length}) and array shape (${shape.length})`
       );
     }
-    this.loader_ = new OmeZarrImageLoader({ metadata, arrays, arrayParams });
-    return this.loader_;
+    return new OmeZarrImageLoader({ metadata, arrays, arrayParams });
   }
 
   public getDimensions(): SourceDimensionMap {
-    if (!this.loader_) {
-      throw new Error(
-        "OmeZarrImageSource.getDimensions() requires the source to be opened first; " +
-          "use `await OmeZarrImageSource.fromHttp({ url })` or `await source.open()`."
-      );
-    }
     return this.loader_.getSourceDimensionMap();
   }
 
   public getChannelCount(): number {
     return this.getDimensions().c?.lods[0].size ?? 1;
   }
 
+  /** The source's loader, for synchronous view creation by the chunk manager. */
+  public getLoader(): OmeZarrImageLoader {
+    return this.loader_;
+  }
+
   /**
    * Creates and opens an OmeZarrImageSource from an HTTP(S) URL.
    *
@@ -101,13 +110,9 @@ export class OmeZarrImageSource {
   public static async fromHttp(
     props: HttpOmeZarrImageSourceProps
   ): Promise<OmeZarrImageSource> {
-    const store = new FetchStore(props.url);
-    const source = new OmeZarrImageSource({
-      location: new Location(store),
-      version: props.version,
-    });
-    await source.open();
-    return source;
+    const location = new Location(new FetchStore(props.url));
+    const loader = await OmeZarrImageSource.openLoader(location, props.version);
+    return new OmeZarrImageSource({ location, version: props.version, loader });
   }
 
   /**
@@ -122,12 +127,11 @@ export class OmeZarrImageSource {
   public static async fromFileSystem(
     props: FileSystemOmeZarrImageSourceProps
   ): Promise<OmeZarrImageSource> {
-    const store = new WebFileSystemStore(props.directory);
-    const source = new OmeZarrImageSource({
-      location: new Location(store, props.path),
-      version: props.version,
-    });
-    await source.open();
-    return source;
+    const location = new Location(
+      new WebFileSystemStore(props.directory),
+      props.path
+    );
+    const loader = await OmeZarrImageSource.openLoader(location, props.version);
+    return new OmeZarrImageSource({ location, version: props.version, loader });
   }
 }

src/layers/image_layer.ts

@@ -73,13 +73,13 @@ export class ImageLayer extends Layer implements ChannelsEnabled {
     this.onPickValue_ = onPickValue;
   }
 
-  public async onAttached(context: IdetikContext) {
+  public attach(context: IdetikContext): void {
     if (this.chunkStoreView_) {
       throw new Error(
-        "ImageLayer cannot be attached to multiple contexts simultaneously."
+        "ImageLayer cannot be attached to multiple viewports simultaneously."
       );
     }
-    this.chunkStoreView_ = await context.chunkManager.addView(
+    this.chunkStoreView_ = context.chunkManager.viewFor(
       this.source_,
       this.policy_
     );
@@ -100,7 +100,7 @@ export class ImageLayer extends Layer implements ChannelsEnabled {
     }
   }
 
-  public onDetached(_context: IdetikContext): void {
+  public detach(): void {
     if (!this.chunkStoreView_) return;
     this.releaseAndRemoveChunks(this.visibleChunks_.keys());
     this.clearObjects();

src/layers/label_layer.ts

@@ -66,13 +66,13 @@ export class LabelLayer extends Layer {
     this.outlineSelected_ = outlineSelected;
   }
 
-  public async onAttached(context: IdetikContext) {
+  public attach(context: IdetikContext): void {
     if (this.chunkStoreView_) {
       throw new Error(
-        "LabelLayer cannot be attached to multiple contexts simultaneously."
+        "LabelLayer cannot be attached to multiple viewports simultaneously."
       );
     }
-    this.chunkStoreView_ = await context.chunkManager.addView(
+    this.chunkStoreView_ = context.chunkManager.viewFor(
       this.source_,
       this.policy_
     );
@@ -86,7 +86,7 @@ export class LabelLayer extends Layer {
     }
   }
 
-  public onDetached(_context: IdetikContext): void {
+  public detach(): void {
     if (!this.chunkStoreView_) return;
     this.releaseAndRemoveChunks(this.visibleChunks_.keys());
     this.clearObjects();

src/layers/volume_layer.ts

@@ -130,8 +130,8 @@ export class VolumeLayer extends Layer implements ChannelsEnabled {
     return volume;
   }
 
-  public async onAttached(context: IdetikContext) {
-    this.chunkStoreView_ = await context.chunkManager.addView(
+  public attach(context: IdetikContext): void {
+    this.chunkStoreView_ = context.chunkManager.viewFor(
       this.source_,
       this.sourcePolicy_
     );
@@ -142,7 +142,7 @@ export class VolumeLayer extends Layer implements ChannelsEnabled {
     );
   }
 
-  public onDetached(_context: IdetikContext): void {
+  public detach(): void {
     if (!this.chunkStoreView_) return;
     for (const volume of this.currentVolumes_.values()) {
       this.releaseAndRemoveVolume(volume);