Re: [Bug 111028] Dealing with reference cycles in GObject
- From: "J. Ali Harlow" <ali avrc city ac uk>
- To: Owen Taylor <otaylor redhat com>
- Cc: gtk-devel-list gnome org
- Subject: Re: [Bug 111028] Dealing with reference cycles in GObject
- Date: Fri, 25 Apr 2003 16:34:09 +0100
On 2003.04.25 16:12 Owen Taylor wrote:
> On Thu, 2003-04-24 at 12:51, J. Ali Harlow wrote:
[snip re. question of gdk_drawable_get_image() returning an image
which shares pixel data with the initial pixmap under win32]
> Hmm, my initial thought was "no, you can't do that optimization,
> the caller is allowed to modify the image", but looking at the docs
> for gdk_drawable_get_image(), they say:
>
> * If the X server or other windowing system backend is on the local
> * machine, this function may use shared memory to avoid copying
> * the image data.
>
> Which strongly implies that you might or might not get the
> actual data, and thus shouldn't count on being able to modify
> it. So, I think the optimization is fine, though it isn't
> 100% clear to me that it is a good idea, since it is going
> to kill the ability of the Win32 port to do *any* hardware
> optimization of drawing.
>
> That being said, I simply don't see the code in gdk/win32 to
> do this. It looks like to me that drawable_get_image() does
> always make a copy on win32.
Nor can I today. I _think_ I was getting confused with the fact that
gdk_image_new_bitmap return the image associated with a pixmap rather
than create a new one. In any event, I'm pretty sure I'm mistaken.
> > The current situation is that the reference counting is simply
> > wrong which causes the bug I reported. The obvious way to fix
> > it might be to count all references. However doing so causes
> > the cycle I describe.
> >
> > Ie., user calls gdk_pixmap_new() which creates a new GdkPixmap
> > with a floating reference and then calls
> > _gdk_win32_setup_pixmap_image() to create a new GdkImage with
> > a floating reference. _gdk_win32_setup_pixmap_image() creates
> > a reference to the associated GdkPixmap so does a g_object_ref
> > followed by a gtk_object_sink.
>
> I don't see any reference. Are you maybe looking at something
> other than the current gtk2 branch code?
I'm looking at the 2.2.1 code. The reference from GdkImage to GdkPixmap
is stored in image->windowing_data (gdkimage-win32.c line 118) and the
reference from GdkPixmap to GdkImage is stored in pixmap_impl->image
(gdkpixmap-win32.c line 310).
> > Finally gdk_pixmap_new() does a g_object_ref/gtk_object_sink on
> > the new GdkImage.
> >
> > Net result: GdkPixmap returned to the user has reference count
> > of 1 and the associated GdkImage also has a reference count
> > of 1. This cycle will prevent the two objects ever being finalized
> > unless somebody calls g_object_dispose() and the dispose()
> > function was implemented.
> >
> > At least, that's how I worked it out in my head. It could be
> > a load of rubbish. I'm very much a GObject beginner.
>
> Well, it sounds a little suspicious, since there are no floating
> references at the GDK layer :-)
>
> I must admit of bit of confusion trying to correlate your
> mail with the current code.
Sorry; I did say I was a beginner with GObject. Yes, something
else I discovered this morning was that Gdk doesn't use floating
references which makes the detail of what I explained above wrong
even if the basic idea is still valid.
In fact when I came to fix this using weak references as advised by
Michael (thanks, btw) I found that the GdkPixmap->GdkImage reference
really isn't important. It's only really there to have somewhere
convenient to store the location of the pixel data in the pixmap
and as an easy way of accessing the dimensions of that data. I
have therefore put together a fix which removes this reference
entirely which seems to solve the problem. Sadly it doesn't seem
to have solved my resource leak but I'm still trying to debug
that to see where it might be coming from.
I'll attach my fix.
--
Ali Harlow Email: ali avrc city ac uk
Research programmer Tel: (020) 7040 4348
Applied Vision Research Centre Intl: +44 20 7040 4348
City University Fax: (020) 7040 5515
London Intl: +44 20 7040 5515
--- ../gtk+-2.2.1/gdk/win32/gdkpixmap-win32.h Sun Feb 17 00:25:05 2002
+++ ./gdk/win32/gdkpixmap-win32.h Fri Apr 25 11:19:40 2003
@@ -54,9 +54,7 @@
gint width;
gint height;
- GdkImage *image; /* A pointer to the GdkImage
- * containing the pixels.
- */
+ guchar *bits; /* A pointer to the memory containing the pixels. */
guint is_foreign : 1;
};
--- ../gtk+-2.2.1/gdk/win32/gdkpixmap-win32.c Thu Jan 23 21:57:04 2003
+++ ./gdk/win32/gdkpixmap-win32.c Fri Apr 25 11:24:12 2003
@@ -103,7 +103,6 @@
{
GdkPixmapImplWin32 *impl = GDK_PIXMAP_IMPL_WIN32 (object);
GdkPixmap *wrapper = GDK_PIXMAP (GDK_DRAWABLE_IMPL_WIN32 (impl)->wrapper);
- GdkImage *image = impl->image;
GDK_NOTE (PIXMAP, g_print ("gdk_pixmap_impl_win32_finalize: %p\n",
GDK_PIXMAP_HBITMAP (wrapper)));
@@ -113,9 +112,6 @@
gdk_win32_handle_table_remove (GDK_PIXMAP_HBITMAP (wrapper));
- image->windowing_data = NULL;
- g_object_unref (image);
-
G_OBJECT_CLASS (parent_class)->finalize (object);
}
@@ -307,9 +303,7 @@
}
drawable_impl->handle = hbitmap;
- pixmap_impl->image = _gdk_win32_setup_pixmap_image (pixmap, drawable,
- width, height,
- depth, bits);
+ pixmap_impl->bits = bits;
gdk_win32_handle_table_insert (&GDK_PIXMAP_HBITMAP (pixmap), pixmap);
@@ -359,7 +353,7 @@
{
GdkPixmap *pixmap;
GdkPixmapImplWin32 *pixmap_impl;
- gint i, j, data_bpl, image_bpl;
+ gint i, j, data_bpl, pixmap_bpl;
guchar *bits;
g_return_val_if_fail (data != NULL, NULL);
@@ -378,13 +372,13 @@
return NULL;
pixmap_impl = GDK_PIXMAP_IMPL_WIN32 (GDK_PIXMAP_OBJECT (pixmap)->impl);
- bits = pixmap_impl->image->mem;
+ bits = pixmap_impl->bits;
data_bpl = ((width - 1) / 8 + 1);
- image_bpl = pixmap_impl->image->bpl;
+ pixmap_bpl = ((width - 1)/32 + 1)*4;
for (i = 0; i < height; i++)
for (j = 0; j < data_bpl; j++)
- bits[i*image_bpl + j] = mirror[(guchar) data[i*data_bpl + j]];
+ bits[i*pixmap_bpl + j] = mirror[(guchar) data[i*data_bpl + j]];
GDK_NOTE (PIXMAP, g_print ("gdk_bitmap_create_from_data: %dx%d=%p\n",
width, height, GDK_PIXMAP_HBITMAP (pixmap)));
@@ -481,11 +475,7 @@
draw_impl->colormap = NULL;
pix_impl->width = size.cx;
pix_impl->height = size.cy;
- pix_impl->image =
- _gdk_win32_setup_pixmap_image (pixmap, _gdk_parent_root,
- size.cx, size.cy,
- gdk_visual_get_system ()->depth,
- NULL);
+ pix_impl->bits = NULL;
gdk_win32_handle_table_insert (&GDK_PIXMAP_HBITMAP (pixmap), pixmap);
--- ../gtk+-2.2.1/gdk/win32/gdkcursor-win32.c Tue Nov 12 22:17:37 2002
+++ ./gdk/win32/gdkcursor-win32.c Fri Apr 25 11:48:23 2003
@@ -129,7 +129,8 @@
GdkCursorPrivate *private;
GdkCursor *cursor;
GdkPixmapImplWin32 *source_impl, *mask_impl;
- GdkImage *source_image, *mask_image;
+ guchar *source_bits, *mask_bits;
+ gint source_bpl, mask_bpl;
HCURSOR hcursor;
guchar *p, *q, *xor_mask, *and_mask;
gint width, height, cursor_width, cursor_height;
@@ -158,12 +159,16 @@
residue = (1 << ((8-(width%8))%8)) - 1;
- source_image = source_impl->image;
- mask_image = mask_impl->image;
+ source_bits = source_impl->bits;
+ mask_bits = mask_impl->bits;
- g_return_val_if_fail (source_image->depth == 1 && mask_image->depth == 1,
+ g_return_val_if_fail (GDK_PIXMAP_OBJECT (source)->depth == 1
+ && GDK_PIXMAP_OBJECT (mask)->depth == 1,
NULL);
+ source_bpl = ((width - 1)/32 + 1)*4;
+ mask_bpl = ((mask_impl->width - 1)/32 + 1)*4;
+
#ifdef G_ENABLE_DEBUG
if (_gdk_debug_flags & GDK_DEBUG_CURSOR)
{
@@ -174,7 +179,7 @@
if (iy == 16)
break;
- p = (guchar *) source_image->mem + iy*source_image->bpl;
+ p = (guchar *) source_bits + iy*source_bpl;
for (ix = 0; ix < width; ix++)
{
if (ix == 79)
@@ -191,7 +196,7 @@
if (iy == 16)
break;
- p = (guchar *) mask_image->mem + iy*source_image->bpl;
+ p = (guchar *) mask_bits + iy*mask_bpl;
for (ix = 0; ix < width; ix++)
{
if (ix == 79)
@@ -222,8 +227,8 @@
*/
for (iy = 0; iy < height; iy++)
{
- p = (guchar *) source_image->mem + iy*source_image->bpl;
- q = (guchar *) mask_image->mem + iy*mask_image->bpl;
+ p = (guchar *) source_bits + iy*source_bpl;
+ q = (guchar *) mask_bits + iy*mask_bpl;
for (ix = 0; ix < ((width-1)/8+1); ix++)
if (bg_is_white)
@@ -237,7 +242,7 @@
for (iy = 0; iy < height; iy++)
{
- p = (guchar *) source_image->mem + iy*source_image->bpl;
+ p = (guchar *) source_bits + iy*source_bpl;
q = xor_mask + iy*cursor_width/8;
for (ix = 0; ix < ((width-1)/8+1); ix++)
@@ -255,7 +260,7 @@
for (iy = 0; iy < height; iy++)
{
- p = (guchar *) mask_image->mem + iy*mask_image->bpl;
+ p = (guchar *) mask_bits + iy*mask_bpl;
q = and_mask + iy*cursor_width/8;
for (ix = 0; ix < ((width-1)/8+1); ix++)
--- ../gtk+-2.2.1/gdk/win32/gdkdrawable-win32.c Mon Dec 9 00:43:42 2002
+++ ./gdk/win32/gdkdrawable-win32.c Fri Apr 25 11:53:25 2003
@@ -1454,13 +1454,13 @@
WIN32_GDI_FAILED ("SelectObject");
else
{
- if (src->image->depth <= 8)
+ if (GDK_PIXMAP_OBJECT(src)->depth <= 8)
{
/* Blitting from a 1, 4 or 8-bit pixmap */
if ((oldtable_size = GetDIBColorTable (srcdc, 0, 256, oldtable)) == 0)
WIN32_GDI_FAILED ("GetDIBColorTable");
- else if (src->image->depth == 1)
+ else if (GDK_PIXMAP_OBJECT(src)->depth == 1)
{
/* Blitting from an 1-bit pixmap */
@@ -1478,7 +1478,7 @@
}
if (GDK_IS_PIXMAP_IMPL_WIN32 (dest) &&
- ((GdkPixmapImplWin32 *) dest)->image->depth <= 8)
+ GDK_PIXMAP_OBJECT(dest)->depth <= 8)
{
/* Destination is also pixmap, get fg and bg from
* its palette. Either use the foreground and
--- ../gtk+-2.2.1/gdk/win32/gdkgc-win32.c Tue Dec 17 01:39:11 2002
+++ ./gdk/win32/gdkgc-win32.c Fri Apr 25 11:59:18 2003
@@ -1077,12 +1077,17 @@
HRGN h;
DWORD maxRects;
RGNDATA *pData;
- GdkImage *image;
+ guchar *bits;
+ gint width, height, bpl;
guchar *p;
gint x, y;
- image = GDK_PIXMAP_IMPL_WIN32 (GDK_PIXMAP_OBJECT (pixmap)->impl)->image;
- g_assert (image->depth == 1);
+ g_assert (GDK_PIXMAP_OBJECT(pixmap)->depth == 1);
+
+ bits = GDK_PIXMAP_IMPL_WIN32 (GDK_PIXMAP_OBJECT (pixmap)->impl)->bits;
+ width = GDK_PIXMAP_IMPL_WIN32 (GDK_PIXMAP_OBJECT (pixmap)->impl)->width;
+ height = GDK_PIXMAP_IMPL_WIN32 (GDK_PIXMAP_OBJECT (pixmap)->impl)->height;
+ bpl = ((width - 1)/32 + 1)*4;
/* For better performances, we will use the ExtCreateRegion()
* function to create the region. This function take a RGNDATA
@@ -1098,15 +1103,15 @@
pData->rdh.nCount = pData->rdh.nRgnSize = 0;
SetRect (&pData->rdh.rcBound, MAXLONG, MAXLONG, 0, 0);
- for (y = 0; y < image->height; y++)
+ for (y = 0; y < height; y++)
{
/* Scan each bitmap row from left to right*/
- p = (guchar *) image->mem + y * image->bpl;
- for (x = 0; x < image->width; x++)
+ p = (guchar *) bits + y * bpl;
+ for (x = 0; x < width; x++)
{
/* Search for a continuous range of "non transparent pixels"*/
gint x0 = x;
- while (x < image->width)
+ while (x < width)
{
if ((((p[x/8])>>(7-(x%8)))&1) == 0)
/* This pixel is "transparent"*/
--- ../gtk+-2.2.1/gdk/win32/gdkimage-win32.c Wed Dec 11 23:04:38 2002
+++ ./gdk/win32/gdkimage-win32.c Fri Apr 25 12:02:07 2003
@@ -104,20 +104,24 @@
}
}
-GdkImage *
-_gdk_win32_setup_pixmap_image (GdkPixmap *pixmap,
- GdkDrawable *drawable,
- gint width,
- gint height,
- gint depth,
- guchar *bits)
+/*
+ * Create a GdkImage _without_ an associated GdkPixmap. The caller is
+ * responsible for creating a GdkPixmap object and making the association.
+ */
+
+static GdkImage *
+_gdk_win32_new_image (GdkVisual *visual,
+ gint width,
+ gint height,
+ gint depth,
+ guchar *bits)
{
GdkImage *image;
image = g_object_new (gdk_image_get_type (), NULL);
- image->windowing_data = pixmap;
+ image->windowing_data = NULL;
image->type = GDK_IMAGE_SHARED;
- image->visual = gdk_drawable_get_visual (drawable);
+ image->visual = visual;
image->byte_order = GDK_LSB_FIRST;
image->width = width;
image->height = height;
@@ -143,7 +147,7 @@
image->bpp = 4;
break;
default:
- g_warning ("_gdk_win32_setup_pixmap_image: depth=%d", image->depth);
+ g_warning ("_gdk_win32_new_image: depth=%d", image->depth);
g_assert_not_reached ();
}
if (depth == 1)
@@ -166,6 +170,7 @@
{
GdkPixmap *pixmap;
GdkImage *image;
+ guchar *bits;
gint data_bpl = (w-1)/8 + 1;
gint i;
@@ -174,10 +179,14 @@
if (pixmap == NULL)
return NULL;
- image = GDK_PIXMAP_IMPL_WIN32 (GDK_PIXMAP_OBJECT (pixmap)->impl)->image;
-
GDK_NOTE (IMAGE, g_print ("gdk_image_new_bitmap: %dx%d=%p\n",
w, h, GDK_PIXMAP_HBITMAP (pixmap)));
+
+ bits = GDK_PIXMAP_IMPL_WIN32 (GDK_PIXMAP_OBJECT (pixmap)->impl)->bits;
+
+ image = _gdk_win32_new_image (visual, w, h, 1, bits);
+
+ image->windowing_data = pixmap;
if (data_bpl != image->bpl)
{
@@ -205,6 +214,8 @@
gint depth)
{
GdkPixmap *pixmap;
+ GdkImage *image;
+ guchar *bits;
g_return_val_if_fail (!visual || GDK_IS_VISUAL (visual), NULL);
g_return_val_if_fail (visual || depth != -1, NULL);
@@ -221,7 +232,13 @@
GDK_NOTE (IMAGE, g_print ("_gdk_image_new_for_depth: %dx%dx%d=%p\n",
width, height, depth, GDK_PIXMAP_HBITMAP (pixmap)));
- return GDK_PIXMAP_IMPL_WIN32 (GDK_PIXMAP_OBJECT (pixmap)->impl)->image;
+ bits = GDK_PIXMAP_IMPL_WIN32 (GDK_PIXMAP_OBJECT (pixmap)->impl)->bits;
+
+ image = _gdk_win32_new_image (visual, width, height, depth, bits);
+
+ image->windowing_data = pixmap;
+
+ return image;
}
GdkImage*
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
