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(imageOutline): fix the bug where outline was not showing without filling #3013

Merged
merged 1 commit into from
Feb 19, 2024

Conversation

sedghi
Copy link
Contributor

@sedghi sedghi commented Feb 6, 2024

Context

The labelmap outline in the volume/image mapper used the condition tColor.a > 0.01 to improve performance by skipping the background. However, this meant that we couldn't render the outline when the fill is 0 (or a small number).You can see the bug here in this code sand box

I changed the logic to not rely on that and only work on segment Index.

Results

Now we can render them fine if the opacity is set to 0

Changes

Changed the logic of outline detection to not rely on the color opacity but rather the segment index itself.

  • Documentation and TypeScript definitions were updated to match those changes

PR and Code Checklist

  • semantic-release commit messages
  • Run npm run reformat to have correctly formatted code

Testing

  • This change adds or fixes unit tests
  • Tested environment:
    • vtk.js: Latest
    • OS: macOS
    • Browser: Latest Chrome Stable

@sedghi sedghi changed the title fix(imageOutline): fix the bug where outline was not showing without … fix(imageOutline): fix the bug where outline was not showing without filling Feb 6, 2024
@finetjul finetjul requested a review from sankhesh February 6, 2024 21:59
Copy link
Collaborator

@sankhesh sankhesh left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Thank you @sedghi

@sankhesh sankhesh added this pull request to the merge queue Feb 16, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 16, 2024
@floryst floryst added this pull request to the merge queue Feb 19, 2024
Merged via the queue into Kitware:master with commit 77ff259 Feb 19, 2024
3 checks passed
Copy link

🎉 This PR is included in version 29.7.2 🎉

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
released Automated label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants