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
This commit is contained in:
Robert Mader 2019-01-07 14:09:16 +01:00
parent 177b4df217
commit 72692b1144

View File

@ -150,8 +150,8 @@ typedef struct _RequestedSize
gfloat natural_size; gfloat natural_size;
} RequestedSize; } RequestedSize;
static gint distribute_natural_allocation (gint extra_space, static float distribute_natural_allocation (float extra_space,
guint n_requested_sizes, unsigned int n_requested_sizes,
RequestedSize *sizes); RequestedSize *sizes);
static void count_expand_children (ClutterLayoutManager *layout, static void count_expand_children (ClutterLayoutManager *layout,
ClutterContainer *container, ClutterContainer *container,
@ -624,7 +624,19 @@ get_preferred_size_for_opposite_orientation (ClutterBoxLayout *self,
else else
{ {
/* Bring children up to size first */ /* 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, /* Calculate space which hasn't distributed yet,
* and is available for expanding children. * and is available for expanding children.
@ -879,17 +891,18 @@ compare_gap (gconstpointer p1,
* *
* Pulled from gtksizerequest.c from Gtk+ * Pulled from gtksizerequest.c from Gtk+
*/ */
static gint static float
distribute_natural_allocation (gint extra_space, distribute_natural_allocation (float extra_space,
guint n_requested_sizes, unsigned int n_requested_sizes,
RequestedSize *sizes) RequestedSize *sizes)
{ {
guint *spreading; unsigned int *spreading;
gint i; int i;
g_return_val_if_fail (isnormal (extra_space) || extra_space == 0, 0);
g_return_val_if_fail (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++) for (i = 0; i < n_requested_sizes; i++)
spreading[i] = i; spreading[i] = i;
@ -913,7 +926,7 @@ distribute_natural_allocation (gint extra_space,
/* Sort descending by gap and position. */ /* Sort descending by gap and position. */
g_qsort_with_data (spreading, g_qsort_with_data (spreading,
n_requested_sizes, sizeof (guint), n_requested_sizes, sizeof (unsigned int),
compare_gap, sizes); compare_gap, sizes);
/* Distribute available space. /* Distribute available space.
@ -925,11 +938,11 @@ distribute_natural_allocation (gint extra_space,
* Sort order and reducing remaining space by assigned space * Sort order and reducing remaining space by assigned space
* ensures that space is distributed equally. * ensures that space is distributed equally.
*/ */
gint glue = (extra_space + i) / (i + 1); int glue = (extra_space + i) / (i + 1);
gint gap = sizes[(spreading[i])].natural_size int gap = sizes[(spreading[i])].natural_size
- sizes[(spreading[i])].minimum_size; - sizes[(spreading[i])].minimum_size;
gint extra = MIN (glue, gap); int extra = MIN (glue, gap);
sizes[spreading[i]].minimum_size += extra; sizes[spreading[i]].minimum_size += extra;
@ -1056,7 +1069,9 @@ clutter_box_layout_allocate (ClutterLayoutManager *layout,
else else
{ {
/* Bring children up to size first */ /* 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, /* Calculate space which hasn't distributed yet,
* and is available for expanding children. * and is available for expanding children.