[mutter] clutter-box-layout: Use floats and assert on denormal numbers



commit 72692b11441e99b6b6dfd72de2f6f4113b901a02
Author: Robert Mader <robert mader posteo de>
Date:   Mon Jan 7 14:09:16 2019 +0100

    clutter-box-layout: Use floats and assert on denormal numbers
    
    `distribute_natural_allocation` expects an input >= 0 of type `gint`. In
    `get_preferred_size_for_opposite_orientation` it is used with an unchecked
    variable `size` of type `gfloat`, which in case it is `Infinity`, gets
    passed on in the macro `MAX (0, size)`. `Infinity` becomes `G_MININT`
    when implicitly casted to `gint` in `distribute_natural_allocation`,
    triggering the assertion `extra_space >= 0`.
    
    The resulting warning in the log is counter intuitive and not very
    helpful.
    
    Use `float` in `distribute_natural_allocation` instead of `gint` and
    assert on denormal values so we can more easily identify bugs.
    
    Additionally change some types while at it and add a even more
    expressive warning referencing the actor at one point.
    
    https://gitlab.gnome.org/GNOME/mutter/merge_requests/375

 clutter/clutter/clutter-box-layout.c | 47 ++++++++++++++++++++++++------------
 1 file changed, 31 insertions(+), 16 deletions(-)
---
diff --git a/clutter/clutter/clutter-box-layout.c b/clutter/clutter/clutter-box-layout.c
index 1e7cb7db5..a3197f43f 100644
--- a/clutter/clutter/clutter-box-layout.c
+++ b/clutter/clutter/clutter-box-layout.c
@@ -150,9 +150,9 @@ typedef struct _RequestedSize
   gfloat natural_size;
 } RequestedSize;
 
-static gint distribute_natural_allocation (gint                  extra_space,
-                                          guint                 n_requested_sizes,
-                                          RequestedSize        *sizes);
+static float distribute_natural_allocation (float          extra_space,
+                                            unsigned int   n_requested_sizes,
+                                            RequestedSize *sizes);
 static void count_expand_children         (ClutterLayoutManager *layout,
                                           ClutterContainer     *container,
                                           gint                 *visible_children,
@@ -624,7 +624,19 @@ get_preferred_size_for_opposite_orientation (ClutterBoxLayout   *self,
   else
     {
       /* Bring children up to size first */
-      size = distribute_natural_allocation (MAX (0, size), nvis_children, sizes);
+      if (isnormal (size) || size == 0)
+        {
+          size = distribute_natural_allocation (MAX (0, size),
+                                                nvis_children,
+                                                sizes);
+        }
+      else
+        {
+          g_critical ("Actor %s (%p) received the invalid "
+                      "value %f as minimum/natural size\n",
+                       G_OBJECT_TYPE_NAME (container), container, size);
+          size = 0;
+        }
 
       /* Calculate space which hasn't distributed yet,
        * and is available for expanding children.
@@ -879,17 +891,18 @@ compare_gap (gconstpointer p1,
  *
  * Pulled from gtksizerequest.c from Gtk+
  */
-static gint
-distribute_natural_allocation (gint           extra_space,
-                               guint          n_requested_sizes,
+static float
+distribute_natural_allocation (float          extra_space,
+                               unsigned int   n_requested_sizes,
                                RequestedSize *sizes)
 {
-  guint *spreading;
-  gint   i;
+  unsigned int *spreading;
+  int i;
 
+  g_return_val_if_fail (isnormal (extra_space) || extra_space == 0, 0);
   g_return_val_if_fail (extra_space >= 0, 0);
 
-  spreading = g_newa (guint, n_requested_sizes);
+  spreading = g_newa (unsigned int, n_requested_sizes);
 
   for (i = 0; i < n_requested_sizes; i++)
     spreading[i] = i;
@@ -913,7 +926,7 @@ distribute_natural_allocation (gint           extra_space,
 
   /* Sort descending by gap and position. */
   g_qsort_with_data (spreading,
-                     n_requested_sizes, sizeof (guint),
+                     n_requested_sizes, sizeof (unsigned int),
                      compare_gap, sizes);
 
   /* Distribute available space.
@@ -925,11 +938,11 @@ distribute_natural_allocation (gint           extra_space,
        * Sort order and reducing remaining space by assigned space
        * ensures that space is distributed equally.
        */
-      gint glue = (extra_space + i) / (i + 1);
-      gint gap = sizes[(spreading[i])].natural_size
-               - sizes[(spreading[i])].minimum_size;
+      int glue = (extra_space + i) / (i + 1);
+      int gap = sizes[(spreading[i])].natural_size
+              - sizes[(spreading[i])].minimum_size;
 
-      gint extra = MIN (glue, gap);
+      int extra = MIN (glue, gap);
 
       sizes[spreading[i]].minimum_size += extra;
 
@@ -1056,7 +1069,9 @@ clutter_box_layout_allocate (ClutterLayoutManager   *layout,
   else
     {
       /* Bring children up to size first */
-      size = distribute_natural_allocation (MAX (0, size), nvis_children, sizes);
+      size = (gint) distribute_natural_allocation (MAX (0, (float) size),
+                                                   nvis_children,
+                                                   sizes);
 
       /* Calculate space which hasn't distributed yet,
        * and is available for expanding children.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]