-
Notifications
You must be signed in to change notification settings - Fork 315
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
wip Feat/norm16 textures #336
Conversation
✅ Deploy Preview for cornerstone-3d-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
* @param {any} [scalarData] - The data to be converted. | ||
* @returns The data type of the scalar data. | ||
*/ | ||
export default function getScalarDataType( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to get expanded to consider scaling parameters to decide on the final data type (if scalings are float, then the type
passed to wado as option should be float)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am assuming this is usually used for PET overlays. Would it be worth it to account for preferSizeOverAccuracy
to perform a scale + cast
That is weird behavior... is the GL context lost just during debugging or will it randomly occur. I can try it on my local too |
@sedghi the divergence of stack and volume might have some consequence for the cache since the stored values are going to be uint16 / int16 in the cache. Is the plan just to cast them to float32 inside the stackViewport on StackViewport doesn't seem to use too much GPU memory anyway so I don't think it is a big deal |
@Ouwen That is an interesting point, maybe then I just add the norm 16 to image mapper as well. I'm having some trouble with the rendering volume right now on Mac and Chrome. If you have time and look for some headache, can you check this branch (with the changes in wado, PR as well there), and see if I'm looking inside Chromium forums to see if it has been reported there too. |
@sedghi did a rebase of this on top of I made some changes to the stack viewport so it should also properly render int16 and uint16 data types. Apologies on the premature commit squashing for now... One of the issues I found was that uint16 support requires some more logic for prescaling. Thoughts on the following: if a targetbuffer is not present we are using the native decoded pixelData (float32, uint8, uint16, int16, int8 not supported yet but maybe later). We try to preserve dynamic range. So if rescale slope and rescaleIntercept are both positive and native type is uint16, we can keep uint16 type. However, if rescale slop and rescaleIntercept are possibly negative we can cast to int16. Alternatively, we can always cast to float32 and recast to the min pixel type to preserve dynamic range (keep track of a min/max) |
@Ouwen great observations. |
I think the only reason target buffer is needed is because for shared array buffers we need to allocate prior to decoding for progressive loading. So to allocate the best type, we'd need to look at scaling, minvalue, maxvalue, and some other dicom metadata tags (signed, bitsAllocated, photometric interpertation) |
this can be closed with #420 |
This relies on this cornerstonejs/cornerstoneWADOImageLoader#500Volume Viewport
volumeAPI
example, the webGL context get lost, and I get a flash of empty screenhttps://github.com/cornerstonejs/cornerstone3D-beta/blob/feat/norm16-textures/packages/core/src/RenderingEngine/vtkClasses/vtkStreamingOpenGLTexture.js#L103
Stack Viewport
CPU
CC @swederik @Ouwen It is very interesting, sometimes when I loose the GL context, the extension becomes unavailable (webgl report v2) I need to restart chrome for it to appear again!)