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

2009-10-31 Thread 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.0 -0400
+++ g_editor.c  2009-10-31 14:01:29.0 -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





___
Pd-dev mailing list
Pd-dev@iem.at
http://lists.puredata.info/listinfo/pd-dev


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

2009-10-31 Thread Ivica Ico Bukvic

> > 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.

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.0 -0400
+++ g_editor.c  2009-10-31 14:01:29.0 -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.

Best wishes,

Ico


___
Pd-dev mailing list
Pd-dev@iem.at
http://lists.puredata.info/listinfo/pd-dev


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

2009-10-31 Thread Hans-Christoph Steiner


On Oct 31, 2009, at 1:39 PM, Ivica Ico Bukvic wrote:

Here are two patches for g_editor.c that fix following issues in  
0.42.5:


1) undo recreates patch cords with wrong color (I posted this one
earlier by mistake on the pd-list)


Accepted and committed.


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  
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)
 {
 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) {
+editor_free(x->gl_editor, x);
+x->gl_editor = 0;
+}
 }
 }


.hc




I have the audacity to believe that peoples everywhere can have three  
meals a day for their bodies, education and culture for their minds,  
and dignity, equality and freedom for their spirits.  - Martin  
Luther King, Jr.




___
Pd-dev mailing list
Pd-dev@iem.at
http://lists.puredata.info/listinfo/pd-dev


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

2009-10-31 Thread Ivica Ico Bukvic
On Sat, 2009-10-31 at 13:39 -0400, Ivica Ico Bukvic wrote:
> Here are two patches for g_editor.c that fix following issues in 0.42.5:
> 
> 1) undo recreates patch cords with wrong color (I posted this one
> earlier by mistake on the pd-list)
> 
> 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)

Oops, discovered another bug:

New patcher->create [pd something]->inside new patcher create any object
(e.g. symbol) and leave it selected->right-click on
canvas->properties->enable gop->apply->disable gop->apply->crash

New patch should supersede the previous patch for the second point
fixing this problem as well.

Ico
--- g_editor_old.c	2009-10-30 22:13:16.0 -0400
+++ g_editor.c	2009-10-31 14:01:29.0 -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)
 {
-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_list) {
+	for (y = x->gl_list; y; y = y->g_next)
+	if (ob = pd_checkobject(&y->g_pd))
+	rtext_free(glist_findrtext(x, ob));
+			if (x->gl_editor) {
+editor_free(x->gl_editor, x);
+x->gl_editor = 0;
+			}
+		}
 }
 }
___
Pd-dev mailing list
Pd-dev@iem.at
http://lists.puredata.info/listinfo/pd-dev