Add volume and area calculations

[casperlamboo]

Add volume, and area calculations to IndexedMesh. Volume calculations are based on https://doi.org/10.1109/ICIP.2001.958278. Added tests to make sure the calculations were valid. For valid volume calculations the input mesh cannot contain any holes. For this reason I've added another bunny model to the testdata folder.

[casperlamboo]

any thoughts @hmeyer? I will gladly change anything if it is not up to your standards.

[hmeyer]

Sorry - this fell off my radar. I'm not sure those metrics belong in this crate, as I intended it to be purely for Io of STL files. Thoughts? Casper Lamboo ***@***.***> schrieb am Di., 18. Mai 2021, 23:15:

…

any thoughts @hmeyer <https://github.com/hmeyer>? I will gladly change anything if it is not up to your standards. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#12 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAE7LAORZ3CSLGQMINKSO3TOLKHVANCNFSM44VHIVBQ> .

[casperlamboo]

I would argue that volume is actually pretty inherent to stl files. Or at least as inherent to stl files as the validate function; valid stl models cannot have negative volume.

Example, a popular tool for fixing meshes (admesh) shows the volume pretty prominently when analysing an stl file.

================= Results produced by ADMesh version 0.98 =================
Input file         : sphere.stl
File type          : Binary STL file
Header             : Processed by ADMesh version 0.98
============== Size ==============
Min X = -1.334557, Max X = 1.370952
Min Y = -1.377953, Max Y = 1.377230
Min Z = -1.373225, Max Z = 1.242838
========= Facet Status ========== Original ============ Final ====
Number of facets                 :  3656                3656
Facets with 1 disconnected edge  :    18                   0
Facets with 2 disconnected edges :     3                   0
Facets with 3 disconnected edges :     0                   0
Total disconnected facets        :    21                   0
=== Processing Statistics ===     ===== Other Statistics =====
Number of parts       :     1        Volume   :  10.889216
Degenerate facets     :     0
Edges fixed           :    24
Facets removed        :     0
Facets added          :     0
Facets reversed       :     0
Backwards edges       :     0
Normals fixed         :     0

Ofcourse io is not the same as "fixing" but I would argue that volume calculations are as foreign to io as they are to mesh-fixing.

In my opinion such elementary operations do fit in nicely in an io library such as this. Tut as the creator/maintainer you have final say ofcourse!

[hmeyer]

I think this method should return an Optional[f32] or a Result[f32, SomeErrorType], and include a call to validate.
Furthermore, for this to be the true volume, I think we'd have to validate that the winding number of all triangles is the same, no?

[casperlamboo]

I think this method should return an Optional[f32] or a Result[f32, SomeErrorType], and include a call to validate.

I think I would switch it around; have the volume > 0 check in the validate function. but it is your call. (if you know your mesh to be valid it is annoying if a library spends computation on validation)

Furthermore, for this to be the true volume, I think we'd have to validate that the winding number of all triangles is the same, no?

I believe that the connected indices check in the validate function ensures that the winding order is the same for all tris.

[hmeyer]

I believe that the connected indices check in the validate function ensures that the winding order is the same for all tris.
Good point. I agree.

I think I would switch it around; have the volume > 0 check in the validate function. but it is your call. (if you know your > mesh to be valid it is annoying if a library spends computation on validation)
How about this:

• rename validate() into is_closed_and_has_correct_winding().
• call is_closed_and_has_correct_winding() inside volume().
• create validate() as volume() > 0

[casperlamboo]

In my opinion it is bad practice to combine these kinds of validate checks inside "computation functions". It is better to execute such validation checks only once during or just after the construction of the data type. For instance coding a ray tracer; the direction of the ray always needs to be a unit length vector, but i'm not going to normalise the vector every time I compute something. Instead the normalisation is executed once when constructing the ray.

I would propose something like this.

impl IndexedMesh {
    ...
    pub fn validate(&self) -> Result<()> {
        self.is_closed()?;
        self.non_zero_tris()?;
        if self.volume() > 0.0 {
            Err(::std::io::Error::new(
                ::std::io::ErrorKind::InvalidData,
                "mesh has a negative volume",
            ))
        } else {
            Ok(())
        }
    }

    fn is_closed() -> Result<()> { ... }
    fn non_zero_tris() -> Result<()> { ... }
}

What you think about this? downside with my approach compared to yours is that if a user wants to know if a model is valid and want to know it's volume the volume is essentially computed twice.

[hmeyer]

I think it is questionable to assume the one winding is the correct and thus the other winding is invalid.
Meaning even a mesh that has negative winding might be valid.

The thing about volume is, that the calculation is only correct, if the mesh is closed and has correct winding.
But the above volume() function will also return a volume for meshes that are not closed or have no correct winding. In my opinion this would be a bug.
If you really wouldn't want to include the other validation checks in volume(), then volume would have to renamed to:
fn volume_assuming_the_mesh_is_closed_and_has_clockwise_winding()

[hmeyer]

Given you already invested this much, and the code is quite compact for now, I think it is fine to add.
But we should draw a line somewhere close, to what are the limits of this library.