Construct `Volume` from loaders; load volume meta at loader creation by frasercl · Pull Request #499 · AllenCell/vole-core

frasercl · 2026-10-14T01:52:27Z

Review time: medium-large (30-35min)

Problem

This PR takes another step towards making Volume the primary client-facing interface for initiating volume data loads, rather than VolumeLoader, by altering the Volume constructor to require a VolumeLoader.

To enable this, all VolumeLoader methods that previously initiated a load of metadata rather than actual data are now synchronous. This has the effect of moving metadata loading to loader creation. Most loaders therefore now follow the OmeZarrLoader pattern of having a private constructor and an async static method that "opens" the loader. Once a loader is open, it must have all the data it needs to construct metadata objects immediately.

Solution

The loader interface change described above was pretty trivial on most VolumeLoader implementers, but two required more intensive changes:

OMEZarrLoader is responsible for computing the extent of a data request and picking a scale level. The metadata that this PR requires it to load in advance is more complex, and the code that loaded the relevant metadata was relatively entangled with the code that converted it into the required types.
WorkerLoader is a handle to a VolumeLoader running on a WebWorker, so everything it does involves message passing and is therefore async. This PR enables WorkerLoader to hold onto metadata on the main thread, so it can return results from metadata methods synchronously.

…structors)

…stency

frasercl · 2026-10-14T20:33:37Z

+  const channels: number[] = [];
+  myState.channelGui.forEach(({ enabled }, idx) => {
+    if (enabled) {
+      channels.push(idx);
+    }
+  });
+  loader.loadVolumeData(volume, { ...volume.loadSpec, channels }, onChannelDataArrived);


The ultimate aim of this stack of PRs is to make this code, which initiates data loading, a call on volume with a simpler signature, rather than a call to loader. This PR doesn't get all the way there and in fact makes this procedure temporarily a bit more complicated; the next PR in this stack is set to clean it up.

Copilot

🟡 Changes recommended

Crop preservation, volume-state initialization, and demo-loading regressions remain unresolved.

5 open findings

What changed in this PR

Moves metadata loading to loader creation, allowing Volume to construct synchronously from an initialized loader.

Changes:

Replaces asynchronous metadata methods with synchronous getters.
Transfers worker-loader metadata to the main thread.
Updates volume construction, demo usage, and tests.

File	Description
src/workers/VolumeLoadWorker.ts	Returns metadata when creating loaders.
src/workers/VolumeLoaderContext.ts	Provides synchronous metadata access.
src/workers/types.ts	Updates worker messages and metadata types.
src/VolumeDrawable.ts	Reads names through image info.
src/Volume.ts	Constructs from loaders and adds loader replacement.
src/test/zarr_utils.test.ts	Updates scale-helper input.
src/test/volume.test.ts	Adapts construction tests and mocks.
src/test/TripleSliceVolume.test.ts	Uses loader-backed test volumes.
src/loaders/zarr_utils/utils.ts	Generalizes scale-helper parameter type.
src/loaders/VolumeLoader.ts	Revises the metadata interface.
src/loaders/TiffLoader.ts	Loads metadata during asynchronous creation.
src/loaders/RawArrayLoader.ts	Makes metadata getters synchronous.
src/loaders/OpenCellLoader.ts	Makes metadata getters synchronous.
src/loaders/OmeZarrLoader.ts	Separates metadata extraction and conversion.
src/loaders/JsonImageInfoLoader.ts	Preloads JSON metadata and exports converters.
src/loaders/index.ts	Uses asynchronous loader factories.
src/ImageInfo.ts	Adds metadata accessors.
public/types.ts	Makes demo loaders optional.
public/index.ts	Adapts demo construction and loading.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

…ll-animated/vole-core into refactor/sync-meta-load

Copilot

🟡 Changes recommended

Metadata initialization, load-spec propagation, and shared translation mutation need correction before approval.

1 open finding

Initialize imageMetadata from loader image info · New

5 resolved since last review

Previously missed (2)

In code that hasn't changed since last review

Pass loadSpec to volume construction

public/index.ts:1108

The supplied loadSpec is no longer passed to volume construction. A non-default time or scale-level constraint is used by getDims above but discarded for the actual volume and data load, unlike the previous createVolume(loadSpec, ...) call. Pass it as the constructor's second argument.

Prevent mutation of cached transform translation

src/loaders/JsonImageInfoLoader.ts:74

jsonToImageInfo now runs against WorkerLoader's cached main-thread metadata, but tr aliases json.transform.translation and is rescaled in place. Creating another volume from the same loader and time point therefore rescales the translation shared by an already-created volume. Previously, worker responses isolated the returned arrays. Copy the translation before rescaling so conversion neither changes cached metadata nor shares this array between volumes.

🧠 Review effort: Balanced

ShrimpCryptid · 2026-10-19T18:09:21Z

@@ -1,3 +1,3 @@
 import { Box3, Vector2, Vector3 } from "three";

 import { CImageInfo, type ImageInfo } from "../ImageInfo.js";


Some unused imports here

ShrimpCryptid · 2026-10-19T18:16:55Z

