cornerstonejs / cornerstoneTools

A framework for tools built on top of Cornerstone.
https://tools.cornerstonejs.org/
MIT License
577 stars 452 forks source link

stackImagePositionSynchronizer should only synchronize images on approximately the same plane #980

Open dannyrb opened 5 years ago

dannyrb commented 5 years ago

I've had a number of users report to me directly that the stackImagePositionSynchronizer exhibits unexpected behavior. For example:

When scrolling one of the axial stack's, I would expect the other to update it's image to the closest slice. I would expect both coronal stack's, because their for a different orientation, to remain on their current image.

Unfortunately, in some instances, we stacks in other orientations "jump" to a new image. Often the first or last one in their stack.

We can accomplish this by updating the synchronizer to discount images that aren't in the same orientation. I think the math here is making sure the row dot product and column dot product are ~1 for the two images being compared (the source image, and each target image in the stack)

rmasad commented 3 years ago

We are implementing this function as a new command in our (public) fork¹ of OHIF (a toggle synchronizer command) and we are having the same problem.

¹ https://gitlab.nursoft.cl/foss/ohif-viewer

chafey commented 3 years ago

You can have multiple synchronizers and it is up to the developer to decide which stacks should be synchronized in this manner. Stacks often have images in the same orientation, but not always (e.g. 3 plane localizer, MRA 3D rotation, etc). I don't think it is cornerstone's responsibility to decide which stacks should and should not be synchronized. It is cornerstones responsibility to document how the synchronization behavior works and to caution developers against using it improperly (e.g. blindly add all stacks to a single synchronizer)

On Tue, Feb 23, 2021 at 11:20 AM rmasad notifications@github.com wrote:

We are implementing this function as a new command in our (public) fork¹ of OHIF (a toggle synchronizer command) and we are having the same problem.

¹ https://gitlab.nursoft.cl/foss/ohif-viewer

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/cornerstonejs/cornerstoneTools/issues/980#issuecomment-784364882, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJVXWX4ZSBV5GWHNWSOR2DTAPPWBANCNFSM4HY7VPLA .

rmasad commented 3 years ago

Thanks @chafey, finally we implement it as you commented. You can see the result here: https://gitlab.nursoft.cl/foss/ohif-viewer/-/commit/6167cbd5731013382d0b29cc1aac6e549ab8ab7d#1d12cde93c37b86545343949bdce2a625b44dbce_0_70

ranasrule commented 2 years ago

We are implementing this function as a new command in our (public) fork¹ of OHIF (a toggle synchronizer command) and we are having the same problem.

¹ https://gitlab.nursoft.cl/foss/ohif-viewer

Im unable to access your public fork of OHIF....can you kindly fix the link

Screenshot 2021-11-05 165708

racacere commented 2 years ago

Hi @ranasrule. Yesterday we had some problems with our Gitlab server, now it's back online. Please try again, sorry for the inconveniences.

ranasrule commented 2 years ago

Hi @ranasrule. Yesterday we had some problems with our Gitlab server, now it's back online. Please try again, sorry for the inconveniences.

thank you...I can access it fine now and have been able to build the source but get the following when I open any study Screenshot 2021-11-06 035052

ranasrule commented 2 years ago

@rmasad @racacere could you help me out with the above issue please? Thanks in advance.

rmasad commented 2 years ago

Sorry for the delay, we change the repository url to: https://gitlab.nur.dev/foss/ohif-viewer

You can see the change in: https://gitlab.nur.dev/foss/ohif-viewer/-/commit/6167cbd5731013382d0b29cc1aac6e549ab8ab7d#1d12cde93c37b86545343949bdce2a625b44dbce_0_70

ranasrule commented 2 years ago

Sorry for the delay, we change the repository url to: https://gitlab.nur.dev/foss/ohif-viewer

You can see the change in: https://gitlab.nur.dev/foss/ohif-viewer/-/commit/6167cbd5731013382d0b29cc1aac6e549ab8ab7d#1d12cde93c37b86545343949bdce2a625b44dbce_0_70

Which change are you referring to? Does the code in your repository contain 3d reconstruction feature?

racacere commented 2 years ago

Hi @ranasrule,

I'm very sorry for the response delay. The commit mentioned above, is our implementation of the solution to the scroll synchronization error between viewports. In brief, we added a sync botton to the toolbar when two or more viewports are active and that component has the needed logic to sync the viewports in a correct way.

The error that you described earlier is because we use a custom field in the ohif settings that indicates the study instance UID that you want to visualize, here are an example of this settings:

  ...
  servers: {
    dicomWeb: [
      {
        name: 'SomeName',
        wadoUriRoot: 'https://example.com/wado',
        qidoRoot: 'https://example.com/dicom-web',
        wadoRoot: 'https://example.com/dicom-web',
        qidoSupportsIncludeField: true,
        imageRendering: 'wadouri',
        thumbnailRendering: 'wadouri',
        enableStudyLazyLoad: true,
        supportsFuzzyMatching: true,
        studyInstanceUids: [
          '1.2.840.113619.2.278.3.2832275436.975.1617744955.830',
        ],
      },
    ],
  },
  ...

Best regards.