[Glade-devel] Signal editor rework

[tristan.van.berkom at gmail.com (Tristan Van Berkom)]

On Thu, Aug 26, 2010 at 3:45 PM, Johannes Schmid <jhs at jsschmid.de> wrote:
> Hi Tristan!
>
> > ? - Firstly I can see you went and implemented the GtkTreeIter->stamp a
> > bit more
> > ? ? robustly than in GladeProject, additionally the code needs to assert
> > for
> > ? ? any input iter: (iter->stamp == model->priv->stamp). I've already
> > done this for
> > ? ? GladeProject and will commit that soon (it helped alot to debug...
> > unfortunately
> > ? ? I could still not completely solve bug 627078 and have not
> > committed that yet,
> > ? ? I'll commit the checking code now regardless).
>
> I filed bug https://bugzilla.gnome.org/show_bug.cgi?id=623879 for that
> some time ago to also fix it in GladeProject. But you might be right that
> we might want to check it with an assertion.
>
> > ? - Can you explain why the model->priv->stamp must be incremented
> > when the overall
> > ? ? model data changes ? (it could be that GladeProject needs this
> > treatment too... or
> > ? ? that the model's stamp should not change... I'm not sure).
>
> IMHO the gtk+ documentation says that an iter is only valid as long as the
> model doesn't change. If you can assure that the iter is valid after a
> change you might not need it but when a node is deleted, the iter of that
> node definitly becomes invalid. I think it's good to catch those things
> with an assertion before running into a memory corruption.

Ok that makes perfect sense to me. Sorry I missed the stamp fix
for GladeProject, I'm going to go over your patch and make sure
I didnt miss anything (for instance I at least missed the incrementing
of the stamp when the project tree changes...).

> > ? - The iter->stamp should be initialized to something random,
> > otherwise every model
> > ? ? starts with stamp == 0 (possibly making the iter from the last
> > destroyed signal model
> > ? ? appear "valid")
>
> I wasn't assuming that someone would use an iter on a wrong model but if
> we want to catch that case we might want to start which a random number. I
> think we should check what GtkTreeStore does.

I guess it is at least important that the initial stamp is not 0, since
0 is what we generally use to represent an invalid iter (when
the iter to return was not found for instance).

Actually I was just guessing at why treestore initializes its stamp
to a non-zero random number... I can vouch that treestore (or liststore that
I checked... not sure) does this but I'm not exactly sure why.

> > ? - I would prefer we stick with firing Glade's generic object
> > selection dialog to select
> > ? ? the project object for a signal user data - I dont like the idea
> > of navigating recursive menus
> > ? ? or dropping down a combo box with a possible > 1000 objects to select.
>
> Yeah, I will change that. I just was easy to implement for now but the UI
> is satisfying at all.
>
> Just some additional note: The dnd code currently uses the old gdk_drawing
> funtions that have been removed from gtk+. I didn't have the time to
> change that as I didn't fully understood how to do that in cairo. But it
> shouldn't be a big deal.

Extra note... I went ahead and copied out some GTK+ internals:
   glade_utils_cairo_draw_rectangle(), glade_utils_cairo_draw_line()

... in the case they may be helpful to you... also feel free to add some
convenience drawing functions to glade-utils.[ch] if you think that makes
sense for what needs drawing.

Cheers,
      -Tristan