Fix #137932: compute correct armature bounds

There were actually two issues here:

1. The dimension reported for armatures were often wildly incorrect,
   including negative values and zero!
2. The dimensions reported for objects are supposed to be invariant with
   rotation, representing the dimensions along the object's local axes.
   However, armature objects' reported dimensions changed with rotation.

The respective causes were:

1. `BKE_armature_min_max()` was using an incorrect formula (acknowledged
   in a comment) for transforming the bounding box between spaces. This
   worked fine for some of the places that `BKE_armature_min_max()` was
   called, since they just reverse the transform using the same(!)
   erroneous formula, but it didn't work for others.
2. `BKE_armature_min_max()` first computed the bounds in world space,
   and then transformed them into object space, rather than computing
   them in object space directly like the respective functions for other
   object types. Even when done correctly, this causes the reported
   dimension to vary with rotation.

This PR fixes these issues by simply computing the armature bounding box
in object space directly instead.

There is one place in the code base that was directly using the
world-space bounds: `view3d_calc_minmax_selected()`. However, for every
object type other than armatures, it takes the object-space bounds and
transforms them (with an incorrect formula!) to world space.  So this PR
also changes `view3d_calc_minmax_selected()`'s armature code to do the
same, except with a correct formula.

Note that the reason for using the correct transform formula (departing
from other object types) is that the world-space bounds for armatures
were already correct prior to this PR due to being computed in that
space. Therefore using the incorrect formula has the potential to
introduce regressions in this case.

Pull Request: https://projects.blender.org/blender/blender/pulls/137961

source/blender/blenkernel/BKE_armature.hh

@@ -159,7 +159,7 @@ void BKE_armature_transform(bArmature *arm, const float mat[4][4], bool do_props
 std::optional<blender::Bounds<blender::float3>> BKE_armature_min_max(const Object *ob);
 
 /**
- * Calculate the axis-aligned bounds of `pchan` in world-space,
+ * Calculate the axis-aligned bounds of `pchan` in object-space,
  * taking into account custom transform when set.
  *
  * `r_min` and `r_max` are expanded to fit `pchan` so the caller must initialize them
@@ -179,7 +179,7 @@ void BKE_pchan_minmax(const Object *ob,
                       blender::float3 &r_min,
                       blender::float3 &r_max);
 /**
- * Calculate the axis aligned bounds of the pose of `ob` in world-space.
+ * Calculate the axis aligned bounds of the pose of `ob` in object-space.
  *
  * This only considers visible bones. When they are either directly (via a flag on the bone) or
  * indirectly (via bone collections) hidden, they are not part of the bounds calculation. When a

source/blender/blenkernel/intern/armature.cc

@@ -3094,18 +3094,7 @@ void BKE_pose_where_is(Depsgraph *depsgraph, Scene *scene, Object *ob)
 
 std::optional<blender::Bounds<blender::float3>> BKE_armature_min_max(const Object *ob)
 {
-  std::optional<blender::Bounds<blender::float3>> bounds_world = BKE_pose_minmax(ob, false);
-
-  if (!bounds_world) {
-    return std::nullopt;
-  }
-
-  /* NOTE: this is not correct (after rotation the AABB may not be the smallest enclosing AABB any
-   * more), but acceptable because this is called via BKE_object_boundbox_get(), which is called by
-   * BKE_object_minmax(), which does the opposite transform. */
-  return blender::Bounds<blender::float3>{
-      math::transform_point(ob->world_to_object(), bounds_world->min),
-      math::transform_point(ob->world_to_object(), bounds_world->max)};
+  return BKE_pose_minmax(ob, false);
 }
 
 void BKE_pchan_minmax(const Object *ob,
@@ -3142,17 +3131,14 @@ void BKE_pchan_minmax(const Object *ob,
                  pchan->custom_translation[0],
                  pchan->custom_translation[1],
                  pchan->custom_translation[2]);
-    mul_m4_series(mat.ptr(), ob->object_to_world().ptr(), tmp.ptr(), rmat.ptr(), smat.ptr());
+    mul_m4_series(mat.ptr(), tmp.ptr(), rmat.ptr(), smat.ptr());
     BoundBox bb;
     BKE_boundbox_init_from_minmax(&bb, bb_custom->min, bb_custom->max);
     BKE_boundbox_minmax(bb, mat, r_min, r_max);
   }
   else {
-    float vec[3];
-    mul_v3_m4v3(vec, ob->object_to_world().ptr(), pchan_tx->pose_head);
-    minmax_v3v3_v3(r_min, r_max, vec);
-    mul_v3_m4v3(vec, ob->object_to_world().ptr(), pchan_tx->pose_tail);
-    minmax_v3v3_v3(r_min, r_max, vec);
+    minmax_v3v3_v3(r_min, r_max, pchan_tx->pose_head);
+    minmax_v3v3_v3(r_min, r_max, pchan_tx->pose_tail);
   }
 }
 

source/blender/blenlib/BLI_bounds.hh

@@ -14,6 +14,7 @@
 
 #include "BLI_bounds_types.hh"
 #include "BLI_index_mask.hh"
+#include "BLI_math_matrix.hh"
 #include "BLI_math_vector.hh"
 #include "BLI_task.hh"
 #include "BLI_virtual_array.hh"
@@ -183,6 +184,45 @@ template<typename T> inline std::optional<T> max(const VArray<T> &values)
       [](const int a, const int b) { return std::max(a, b); });
 }
 
+/**
+ * Return the eight corners of a 3D bounding box.
+ */
+template<typename T>
+inline std::array<VecBase<T, 3>, 8> corners(const Bounds<VecBase<T, 3>> &bounds)
+{
+  return {
+      VecBase<T, 3>{bounds.min[0], bounds.min[1], bounds.min[2]},
+      VecBase<T, 3>{bounds.min[0], bounds.min[1], bounds.max[2]},
+      VecBase<T, 3>{bounds.min[0], bounds.max[1], bounds.min[2]},
+      VecBase<T, 3>{bounds.min[0], bounds.max[1], bounds.max[2]},
+      VecBase<T, 3>{bounds.max[0], bounds.min[1], bounds.min[2]},
+      VecBase<T, 3>{bounds.max[0], bounds.min[1], bounds.max[2]},
+      VecBase<T, 3>{bounds.max[0], bounds.max[1], bounds.min[2]},
+      VecBase<T, 3>{bounds.max[0], bounds.max[1], bounds.max[2]},
+  };
+}
+
+/**
+ * Transform a 3D bounding box.
+ *
+ * Note: this necessarily grows the bounding box, to ensure that the transformed
+ * bounding box fully contains the original. Therefore, calling this iteratively
+ * to transform from space A to space B, and then from space B to space C, etc.,
+ * will also iteratively grow the bounding box on each call. Try to avoid doing
+ * that, and instead first compose the transform matrices and then use that to
+ * transform the bounding box.
+ */
+template<typename T, int D>
+inline Bounds<VecBase<T, 3>> transform_bounds(const MatBase<T, D, D> &matrix,
+                                              const Bounds<VecBase<T, 3>> &bounds)
+{
+  std::array<VecBase<T, 3>, 8> points = corners(bounds);
+  for (VecBase<T, 3> &p : points) {
+    p = math::transform_point(matrix, p);
+  }
+  return {math::min(Span(points)), math::max(Span(points))};
+}
+
 }  // namespace bounds
 
 namespace detail {

source/blender/blenlib/BLI_math_vector.hh

@@ -59,12 +59,62 @@ template<typename T, int Size>
   BLI_UNROLL_MATH_VEC_FUNC_VEC_VEC(math::min, a, b);
 }
 
+/**
+ * Element-wise minimum of the passed vectors.
+ */
+template<typename T, int Size>
+[[nodiscard]] inline VecBase<T, Size> min(Span<VecBase<T, Size>> values)
+{
+  BLI_assert(!values.is_empty());
+
+  VecBase<T, Size> result = values[0];
+  for (const VecBase<T, Size> &v : values.drop_front(1)) {
+    result = min(result, v);
+  }
+
+  return result;
+}
+
+/**
+ * Element-wise minimum of the passed vectors.
+ */
+template<typename T, int Size>
+[[nodiscard]] inline VecBase<T, Size> min(std::initializer_list<VecBase<T, Size>> values)
+{
+  return min(Span(values));
+}
+
 template<typename T, int Size>
 [[nodiscard]] inline VecBase<T, Size> max(const VecBase<T, Size> &a, const VecBase<T, Size> &b)
 {
   BLI_UNROLL_MATH_VEC_FUNC_VEC_VEC(math::max, a, b);
 }
 
