Export BitmapLayer

[andreasg123]

User story

To better support the transition between z-slices, I implemented my own version of MultiscaleImageLayer. I would like to maintain support for interlaced RGB images.

If you are interested, I could work on a PR for porting my approach to MultiscaleImageLayer. I create a second tile layer for the newly selected z-slice on top of the previous tile layer and remove the old tile layer in response to onViewportLoad. However, that approach only works for opacity 1.

Preferred solution

Export BitmapLayer

Possible alternatives

I could use your version of MultiscaleImageLayer just for interlaced RGB images because those are less likely to contain z-slices.

[ilan-gold]

@andreasg123 At a high level, we would like to fix this issue with transitioning between z-slices. A few questions just so I can understand:

1. Are you saying you have another composite layer that then calls our MultiscaleImageLayer that has an onViewportLoad function? Just trying to understand what is going on here. I ask because I wonder if we could create a small patch for this z-slicing issue by adding the z-selection to the ids so that new layers are created every time it changes (haven't tried this, but sounds very similar to creating new layers for each z-slice): https://github.com/hms-dbmi/viv/blob/8efa2b4483b8a6103ec2ef5864ed4410ec2177ae/src/views/DetailView.js#L21
2. How does this relate to the fix proposed in #337 (specifically the deck.gl issue there)? Is this a stopgap measure in the meantime? I think deck.gl is at the end of a release cycle so it might be good to make the contribution there now instead of this or we will have to wait until the 8.5 release cycle starts alpha/beta release. Of course we can and probably should export our BitmapLayer - that is certainly an oversight.

[ilan-gold]

Just to follow up to 1., my idea (if it works) would isolate the fix to just changing our VivView classes instead of editing the actual MultiscaleImageLayer, which would be better than editing the MultiscaleImageLayer - if it doesn't work, then I would consider your change once I understand it better.

[andreasg123]

1. I created my own versions of MultiscaleImageLayerBase and MultiscaleImageLayer that started as copies of yours. I had to do that anyway to get access to onViewportLoad in TileLayer. If your layers exposed the TileLayer callbacks, I may have found a different solution with changes just to DetailView. The issue was that a change in z-selection caused the really coarse pixels from ImageLayer to be displayed. My users preferred the fade-to-black after I removed ImageLayer (initially by setting opacity=0.99) because the coarse pixels didn't line up with image features. I didn't find any negative effects after removing it, even when zooming out a lot. The new approach displays the tiles from the new z-selection on top of the previous ones so that there is a smooth transition, allowing users to see the pixel changes, e.g., registration shifts from one z-slice to the next. I set refinementStrategy=never for the top layer to make sure that only the desired tiles are being displayed.

2. This is separate from #337. I managed to address the loading of tiles by putting a 50 ms, trailing edge bounce on the global selection sliders (in addition to the comparison whether the global selection changed). That deals with the situation where the user rapidly moves the slider by several stops, avoiding the loading for tiles from intermediate stops. I also set maxRequests=4. It would still be desirable to get the deck.gl fix but it isn't urgent.

[ilan-gold]

@andreasg123 I think I understand - this issue has to do with the transitions between selections, rather than the actual process of making selections? Do you have a feature branch I could test out? I think if we were to hoist this out the DetailView, it could be a nice feature to have. Love incorporating user feedback into what we build so this is really great!

[andreasg123]

@ilan-gold, can you point me to a public OME-TIFF or OME-Zarr image with z-slices?

As I wrote before, at the minimum, this requires the onViewportLoad property from TileLayer to be exposed all the way out to a React component (VivViewer or PictureInPictureViewer) so that the last loaded selection can be stored in a state hook or in the state.

onViewportLoad = () => {
  // Slightly delay to avoid issues with a render in the middle of a deck.gl layer state update.
  setTimeout(() => {
    setViewportSelection(loaderSelection);
  }, 0);
};

While DetailView could create a second instance of MultiscaleImageLayer, having MultiscaleImageLayer create two instances of MultiscaleImageLayerBase should provide better performance and may even be the only way to accomplish this. The reason is that I added the global selection to the layer ID there. When the layer for the new global selection becomes the only layer after the viewport tiles are loaded, it keeps its ID such that all the deck.gl state remains with it.

const suffix = GLOBAL_SLIDER_DIMENSION_FIELDS.map(f => loaderSelection[0][f]).join('-');
const layer = new MultiscaleImageLayerBase({
  ...this.props,
  id: `Tiled-Image-${id}-${suffix}`,

One could split the change into two parts, i.e., change MultiscaleImageLayer to include the global selection in the ID and have DetailView create two instances of MultiscaleImageLayer with different loaderSelection. However, that approach would still prevent having refinementStrategy=never for one of them.

[ilan-gold]

@andreasg123 Here is an image you can test against: http://avivator.gehlenborglab.org/?image_url=https://viv-demo.storage.googleapis.com/2018-12-18_ASY_H2B_bud_05_3D_8_angles.ome.tif

I think without seeing a diff/demo it's hard to say exactly what I would want and not want out of this, but my general sense is that I don't think I would want to do anything loaderSelection-specific to the layers - exposing TileLayer callbacks, editing the VivView classes, exposing props (like refinementStrategy or a useBackgroundImage prop) all feel doable though. If you don't want to make the contribution though without the "full change" you have, I understand that and I can just expose the BitmapLayer (and MultiscaleImageLayerBase if that would help).

[andreasg123]

For my internal PR, I created a diff of what I changed in the copied Viv code (see below; I reverted the minZoom change after creating the diff). Some of those changes were only necessary because I needed to access internal symbols such as MultiscaleImageLayerBase, renderSubLayers, onPointer. In some cases, I renamed the copied classes to avoid future name clashes. Some of the other changes are just due to TypeScript. There are a few changes due to my different Prettier settings. The diff doesn't show my changes to DetailView and above because I forked that earlier.

If the change to MultiscaleImageLayer does look reasonable to you (new prop newLoaderSelection, pass-through of onViewportLoad, include global selection in layer ID, loop over two selections to create layers), I can create a PR. In that PR, I would attempt to figure out how to convert my React state hook for the last selection where the tiles loaded to something that could be put into VivViewer.state (I've stopped using class-based React components a while ago).

--- .../viv/src/layers/MultiscaleImageLayer/MultiscaleImageLayer.js 2021-01-07 09:41:02.000000000 -0800
+++ src/components/layers/ImagePyramidLayer.ts  2021-01-09 19:42:02.000000000 -0800
@@ -1,11 +1,16 @@
-import { CompositeLayer } from '@deck.gl/core';
+import { CompositeLayer, COORDINATE_SYSTEM } from '@deck.gl/core';
+import { CompositeLayerProps } from '@deck.gl/core/lib/composite-layer';
+import { TileLayer } from '@deck.gl/geo-layers';
+import { TileLayerProps } from '@deck.gl/geo-layers/tile-layer/tile-layer';
 import { isWebGL2 } from '@luma.gl/core';
-import { Matrix4 } from 'math.gl';
 import GL from '@luma.gl/constants';
+import { XRLayer } from '@hms-dbmi/viv';
+import { ChannelSelection, GLOBAL_SLIDER_DIMENSION_FIELDS } from '../types';

-import MultiscaleImageLayerBase from './MultiscaleImageLayerBase';
-import ImageLayer from '../ImageLayer';
-import { to32BitFloat, onPointer } from '../utils';
+// This started as a copy of https://github.com/hms-dbmi/viv/blob/master/src/layers/MultiscaleImageLayer/MultiscaleImageLayer.js
+// It handles global selection changes more smoothly by adding a layer for the new selection.
+
+const noop = () => {};

 const defaultProps = {
   pickable: true,
@@ -23,11 +28,11 @@
   lensBorderColor: { type: 'array', value: [255, 255, 255], compare: true },
   lensBorderRadius: { type: 'number', value: 0.02, compare: true },
   maxRequests: { type: 'number', value: 10, compare: true },
-  onClick: { type: 'function', value: null, compare: true }
+  onClick: { type: 'function', value: null, compare: true },
 };

 /**
- * This layer generates a MultiscaleImageLayer (tiled) and a ImageLayer (background for the tiled layer)
+ * This layer generates a ViewportTileLayer (tiled)
  * @param {Object} props
  * @param {Array} props.sliderValues List of [begin, end] values to control each channel's ramp function.
  * @param {Array} props.colorValues List of [r, g, b] values for each channel.
@@ -51,19 +56,50 @@
  * @param {number} props.modelMatrix Math.gl Matrix4 object containing an affine transformation to be applied to the image.
  */

-export default class MultiscaleImageLayer extends CompositeLayer {
+interface ImagePyramidLayerProps extends CompositeLayerProps<any> {
+  pickable: boolean;
+  onHover: () => void;
+  sliderValues: any[];
+  colorValues: any[];
+  channelIsOn: boolean[];
+  opacity: number;
+  colormap: string;
+  domain: number[][];
+  viewportId: string;
+  isLensOn: boolean;
+  lensSelection: number;
+  lensRadius: number;
+  lensBorderColor: number[];
+  lensBorderRadius: number;
+  maxRequests: number;
+  onClick: () => void;
+  loader: any;
+  loaderSelection: ChannelSelection[];
+  newLoaderSelection: ChannelSelection[];
+  onTileError: () => void;
+  // eslint-disable-next-line no-unused-vars
+  onViewportLoad: (data: any[]) => void;
+}
+
+export default class ImagePyramidLayer extends CompositeLayer<
+  any,
+  ImagePyramidLayerProps
+> {
+  static layerName = 'ImagePyramidLayer';
+  static defaultProps = defaultProps;
+
   initializeState() {
     this.state = {
-      unprojectLensBounds: [0, 0, 0, 0]
+      unprojectLensBounds: [0, 0, 0, 0],
     };
-    if (this.context.deck) {
-      this.context.deck.eventManager.on({
+    // "eventManager" isn't declared in Deck
+    const deck: any = this.context.deck;
+    deck?.eventManager.on({
         pointermove: () => onPointer(this),
         pointerleave: () => onPointer(this),
-        wheel: () => onPointer(this)
+      wheel: () => onPointer(this),
       });
     }
-  }

   renderLayers() {
     const {
@@ -72,6 +108,7 @@
       colorValues,
       channelIsOn,
       loaderSelection,
+      newLoaderSelection,
       domain,
       opacity,
       colormap,
@@ -86,50 +123,59 @@
       lensBorderRadius,
       maxRequests,
       onClick,
-      modelMatrix
+      modelMatrix,
+      onViewportLoad,
     } = this.props;
+    if (!loaderSelection?.length) {
+      return [];
+    }
     const { tileSize, numLevels, dtype, isInterleaved, isRgb } = loader;
     const { unprojectLensBounds } = this.state;
     const noWebGl2 = !isWebGL2(this.context.gl);
-    const getTileData = async ({ x, y, z, signal }) => {
+    // newLoaderSelection != null indicates that there is a pending change of global selections that require loading new tiles.
+    // Even though the deck.gl documentation is sparse on this topic, it has been verified that the later layer is on top.
+    return [loaderSelection, newLoaderSelection]
+      .filter(s => s)
+      .map((s, i) => {
+        // The declaration from TileLayer is missing "signal" and has the wrong return type.
+        const getTileData: any = async ({ x, y, z, signal }) => {
       const tile = await loader.getTile({
         x,
         y,
-        // I don't fully undertstand why this works, but I have a sense.
-        // It's basically to cancel out:
-        // https://github.com/visgl/deck.gl/pull/4616/files#diff-4d6a2e500c0e79e12e562c4f1217dc80R128,
-        // which felt odd to me to beign with.
-        // The image-tile example works without, this but I have a feeling there is something
-        // going on with our pyramids and/or rendering that is different.
         z: Math.round(-z + Math.log2(512 / tileSize)),
-        loaderSelection,
-        signal
+            // The different loaderSelection is the only difference for getTileData.
+            loaderSelection: s,
+            signal,
       });
       if (isInterleaved && isRgb) {
         // eslint-disable-next-line prefer-destructuring
         tile.data = tile.data[0];
         if (tile.data.length === tile.width * tile.height * 3) {
           tile.format = GL.RGB;
-          tile.dataFormat = GL.RGB; // is this not properly inferred?
+              tile.dataFormat = GL.RGB;
         }
-        // can just return early, no need  to check for webgl2
         return tile;
       }
       if (noWebGl2) {
-        tile.data = to32BitFloat(tile.data);
+            tile.data = tile.data.map(arr => new Float32Array(arr));
       }
       return tile;
     };
+        // Tile layers are now identified by the global selection to properly match them in the deck.gl lifecycle.
+        const suffix = GLOBAL_SLIDER_DIMENSION_FIELDS.map(f => s[0][f]).join(
+          '-',
+        );
     const { height, width } = loader.getRasterSize({ z: 0 });
-    const tiledLayer = new MultiscaleImageLayerBase(this.props, {
-      id: `Tiled-Image-${id}`,
+        // TODO: Removed "s" from updateTriggers.getTileData for now. Check if it's needed when channels are added.
+        return new ViewportTileLayer({
+          ...this.props,
+          id: `Tiled-Image-${id}-${suffix}`,
       getTileData,
       dtype,
       tileSize,
       onClick,
       extent: [0, 0, width, height],
-      // See the above note within getTileData for why the division with 512 and the rounding necessary.
-      minZoom: Math.round(-(numLevels - 1) + Math.log2(512 / tileSize)),
+          minZoom: -(numLevels - 1),
       maxZoom: Math.min(0, Math.round(Math.log2(512 / tileSize))),
       colorValues,
       sliderValues,
@@ -138,12 +184,13 @@
       domain,
       // We want a no-overlap caching strategy with an opacity < 1 to prevent
       // multiple rendered sublayers (some of which have been cached) from overlapping
-      refinementStrategy: opacity === 1 ? 'best-available' : 'no-overlap',
+          refinementStrategy:
+            i !== 0 ? 'never' : opacity === 1 ? 'best-available' : 'no-overlap',
       // TileLayer checks `changeFlags.updateTriggersChanged.getTileData` to see if tile cache
       // needs to be re-created. We want to trigger this behavior if the loader changes.
       // https://github.com/uber/deck.gl/blob/3f67ea6dfd09a4d74122f93903cb6b819dd88d52/modules/geo-layers/src/tile-layer/tile-layer.js#L50
       updateTriggers: {
-        getTileData: [loader, loaderSelection]
+            getTileData: [loader],
       },
       onTileError: onTileError || loader.onTileError,
       opacity,
@@ -156,31 +203,10 @@
       lensSelection,
       lensBorderColor,
       lensBorderRadius,
-      modelMatrix
+          modelMatrix,
+          onViewportLoad: i === 0 ? noop : onViewportLoad,
     });
-    // This gives us a background image and also solves the current
-    // minZoom funny business.  We don't use it for the background if we have an opacity
-    // paramteter set to anything but 1, but we always use it for situations where
-    // we are zoomed out too far.
-    const implementsGetRaster = typeof loader.getRaster === 'function';
-    const layerModelMatrix = modelMatrix ? modelMatrix.clone() : new Matrix4();
-    const baseLayer =
-      implementsGetRaster &&
-      new ImageLayer(this.props, {
-        id: `Background-Image-${id}`,
-        modelMatrix: layerModelMatrix.scale(2 ** (numLevels - 1)),
-        visible:
-          opacity === 1 &&
-          (!viewportId || this.context.viewport.id === viewportId),
-        z: numLevels - 1,
-        pickable: true,
-        onHover,
-        onClick
       });
-    const layers = [baseLayer, tiledLayer];
-    return layers;
   }
 }

-MultiscaleImageLayer.layerName = 'MultiscaleImageLayer';
-MultiscaleImageLayer.defaultProps = defaultProps;
--- .../viv/src/layers/MultiscaleImageLayer/utils.js    2020-12-22 15:36:11.000000000 -0800
+++ src/components/layers/ImagePyramidLayer.ts  2021-01-09 19:42:02.000000000 -0800
@@ -1,9 +1,9 @@
-export function renderSubLayers(props) {
+function renderSubLayers(props) {
   const {
     bbox: { left, top, right, bottom },
     x,
     y,
-    z
+    z,
   } = props.tile;
   const {
     colorValues,
@@ -29,9 +353,9 @@
     lensSelection,
     onClick,
     loader,
-    modelMatrix
+    modelMatrix,
   } = props;
-  // Only render in positive coorinate system
+  // Only render in positive coordinate system
   if ([left, bottom, right, top].some(v => v < 0) || !data) {
     return null;
   }
@@ -42,26 +366,9 @@
     left,
     data.height < loader.tileSize ? height : bottom,
     data.width < loader.tileSize ? width : right,
-    top
+    top,
   ];
-  const { isRgb, isInterleaved, photometricInterpretation } = loader;
-  if (isRgb && isInterleaved) {
-    return new BitmapLayer(props, {
-      image: data,
-      photometricInterpretation,
-      // Shared props with XRLayer:
-      bounds,
-      id: `tile-sub-layer-${bounds}-${id}`,
-      tileId: { x, y, z },
-      onHover,
-      pickable,
-      onClick,
-      modelMatrix,
-      opacity,
-      visible
-    });
-  }
-  return new XRLayer(props, {
+  const layer = new XRLayer(props, {
     channelData: data,
     // Uncomment to help debugging - shades the tile being hovered over.
     // autoHighlight: true,
@@ -84,6 +391,7 @@
     onClick,
     modelMatrix,
     opacity,
-    visible
+    visible,
   });
+  return layer;
 }
--- .../viv/src/layers/utils.js 2020-12-28 12:04:38.000000000 -0800
+++ src/components/layers/ImagePyramidLayer.ts  2021-01-09 19:42:02.000000000 -0800
@@ -1,25 +1,26 @@
-export function onPointer(layer) {
+function onPointer(layer: ImagePyramidLayer) {
   const { viewportId, lensRadius } = layer.props;
   // If there is no viewportId, don't try to do anything.
   if (!viewportId) {
     layer.setState({ unprojectLensBounds: [0, 0, 0, 0] });
     return;
   }
-  const { mousePosition } = layer.context;
-  const layerView = layer.context.deck.viewManager.views.filter(
-    view => view.id === viewportId
+  const mousePosition: any = layer.context.mousePosition;
+  const deck: any = layer.context.deck;
+  const layerView = deck.viewManager.views.filter(
+    view => view.id === viewportId,
   )[0];
-  const viewState = layer.context.deck.viewManager.viewState[viewportId];
+  const viewState = deck.viewManager.viewState[viewportId];
   const viewport = layerView.makeViewport({
     ...viewState,
-    viewState
+    viewState,
   });
   // If the mouse is in the viewport and the mousePosition exists, set
   // the state with the bounding box of the circle that will render as a lens.
   if (mousePosition && viewport.containsPixel(mousePosition)) {
     const offsetMousePosition = {
       x: mousePosition.x - viewport.x,
-      y: mousePosition.y - viewport.y
+      y: mousePosition.y - viewport.y,
     };
     const mousePositionBounds = [
       // left
@@ -99,14 +242,14 @@
       // right
       [offsetMousePosition.x + lensRadius, offsetMousePosition.y],
       // top
-      [offsetMousePosition.x, offsetMousePosition.y - lensRadius]
+      [offsetMousePosition.x, offsetMousePosition.y - lensRadius],
     ];
     // Unproject from screen to world coordinates.
     const unprojectLensBounds = mousePositionBounds.map(
-      (bounds, i) => viewport.unproject(bounds)[i % 2]
+      (bounds, i) => viewport.unproject(bounds)[i % 2],
     );
     layer.setState({ unprojectLensBounds });
   } else {
     layer.setState({ unprojectLensBounds: [0, 0, 0, 0] });
   }
 }
--- .../viv/src/layers/MultiscaleImageLayer/MultiscaleImageLayerBase.js 2020-12-22 15:36:11.000000000 -0800
+++ src/components/layers/ImagePyramidLayer.ts  2021-01-09 19:42:02.000000000 -0800
@@ -1,5 +1,5 @@
-const defaultProps = {
+const defaultTileProps = {
   pickable: true,
   coordinateSystem: COORDINATE_SYSTEM.CARTESIAN,
   sliderValues: { type: 'array', value: [], compare: true },
   colorValues: { type: 'array', value: [], compare: true },
@@ -21,13 +273,40 @@
   lensSelection: { type: 'number', value: 0, compare: true },
   lensRadius: { type: 'number', value: 100, compare: true },
   lensBorderColor: { type: 'array', value: [255, 255, 255], compare: true },
-  lensBorderRadius: { type: 'number', value: 0.02, compare: true }
+  lensBorderRadius: { type: 'number', value: 0.02, compare: true },
 };

+interface ViewportTileLayerProps extends TileLayerProps<any> {
+  pickable: boolean;
+  sliderValues: any[];
+  colorValues: any[];
+  channelIsOn: boolean[];
+  minZoom?: number;
+  maxZoom?: number;
+  opacity?: number;
+  colormap?: string;
+  dtype?: string;
+  domain: number[][];
+  viewportId?: string;
+  unprojectLensBounds?: number[];
+  isLensOn: boolean;
+  lensSelection?: number;
+  lensRadius?: number;
+  lensBorderColor?: number[];
+  lensBorderRadius?: number;
+}
+
 /**
  * This layer serves as a proxy of sorts to the rendering done in renderSubLayers, reacting to viewport changes in a custom manner.
  */
-export default class MultiscaleImageLayerBase extends TileLayer {
+class ViewportTileLayer extends TileLayer<any, ViewportTileLayerProps> {
+  static layerName = 'ViewportTileLayer';
+  static defaultProps = defaultTileProps;
+
+  constructor(props: ViewportTileLayerProps) {
+    super(props);
+  }
+
   /**
    * This function allows us to controls which viewport gets to update the Tileset2D.
    * This is a uniquely TileLayer issue since it updates based on viewport updates thanks
@@ -50,5 +329,3 @@
   }
 }

-MultiscaleImageLayerBase.layerName = 'MultiscaleImageLayerBase';
-MultiscaleImageLayerBase.defaultProps = defaultProps;

[ilan-gold]

@andreasg123 I would like to understand the use-case more. Are the images you are using large and thin (small z stack) or more medium sized and very large in z, such as a lightsheet dataset (like the one I linked to above)? Does this fix work with a lot of rapid changes? It seems that for very rapid changes, the issue highlighted in #337 would prevent this change from having a real effect since intermediate tiles would be aborted (if you are trying to view registration between serial selections, this seems like the opposite of what you wantx). I think a demo + test data would really help me understand. Otherwise, I would accept a generalization of the MultiscaleImageLayer API to allow you to do what you need here (you highlighted onViewportLoad but we could also have an editLayers function that gets called on every render to add/edit the layers that would be returned by MultiscaleImageLayer).

I guess what I am trying to say is that without a demo/a screen-movie/gif it is hard for me to conceptualize this, but I would accept a PR that generalizes our API via callbacks or exposing props to allow you to do what you need - if you need to make changes in this way, others could too. I know for a fact that turning on and off (or just altering) the background layer is something that could be of interest to others, for example.

[andreasg123]

In this particular image, I have 27 z-slices. However, the number of z-slices isn't relevant for the change that I made. Below are three videos that show the effect. The zoom is 0 (largest resolution with actual pixels). The tile size is 1024. All tiles are in the browser cache. Without the cache, the transition would be much slower.

The first two videos are from your version with an opacity of 1 and 0.99, respectively. With an opacity of 1, the ImageLayer shows very coarse pixels during the transition. With on opacity of 0.99, the ImageLayer isn't included so that a fade to black happens during the transition.

The third video shows my version. The only perceivable change is the imperfect registration of the z-slices.

I still think that the best option is to modify MultiscaleImageLayer (called ImagePyramidLayer in my version in the diff above). The two main differences are that it has an extra parameter newLoaderSelection, containing the loader selection for the newly selected z-slice, and the pass-through of onViewportLoad. It also drops ImageLayer because I couldn't find a situation where it was helpful.

https://user-images.githubusercontent.com/15636985/104655200-33c8f300-5672-11eb-8166-fc5a4cdd98cf.mov

https://user-images.githubusercontent.com/15636985/104655215-3cb9c480-5672-11eb-87cf-4163c1afdd2b.mov

https://user-images.githubusercontent.com/15636985/104655227-44796900-5672-11eb-9c1f-8a126800ced6.mov

[andreasg123]

None of VivViewer, PictureInPictureViewer, and DetailView document the opacity property. Would it be reasonable to assume that opacity==1 if my proposed newLoaderSelection property is passed to MultiscaleImageLayer (or that blending during the transition would be acceptable)?

[ilan-gold]

@andreasg123 Could we avoid the extra parameter newLoaderSelection by storing loaderSelection in state (or on the object somehow)? If that plus the onViewportLoad prop are the only changes, that would be ok. Would those two things solve the issue? It seemed like your diff included created two ViewportTileLayer instances within MultiscaleImageLayer, which I don't think we would want to have as part of the layer. I think it would be easiest if you opened a PR and we could communicate over the code diff - this seems like a feature we want somewhere, it is just a matter of working out exactly what the API will look like and where the "feature" lives. Can you share what the layer looks like without the caching?

[ilan-gold]

I think the other thing is that having an explicit dependency on GLOBAL_SLIDER_DIMENSION_FIELDS within the MultiscaleImageLayer is not optimal. Sorry to be particular, but I want to keep MultiscaleImageLayer as general as possible with as few dependencies as possible. I do think there is a solution here that could really work for us, and I really appreciate the discussion here.

[andreasg123]

For my approach to work, there have to be two instances of MultiscaleImageLayerBase, one for each of the two loader selections that are displayed at the same time during the transition. Those could be created by the same instance of MultiscaleImageLayer like in my implementation. Alternatively, DetailView could create two instances of MultiscaleImageLayer, one for each loader selection. The top instance of MultiscaleImageLayer must not create an ImageLayer. That probably means that there would have to be a Boolean property for MultiscaleImageLayer to indicate whether it is the top layer. onViewportLoaded could be used for that purpose because only the top layer would get it.

It would be desirable if the top instance of MultiscaleImageLayerBase would specify refinementStrategy=never to make sure that only the requested pixels are being displayed.

Regarding the global selection, deck.gl copies layer properties by ID, requiring different IDs for the two tile layers. That also means that after the new tiles are loaded, the top layer needs to pass on all its properties to the remaining tile layer. I initially considered toggling back and forth between two IDs but I think that including the global selection in the layer ID is superior. Can you suggest an alternative for determining what the global selection is, maybe just everything that isn't channel?

I'll create another set of videos tomorrow with the cache disabled in DevTools.

[ilan-gold]

Thanks @andreasg123 - it seems like we're converging on something here.

For my approach to work, there have to be two instances of MultiscaleImageLayerBase, one for each of the two loader selections that are displayed at the same time during the transition. Those could be created by the same instance of MultiscaleImageLayer like in my implementation. Alternatively, DetailView could create two instances of MultiscaleImageLayer, one for each loader selection. The top instance of MultiscaleImageLayer must not create an ImageLayer. That probably means that there would have to be a Boolean property for MultiscaleImageLayer to indicate whether it is the top layer. onViewportLoaded could be used for that purpose because only the top layer would get it.

It would be desirable if the top instance of MultiscaleImageLayerBase would specify refinementStrategy=never to make sure that only the requested pixels are being displayed.

I think we can expose the necessary props on the layer for these things (i.e a prop for whether or not to use the background ImageLayer and/or a prop for specifying refinementStrategy).

Regarding the global selection, deck.gl copies layer properties by ID, requiring different IDs for the two tile layers. That also means that after the new tiles are loaded, the top layer needs to pass on all its properties to the remaining tile layer. I initially considered toggling back and forth between two IDs but I think that including the global selection in the layer ID is superior. Can you suggest an alternative for determining what the global selection is, maybe just everything that isn't channel?

I would be ok with GLOBAL_SLIDER_DIMENSION_FIELDS in the DetailView, or perhaps some sort of prop (whose default is GLOBAL_SLIDER_DIMENSION_FIELDS) to specify those dimension fields to the react component PictureInPictureViewer, for example.

[andreasg123]

Just to clarify, you prefer DetailView creating two instances of MultiscaleImageLayer over MultiscaleImageLayer creating two instances of MultiscaleImageLayerBase?

I would be ok with GLOBAL_SLIDER_DIMENSION_FIELDS in the DetailView, or perhaps some sort of prop (whose default is GLOBAL_SLIDER_DIMENSION_FIELDS) to specify those dimension fields to the react component PictureInPictureViewer, for example.

DetailView could include the global selection in the ID passed to MultiscaleImageLayer. Would it be better to have a constant indicating the non-global selection that would contain 'channel' to filter out what not to include in the ID?

I haven't tried this with non-pyramid images so that I would only implement it for the isPyramid case.

[andreasg123]

Here are the new videos with cache disabled and a remote image. The delay is the same for all three, just what's displayed during the transition differs.

https://user-images.githubusercontent.com/15636985/104675942-4efc2880-569b-11eb-96e4-0f1631b06373.mov

https://user-images.githubusercontent.com/15636985/104675948-51f71900-569b-11eb-9f4d-1c1f0a97b277.mov

https://user-images.githubusercontent.com/15636985/104675953-54597300-569b-11eb-89a5-80091ec89959.mov

[ilan-gold]

Just to clarify, you prefer DetailView creating two instances of MultiscaleImageLayer over MultiscaleImageLayer creating two instances of MultiscaleImageLayerBase?

Yes, I think this is the route to go, for now at least.

DetailView could include the global selection in the ID passed to MultiscaleImageLayer. Would it be better to have a constant indicating the non-global selection that would contain 'channel' to filter out what not to include in the ID?

I think we could include a transitionFields prop to the react component that gets passed on to the VivView which can be used to determine which kinds of selections cause these sorts of transitions, which would default to GLOBAL_SLIDER_DIMENSION_FIELDS. I do like this more than a negative prop for filtering out what not to include since inclusion -> new kind of transition (maybe your PR will change my mind once I have the code in front of me).

I haven't tried this with non-pyramid images so that I would only implement it for the isPyramid case.

I think the code should be relatively similar to the multi-scale, but yes, this would need to be resolved. The branch I shared in #337 might be of help, but I am not sure.

[ilan-gold]

Fixed in #357