Re: parse geometry

[Owen Taylor <otaylor redhat com>]

Havoc Pennington <hp redhat com> writes:

> +gboolean
> +gtk_window_parse_geometry (GtkWindow   *window,
> +                           const gchar *geometry)
> +{
> +  gint result, x, y;
> +  guint uw, uh;
> +  gint w, h;
> +  GdkGravity grav;
> +  GtkWidget *widget;
> +  gint extra_width = 0;
> +  gint extra_height = 0;
> +  GtkWindowGeometryInfo *geometry_info;
> +  GtkRequisition requisition;
> +  
> +  g_return_val_if_fail (GTK_IS_WINDOW (window), FALSE);
> +  g_return_val_if_fail (geometry != NULL, FALSE);
> +  
> +  result = gtk_XParseGeometry (geometry, &x, &y, &uw, &uh);
> +  w = uw;
> +  h = uh;

uw,uh will not be initialized here if !result & WidthValue, !result
&HeightValue not set, so you probably want to avoid accessing them
even though you eventually do check the flags. (It will make purify
unhappy, etc)

> +  widget = GTK_WIDGET (window);
> +  
> +  gtk_widget_get_child_requisition (widget, &requisition);
> +  geometry_info = gtk_window_get_geometry_info (GTK_WINDOW (widget), FALSE);
                                                   ^^^^^^^^^^^^^^^^^^^

Excessive casting copied from gtk_window_compute_hints().

> +  g_return_val_if_fail (geometry_info != NULL, FALSE);

I don't think this assertion is valid here - geometry info could well
never have been referenced before. I think it is probably simplest to
just change the FALSE above to TRUE.

> +  if (geometry_info->mask & GDK_HINT_RESIZE_INC)
> +    {
> +      w *= geometry_info->geometry.width_inc;
> +      h *= geometry_info->geometry.height_inc;
> +    }

> +  if (geometry_info->widget)
> +    {
> +      extra_width = widget->requisition.width - geometry_info->widget->requisition.width;
> +      extra_height = widget->requisition.height - geometry_info->widget->requisition.height;
> +    }
> +
> +  w -= extra_width;
> +  h -= extra_height;
> +
> +  if ((result & WidthValue) && w > 0)
> +    gtk_window_set_default_size (window, w, -1);
> +  if ((result & HeightValue) && h > 0)
> +    gtk_window_set_default_size (window, -1, h);
> +
> +  grav = GDK_GRAVITY_NORTH_WEST;
> +
> +  if ((result & XNegative) && (result & YNegative))
> +    grav = GDK_GRAVITY_SOUTH_EAST;
> +  else if (result & XNegative)
> +    grav = GDK_GRAVITY_NORTH_EAST;
> +  else if (result & YNegative)
> +    grav = GDK_GRAVITY_SOUTH_WEST;

     else
       grav = GDK_GRAVITY_NORTH_WEST;

?

> +  gtk_window_set_gravity (window, grav);

Reminds me that gravity is actually in GdkGeometry so having
gtk_window_set_geometry_hints() not affecting gravity and
a separate function to set the gravity seems bizarre.

> +  if (grav == GDK_GRAVITY_SOUTH_WEST ||
> +      grav == GDK_GRAVITY_SOUTH_EAST)
> +    y = gdk_screen_height () + y;
> +
> +  if (grav == GDK_GRAVITY_SOUTH_EAST ||
> +      grav == GDK_GRAVITY_NORTH_EAST)
> +    x = gdk_screen_width () + x;
> +
> +  if (y < 0)
> +    y = 0;
> +
> +  if (x < 0)
> +    x = 0;
> +  
> +  if (result & XValue)
> +    gtk_window_set_location (window, x, -1);
> +  if (result & YValue)
> +    gtk_window_set_location (window, -1, y);

Hrmm, I intentionally made set_location() not honour -1 because
-1 is a perfectly valid window position. Also, I don't think
we need to handle X-only or Y-only settings of the position.

Regards,
                                        Owen