Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade Viv@0.9.2 #73

Merged
merged 25 commits into from
Feb 23, 2021
Merged

Upgrade Viv@0.9.2 #73

merged 25 commits into from
Feb 23, 2021

Conversation

manzt
Copy link
Member

@manzt manzt commented Feb 17, 2021

Changes:

  • Support for signed dtypes in viv; use PixelSource rather than ZarrLoader.
  • Cleans up OME types.
  • Upgrades zarr.js (uses of openGroup + openArray and walks hierarchy). Rather than relying on URLs, we can use the zarr path to link to different nodes in the store. This will allow for non-HTTP backends (e.g. via imjoy-rpc).

Known issues:

  • src/utils.ts includes a new method trimPyramid that tries to trim an image pyramid to resolutions that Viv can render. Notably, the MutliscaleImageLayer in Viv can only support resolutions that have the same chunk shape in x and y except for the lowest resolution. This type of "trimming" occured in createLoader before, and ideally that logic should be replicated identically here (or improved). We upgraded the ZarrPixelSource in Viv to allow for a declarative "tileSize". We now guess a tile size given the chunkshape of the highest resolution, which allows us to use all levels of the pyramid.
  • src/components/AcquisitionController.tsx has a callback that is specific to an HTTPStore for an OME-Zarr acquisition. Ideally the logic in our components (and UI elements) should be agnostic of the type of image. Perhaps there is a better way to declare UI elements per layer at the config level than hard-coding this in the UI. I have commented this out for now, but will fix.
  • the pickable layer seems to be disabled for our custom GridLayer. Need to investigate what's going on. This seems to be working now. I still see a "Too many pickable layers" error in the console for a large grid, however, although the linking is working. Resolved. I disabled picking on the grid elements, and enabled picking on the polygon layer.

I'm eager to find the time to finish this, but unfortunately it's not a priority for me at the moment. I'll hopefully be able to set aside a day and finish in the next week. I wanted to make something public towards migrating to the newest release of Viv. Feel free to test previous examples (using the netlify link below) and report here if something isn't working.

@manzt manzt changed the title Upgrade Viv@0.9.1 Upgrade Viv@0.9.2 Feb 21, 2021
@manzt manzt marked this pull request as ready for review February 21, 2021 19:16
@manzt
Copy link
Member Author

manzt commented Feb 21, 2021

@will-moore, I'm about ready to merge this. A few highlights:

  • 1.) Latest version of zarr.js is much smarter about loading nodes and doesn't make any unnecessary requests. This means that there are many fewer 404s when loading a plate (only 2 from vizarr checking that it's a plate).
  • 2.) I refactored a bit of the src/ome.ts. Mainly, I now use openGroup from zarr.js and walk the hierarchy. There is an implicit assumption that a source URL needs to contain the string .zarr. This is assumed to be the "root" of the store, and then all path walking is relative to the root.
  • 3.) I finally figured out how to get various layers to fit the viewport. Now, whatever node is opened (multiscales, plate, well), the grid or multiscale layer fits the whole viewport. This is much nicer than before IMO, and took refactoring the gridLayer.ts to be un-centered.

I've tried out almost all the examples from the OME-Blog and things seems to be working (much faster loading too!). The final issue I need to tackle is that all plates/wells are assumed to be complete. I think we actually need a better data-structure for loaders that allows for missing wells:

// src/ome.ts
const plateAttrs = attrs.plate;
const rows = plateAttrs.rows.map(row => row.name);
const plate = plateAttrs.columns.map(col => col.name);
const promises = plateAttrs.wells.map(well => grp.getItem(well.path).then(arr => [well.path, arr]));
const loaders: Map<string, ZarrPixelSource> = new Map(await Promise.all(promises));
// loaders.get('A/0').getTile(...)


// gridLayer.ts
const gridLayers = rows.flatMap((row, i) => {
      return columns.map((col, j) => {
        const y = top + i * (height + spacer);
        const x = left + j * (width + spacer);
        const offset = col + row * columns;
        const layerProps = {
          channelData: gridData.get(`${row}/${col}`) || null, // coerce to null if no data
          bounds: scaleBounds(width, height, [x, y]),
          id: `${id}-GridLayer-${row}-${col}`,
          dtype: loaders[offset]?.dtype || '<u2', // fallback if missing,
        };
        return new XRLayer({ ...this.props, ...layerProps });
      });
    });

What are your thoughts?

@manzt
Copy link
Member Author

manzt commented Feb 21, 2021

Still experimenting... but using a different data structure might also allow us keep names around for an additional text layer.

image

image

Maybe something like this would be easiest,

const loaders = [
  { label: 'A0', row: 0, col: 0, data: new ZarrPixelSource(...) },
  /* ... */
]

@manzt
Copy link
Member Author

manzt commented Feb 23, 2021

@will-moore
Copy link
Collaborator

Hi @manzt - Thanks, it's looking great. Centering is improved.
Nice to have fewer 404s and the labels are cool tool 👍

@manzt manzt merged commit 1ef2163 into master Feb 23, 2021
@manzt manzt mentioned this pull request Feb 23, 2021
@manzt manzt deleted the upgrade-viv branch February 26, 2021 15:56
@manzt manzt mentioned this pull request Mar 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants