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] StackImageSynchronizer silently fails in versions 1.14.4+ if synchronizerInstance options are undefined #795

Open
vnosikov opened this issue Sep 22, 2023 · 3 comments
Assignees

Comments

@vnosikov
Copy link

Describe the Bug

This commit cd5efa0 introduced breaking changes. After an update of cornerstonetools from 1.14.3 to 1.14.4 position synchronization between StackViewports in our app silently stopped working. It happened because of this line in stackIageSyncCallback

const options = synchronizerInstance.getOptions(targetViewport.viewportId);
[...]
options.useInitialPosition !== false

If options are undefined as is in our case the app fails.

And what is even worse it is called from Synchronizer in such way that prevents errors or warnings to appear. The error happens inside a promise and there is absolutely no feedback to user. It just looks that synchronization doesn't work.

  private fireEvent(sourceViewport: Types.IViewportId, sourceEvent: any): void {
    if (this.isDisabled() || this._ignoreFiredEvents) {
      return;
    }

    this._ignoreFiredEvents = true;
    const promises = [];
    try {
      for (let i = 0; i < this._targetViewports.length; i++) {
        const targetViewport = this._targetViewports[i];
        const targetIsSource =
          sourceViewport.viewportId === targetViewport.viewportId;

        if (targetIsSource) {
          continue;
        }
        promises.push(
          this._eventHandler(
            this,
            sourceViewport,
            targetViewport,
            sourceEvent,
            this._options
          )
        );
      }
    } catch (ex) {
      console.warn(`Synchronizer, for: ${this._eventName}`, ex);
    } finally {
      if (promises.length) {
        Promise.allSettled(promises).then(() => {
          this._ignoreFiredEvents = false;
        });
      } else {
        this._ignoreFiredEvents = false;
      }
    }
  }

I'd like also to call attention to a fact that this happened in a supposedly trivial update from 1.14.3 to 1.14.4. I don't know, guys, maybe these options should always exist. But I can't find anything in docs about them for createStackImageSynchronizer and our app worked fine previously without them. And a patch update that changes the last number in a version is not supposed to have breaking changes. Especially when they break things silently.

For now I rollbacked. I hope you fix this error or at least provide me some information how to configure these options for a Synchronizer.

Thank you

Steps to Reproduce

I can't provide the whole setup of our app and I am too lazy to create a fresh app to reproduce it there. I believe I provided enough information in a previous section

The current behavior

Position synchronization between StackViewports doesn't work and shows no errors

The expected behavior

Position synchronization between StackViewports should work or at least inform a user that something went wrong

OS

macOS 13.4

Node version

16.15.1

Browser

Chrome 116.0.5845.187

@idemopacs
Copy link

idemopacs commented Sep 24, 2023

I was also facing same issue after update of cornerstonetools from 1.14.3 to 1.14.4.
I have resolved this with synchronizer.setOption() method while adding Viewport to Synchronizer.

See code below
const options = { useInitialPosition: false };
synchronizer.add({ renderingEngineId, viewportId });
synchronizer.setOptions(viewportId, options)

@vnosikov
Copy link
Author

@idemopacs Thank you, it helps.

Still nowhere in docs it is stated that synchronizer's options are basically mandatory now.

@wayfarer3130
Copy link
Collaborator

Fixed by #799

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants