How to consistently handle custom shapes line_width?

[Hans Breuer <hans breuer org>]

While creating my own custom shapes and improving the documentation of 
custom-shapes [1] I noticed that the "stroke-width" given by the shape file 
is not consistently handled.
Only the very first line-width used during shape drawing was replaced by 
the one given in the shapes property. Further strokes were drawn with the 
product of stroke-width from the shape file and the "border_width" 
(line-width) given by the properties.
Apparently this is broken since the beginning of time. It only seemed to 
work when in the drawing sequence there was just one "stroke".

I see three options:

 * always use the stroke-width given by the shape-file
 * always use the line_width given by the properties dialog
 * something in between ;)

The last option seems most usefull but needs some further definition.
I currently have implmented a version where every line-width gets weighted 
by the ratio ("border_width" / "first-stroke-width-in-file"). It should 
give the same result for single-stroke-shapes as before (e.g. apparently 
all of Assorted [2]), but will produce artifacts for shapes adding a 
workaround like "Cisco - PC".

Of course it would also be possible to accept and document the current 
workaround apparently added to a lot of shapes. But IMO this would be the 
least compelling approach.

Comments?

BTW: if you want to test it yourself apply the attached patch.

Thanks,
        Hans

[1] http://svn.gnome.org/viewvc/dia/trunk/doc/custom-shapes?view=log
[2] http://bugzilla.gnome.org/show_bug.cgi?id=120784

-------- Hans "at" Breuer "dot" Org -----------
Tell me what you need, and I'll tell you how to
get along without it.                -- Dilbert

