Hi, "Garry R. Osgood" <grosgood@xxxxxxx> writes: > What bemuses me is this "sink-but-not-really-sink" policy for > layers and channels that prevails around 1.2.1 tile mismanagement, and > which finds partial realization in layer_ref(). [layer.c-CVS-1.72.2.1 gimp-1-2] > This brief little snippet explicitly increments the object reference > count of layers (gtk_object_ref()) before invoking gtk_object_sink(). > After this intrusion into private GTK object management has been pulled off, > we are left with a special layer that is not floating, but not quite sunk > either (it has a positive reference count). calling gtk_object_ref(); gtk_object_sink(); is definitely correct and not strange at all. It is what containers that take ownership of an object are supposed to do. I'm not saying that everything is fine (it apparantly is not), but I wouldn't say we are messing with the GTK+ object system here. > The strange manipulations of layer_ref() is limited to gimp_image_add_layer(), > where it is applied on layers that are being addedto the list in GimpImage::layers Yep. GimpImage::layers is a container and thus sinks the floating object after referencing it to make clear that this object hads found its home and now belongs to the container. Any attempt to delete it using layer_delete() will fail now since the layer is associated to an image and bad things will happen if you delete it without removing it from the container. This is what the comment in layer.c (around line 590) tries to make clear. The problem is most probably located in the undo code. If you remove a layer from an image the image should drop its reference to it. To avoid that it gets deleted and thus can never be readded by an Undo operation the undo stack should reference the layer before removing it from the image. The undo system then has to take care of dropping its reference as soon as the relevant undo step is removed from the stack. I guess this is where the real problem is located. > Happy Gimp. The motivation for this layer_ref() trickery appears to > date to Feb - August 1998 (Gimp 0.99.x) when Scott Goehring first > Objectified Gimp drawables. GTK was in flux then as well, and I surmise > (reading mail from the period is not clear on this) that GTK object referencing > was a bit queer anyway. If so, is layer_ref() (and its effects) a kludge that > has outlived its usefulness? Quite frankly, I get better memory management > when I disable this non-standard reference manipulation and leave > GTK object management internals undisturbed. You will get very bad effects if someone calls layer_delete() on a layer that has been added to an image and on the other hand you want to allow to delete layers that are not part of an image. This situation is exactly why the concept of floating objects was introduced to GTK+ and I think we are using it the way it was designed for. > Personally, I think this is a must-fix that justifies a 1.2.2 release, Yes, we should definitely fix this. I'll try to have a closer look later today. Salut, Sven
