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

fix: colored images using rgba in CPU rendering #338 #345

Merged
merged 1 commit into from
Jan 11, 2023
Merged

Conversation

Zaid-Safadi
Copy link
Contributor

flip the useRGBA in StackViewport to true in the CPU path as discussed here: #338

@netlify
Copy link

netlify bot commented Jan 5, 2023

Deploy Preview for cornerstone-3d-docs ready!

Name Link
🔨 Latest commit e0c1e98
🔍 Latest deploy log https://app.netlify.com/sites/cornerstone-3d-docs/deploys/63be2b03480ec900082e6a15
😎 Deploy Preview https://deploy-preview-345--cornerstone-3d-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

Copy link
Member

@sedghi sedghi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default is true, I don't think we need to specify here (please check wado).

@Zaid-Safadi
Copy link
Contributor Author

default is true, I don't think we need to specify here (please check wado).

I think I see where you default it to true here

Isn't the StackViewport in cs3d? The comment says true only required for cs-legacy, sorry maybe I am not understanding the comment very well but it sounds like it should have worked if we set it to false in the StackViewport.

yeah, I can remove it from the options!

function createImage(imageId, pixelData, transferSyntax, options = {}) {
  // whether to use RGBA for color images, default true as cs-legacy uses RGBA
  // but we don't need RGBA in cs3d, and it's faster, and memory-efficient
  // in cs3d
  let useRGBA = true;

  if (options.useRGBA !== undefined) {
    useRGBA = options.useRGBA;
  }


@sedghi
Copy link
Member

sedghi commented Jan 6, 2023

My comment was you don't need to have useRGBA:true in your changed files, since the default is true, you can just remove it

@Zaid-Safadi
Copy link
Contributor Author

My comment was you don't need to have useRGBA:true in your changed files, since the default is true, you can just remove it

Lets keep it explicitly set to true so we are always sure it will work even if the wado library changes

@sedghi sedghi merged commit 90bcc7b into main Jan 11, 2023
rodrigobasilio2022 pushed a commit to RadicalImaging/basilioCornerstone3D that referenced this pull request Jan 21, 2023
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

Successfully merging this pull request may close these issues.

2 participants