+  private constructor(
+    private urls: string[],
+    public jsonInfo: JsonImageInfo[],
+    private cache?: VolumeCache
+  ) {


First time seeing parameter properties, neat!

ShrimpCryptid · 2026-10-19T21:52:45Z

+function createTestLoader(): VolumeLoader {
+  return {
+    getDims: vi.fn(() => imageInfo.multiscaleLevelDims),
+    getImageInfo: vi.fn((loadSpec) => ({ imageInfo, loadSpec })),
+    loadVolumeData: vi.fn(),
+    setPrefetchPriority: vi.fn(),
+    syncMultichannelLoading: vi.fn(),
+    loadRawChannelData: vi.fn(),
+    updateFetchOptions: vi.fn(),
+  };
+}


Is this repeated in enough places to justify making a shared utility?

ShrimpCryptid · 2026-10-19T21:56:17Z

@@ -1146,12 +1102,19 @@ async function createLoader(data: TestDataSpec): Promise<VolumeLoader[]> {
 }

 async function loadVolume(name: string, loadSpec: LoadSpec, loader: VolumeLoader): Promise<void> {


Does this still need to be async?

ShrimpCryptid · 2026-10-19T22:11:53Z

+    z: imageInfo.physicalPixelSize.z + imageInfo.spatialUnit,
+  };
+  metadata["Multiresolution levels"] = rawImageInfo.multiscaleLevelDims;
+  // TODO decide???? combined or not?


Did we ever decide on whatever this was about?

ShrimpCryptid · 2026-10-19T22:47:36Z

-
-  constructor(url: string[]) {
+  private constructor(
+    private url: string[],


Should this be stored as a remapped URL?

ShrimpCryptid · 2026-10-19T23:03:25Z

+      meta = { type: VolumeFileFormat.JSON, info: (loader as JsonImageInfoLoader).jsonInfo };
+    } else {
+      meta = {
+        type: undefined,


Should fileType be stored here?

ShrimpCryptid · 2026-10-19T23:41:12Z

+    // We ensured most info (dims, chunks, etc.) matched between sources earlier, so we can just use the first source.
+    // const source0 = meta.sources[0];


frasercl added 19 commits October 5, 2026 13:57

add getters for remaining ImageInfo properties in CImageInfo

7ed101d

de-async-ize multiple VolumeLoader methods

8603873

apply de-async-ization to loader impls (mostly with new async con…

eac11c1

…structors)

construct Volumes with loaders

8611ece

use async constructors in createVolumeLoader

90e011f

rename: OMEZarrLoader.createLoader -> OMEZarrLoader.new for consi…

8bd7832

…stency

little tweaks to WorkerLoader

19ddfcc

remove VolumeLoader.createVolume

3f2ef98

set up metadata export for zarr loader

5fb345c

update zarr utils

902ea65

derive dims from zarr metadata too

32c3d8d

export conversions for JsonImageInfo

52f7569

make WorkerLoader metadata methods synchronous

d1d2d7d

remove now-obsolete message types for getting metadata

17bcee9

easiest fixes to public/index.ts

b0ada03

make loader optional

980e2fa

fix up loader API use in test app

1ba7f6c

rename methods; fix test

e65db8f

fix more tests and fix them better

b269f5f

frasercl added this pull request to stack #500 October 14, 2026 01:52

fix types

95273d1

frasercl commented Oct 14, 2026

View reviewed changes

frasercl requested a balanced review from Copilot October 14, 2026 21:00

Copilot started reviewing on behalf of frasercl October 14, 2026 21:01

View session

Copilot AI reviewed Oct 14, 2026

View reviewed changes

Comment thread src/Volume.ts Outdated

Comment thread src/Volume.ts Outdated

Comment thread src/loaders/OmeZarrLoader.ts

Comment thread public/index.ts Outdated

Comment thread public/index.ts Outdated

frasercl and others added 5 commits October 14, 2026 15:11

remove maxExtent from OMEZarrLoader once and for all

a529d6a

remove now-unused util function composeSubregion

b5212d2

move createDefaultMetadata

21fd790

generate default metadata on volume construction

9f6484f

Fix maxSceneIndex calculation in updateScenesUI

aee3709

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

frasercl and others added 3 commits October 14, 2026 16:38

Refactor volume data loading with callback assignment

e2a42ab

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

just get rid of setLoader

aec5ad6

Merge branch 'refactor/sync-meta-load' of https://github.com/allen-ce…

091d328

…ll-animated/vole-core into refactor/sync-meta-load

frasercl requested a balanced review from Copilot October 14, 2026 23:58

Copilot started reviewing on behalf of frasercl October 14, 2026 23:59

View session

Copilot AI reviewed Oct 15, 2026

View reviewed changes

Comment thread src/Volume.ts Outdated

fix image metadata

f723181

frasercl force-pushed the refactor/sync-meta-load branch from d81c5d0 to f723181 Compare October 15, 2026 18:25

frasercl marked this pull request as ready for review October 16, 2026 22:05

frasercl requested a review from a team as a code owner October 16, 2026 22:06

ShrimpCryptid self-requested a review October 19, 2026 17:36

ShrimpCryptid reviewed Oct 19, 2026

View reviewed changes

		@@ -1,3 +1,3 @@
		import { Box3, Vector2, Vector3 } from "three";

		import { CImageInfo, type ImageInfo } from "../ImageInfo.js";

		@@ -1146,12 +1102,19 @@ async function createLoader(data: TestDataSpec): Promise<VolumeLoader[]> {
		}

		async function loadVolume(name: string, loadSpec: LoadSpec, loader: VolumeLoader): Promise<void> {

		// We ensured most info (dims, chunks, etc.) matched between sources earlier, so we can just use the first source.
		// const source0 = meta.sources[0];

Conversation

frasercl commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

frasercl commented Oct 14, 2026 •

edited

Loading