diff --git a/frontend/src/__tests__/componentTests/useAddToView.test.tsx b/frontend/src/__tests__/componentTests/useAddToView.test.tsx index 7e5b05f17..eebad74fd 100644 --- a/frontend/src/__tests__/componentTests/useAddToView.test.tsx +++ b/frontend/src/__tests__/componentTests/useAddToView.test.tsx @@ -12,6 +12,9 @@ const buildViewState = vi.hoisted(() => vi.fn()); vi.mock('@/hooks/useCartCheckout', () => ({ useCartCheckout: () => ({ resolveCartDatasets }) })); +vi.mock('@/contexts/PreferencesContext', () => ({ + usePreferencesContext: () => ({ disableHeuristicalLayerTypeDetection: false }) +})); vi.mock('@/utils/viewCheckout', async importOriginal => ({ ...(await importOriginal()), buildViewState diff --git a/frontend/src/__tests__/componentTests/useCartCheckout.test.tsx b/frontend/src/__tests__/componentTests/useCartCheckout.test.tsx index 4259a6b2b..b91891d76 100644 --- a/frontend/src/__tests__/componentTests/useCartCheckout.test.tsx +++ b/frontend/src/__tests__/componentTests/useCartCheckout.test.tsx @@ -18,6 +18,9 @@ vi.mock('@/contexts/ViewsContext', () => ({ createViewMutation: { mutateAsync: createViewAsync } }) })); +vi.mock('@/contexts/PreferencesContext', () => ({ + usePreferencesContext: () => ({ disableHeuristicalLayerTypeDetection: false }) +})); vi.mock('@/utils/viewCheckout', () => ({ buildViewState })); diff --git a/frontend/src/__tests__/componentTests/useCreateViewFlow.test.tsx b/frontend/src/__tests__/componentTests/useCreateViewFlow.test.tsx index 758940ed0..4d2207e59 100644 --- a/frontend/src/__tests__/componentTests/useCreateViewFlow.test.tsx +++ b/frontend/src/__tests__/componentTests/useCreateViewFlow.test.tsx @@ -52,9 +52,12 @@ function Harness({ apiRef }: { apiRef: { current: FlowApi | null } }) { return <>{api.dialog}; } +// The pre-opened tab the draft is handed to. +let tab: { location: { href: string }; close: ReturnType }; beforeEach(() => { localStorage.clear(); - vi.spyOn(window, 'open').mockReturnValue({} as Window); + tab = { location: { href: '' }, close: vi.fn() }; + vi.spyOn(window, 'open').mockReturnValue(tab as unknown as Window); checkout .mockReset() .mockResolvedValue({ short_key: 'v1', read_key: 'rk1', name: 'A' }); @@ -83,8 +86,9 @@ describe('useCreateViewFlow draft', () => { }); expect(screen.queryByRole('textbox')).not.toBeInTheDocument(); - await waitFor(() => expect(window.open).toHaveBeenCalled()); - const url = vi.mocked(window.open).mock.calls[0][0] as string; + expect(window.open).toHaveBeenCalledWith('', '_blank'); + await waitFor(() => expect(tab.location.href).not.toBe('')); + const url = tab.location.href; const id = new URL(url, 'http://x').searchParams.get('draft')!; expect(url).toBe(`/view/new?draft=${id}`); expect( @@ -111,7 +115,7 @@ describe('useCreateViewFlow draft', () => { apiRef.current!.startDraftView(datasets, 'A'); }); - await waitFor(() => expect(window.open).toHaveBeenCalled()); + await waitFor(() => expect(tab.location.href).not.toBe('')); expect(localStorage.getItem('fg-view-draft:recent')).not.toBeNull(); expect(localStorage.getItem('fg-view-draft:old')).toBeNull(); }); @@ -150,7 +154,7 @@ describe('useCreateViewFlow draft', () => { await userEvent .setup() .click(screen.getByRole('button', { name: /continue/i })); - await waitFor(() => expect(window.open).toHaveBeenCalled()); + await waitFor(() => expect(tab.location.href).not.toBe('')); }); it('toasts and stays put when the draft cannot be built', async () => { @@ -163,7 +167,8 @@ describe('useCreateViewFlow draft', () => { }); await waitFor(() => expect(toast.error).toHaveBeenCalledWith('No layers')); - expect(window.open).not.toHaveBeenCalled(); + expect(tab.close).toHaveBeenCalled(); + expect(tab.location.href).toBe(''); expect(navigate).not.toHaveBeenCalled(); }); }); diff --git a/frontend/src/__tests__/unitTests/viewCheckout.test.ts b/frontend/src/__tests__/unitTests/viewCheckout.test.ts index cd6ae2584..a9e3a9b0e 100644 --- a/frontend/src/__tests__/unitTests/viewCheckout.test.ts +++ b/frontend/src/__tests__/unitTests/viewCheckout.test.ts @@ -4,6 +4,8 @@ const encoded = (state: unknown) => encodeURIComponent(JSON.stringify(state)); vi.mock('@/omezarr-helper', () => ({ getOmeZarrMetadata: vi.fn(), + getOmeZarrThumbnail: vi.fn(), + determineLayerType: vi.fn(), generateNeuroglancerStateForOmeZarr: vi.fn(), generateNeuroglancerStateForDataURL: vi.fn(), generateStateForPlainZarr: vi.fn() @@ -11,21 +13,29 @@ vi.mock('@/omezarr-helper', () => ({ import { getOmeZarrMetadata, + getOmeZarrThumbnail, + determineLayerType, generateNeuroglancerStateForOmeZarr, generateStateForPlainZarr } from '@/omezarr-helper'; import { appendLayers, - buildViewState, + buildViewState as buildViewStateImpl, dataLinkSegment, rewriteDataLinkSegment, probeDataset } from '@/utils/viewCheckout'; const md = { multiscales: [{}], arr: {}, zarrVersion: 2 }; +const buildViewState = ( + datasets: Parameters[0], + detectLayerType = true +) => buildViewStateImpl(datasets, detectLayerType); beforeEach(() => { (getOmeZarrMetadata as any).mockReset().mockResolvedValue(md); + (getOmeZarrThumbnail as any).mockReset().mockResolvedValue(['thumb', null]); + (determineLayerType as any).mockReset().mockResolvedValue('image'); (generateStateForPlainZarr as any) .mockReset() .mockRejectedValue(new Error('not a plain array')); @@ -41,6 +51,32 @@ beforeEach(() => { }); describe('buildViewState', () => { + const one = { + url: 'a', + sharing_key: 'ka', + url_prefix: 'a', + fsp_name: 'f', + path: '/a', + label: 'A' + }; + + it('uses the inferred layer type for OME-Zarr layers', async () => { + (determineLayerType as any).mockResolvedValue('segmentation'); + await buildViewState([one], true); + expect(determineLayerType).toHaveBeenCalledWith(true, 'thumb'); + expect((generateNeuroglancerStateForOmeZarr as any).mock.calls[0][2]).toBe( + 'segmentation' + ); + }); + + it('skips inference when detection is disabled', async () => { + await buildViewState([one], false); + expect(getOmeZarrThumbnail).not.toHaveBeenCalled(); + expect((generateNeuroglancerStateForOmeZarr as any).mock.calls[0][2]).toBe( + 'image' + ); + }); + it('concatenates layers across datasets and maps ViewLayerInput', async () => { const { ng_state, layers } = await buildViewState([ { diff --git a/frontend/src/hooks/useAddToView.ts b/frontend/src/hooks/useAddToView.ts index 59f4644b1..2f4e0977f 100644 --- a/frontend/src/hooks/useAddToView.ts +++ b/frontend/src/hooks/useAddToView.ts @@ -2,6 +2,7 @@ import { useCallback, useEffect, useRef } from 'react'; import toast from 'react-hot-toast'; import { useCartCheckout } from '@/hooks/useCartCheckout'; +import { usePreferencesContext } from '@/contexts/PreferencesContext'; import { isRelinkableLayer } from '@/queries/viewQueries'; import { appendLayers, @@ -39,6 +40,7 @@ export function useAddToView( } ): (items: CartItem[], options?: { shown?: boolean }) => Promise { const { resolveCartDatasets } = useCartCheckout(); + const { disableHeuristicalLayerTypeDetection } = usePreferencesContext(); // The sidebar unmounts when NG reloads (the bridge leaves `ready`). An add // still in flight then holds the dead viewer, so it must stop. const mounted = useRef(true); @@ -55,7 +57,10 @@ export function useAddToView( // broken layers on that dataset server-side and refetches the rows. const broken = viewLayers.filter(isRelinkableLayer); const resolved = await resolveCartDatasets(items); - const { ng_state, layers } = await buildViewState(resolved); + const { ng_state, layers } = await buildViewState( + resolved, + !disableHeuristicalLayerTypeDetection + ); const added = (ng_state.layers ?? []) as NgLayer[]; const unsupportedKeys = [ ...new Set( @@ -117,6 +122,14 @@ export function useAddToView( `Added ${count} ${noun}${count === 1 ? '' : 's'}. Save to keep ${count === 1 ? 'it' : 'them'}.${skipped}` ); }, - [resolveCartDatasets, bridge, viewLayers, onEdited, onAddSources, onAdded] + [ + resolveCartDatasets, + disableHeuristicalLayerTypeDetection, + bridge, + viewLayers, + onEdited, + onAddSources, + onAdded + ] ); } diff --git a/frontend/src/hooks/useCartCheckout.ts b/frontend/src/hooks/useCartCheckout.ts index 97aa0e0ff..67b4b6533 100644 --- a/frontend/src/hooks/useCartCheckout.ts +++ b/frontend/src/hooks/useCartCheckout.ts @@ -4,6 +4,7 @@ import { useAllProxiedPathsQuery, useCreateProxiedPathMutation } from '@/queries/proxiedPathQueries'; +import { usePreferencesContext } from '@/contexts/PreferencesContext'; import { useViewsContext } from '@/contexts/ViewsContext'; import { useZoneAndFspMapContext } from '@/contexts/ZonesAndFspMapContext'; import { buildViewState } from '@/utils/viewCheckout'; @@ -52,6 +53,7 @@ export function useCartCheckout() { const createProxiedPath = useCreateProxiedPathMutation(); const { createViewMutation } = useViewsContext(); const { zonesAndFspQuery } = useZoneAndFspMapContext(); + const { disableHeuristicalLayerTypeDetection } = usePreferencesContext(); const bucketFor = useCallback( (ds: Pick): FileSharePath | null => { @@ -128,10 +130,17 @@ export function useCartCheckout() { const checkout = useCallback( async (rawDatasets: CartItem[], name: string): Promise => { const resolved = await resolveCartDatasets(rawDatasets); - const { ng_state, layers } = await buildViewState(resolved); + const { ng_state, layers } = await buildViewState( + resolved, + !disableHeuristicalLayerTypeDetection + ); return createViewMutation.mutateAsync({ name, ng_state, layers }); }, - [resolveCartDatasets, createViewMutation] + [ + resolveCartDatasets, + createViewMutation, + disableHeuristicalLayerTypeDetection + ] ); return { checkout, resolveCartDatasets, isBucketDataset }; diff --git a/frontend/src/hooks/useCreateViewFlow.tsx b/frontend/src/hooks/useCreateViewFlow.tsx index f3cf81963..f7c325e0e 100644 --- a/frontend/src/hooks/useCreateViewFlow.tsx +++ b/frontend/src/hooks/useCreateViewFlow.tsx @@ -84,7 +84,8 @@ export function useCreateViewFlow() { const { areDataLinksAutomatic, dataLinkSubpathMode, - toggleAutomaticDataLinks + toggleAutomaticDataLinks, + disableHeuristicalLayerTypeDetection } = usePreferencesContext(); const allProxiedPathsQuery = useAllProxiedPathsQuery(); const navigate = useNavigate(); @@ -99,13 +100,26 @@ export function useCreateViewFlow() { request?.mode === 'add' ? request.hasMismatch : cartCheck.hasMismatch; const openDraft = async (datasets: CartItem[], name: string) => { - const resolved = await resolveCartDatasets(datasets); - const { ng_state, layers } = await buildViewState(resolved); - const draft: ViewDraft = { name, ng_state, layers, datasets: resolved }; - // Opens in a new tab; falls back to this tab if the popup was blocked. + // Open the tab now, while the click's user activation is still fresh: + // building the state renders a thumbnail per dataset, which can outlast + // it and get the popup blocked. Falls back to this tab if blocked anyway. // No 'noopener': it makes window.open return null, which reads as blocked. - if (!window.open(storeDraft(draft), '_blank')) { - navigate(DRAFT_VIEW_PATH, { state: { draft } }); + const tab = window.open('', '_blank'); + try { + const resolved = await resolveCartDatasets(datasets); + const { ng_state, layers } = await buildViewState( + resolved, + !disableHeuristicalLayerTypeDetection + ); + const draft: ViewDraft = { name, ng_state, layers, datasets: resolved }; + if (tab) { + tab.location.href = storeDraft(draft); + } else { + navigate(DRAFT_VIEW_PATH, { state: { draft } }); + } + } catch (error) { + tab?.close(); + throw error; } }; diff --git a/frontend/src/omezarr-helper.ts b/frontend/src/omezarr-helper.ts index ff5cd3821..ecb03d9e8 100644 --- a/frontend/src/omezarr-helper.ts +++ b/frontend/src/omezarr-helper.ts @@ -889,36 +889,20 @@ async function getOmeZarrLabels( * checkout when a cart dataset is a bare array, so each added directory still * becomes one layer. */ -/** - * Guess a Neuroglancer layer type for a plain array from its name and dtype. - * Label/segmentation volumes are integer-typed and usually named seg/label/mask; - * everything else is treated as an image. This only sets the default so the - * layer resolves without Neuroglancer's "new layer" type picker — the user can - * still switch the type in the viewer. - */ -function guessPlainLayerType(dataUrl: string, dtype: string): LayerType { - const name = getLayerName(dataUrl).toLowerCase(); - const isInteger = /^(u?int)/.test(dtype); - if (isInteger && /seg|label|mask/.test(name)) { - return 'segmentation'; - } - return 'image'; -} - async function generateStateForPlainZarr(dataUrl: string): Promise { // Probe the storage version (needed for the |zarrN: source suffix): try v2 // first (most fileglancer data), then v3. If neither opens, this throws and // the caller skips the dataset. let zarrVersion: 2 | 3 = 2; - let arr; try { - arr = await getZarrArray(dataUrl, 2); + await getZarrArray(dataUrl, 2); } catch { - arr = await getZarrArray(dataUrl, 3); + await getZarrArray(dataUrl, 3); zarrVersion = 3; } - const layerType = guessPlainLayerType(dataUrl, String(arr.dtype)); - return generateNeuroglancerStateForZarrArray(dataUrl, zarrVersion, layerType); + // Plain arrays have no thumbnail for the edge heuristic, so the standalone + // Neuroglancer button always opens them as 'image'; match it. + return generateNeuroglancerStateForZarrArray(dataUrl, zarrVersion, 'image'); } /** @@ -955,7 +939,7 @@ type ThumbnailResult = [thumbnail: string | null, errorMessage: string | null]; async function getOmeZarrThumbnail( dataUrl: string, - signal: AbortSignal, + signal?: AbortSignal, thumbnailSize: number = 300, maxThumbnailSize: number = 1024, autoBoost: boolean = true diff --git a/frontend/src/utils/viewCheckout.ts b/frontend/src/utils/viewCheckout.ts index 7ebae1582..bca92e601 100644 --- a/frontend/src/utils/viewCheckout.ts +++ b/frontend/src/utils/viewCheckout.ts @@ -1,10 +1,12 @@ import { getOmeZarrMetadata, + getOmeZarrThumbnail, + determineLayerType, generateNeuroglancerStateForOmeZarr, generateNeuroglancerStateForDataURL, generateStateForPlainZarr } from '@/omezarr-helper'; -import type { Metadata } from '@/omezarr-helper'; +import type { LayerType, Metadata } from '@/omezarr-helper'; import type { ViewLayer, ViewLayerInput } from '@/queries/viewQueries'; import { default as log } from '@/logger'; @@ -63,8 +65,21 @@ function decodeState(encoded: string | null): NgState | null { } } +// Same thumbnail-edge heuristic the single-dataset preview uses, so the +// metadata panel and the View agree on image vs segmentation. +// ponytail: renders a fresh thumbnail per dataset (not the preview's cached +// one, whose URL differs); cache by dataset if checkout of big carts drags. +async function inferLayerType(url: string): Promise { + const [thumbnail, error] = await getOmeZarrThumbnail(url); + if (error) { + log.debug(`No thumbnail for ${url}, using image layer type: ${error}`); + } + return determineLayerType(true, thumbnail); +} + async function generateStateForDataset( - ds: ResolvedCheckoutDataset + ds: ResolvedCheckoutDataset, + detectLayerType: boolean ): Promise { const probe = await probeDataset(ds.url); if (probe.kind === 'unsupported') { @@ -80,13 +95,13 @@ async function generateStateForDataset( } const { metadata } = probe; const multiscale = metadata.multiscales?.[0]; - // ponytail: default layerType 'image' — the thumbnail-edge heuristic used - // for the single-dir preview needs a rendered thumbnail we don't have here. + const layerType = + multiscale && detectLayerType ? await inferLayerType(ds.url) : 'image'; const encoded = multiscale ? generateNeuroglancerStateForOmeZarr( ds.url, metadata.zarrVersion, - 'image', + layerType, multiscale, metadata.arr, metadata.labels, @@ -129,7 +144,9 @@ function startsArchived(layer: NgLayer, index: number): boolean { return layer.type !== 'segmentation' && index >= 4; } export async function buildViewState( - datasets: ResolvedCheckoutDataset[] + datasets: ResolvedCheckoutDataset[], + // Mirrors the disableHeuristicalLayerTypeDetection preference (inverted). + detectLayerType: boolean ): Promise<{ ng_state: Record; layers: ViewLayerInput[] }> { const combinedLayers: NgLayer[] = []; const viewLayers: ViewLayerInput[] = []; @@ -138,17 +155,23 @@ export async function buildViewState( const taken = new Set(); // Every channel entry of a dataset generates the same state; generate it - // once per dataset and layer path, then slice it per entry. + // once per dataset and layer path, then slice it per entry. All datasets + // start up front: each one probes metadata and renders a thumbnail, so + // awaiting them one at a time would make checkout time grow with cart size. const stateCache = new Map>(); - + const cacheKeyFor = (ds: ResolvedCheckoutDataset) => + `${ds.url}::${ds.channel !== undefined}`; for (const ds of datasets) { - const cacheKey = `${ds.url}::${ds.channel !== undefined}`; - let pending = stateCache.get(cacheKey); - if (!pending) { - pending = generateStateForDataset(ds); - stateCache.set(cacheKey, pending); + if (!stateCache.has(cacheKeyFor(ds))) { + stateCache.set( + cacheKeyFor(ds), + generateStateForDataset(ds, detectLayerType) + ); } - const state = await pending; + } + + for (const ds of datasets) { + const state = await stateCache.get(cacheKeyFor(ds))!; if (!state) { unsupported.push(ds); continue;