imagej / imagej-ops

ImageJ Ops: "Write once, run anywhere" image processing
https://imagej.net/libs/imagej-ops
BSD 2-Clause "Simplified" License
88 stars 42 forks source link

Use non-normalized normals in Mesh centroid #655

Open gselzer opened 2 months ago

gselzer commented 2 months ago

This PR adds the fixes I discussed with @tibuch on Zulip (here), while trying to determine the difference in return between the geom.centroid Op operating on Meshes and the static algorithm MeshStats.centroid() from imglib2-mesh. This change does not need to be ported forward to SciJava Ops Image, as it is our plan to remove these Ops entirely, instead leaning upon imglib2-mesh (where we'd just use MeshStats.centroid())

@Tim-Oliver Buchholz I have finally figured out the difference - if you call mesh.triangles().add(idx1, idx2, idx3), the library will automatically normalize the normal vector. The geom.centroid Op expects the normal vector of each triangle to be non-normalized, as it uses those values directly in the computation (SciJava Ops Image does the same thing), however the algorithm in the paper requires non-normlized normal vectors. If you recompute the normal vector within the CentroidMesh Op, then you get the negative of MeshStats.centroid, because of the inversion here

The reason for the draft PR is that I am unsure how to fix the tests - there are three failures in Mesh features that depend upon the centroid Op which fail with these changes, and two suggest that the values were verified against MATLAB. I don't know which MATLAB code was used/how to verify.