fix #523: crash with manual width adjustment on subpatch

pd/src/g_canvas.c

@@ -2186,39 +2186,52 @@ int canvas_path_iterate(t_canvas*x, t_canvas_path_iterator fun, void *user_data)
     return(result);
 }*/
 
-extern t_symbol *last_typedmess; // see g_readwrite.c for the explanation of this ugly hack
+extern t_symbol *last_typedmess; // see m_class.c for details on this ugly hack
+extern t_pd *last_typedmess_pd; // same as above
 
 static void canvas_f(t_canvas *x, t_symbol *s, int argc, t_atom *argv)
 {
-    static int warned;
-    //fprintf(stderr,"canvas_f %lx %d current=%lx %s\n", (t_int)x, argc, canvas_getcurrent(), last_typedmess != NULL ? last_typedmess->s_name : "none");
+    static int warned_future_version;
+    static int warned_old_syntax;
+    //fprintf(stderr,"canvas_f %lx %d current=%lx %s\n",
+    //    (t_int)x, argc, canvas_getcurrent(),
+    //    last_typedmess != NULL ? last_typedmess->s_name : "none");
     t_canvas *xp = x; //parent window for a special case dealing with subpatches
     t_gobj *g, *g2;
     t_object *ob;
-    if (argc > 1 && !warned)
+    if (argc > 1 && !warned_future_version)
     {
         post("** ignoring width or font settings from future Pd version **");
-        warned = 1;
-    }
-    // if we are either an empty canvas or are a part of a restore message of a subpatch
-    if (!x->gl_list || !strcmp(last_typedmess->s_name, "restore")) {
-        if (x->gl_owner && !x->gl_isgraph) {
-            // this means that we are a canvas that was just created or have just
-            // fnished restoring and that our width applies to our appearance on
-            // our parent
-            xp = x->gl_owner;
-            for (g = xp->gl_list; g != (t_gobj *)x; g = g->g_next) {
-                //fprintf(stderr,".x%lx .x%lx\n", (t_int)g, (t_int)x);
-                ;
+        warned_future_version = 1;
+    }
+    // if we are part of a restore message
+    // of a subpatch in the form "#X restore..., f 123456789+;"
+    if (pd_class(last_typedmess_pd) == canvas_class &&
+        (t_canvas *)last_typedmess_pd == x &&
+        last_typedmess == gensym("restore"))
+    {
+        if (x->gl_owner && !x->gl_isgraph)
+        {
+            // this means that we are a canvas that
+            // just finished restoring and that our width applies to our
+            // appearance on our parent.
+            if (!warned_old_syntax)
+            {
+                post("warning: old box-width syntax detected. Please save the "
+                     "patch to switch to the new syntax.");
+                warned_old_syntax = 1;
             }
-            //fprintf(stderr,"done %d\n", (g != NULL ? 1: 0));
-        } else return;
-    } else {
-    for (g = x->gl_list; g2 = g->g_next; g = g2)
-        ;
+            xp = x->gl_owner;
+            g = &x->gl_gobj;
+        }
+        else return;
+    }
+    else
+    {
+        for (g = x->gl_list; g2 = g->g_next; g = g2)
+            ;
         //fprintf(stderr,"same canvas .x%lx .x%lx\n", (t_int)g, (t_int)x);
     }
-    //fprintf(stderr,"is canvas_class? %d\n", (pd_class(&g->g_pd) == canvas_class ? 1:0));
     if ((ob = pd_checkobject(&g->g_pd)) || pd_class(&g->g_pd) == canvas_class)
     {
         //fprintf(stderr,"f received\n");

pd/src/g_text.c

@@ -2186,6 +2186,7 @@ void text_save(t_gobj *z, t_binbuf *b)
 {
     //fprintf(stderr, "text_save\n");
     t_text *x = (t_text *)z;
+    int savedacanvas = 0;
     if (x->te_type == T_OBJECT)
     {
             /* if we have a "saveto" method, and if we don't happen to be
@@ -2199,6 +2200,7 @@ void text_save(t_gobj *z, t_binbuf *b)
             mess1(&x->te_pd, gensym("saveto"), b);
             binbuf_addv(b, "ssii", gensym("#X"), gensym("restore"),
                 (int)x->te_xpix, (int)x->te_ypix);
+            savedacanvas = 1;
         }
         else    /* otherwise just save the text */
         {
@@ -2283,7 +2285,12 @@ void text_save(t_gobj *z, t_binbuf *b)
         binbuf_addbinbuf(b, x->te_binbuf);
     }
     if (x->te_width)
-        binbuf_addv(b, ",si", gensym("f"), (int)x->te_width);
+    {
+        if (savedacanvas)
+            binbuf_addv(b, ";ssi", gensym("#X"), gensym("f"), (int)x->te_width);
+        else
+            binbuf_addv(b, ",si", gensym("f"), (int)x->te_width);
+    }
     binbuf_addv(b, ";");
 }
 

pd/src/m_class.c

@@ -697,11 +697,24 @@ typedef t_pd *(*t_fun6)(t_int i1, t_int i2, t_int i3, t_int i4, t_int i5, t_int
 /* needed for proper error reporting */
 extern t_pd *pd_mess_from_responder(t_pd *x);
 
-// hackish way to figure out what was the last command we invoked so that commands
-// that follow it (via commas, like the new width ability), can understand whom
-// they belong to. This is a problem with subpatches whose values get assigned to
-// their last created object, as opposed themselves
+/* This is a hack for the messages in Pd files that follow a comma, such as:
+     #X restore..., f 12
+   In that case the "f 12" would get sent to the subcanvas where we actually
+   want it to apply to the parent.
+
+   Pd Vanilla apparently made a special case to handle this-- instead of using
+   the comma after the "restore" message it starts a new "#X f 12;" which
+   ensures that the #X is bound to the correct canvas. But that means it fails
+   for the old the old syntax, and there are patches in the wild that use it.
+
+   So we support _both_ of these styles in Purr Data by doing the following:
+   1. Checking if the last typedmess was "restore" selector.
+   2. Checking if the last t_pd was a canvas that matches the current one.
+      If so, we're dealing with the old "#X..., f 12;" syntax. If not, it's
+      the new Pd Vanilla syntax.
+*/
 t_symbol *last_typedmess;
+t_pd *last_typedmess_pd;
 
 void pd_typedmess(t_pd *x, t_symbol *s, int argc, t_atom *argv)
 {
@@ -878,6 +891,7 @@ badarg:
         s->s_name, c->c_name->s_name);
 lastmess:
     last_typedmess = s;    
+    last_typedmess_pd = x;
     return;
 }
 

scripts/regression_tests/canvas_f.pd

+#N canvas 447 229 471 300 12;
+#X obj 29 33 inlet;
+#X obj 29 102 ../utils/creation-test;
+#X msg 29 62 symbol ../regression_tests/canvas_f_abstraction;
+#X obj 29 131 list prepend apply manual width setting to subpatch with
+'#X f' syntax;
+#X obj 29 176 outlet;
+#X connect 0 0 2 0;
+#X connect 1 0 3 0;
+#X connect 2 0 1 0;
+#X connect 3 0 4 0;

scripts/regression_tests/canvas_f_abstraction.pd

+#N canvas 464 271 450 300 12;
+#N canvas 0 0 450 300 test 0;
+#X restore 106 71 pd test;
+#X f 8;