cornerstonejs / dicomParser

JavaScript parser for DICOM Part 10 data
MIT License
712 stars 228 forks source link

Slice the byteArray.Array in sharedCopy #251

Closed OzgeYurtsever closed 1 year ago

yagni commented 1 year ago

@OzgeYurtsever any more details on this? Does this solve your crash? I can't merge it as-is because it copies bytes instead of just creating a new view over the existing bytes (see the documentation on ArrayBuffer.slice() vs. TypedArray) which changes the fundamental behavior of this method.

OzgeYurtsever commented 1 year ago

@yagni we are still working on the fix. I May update this PR during the project week. Thank you

sedghi commented 1 year ago

Let me chime in here a little bit.

This "fix" address the issue of multiframe dicom where the whole array buffer is passed to the webworker and every time we are unnecessarily decoding the whole buffer instead of the frame itself and everything is slow right now as a result.

So by slicing the buffer here, the worker will have much less problem and only work on the portion of the buffer that it needs to work.

Having said that I don't think we should change the sharedCopy and we only need to pass an option of useCopy or similar wording to the function that is used which internally uses sharedCopy. @OzgeYurtsever You should put debuggers and see which function is getting called for the compressed data

yagni commented 1 year ago

@OzgeYurtsever Any more developments on this post-project week? If not, I'll go ahead and close.