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

[Bug] createSlabThicknessSynchronizer initial slice index issue #4485

Closed
salkz opened this issue Nov 12, 2024 · 3 comments
Closed

[Bug] createSlabThicknessSynchronizer initial slice index issue #4485

salkz opened this issue Nov 12, 2024 · 3 comments
Assignees
Labels
Awaiting Reproduction Can we reproduce the reported bug?

Comments

@salkz
Copy link

salkz commented Nov 12, 2024

Describe the Bug

We noticed a peculiar issue with createSlabThicknessSynchronizer - if you add this synchronizer to a viewport then the initial loaded slice index will be some very weird number instead of the usual middle slice.

Steps to Reproduce

Adding synchronizer to a viewport immediately on a VIEWPORT_DATA_CHANGED event it works unexpectedly:

const synchronizerId = 'SLAB_THICKNESS_SYNCHRONIZER_ID';
const synchronizer = synchronizers.createSlabThicknessSynchronizer(synchronizerId);
const renderingEngineId = 'OHIFCornerstoneRenderingEngine';

...

useEffect(() => {
  cornerstoneViewportService.subscribe(
    cornerstoneViewportService.EVENTS.VIEWPORT_DATA_CHANGED,
    event => {
      synchronizer.add({
        renderingEngineId,
        viewportId: event.viewportId,
      });
    }
  );
}, []);

However, when added with a setTimeout(..., 2000), it works as expected:

useEffect(() => {
  cornerstoneViewportService.subscribe(
    cornerstoneViewportService.EVENTS.VIEWPORT_DATA_CHANGED,
    event => {
      setTimeout(() => {
        synchronizer.add({
          renderingEngineId,
          viewportId: event.viewportId,
        });
      }, 2000);
    }
  );
}, []);

The current behavior

The initial slice indexes are some weird number as seen on the image:
image

The expected behavior

The initial slice indexes are for the middle slices in all viewports.

OS

macOS 14.5 (23F79)

Node version

v18.20.4

Browser

Version 130.0.6723.70 (Official Build) (arm64)

@salkz salkz added the Awaiting Reproduction Can we reproduce the reported bug? label Nov 12, 2024
@vlasn
Copy link

vlasn commented Nov 13, 2024

After experiencing very similar issues in one of our workflows that programmatically removes & restores viewports,
I was able to reproduce the issue (or something extremely similar) with the currently live OHIF version (
3.9.1, commit hash b417063) by

  1. Navigating HERE
  2. Executing the below code block in the browser console
  3. Resizing the window ever so slightly

Repro code:

((cvs) => {
    const firstViewportID = cvs.getViewportIds()[0];
    const firstViewport = cvs.getCornerstoneViewport(firstViewportID);
    firstViewport.setSlabThickness(5);
    firstViewport.render()
})(services.cornerstoneViewportService)

Expected behaviour: Viewports get rescaled according to new window width, mostly retain camera configs & the currently scrolled positions
Current behaviour: The (axial) viewport that I updated slice thicknesses in jumped to a different location in the volume

Notes:

  • no synchronizers are involved in the repro
  • if I destroy all implicit synchronizers (mpr, sameFORId) via cornerstoneTools.SynchronizerManager.destroy(id), the issue still persists
  • We later also found a no-code repro by
    • activating crosshairs
    • using one of the inner handles to change a viewport's slice thickness
    • resizing the window

@sedghi sedghi mentioned this issue Nov 13, 2024
12 tasks
@vlasn
Copy link

vlasn commented Nov 13, 2024

After some binary search with your published docker images, it looks like the issue was introduced in v3.9.0-beta.78

@sedghi
Copy link
Member

sedghi commented Nov 21, 2024

I pushed a fix, but the real solution is to recompute the image index when the slab thickness changes. We shouldn't keep the old slice numbers when the slab changes; instead, we should recalculate the number of slices after the slab thickness changes.

@sedghi sedghi closed this as completed Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Reproduction Can we reproduce the reported bug?
Projects
None yet
Development

No branches or pull requests

3 participants