+/**
+ * Element-wise maximum of the passed vectors.
+ */
+template<typename T, int Size>
+[[nodiscard]] inline VecBase<T, Size> max(Span<VecBase<T, Size>> values)
+{
+  BLI_assert(!values.is_empty());
+
+  VecBase<T, Size> result = values[0];
+  for (const VecBase<T, Size> &v : values.drop_front(1)) {
+    result = max(result, v);
+  }
+
+  return result;
+}
+
+/**
+ * Element-wise maximum of the passed vectors.
+ */
+template<typename T, int Size>
+[[nodiscard]] inline VecBase<T, Size> max(std::initializer_list<VecBase<T, Size>> values)
+{
+  return max(Span(values));
+}
+
 template<typename T, int Size>
 [[nodiscard]] inline VecBase<T, Size> clamp(const VecBase<T, Size> &a,
                                             const VecBase<T, Size> &min,

source/blender/blenlib/tests/BLI_math_vector_test.cc

@@ -6,6 +6,7 @@
 
 #include "BLI_math_vector.h"
 #include "BLI_math_vector.hh"
+#include "BLI_vector.hh"
 
 namespace blender::tests {
 
@@ -65,6 +66,44 @@ TEST(math_vector, Clamp)
   EXPECT_EQ(result_2.z, -50);
 }
 
+TEST(math_vector, MinList)
+{
+  EXPECT_EQ(float3(1.0, 2.0, 3.0), math::min({float3(1.0, 2.0, 3.0)}));
+  EXPECT_EQ(float3(0.0, 2.0, 2.0), math::min({float3(1.0, 2.0, 3.0), float3(0.0, 5.0, 2.0)}));
+  EXPECT_EQ(float3(0.0, 2.0, 1.5),
+            math::min({float3(1.0, 2.0, 3.0), float3(0.0, 5.0, 2.0), float3(2.0, 4.0, 1.5)}));
+
+  const float inf = std::numeric_limits<float>::infinity();
+  EXPECT_EQ(float3(0.0, -inf, -inf),
+            math::min({float3(inf, 2.0, 3.0), float3(0.0, -inf, inf), float3(2.0, 4.0, -inf)}));
+
+  const float nan = std::numeric_limits<float>::quiet_NaN();
+  const float3 result = math::min(
+      {float3(nan, 2.0, 3.0), float3(0.0, nan, 2.0), float3(2.0, 4.0, nan)});
+  EXPECT_TRUE(std::isnan(result.x));
+  EXPECT_EQ(result.y, 2.0);
+  EXPECT_EQ(result.z, 2.0);
+}
+
+TEST(math_vector, MaxList)
+{
+  EXPECT_EQ(float3(1.0, 2.0, 3.0), math::max({float3(1.0, 2.0, 3.0)}));
+  EXPECT_EQ(float3(1.0, 5.0, 3.0), math::max({float3(1.0, 2.0, 3.0), float3(0.0, 5.0, 2.0)}));
+  EXPECT_EQ(float3(2.0, 5.0, 3.0),
+            math::max({float3(1.0, 2.0, 3.0), float3(0.0, 5.0, 2.0), float3(2.0, 4.0, 1.5)}));
+
+  const float inf = std::numeric_limits<float>::infinity();
+  EXPECT_EQ(float3(inf, 4.0, inf),
+            math::max({float3(inf, 2.0, 3.0), float3(0.0, -inf, inf), float3(2.0, 4.0, -inf)}));
+
+  const float nan = std::numeric_limits<float>::quiet_NaN();
+  const float3 result = math::max(
+      {float3(nan, 2.0, 3.0), float3(0.0, nan, 2.0), float3(2.0, 4.0, nan)});
+  EXPECT_TRUE(std::isnan(result.x));
+  EXPECT_EQ(result.y, 4.0);
+  EXPECT_EQ(result.z, 3.0);
+}
+
 TEST(math_vector, InterpolateInt)
 {
   const int3 a(0, -100, 50);

source/blender/editors/space_view3d/view3d_navigate_view_all.cc

@@ -317,8 +317,10 @@ std::optional<blender::Bounds<float3>> view3d_calc_minmax_selected(Depsgraph *de
     {
       const std::optional<blender::Bounds<float3>> bounds = BKE_pose_minmax(ob_eval_iter, true);
       if (bounds) {
-        minmax_v3v3_v3(min, max, bounds->min);
-        minmax_v3v3_v3(min, max, bounds->max);
+        const blender::Bounds<float3> world_bounds = blender::bounds::transform_bounds(
+            ob_eval->object_to_world(), *bounds);
+        minmax_v3v3_v3(min, max, world_bounds.min);
+        minmax_v3v3_v3(min, max, world_bounds.max);
         changed = true;
       }
     }