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 #1176 Pan and Zoom issue with segmentation on image with non-square pixels #1183

Merged
merged 3 commits into from
Mar 2, 2020

Conversation

MickeyMiao7
Copy link
Contributor

  • Please check if the PR fulfills these requirements
  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Bug fix0

  • What is the current behavior? (You can also link to an open issue here)

#1176

  • What is the new behavior (if this is a feature change)?

The segmentation boundary and semi-transparent infill undergo the same same translation, providing an accurate view of which pixels have been highlighted.

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

no

  • Other information:

@codecov
Copy link

codecov bot commented Mar 2, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@98a32d6). Click here to learn what that means.
The diff coverage is 9.9%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1183   +/-   ##
=========================================
  Coverage          ?   18.83%           
=========================================
  Files             ?      284           
  Lines             ?     8510           
  Branches          ?     1451           
=========================================
  Hits              ?     1603           
  Misses            ?     5722           
  Partials          ?     1185
Impacted Files Coverage Δ
src/store/modules/segmentationModule/colorLUT.js 12.28% <ø> (ø)
src/tools/segmentation/strategies/correction.js 0.57% <ø> (ø)
src/store/modules/segmentationModule/index.js 0% <ø> (ø)
...dules/segmentationModule/getSegmentsOnPixeldata.js 0% <ø> (ø)
src/stateManagement/toolState.js 0% <0%> (ø)
...eventListeners/internals/renderSegmentationFill.js 1.81% <0%> (ø)
src/tools/OverlayTool.js 0% <0%> (ø)
...e/modules/segmentationModule/getLabelmapBuffers.js 2.56% <10%> (ø)
src/store/modules/segmentationModule/arrayTypes.js 100% <100%> (ø)
...modules/segmentationModule/defaultConfiguration.js 100% <100%> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 98a32d6...716cac7. Read the comment docs.

@JamesAPetts
Copy link
Member

Hey @MickeyMiao7 , thanks for the PR! This certainly looks like the right thing to do, and we get the same result as before on the square-images in the demo, so that's great.

Have you tried this new version with your non-square pixel examples?

@MickeyMiao7
Copy link
Contributor Author

Hey @JamesAPetts, yes the new version has solved the problem in non-square pixel examples.

I build the cornerstoneTools.js locally with the new version and upload it onto codesandfbox, and then import it in the index.js. Other codes remain the same as the one in #1176.

You can see the problem has been solved from the following sandbox link:

https://codesandbox.io/s/segmentation-boundary-and-infill-not-translating-at-the-same-rate-in-images-with-non-square-pixels-hcp67?fontsize=14&hidenavigation=1&theme=dark

@JamesAPetts
Copy link
Member

Thanks for swift reply and the live demo! This looks good to me.

@JamesAPetts
Copy link
Member

@all-contributors, please add @MickeyMiao7 for this great contribution!

@JamesAPetts JamesAPetts merged commit e70b894 into cornerstonejs:master Mar 2, 2020
@dannyrb
Copy link
Member

dannyrb commented Mar 2, 2020

🎉 This PR is included in version 4.12.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

@dannyrb
Copy link
Member

dannyrb commented Mar 2, 2020

@all-contributors, please add @MickeyMiao7 for this awesome code

@JamesAPetts
Copy link
Member

Apparently the bot is not as appreciative as we are haha.

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

Successfully merging this pull request may close these issues.

4 participants