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

Visualizations that use zoom-level precisely are quite off #806

Open
ilan-gold opened this issue Jun 25, 2024 · 16 comments
Open

Visualizations that use zoom-level precisely are quite off #806

ilan-gold opened this issue Jun 25, 2024 · 16 comments
Labels
bug Something isn't working

Comments

@ilan-gold
Copy link
Collaborator

ilan-gold commented Jun 25, 2024

Describe the bug
The scale bar is jumpy on the Avivator site (but somehow not on Vitessce). I imaging there is some sort of state management issue. In any case we should be rendering the scale bar in its own view anyway as suggested here

To Reproduce
avivator demo with a scale bar shows jumpiness

Expected behavior
The scale bar should be fixed like vitessce

Environment:

  • Release or git hash:
  • Browser: FF
  • Browser version: 127.0.1
@ilan-gold ilan-gold added the bug Something isn't working label Jun 25, 2024
@ilan-gold ilan-gold changed the title Jumpu scale bar Jumpy scale bar Jun 25, 2024
@ilan-gold
Copy link
Collaborator Author

Ok still getting my web development feet wet again. This only happens on FF (of course).

@ilan-gold
Copy link
Collaborator Author

This happens also on google chrome by using the scale bar. So I would venture a guess it has something to do with bounding box calculations.

@ilan-gold ilan-gold changed the title Jumpy scale bar Visualizations that use zoom-level precisely are quite off Jun 28, 2024
@ilan-gold
Copy link
Collaborator Author

Ok this also happens with the side-by-side viewer where one side will get thrown off. So I think there is something going on with the zoom level.

@ilan-gold
Copy link
Collaborator Author

It looks like this actually happens all the time, but is only noticed by things that use zoom state explicitly. These numbers should be monotonic
Screenshot 2024-06-28 at 12 35 21

@ilan-gold
Copy link
Collaborator Author

@xinaesthete See here for more discussion. I have a suspicion this is related to visgl/deck.gl#8989

@xinaesthete
Copy link
Contributor

I think I might have another look into this in combination with updating React / MUI / zustand etc as I still think these things may be somewhat related. In MDV we don't have the janky scale-bar issue and can also load views with appropriate zoom, as well as linking viewState between charts. The latter gets confused if the images have different physical pixel size, but apart from that it seems to work reasonably well and most of the relevant code is based on refactored version of Avivator...

There are some warnings from Zustand currently which it'd be good to get rid of and I'm not sure it's in-scope for the other PR, but it'd be nice to get a new release with all this somewhat cleaned up relatively soon... so I think I'll crack on with that now.

@xinaesthete
Copy link
Contributor

I've just been looking at an image that was encoded in a not terribly useful way - it has a few hundred channels which should really be z-slices etc, and no physical size metadata for the pixels... that last point seems to be helping viv keep the side-by-side views in sync perfectly where others go badly wrong.

@ilan-gold
Copy link
Collaborator Author

This has to be something with Avivator specifcially. https://portal.hubmapconsortium.org/browse/dataset/8690897fced9931da34d66d669c1d698?redirected=True#section-kaggle-1glomerulussegmentation-published for example works fine on safari.

@ilan-gold
Copy link
Collaborator Author

ilan-gold commented Sep 26, 2024

import React, { useState, useEffect } from 'react';
import ReactDOM from 'react-dom/client';
import {
  getChannelStats,
  loadOmeTiff,
  MultiscaleImageLayer,
  ImageLayer
} from '@hms-dbmi/viv';
import DeckGL from '@deck.gl/react';
import { OrthographicView } from '@deck.gl/core'
const url = 'https://viv-demo.storage.googleapis.com/Vanderbilt-Spraggins-Kidney-MxIF.ome.tif'; // OME-TIFF

const useImageLayer = true

// Hardcoded rendering properties.
const props = {
  selections: [
    { z: 0, t: 0, c: 0 },
    { z: 0, t: 0, c: 1 },
    { z: 0, t: 0, c: 2 },
  ],
  colors: [
    [0, 0, 255],
    [0, 255, 0],
    [255, 0, 0],
  ],
  contrastLimits: [
    [0, 10005],
    [0, 10005],
    [0, 10005],
  ],
  channelsVisible: [true, true, true],
}
const INITIAL_VIEW_STATE = useImageLayer ? { target: [100, 100, 0], zoom: -1 } : { target: [10000, 10000, 0], zoom: -7 }

