Description

What is this PR

[x] Bug fix
[ ] Addition of a new feature
[ ] Other

Why is this PR needed? fixes a problem when intersecting some planes with the atlases. The following code crashes without this PR:

import brainrender as br
import bgheatmaps as bgh
import numpy as np

atlas = br.Atlas(atlas_name="allen_mouse_25um")
root = atlas.get_region("root", alpha=1, color=None)
major_divisions = ["Isocortex", "OLF", "HPF", "CTXsp", "STR", "PAL", "TH", "HY", "MB", "P", "MY", "CB"]
regions_meshes = atlas.get_region(*major_divisions, alpha=1, color=None)
if not isinstance(regions_meshes, list):
    regions_meshes = [regions_meshes]

for actor in (root, *regions_meshes):
    # from Render._prepare_actor()
    mtx = [[1, 0, 0, 0], [0, 1, 0, 0], [0, 0, -1, 0], [0, 0, 0, 1]]
    actor._mesh = actor.mesh.clone()
    actor._mesh.applyTransform(mtx) # actor._mesh.apply_transform(mtx) in newer vedo
    actor._is_transformed = True

regions_meshes = [r for r in regions_meshes if r.br_class == "brain region"]

s = bgh.slicer.Slicer(6000, "frontal", 100, root)
# CRASHES
print(s.get_structures_slice_coords(regions_meshes, root))

What does this PR do? Instead of using a mesh as a plane, this PR uses a proper infinite plane for slicing the atlas. This effectively bypasses an open issue of vtk regarding the intersection of two meshes. See: https://discourse.vtk.org/t/vtkintersectionpolydatafilter-crashing/9428 As a bonus it is conceptually a "better" way to represent the planes. In order to do so, I had to use a vtk function by backporting a binding from vedo=2023.4.6.

References

11

How has this PR been tested?

I ran all examples from this repository + extensive usage in my work project. Additionally, I made sure every combination of (frontal, horizontal, sagittal) orientations and (allen_cord_20um_v1.0, allen_mouse_25um_v1.2, mpin_zfish_1um_v1.0) altases had their plane's normal vector as well as u and v (x and y in 2D projections, respectively) in the correct direction by using the modified planner.

Is this a breaking change?

No, it should keep all the previous APIs and functionalities.

Does this PR require an update to the documentation?

No

Checklist:

[x] The code has been tested locally
[ ] Tests have been added to cover all new functionality (unit & integration)
[ ] The documentation has been updated to reflect any changes
[ ] The code has been formatted with pre-commit

carlocastoldi commented 1 year ago

@LeonardoLupori I knew you were interested in this fix. Albeit not being merged, if you need it in the meantime you should be able to use this solution

adamltyson commented 11 months ago

@carlocastoldi Thanks for this. I've closed this PR, but included your commits in #21. If you could test that PR that would be very helpful!

brainglobe / brainglobe-heatmap

Use infinite planes for slicing #16