GNOME Bugzilla – Bug 709434
[REGRESSION] clutter_box_layout_allocate: Process /usr/bin/gnome-shell was killed by signal 5 (SIGTRAP)
Last modified: 2013-10-07 11:30:08 UTC
Trace: https://bugzilla.redhat.com/show_bug.cgi?id=1012844 Probably on of: https://git.gnome.org/browse/gnome-shell/commit/src/st/st-box-layout.c?id=53d268a7ef2329689793c43c83986f7636ac052d https://git.gnome.org/browse/gnome-shell/commit/src/st/st-box-layout.c?id=2fa40555e6dc43c508caa002bd57c56f67230478 https://git.gnome.org/browse/gnome-shell/commit/src/st/st-box-layout.c?id=aa394754b6d78e682f99d3aff7f170bea04b0a8a I'll revert this commits and bisecting.
probably https://git.gnome.org/browse/gnome-shell/commit/src/st/st-box-layout.c?id=15f881f9679adc18259a96a17b6ca1c33dffdd3b
Created attachment 256505 [details] [review] table-layout: Fix size request when there are no visible rows/cols The calculation (n - 1) * spacing to compute the total spacing is only correct for n >= 1 - if there are no visible rows/cols, the required spacing is 0 rather than negative.
Created attachment 256506 [details] [review] table-layout: Base space calculations on visible children This is what we already do in the actual size requests, it makes sense to do the same in the space calculations.
The first patch fixes the issue, the seconds is just a drive-by change that looks correct to me. Reassigning to clutter.
Review of attachment 256505 [details] [review]: Florian, thank you. I see that clutter code-style uses: func [space] (args), but you're using func(args) and func [space] (args) that weird. I think you could fix this at commiting.. Please add: Reported-and-Tested-by: Igor Gnatenko <i.gnatenko.brain@gmail.com>
Review of attachment 256506 [details] [review]: Florian, thank you. I see that clutter code-style uses: func [space] (args), but you're using func(args) and func [space] (args) that weird. I think you could fix this at commiting.. Please add: Reported-and-Tested-by: Igor Gnatenko <i.gnatenko.brain@gmail.com>
Review of attachment 256505 [details] [review]: looks okay, with minor comments. ::: clutter/clutter-table-layout.c @@ +1292,3 @@ columns = (DimensionData *) (void *) priv->columns->data; + total_min_width = MAX(priv->visible_cols - 1, 0) * (float) priv->col_spacing; coding style: missing space. also, this could simply be: total_min_width = MAX (((priv->visible_cols - 1) * (float) priv->col_spacing), 0) as it's pretty much pointless to multiply by 0. @@ +1332,3 @@ rows = (DimensionData *) (void *) priv->rows->data; + total_min_height = MAX (priv->visible_rows - 1, 0) * (float) priv->row_spacing; same as above, minus the coding style issue.
Review of attachment 256506 [details] [review]: looks good, with minor comments. ::: clutter/clutter-table-layout.c @@ +884,3 @@ } + pref_width += priv->col_spacing * MAX(priv->visible_cols - 1, 0); coding style: missing space. @@ +885,3 @@ + pref_width += priv->col_spacing * MAX(priv->visible_cols - 1, 0); + min_width += priv->col_spacing * MAX(priv->visible_cols - 1, 0); same as above. @@ +1174,3 @@ } + pref_height += priv->row_spacing * MAX(priv->visible_rows - 1, 0); same as above. @@ +1175,3 @@ + pref_height += priv->row_spacing * MAX(priv->visible_rows - 1, 0); + min_height += priv->row_spacing * MAX(priv->visible_rows - 1, 0); same as above.
no need to use Reviewed-by; Tested-by is appreciated, by unnecessary as well: the link to Bugzilla contains all the required information.
Attachment 256505 [details] pushed as 44b1a80 - table-layout: Fix size request when there are no visible rows/cols Attachment 256506 [details] pushed as 3435d01 - table-layout: Base space calculations on visible children (In reply to comment #7) > also, this could simply be: > > total_min_width = MAX (((priv->visible_cols - 1) * (float) > priv->col_spacing), 0) > > as it's pretty much pointless to multiply by 0. Yeah, I think MAX (cols - 1, 0) is a tad bit more clear, but not that much, so changed.