Index: objects/custom/custom_object.c
===================================================================
--- objects/custom/custom_object.c      (revision 4094)
+++ objects/custom/custom_object.c      (working copy)
@@ -554,7 +554,7 @@
 
   for (tmp = custom->info->display_list; tmp != NULL; tmp = tmp->next) {
     GraphicElement *el = tmp->data;
-    real line_width = el->any.s.line_width * custom->border_width;
+    real line_width = el->any.s.line_width * (custom->border_width / custom->info->default_border_width);
 
     switch (el->type) {
     case GE_LINE:
@@ -688,9 +688,7 @@
 custom_adjust_scale(Custom *custom, Handle *handle,
                   Point *to, ConnectionPoint *cp,
                   HandleMoveReason reason, ModifierKeys modifiers)
-{
-#define IS_MODIFIER_PRESSED(m) ((modifiers&(m)) != 0)
-  
+{  
   static int uniform_scale = FALSE;
   static Point orig_pos;
 
@@ -703,11 +701,11 @@
       orig_pos.y = to->y;
     }
     
-    if (!uniform_scale && IS_MODIFIER_PRESSED(MODIFIER_SHIFT)) {
+    if (!uniform_scale && ((modifiers & MODIFIER_SHIFT) != 0)) {
       custom->old_subscale = MAX(custom->subscale, 0.0);
-    }
-    
-    uniform_scale = IS_MODIFIER_PRESSED(MODIFIER_SHIFT);
+      uniform_scale = TRUE;
+    } else 
+      uniform_scale = FALSE;
 
     delta_max = (to->x - orig_pos.x);
     
@@ -726,7 +724,6 @@
     if( custom->subscale < SUBSCALE_MININUM_SCALE )
       custom->subscale = SUBSCALE_MININUM_SCALE;
 
-#undef IS_MODIFIER_PRESSED
     break;
   case HANDLE_MOVE_USER_FINAL:
     uniform_scale = FALSE;
@@ -897,7 +894,7 @@
   if (el->any.s.line_width != (*cur_line)) {
     (*cur_line) = el->any.s.line_width;
     renderer_ops->set_linewidth(renderer,
-                 custom->border_width*(*cur_line));
+                 (*cur_line)*(custom->border_width / custom->info->default_border_width));
   }
   if ((el->any.s.linecap == DIA_SVG_LINECAPS_DEFAULT && (*cur_caps) != LINECAPS_BUTT) ||
     el->any.s.linecap != (*cur_caps)) {
@@ -1174,9 +1171,9 @@
       
     text_calc_boundingbox(custom->text, NULL);
     text_width = 
-      custom->text->max_width + 2*custom->padding+custom->border_width;
+      custom->text->max_width + 2*custom->padding+(custom->border_width / 
custom->info->default_border_width);
     text_height = custom->text->height * custom->text->numlines +
-      2 * custom->padding + custom->border_width;
+      2 * custom->padding + (custom->border_width / custom->info->default_border_width);
 
     transform_rect(custom, &info->text_bounds, &tb);
     xscale = text_width / (tb.right - tb.left);
@@ -1329,7 +1326,7 @@
   for (tmp = custom->info->display_list; tmp != NULL; tmp = tmp->next) {
     GraphicElement *el = tmp->data;
     Rectangle rect;
-    real lwfactor = custom->border_width / 2;
+    real lwfactor = (custom->border_width / custom->info->default_border_width) / 2;
 
     switch(el->type) {
     case GE_SUBSHAPE :
@@ -1558,7 +1555,7 @@
   custom->subscale = 1.0;
   custom->current_subshape = NULL;
 
-  custom->border_width =  attributes_get_default_linewidth();
+  custom->border_width = attributes_get_default_linewidth();
   custom->border_color = attributes_get_foreground();
   custom->inner_color = attributes_get_background();
   custom->show_background = default_properties.show_background;
Index: objects/custom/shape_info.c
===================================================================
--- objects/custom/shape_info.c (revision 4092)
+++ objects/custom/shape_info.c (working copy)
@@ -223,6 +223,10 @@
       continue;
     dia_svg_style_init (&s, style);
     dia_svg_parse_style(node, &s, -1);
+    /* first line_width found is default for scaling with border_width */
+    if (info->default_border_width == 0 && s.line_width != 0)
+      info->default_border_width = s.line_width;
+
     if (!xmlStrcmp(node->name, (const xmlChar *)"line")) {
       GraphicElementLine *line = g_new0(GraphicElementLine, 1);
 
@@ -460,16 +464,11 @@
           /* add elements from the group element, but make it a subshape */
         GraphicElementSubShape *subshape = g_new0(GraphicElementSubShape, 1);
         ShapeInfo* tmpinfo = g_new0(ShapeInfo, 1);
-        DiaSvgStyle tmp_s = {
-                1.0, DIA_SVG_COLOUR_FOREGROUND, 
-                DIA_SVG_COLOUR_NONE,
-                DIA_SVG_LINECAPS_DEFAULT, 
-                DIA_SVG_LINEJOIN_DEFAULT,
-                DIA_SVG_LINESTYLE_DEFAULT, 1.0
-        };
+        DiaSvgStyle tmp_s;
         xmlChar *v_anchor_attr = xmlGetProp(node, (const xmlChar*)"v_anchor");
         xmlChar *h_anchor_attr = xmlGetProp(node, (const xmlChar*)"h_anchor");
       
+        dia_svg_style_init (&tmp_s, style);
         parse_svg_node(tmpinfo, node, svg_ns, &tmp_s, filename);
         
         tmpinfo->shape_bounds.top = DBL_MAX;
@@ -529,6 +528,11 @@
     if (s.font)
       dia_font_unref (s.font);
   }
+  /* if there is no stroke-width in the file, choose the default of attributes_default_linewidth()  
+   * to avoid normalizing with 0. We ar not using the function because it would make the 
+   * shapes appearance depend on some UI state (and can be zero again) */
+  if (info->default_border_width == 0)
+    info->default_border_width = 0.1;
 }
 
 static void
@@ -838,15 +842,17 @@
       xmlFree(tmp);
     } else if (node->ns == svg_ns && !xmlStrcmp(node->name, (const xmlChar *)"svg")) {
       DiaSvgStyle s = {
-       1.0, DIA_SVG_COLOUR_FOREGROUND, DIA_SVG_COLOUR_NONE,
+       0.0, DIA_SVG_COLOUR_FOREGROUND, DIA_SVG_COLOUR_NONE,
        DIA_SVG_LINECAPS_DEFAULT, DIA_SVG_LINEJOIN_DEFAULT, DIA_SVG_LINESTYLE_DEFAULT, 1.0
       };
 
       dia_svg_parse_style(node, &s, -1);
+      /* first line_width found is default for scaling with border_width */
+      if (info->default_border_width == 0 && s.line_width != 0)
+        info->default_border_width = s.line_width;
       parse_svg_node(info, node, svg_ns, &s, filename);
       update_bounds(info);
-    }
-       else if (!xmlStrcmp(node->name, (const xmlChar *)"ext_attributes")) {
+    } else if (!xmlStrcmp(node->name, (const xmlChar *)"ext_attributes")) {
       ext_node = node;
     }
   }
Index: objects/custom/shape_info.h
===================================================================
--- objects/custom/shape_info.h (revision 4092)
+++ objects/custom/shape_info.h (working copy)
@@ -186,6 +186,7 @@
 
   real default_width;  /* default_width normalized in points */
   real default_height; /* default_height normalized in points */
+  real default_border_width;
 
   GList *display_list;