[PD-dev] two patches for pd vanilla and pd-extended 0.42.5

[Hans-Christoph Steiner]

On Oct 31, 2009, at 9:12 PM, Ivica Ico Bukvic wrote:

>
>>> 2) graph on parent (GOP) enable and then immediately disable crashes
>>> patches that haven't been closed prior to disabling GOP (to  
>>> reproduce,
>>> open new patch->right-click->properties->enable gop->apply->disable
>>> gop->apply->crash). This one may also affect pd vanilla (haven't
>>> checked)
>>
>> Looks good to me.   Is it still present in 0.43?  Submit this to the
>
> Just tested it and yes it is present in 0.43 (pd-gui-rewrite version I
> checked out less than a week ago from svn).
>
>>
>> patch tracker and assign it to Miller.  Then I can accept it in Pd-
>> extended.  One change, I'd do this patch like this, I think its  
>> cleaner:
>>
>> Index: g_editor.c
>> ===================================================================
>> --- g_editor.c	(revision 12704)
>> +++ g_editor.c	(working copy)
>> @@ -907,13 +907,15 @@
>>  {
>>      t_gobj *y;
>>      t_object *ob;
>> -    if (x->gl_editor)
>> +    if (x->gl_editor && x->gl_list) <---------HERE
>>      {
>>          for (y = x->gl_list; y; y = y->g_next)
>>              if (ob = pd_checkobject(&y->g_pd))
>>                  rtext_free(glist_findrtext(x, ob));
>> -        editor_free(x->gl_editor, x);
>> -        x->gl_editor = 0;
>> +        if (x->gl_editor) {
>
> I think this is superfluous, the code is already checking that in the
> previous if statement (see ASCII arrow above). The code in between the
> arrow and this if statement TTBOMK does not touch gl_editor. So, my  
> vote
> would be to leave the patch as-is.

Stylistically, the format of your patch doesn't fit in with most Pd  
code, the change I suggest makes the patch fit into the Pd style.   
They are both functionally the same.

> However, please note another more important omission which deals with
> the other bug I reported in the follow-up email:
>
> --- g_editor_old.c      2009-10-30 22:13:16.000000000 -0400
> +++ g_editor.c  2009-10-31 14:01:29.000000000 -0400
> @@ -907,13 +907,18 @@ void canvas_destroy_editor(t_glist *x)
> {
>     t_gobj *y;
>     t_object *ob;
> +       glist_noselect(x);
>     if (x->gl_editor)
>
> It appears if noselect is not called prior to running the rest of the
> code, it crashes in deselect part. This simply deselects selected
> objects when disabling GOP. To reproduce this problem do the  
> following:
>
> New patcher->create [pd something]->inside new patcher create any  
> object
> (e.g. symbol) *and leave the object selected*->right-click on
> canvas->properties->enable gop->apply->disable gop->apply->crash
>
>> +            editor_free(x->gl_editor, x);
>> +            x->gl_editor = 0;
>> +        }
>
> Obviously I would also remove the last line if you agree to remove the
> "if" statement in question.

I don't really know the ramifications of this patch, hence submitting  
it to the patch tracker.  But fixing crasher bugs is always good.

.hc

>
> Best wishes,
>
> Ico
>




----------------------------------------------------------------------------

You can't steal a gift. Bird gave the world his music, and if you can  
hear it, you can have it. - Dizzy Gillespie