Issue : Cine infinite rendering loop

[IbrahimCSAE]

trafficstars

This line in the code causes CINE to be in an infinite loop and render the same frame over and over. It happens when you activate the CINE tool and you are on the last image of a series.

    if (newImageIdIndex >= imageCount) {
      newImageIdIndex = 0;
    }

and since delta is const delta = newImageIdIndex - stackData.targetImageIdIndex; It causes it to go in the negative when it should always be 1 from my understanding.

        if (newImageIdIndex !== stackData.targetImageIdIndex) {
            const delta = newImageIdIndex - stackData.targetImageIdIndex;
            viewport.scroll(delta, debounced, loop);
        }
    };

If you would like to replicate the issue, visit this study

https://v3-demo.ohif.org/viewer?StudyInstanceUIDs=1.3.6.1.4.1.25403.345050719074.3824.20170126083429.2

load any series and go to the last image in it by scrolling, so 53/53 for example, and trigger the CINE tool, you will notice it freezes, and it's frozen not because it's stopped but it's just really rendering the same frame over and over.

You can check by recording the performance and looking at the call stack, or you will just notice the CPU usage rising up.

After removing those lines the issue doesn't happen anymore since the calculation for delta comes out with 1 instead of negative values, so 53- 52 = 1, with the previous code it would've been 0-52 = -52 which makes it get stuck.

The way I fixed it could be wrong, so feel free to fix it some other way, but it's an issue to lookout for anyway.

[netlify[bot]]

✅ Deploy Preview for cornerstone-3d-docs ready!

Name	Link
🔨 Latest commit	5c265cb79da88ce85dfce5cf5a8531f7e409ea4e
🔍 Latest deploy log	https://app.netlify.com/sites/cornerstone-3d-docs/deploys/6357e29df292fc0008414d5e
😎 Deploy Preview	https://deploy-preview-249--cornerstone-3d-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[sedghi]

Interesting bug, thanks for your PR. I'm not sure if this is a fix, but I will think about it

[IbrahimCSAE]

Interesting bug, thanks for your PR. I'm not sure if this is a fix, but I will think about it

No problem, thanks

[IbrahimCSAE]

Interesting bug, thanks for your PR. I'm not sure if this is a fix, but I will think about it

Yeah I think the problem wasn’t what I said earlier, It looks like it’s because of this

        const stackData = {
            targetImageIdIndex: viewport.getTargetImageIdIndex(),
            imageIds: viewport.getImageIds(),
        };

This doesn't update inside

   const playClipAction = () => {
       let newImageIdIndex = stackData.targetImageIdIndex;
       const imageCount = stackData.imageIds.length;

It keeps using the initial value that was defined when playClip(element, playClipOptions) was called , It should update after every iteration?

I pushed another commit with the new fix.

const playClipAction = () => {
        const stackData = {
            targetImageIdIndex: viewport.getTargetImageIdIndex(),
            imageIds: viewport.getImageIds(),
        };
        let newImageIdIndex = stackData.targetImageIdIndex;
        const imageCount = stackData.imageIds.length;

let me know what you think, I also noticed it was also breaking when you set loop to false, it would loop around anyway, but after fixing the variable both issues are resolved.

[sedghi]

@IbrahimCSAE I like the new fix better, but seems like your eslint/prettier is not setup properly as I see the indentation is wrong. Can you fix that and we merge it?

[IbrahimCSAE]

@IbrahimCSAE I like the new fix better, but seems like your eslint/prettier is not setup properly as I see the indentation is wrong. Can you fix that and we merge it?

Awesome, I just fixed the indentation, does it look alright now? @sedghi

Also there's an issue I opened with the MagnifyTool, it seems to break all SVG rendering for all tools for some reason, all of them stop working because of it.

[IbrahimCSAE]

Thanks

I pushed another commit, I had the variable used before its declaration when I was fixing the indentation, sorry about that.