Re: PATCH: ellipse.c : keep aspect ration / circle.

[Grégoire Dooms <dooms info ucl ac be>]

Lars Clausen wrote:

> I tried it out, and while the core code is useful, I ended up doing a
> dropdown menu.  It was too confusing that two boolean settings could
> silently change each other.  I also added it to the object menu and
> added undo, and (while I was at it) did the same for box.  It's great to
> have working code to build from.  Thanks!
>
> -Lars
>  
Great.
However there is still a shortcoming/bug:
the undo code only works for the object menu not for the properties menu:
if the ellipse is forced into a circle from the properties menu, its old 
dimensions are not saved/restored.
It looks like this comes from
(ApplyPropertiesFunc) object_apply_props_from_dialog,
which should save the width/height.

It looks to me there are two ways of doing it:
- redefine this function with an ad-hoc one. The problem is we have to 
go inside the PropDialog to get the values of the properties and we 
can't use the AspectChange since the change can also be an other change 
(e.g line width).
- redefine props to include width and height in the GPtrArray props  
such as get/set props (using props_from_offset) used by 
object_apply_props_from_dialog saves/restores the width and height. The 
problem is then the width and height must be visible in the widget if we 
want prop_get_data_from_widgets to store the width and height.

If one thinks it is useful to have the width/height visible/changeable 
in the widget, then there is another problem:
When selecting circle the width/height should be updated.
It looks like this is the job of an event handler: (excerpt from 
properties.h line 214):
/* if the property widget can send events when it's somehow interacted 
with,
    control will be passed to object_type-supplied event_handler, and
    event->dialog->obj_copy will be current with the dialog's values.
    When control comes back, event->dialog->obj_copy's properties will be
    brought back into the dialog. */
 PropEventHandler event_handler;

But PropEventHandler is

typedef gboolean (*PropEventHandler) (Object *obj, Property *prop);

So I don't see where to get the event->dialog->obj_copy from. On the 
other hand defining and binding an empty event handler makes the app 
crash (segfault).

Attached is a patch implementing the visible width height properties in 
ellipse.c; the event handler is declared and defined but not bound to 
the widget so that dia doesn't crash ( (un)commenting two lines 
illustrates the segfault). This patch fixes the undo bug in the 
properties but introduces an inconsistency in the property widget 
between aspect==circle and width!=height (like with the former patch and 
the two booleans).

--
Grégoire Dooms

Index: ellipse.c
===================================================================
RCS file: /cvs/gnome/dia/objects/standard/ellipse.c,v
retrieving revision 1.37
diff -c -r1.37 ellipse.c
*** ellipse.c   15 Mar 2004 14:48:59 -0000      1.37
--- ellipse.c   16 Mar 2004 17:15:47 -0000
***************
*** 90,95 ****
--- 90,96 ----
  static void ellipse_save(Ellipse *ellipse, ObjectNode obj_node, const char *filename);
  static Object *ellipse_load(ObjectNode obj_node, int version, const char *filename);
  static DiaMenu *ellipse_get_object_menu(Ellipse *ellipse, Point *clickedpoint);
+ static gboolean aspect_event_handler(Object *obj, Property *prop);
  
  static ObjectTypeOps ellipse_type_ops =
  {
***************
*** 142,150 ****
--- 143,161 ----
    PROP_STD_LINE_STYLE,
    { "aspect", PROP_TYPE_ENUM, PROP_FLAG_VISIBLE,
      N_("Aspect ratio"), NULL, prop_aspect_data },
+ //  N_("Aspect ratio"), NULL, prop_aspect_data, aspect_event_handler },
+  { "ellipse_width", PROP_TYPE_REAL, PROP_FLAG_VISIBLE,
+     N_("Width"), NULL, NULL },
+  { "ellipse_height", PROP_TYPE_REAL, PROP_FLAG_VISIBLE,
+     N_("Height"), NULL, NULL },
    PROP_DESC_END
  };
  
+ static gboolean 
+ aspect_event_handler(Object *obj, Property *prop)
+ {
+       return TRUE;
+ }
  static PropDescription *
  ellipse_describe_props(Ellipse *ellipse)
  {
***************
*** 162,167 ****
--- 173,180 ----
    { "aspect", PROP_TYPE_ENUM, offsetof(Ellipse, aspect) },
    { "line_style", PROP_TYPE_LINESTYLE,
      offsetof(Ellipse, line_style), offsetof(Ellipse, dashlength) },
+   { "ellipse_width", PROP_TYPE_REAL, offsetof(Ellipse,element) + offsetof(Element,width) },
+   { "ellipse_height", PROP_TYPE_REAL, offsetof(Ellipse,element) + offsetof(Element,height) },
    { NULL, 0, 0 }
  };