OHIF / Viewers

OHIF zero-footprint DICOM viewer and oncology specific Lesion Tracker, plus shared extension packages
https://docs.ohif.org/
MIT License
3.35k stars 3.37k forks source link

[Bug] Stack viewport broken when switch from volume viewport if set config useNorm16Texture = true #4167

Closed tunglt1810 closed 7 hours ago

tunglt1810 commented 5 months ago

Describe the Bug

Hi guys, I am working with a fork version which forked since 4 months ago. I faced with an issue when trying switch hanging protocol from mpr to default. The first time loaded and render is ok. I switch to protocol primaryAxial is ok. But when switch back to protocol default, it looks like image is crashed. But one more time when I switch to protocol primaryAxial, the viewports look fine.

first load ok primaryAxial ok errors

Maybe I was wrong in wording, so I raise this issue to ask about some hints to debug this issue

Steps to Reproduce

  1. Run app
  2. Open the first study CTA Head and Neck image
  3. Switch to hanging protocol primaryAxial
  4. Switch back to handing protocol default

The current behavior

After switch hanging protocol from primaryAxial to default, viewport image looks like broken

The expected behavior

I expect that viewport image will be not broken

OS

macOS 14.5

Node version

v18.16.1

Browser

Chrome 125.0.6422.61

ahlaughland commented 5 months ago

Not sure if this is related but it sounds similar to what I'm seeing.

I'm also on a forked branch of OHIF. We're at 3.9.0-beta.0 and usng the new layout selector.

When I switch protocols using the new layout switcher the viewports don't seem to update, like above. For example, if I swtich from a view of with 3 viewports like: [axial][saggital][coronal] and switch to just an [axial] and back to [axial][saggital][coronal] the [saggital][coronal] viewports don't update. I have to re-select the [axial][saggital][coronal] option then the viewports update and look correct.

We are calling a function to update to viewports' opacity threshold from the protocol callbacks.onProtocolEnter.

We use some VTK code directly and it looks like the VTK Actor entry is not ready and is missing the getProperty() from volumeActor.getProperty()...

If I wrap our function calling the actor using a setTimeout(() => ,500); the actor is then available and the viewports are updated.

Again, not sure if this is related to the original post but thought I would pass it along.

tunglt1810 commented 5 months ago

Hi @ahlaughland , Thank for your sharing. I also call a command to enable crosshairs tool when onProtocolEnter. And the calling setHangingProtocol with id is default after that. Do you think I need to call something before calling command to active crosshairs tool?

https://github.com/OHIF/Viewers/assets/19906349/4229fb2c-44c3-4d5d-b0fb-0c84470236ad

ahlaughland commented 5 months ago

Hi @ahlaughland , Thank for your sharing. I also call a command to enable crosshairs tool when onProtocolEnter. And the calling setHangingProtocol with id is default after that. Do you think I need to call something before calling command to active crosshairs tool?

Screen.Recording.2024-05-28.at.22.23.52.mov

I would think activating the tool for the correct toolGroupId would be enough.

tunglt1810 commented 5 months ago

Now I can reproduce the error. I made a command that toggle between volume and stack viewport. Now the stack viewport will be crashed after switch from volume viewport. Do I miss something or some function to call when setting up volume viewport ? I also add log debug from renderingEngine. It looks like the pixel data has decreased by 4 times

toggleSwitchMPR: async ({ orientation }) => {
    const activeViewportID = viewportGridService.getActiveViewportId();
    const viewportInfo = cornerstoneViewportService.getViewportInfo(activeViewportID);
    const currentViewportData = viewportInfo.viewportData;
    const displaySetInstanceUID = currentViewportData.data.length
        ? currentViewportData.data[0].displaySetInstanceUID
        : currentViewportData.data.displaySetInstanceUID;
    const displaySet = displaySetService.getDisplaySetByUID(displaySetInstanceUID);
    const dataSource = extensionManager.getActiveDataSource()[0];
    const renderingEngine = cornerstoneViewportService.getRenderingEngine();
    if (currentViewportData.viewportType === 'stack') {
        const offScreen = renderingEngine._debugRender();
        console.log(`debug render1`, offScreen);
        const viewportOptions = {
            viewportType: 'volume',
            orientation: orientation
        };
        const viewportData = await cornerstoneCacheService.createViewportData([displaySet], viewportOptions, dataSource);
        const publicViewportOptions: PublicViewportOptions = {
            id: orientation,
            viewportType: 'volume',
            toolGroupId: 'mpr',
            orientation: orientation
        };
        cornerstoneViewportService.setViewportData(
            activeViewportID,
            viewportData,
            publicViewportOptions,
            viewportInfo.displaySetOptions
        );
    } else {
        const viewportOptions = {
            viewportType: 'stack'
        };
        const viewportData = await cornerstoneCacheService.createViewportData([displaySet], viewportOptions, dataSource);
        const publicViewportOptions: PublicViewportOptions = {
            viewportType: 'stack',
            toolGroupId: 'default',
            initialImageOptions: {
                preset: 'middle'
            }
        };
        cornerstoneViewportService.setViewportData(
            activeViewportID,
            viewportData,
            publicViewportOptions,
            viewportInfo.displaySetOptions
        );
        window.setTimeout(() => {
            const offScreen = renderingEngine._debugRender();
            console.log(`debug render2`, offScreen);
        }, 500);
    }
}

Before switch from stack viewport to volume viewport

image

After switch from volume viewport to stack viewport

image
tunglt1810 commented 5 months ago

@ahlaughland I reallied that I am using this config to reduce memory sizing

useNorm16Texture: true,
preferSizeOverAccuracy: true

Also found a config about rendering engine

image

If I config cornerstone like this, the issue is solved

cornerstone.setConfiguration({
    ...cornerstone.getConfiguration(),
    rendering: {
        ...cornerstone.getConfiguration().rendering,
        strictZSpacingForVolumeViewport: appConfig.strictZSpacingForVolumeViewport
    },
    enableCacheOptimization: false
});
ahlaughland commented 5 months ago

@ahlaughland I reallied that I am using this config to reduce memory sizing

useNorm16Texture: true,
preferSizeOverAccuracy: true

Also found a config about rendering engine image

If I config cornerstone like this, the issue is solved

cornerstone.setConfiguration({
  ...cornerstone.getConfiguration(),
  rendering: {
      ...cornerstone.getConfiguration().rendering,
      strictZSpacingForVolumeViewport: appConfig.strictZSpacingForVolumeViewport
  },
  enableCacheOptimization: false
});

Thanks, I will research this enableCacheOptimization option.

sedghi commented 1 week ago

Can you check latest OHIF which has latest Cornerstone3D 2.0 additions?

sedghi commented 7 hours ago

We released OHIF 3.9 which address this issue.