search

jerry73204 / apriltag-rust

High-level AprilTag library in Rust
BSD 2-Clause "Simplified" License
16 stars 9 forks source link

Set cos_critical_rad correctly #8

Open schuyler-cohen-zipline opened 1 year ago

schuyler-cohen-zipline commented 1 year ago

In QuadThresholds, the parameter min_opposite_angle was being used to directly set cos_critical_rad, but that's just a cached version of cos(critical_rad), not an entierly different parameter.

This sets it to the proper value.

schuyler-cohen-zipline commented 1 year ago

fyi @Christopher22 I believe you added this if you can verify this? I don't have a great way to test/prove its correct other than the name/looking through the c code at how it's used.

jerry73204 commented 1 year ago

The apriltag source code sets cos_critical_rad but does not make any use of critical_rad. It makes sense because the valid angle is 180 degrees at maximum. I prefer to let the user set cosine value directory like what apriltag.c does, and cache the acos(min_angle).

schuyler-cohen-zipline commented 1 year ago

The apriltag source code sets cos_critical_rad but does not make any use of critical_rad. It makes sense because the valid angle is 180 degrees at maximum. I prefer to let the user set cosine value directory like what apriltag.c does, and cache the acos(min_angle).

I think from an api perspective, setting an angle is much more intuitive than trying to decide what an appropriate "cos of an angle" to pass a function is, but I agree with you, the C version doesn't use it, and it almost seems like it's not really a fully formed tunable parameter in the C version. I've updated it to take the cos value, what do you think?

jerry73204 commented 1 year ago

We can add a checked set_min_angle() to get the best of both.