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(SegRendering): preserve canvas transform matrix after seg render #1550

Merged

Conversation

ViniciusResende
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, ...)
    This PR contains a bug fix.

  • What is the current behavior? (You can also link to an open issue here)
    The current behavior is that my overlay (rendered using the Overlay Tool) is getting resized and translated once the segmentation is rendered.

image
Note that the text is not even entirely on the screen, also there are two ellipsis that aren't even being shown, they appear on the screenshot below.

  • What is the new behavior (if this is a feature change)?
    Now my overlay is not being resized nor translated and renders on the position where I expected it to be, also the segmentation kept working fine.

image
Now we can see that the text is well positioned and that the ellipsis are in the right place.

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    This PR shoudn't introduce any breaking change.

  • Other information:
    Previously after rendering the segmentation cornerstone was always transforming the canvas to follow the identity matrix, but in cases where we had an overlay using a different transformation matrix, it would undesirably affect the original overlay.
    Also did some minor refactorings on the renderFill function of the src/eventListeners/internals/renderSegmentationFill.js file.

Previously after rendering the segmentation cornerstone was always transforming the canvas to follow
the identity matrix, but in cases where we had an overlay using a different transformation matrix,
it old undesirably affect the original overlay.
@codecov
Copy link

codecov bot commented Jul 21, 2023

Codecov Report

Merging #1550 (4363285) into master (73cd522) will increase coverage by 0.55%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1550      +/-   ##
==========================================
+ Coverage   23.30%   23.86%   +0.55%     
==========================================
  Files         287      287              
  Lines       10138    10141       +3     
  Branches     2082     2082              
==========================================
+ Hits         2363     2420      +57     
+ Misses       6625     6575      -50     
+ Partials     1150     1146       -4     
Impacted Files Coverage Δ
...eventListeners/internals/renderSegmentationFill.js 90.32% <100.00%> (+88.68%) ⬆️
...ntListeners/internals/renderSegmentationOutline.js 81.18% <100.00%> (+0.18%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@igoroctaviano
Copy link
Contributor

LGTM 🏅

@igoroctaviano igoroctaviano merged commit dbf874f into cornerstonejs:master Jul 21, 2023
@sedghi
Copy link
Member

sedghi commented Jul 21, 2023

🎉 This PR is included in version 6.0.10 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

3 participants