function App() {
  const [loader, setLoader] = useState(null);
  const [viewState, setViewState] = useState(INITIAL_VIEW_STATE);
  const [autoProps, setAutoProps] = useState(null);
  useEffect(() => {
    loadOmeTiff(url).then(setLoader);
  }, []);

  // Viv exposes the getChannelStats to produce nice initial settings
  // so that users can have an "in focus" image immediately.

  async function computeProps(loader) {
    if (!loader) return null;
    // Use lowest level of the image pyramid for calculating stats.
    const source = loader.data[loader.data.length - 1];
    const stats = await Promise.all(props.selections.map(async selection => {
      const raster = await source.getRaster({ selection });
      return getChannelStats(raster.data);
    }));
    // These are calculated bounds for the contrastLimits
    // that could be used for display purposes.
    // domains = stats.map(stat => stat.domain);

    // These are precalculated settings for the contrastLimits that
    // should render a good, "in focus" image initially.
    const contrastLimits = stats.map(stat => stat.contrastLimits);
    const newProps = { ...props, contrastLimits };
    return newProps
  }

  useEffect(() => {

    computeProps(loader).then(setAutoProps)

  }, [loader])

  if (!loader || !autoProps) return null;
  const layer = loader && new (useImageLayer ? ImageLayer : MultiscaleImageLayer)({
    id: 'layer',
    loader: useImageLayer ? loader.data.slice(-1)[0] : loader.data,
    contrastLimits: props.contrastLimits,
      // Default extension is ColorPaletteExtension so no need to specify it if
      // that is the desired rendering, using the `colors` prop.
    colors: props.colors,
    channelsVisible: props.channelsVisible,
    selections: props.selections
  });

  return (
    <DeckGL
      layers={[layer]}
      views={[new OrthographicView({ id: 'ortho', controller: true })]}
      viewState={viewState}
      onViewStateChange={arg => setViewState(arg.viewState) && arg}
    />
  );
}
ReactDOM.createRoot(document.getElementById('root')).render(<App />);

is a reproducer without VivViewer and the same thing is still happening. Separately, I don't know why setViewState needs to be used like this but ok.

@ilan-gold
Copy link
Collaborator Author

ilan-gold commented Sep 26, 2024

Some things ruled out:

  1. VivViewer or related react-based mechanisms (so zustand included)
  2. ScaleBarLayer even though it's very janky
  3. Removing signal appears to have no effect so it's not the signal aborting mechanism
  4. This happens both with MultiscaleImageLayer and ImageLayer so it's probably XRLayer specific
  5. Related to the above, this isn't a data loading issue then

@xinaesthete
Copy link
Contributor

I've just been doing some debugging of viewState in MDV and it occurs to me that transitionInterpolator / transitionDuration may be potentially implicated in interfering with explicit precise setting of it.

@ilan-gold
Copy link
Collaborator Author

You mean trying to set them? I think I tried but can try again...I did notice that things got better for my other issue on FF. To confirm, are you seeing this?

@xinaesthete
Copy link
Contributor

I don't generally see janky scale-bar or zoom in MDV viv charts, and I do serialise viewState and haven't seen any issues with it looking right when loaded (so I haven't exactly tried to use 'precise zoom levels' in a scientific sense, but it seems like viewState is behaving pretty well there).

I was just implementing non-viv deck charts and handling viewState via mobx, which lead to me having issues with de-serialising unwanted transition properties, and also at some point I saw some slightly janky zoom behaviour (which I think has fixed itself now I've sorted out other issues). It occurred to me that it could conceivably be the case that if there was some kind of internal fighting between transitioning vs explicitly setting the state that could lead to the kind of issues we see in Avivator.

In a way it'd be nice to do more viewState handling internally to deck to reduce react re-renders, but that then means needing a different approach to linked views / serialisation etc.

@xinaesthete
Copy link
Contributor

In Avivator, the scale-bar has some one-frame-behind or similar jank particularly when devtools are open. Also some non-monotonic zoom glitches (I was maybe briefly seeing something a bit like this with my non-viv, mobx-state scatterplots earlier today).

I believe the main noticeable viv smoothness issue in MDV is blocking as data is loaded - which is a separate issue.

@ilan-gold
Copy link
Collaborator Author

In a way it'd be nice to do more viewState handling internally to deck to reduce react re-renders, but that then means needing a different approach to linked views / serialisation etc.

Open to ideas! Could be a nice new package!

Re: any perf issues, could you make a screen grab? It's super tough to compare experiences over text haha

@xinaesthete
Copy link
Contributor

Avivator in Chrome on M2 MBP, first without then with devtools:

2024-10-16.16-20-56.mp4

Maybe a little bit hard to tell what expected behaviour would be when you can't see what the input gestures are - I can barely see here, but there is some occasional visible zoom-glitch. When devtools is open, the scalebar is all over the place and there are more glitches in the other behaviour.

Here's another that catches a non-devtools zoom glitch as it eases-out of the zooming out:

2024-10-16.16-38-07.mov

Here's MDV (with devtools open), obviously showing different data but the only flaws are the blocking as it loads (and the viewState handling is mostly equivalent but with some differences in zustand version and use - I'm still not sure we've totally ruled that stuff out).

2024-10-16.16-25-30.mov

Also the scale-bar is obscured by the scatterplot, but that's another issue entirely - at least the transform isn't glitchy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants