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

Two small fixes #7265

Merged
merged 2 commits into from Oct 10, 2023
Merged

Two small fixes #7265

merged 2 commits into from Oct 10, 2023

Conversation

lassoan
Copy link
Contributor

@lassoan lassoan commented Oct 8, 2023

This PR contains small fixes, see details in the commits. Do not squash merge.

Add masked median filter needed for RGBA volume generation - see Slicer/VTK#43. moved masked median filter to VTK (Merge Slicer/VTK#44)

@lassoan lassoan added this to the Slicer 5.5 milestone Oct 8, 2023
@lassoan lassoan requested a review from pieper October 8, 2023 17:02
@lassoan lassoan self-assigned this Oct 8, 2023
Copy link
Member

@pieper pieper left a comment

Choose a reason for hiding this comment

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

This looks good, but wouldn't it be cleaner to just add a flag to the existing vtk class so there's less code duplication.

* @brief Median Filter
*
* vtkImageMaskedMedian3D is a modified a Median filter that works the same way as
* vtkImageMaskedMedian3D filter except that it ignores 0 values. It can be
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* vtkImageMaskedMedian3D filter except that it ignores 0 values. It can be
* vtkImageMedian3D filter except that it ignores 0 values. It can be

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was not a lot of repeated code (very small filter) but I've submitted a pull request to VTK instead - see Slicer/VTK#44 and https://gitlab.kitware.com/vtk/vtk/-/merge_requests/10588

@lassoan lassoan changed the title Add masked mdedian filter Two small fixes Oct 10, 2023
@lassoan lassoan merged commit 83feea7 into Slicer:main Oct 10, 2023
7 checks passed
@lassoan lassoan deleted the add-masked-mdedian-filter branch October 10, 2023 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants