Hi, regarding Jake's unlock() report I changed the locking behaviour to be more strict about who can unlock a locked surface. While it was possible to completely unlock a surface at any time from anywhere, it is now strongly tied to the object, which caused the lock. While it was possible to do stuff like subsurface = surface.subsurface (...) subsurface.lock () surface.unlock () # surface is unlocked again although subsurface is locked ... sfarray = pygame.surfarray.pixels3d(surface) surface.unlock () # Monsters live here ... you now have to explicitly release the lock using the object that caused it: subsurface = surface.subsurface (...) subsurface.unlock () surface.unlock () # No effect subsurface.unlock () # Now the lock is released. # surface is unlocked again ... sfarray = pygame.surfarray.pixels3d(surface) surface.unlock () # Nothing happens del sfarray # Surface will be unlocked # Rainbowland's coming! ... The surface.get_locked() method was adjusted accordingly and works as supposed now. Additionally the surface got a new get_locks() method, which returns a tuple with the object holding a lock. Though this is not always exact or very helpful, it can give you a quick overview about the reason for a dangling lock (numpy surfarrays for example are not listed, but a surface buffer instead as that one caused the lock). As this is tightly bound to reference counts, weakref pointers and other big sources for errors, I beg anyone to test it extensively with your code. Attached you'll find the patch, which hopefully brings love, joy, happiness and less errors to pygame. Regards Marcus
Index: src/pygame.h =================================================================== --- src/pygame.h (Revision 1297) +++ src/pygame.h (Arbeitskopie) @@ -350,7 +350,8 @@ SDL_Surface* surf; struct SubSurface_Data* subsurface; /*ptr to subsurface data (if a * subsurface)*/ - PyObject* weakreflist; + PyObject *weakreflist; + PyObject *locklist; PyObject *dependency; } PySurfaceObject; #define PySurface_AsSurface(x) (((PySurfaceObject*)x)->surf) @@ -401,7 +402,7 @@ /* SURFLOCK */ /*auto import/init by surface*/ #define PYGAMEAPI_SURFLOCK_FIRSTSLOT \ (PYGAMEAPI_SURFACE_FIRSTSLOT + PYGAMEAPI_SURFACE_NUMSLOTS) -#define PYGAMEAPI_SURFLOCK_NUMSLOTS 5 +#define PYGAMEAPI_SURFLOCK_NUMSLOTS 8 struct SubSurface_Data { PyObject* owner; @@ -409,23 +410,41 @@ int offsetx, offsety; }; +typedef struct +{ + PyObject_HEAD + PyObject *surface; + PyObject *lockobj; + PyObject *weakrefs; +} PyLifetimeLock; + #ifndef PYGAMEAPI_SURFLOCK_INTERNAL +#define PyLifetimeLock_Check(x) \ + ((x)->ob_type == (PyTypeObject*) \ + PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 0]) #define PySurface_Prep(x) \ if(((PySurfaceObject*)x)->subsurface) \ (*(*(void(*)(PyObject*)) \ - PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 0]))(x) + PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 1]))(x) #define PySurface_Unprep(x) \ if(((PySurfaceObject*)x)->subsurface) \ (*(*(void(*)(PyObject*)) \ - PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 1]))(x) + PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 2]))(x) #define PySurface_Lock \ - (*(int(*)(PyObject*))PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 2]) + (*(int(*)(PyObject*))PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 3]) #define PySurface_Unlock \ - (*(int(*)(PyObject*))PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 3]) + (*(int(*)(PyObject*))PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 4]) +#define PySurface_LockBy \ + (*(int(*)(PyObject*,PyObject*)) \ + PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 5]) +#define PySurface_UnlockBy \ + (*(int(*)(PyObject*,PyObject*)) \ + PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 6]) #define PySurface_LockLifetime \ - (*(PyObject*(*)(PyObject*))PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 4]) + (*(PyObject*(*)(PyObject*,PyObject*)) \ + PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 7]) #endif @@ -501,6 +520,18 @@ #endif /* BufferProxy */ +typedef struct +{ + PyObject_HEAD + PyObject *dict; /* dict for subclassing */ + PyObject *weakrefs; /* Weakrefs for subclassing */ + void *buffer; /* Pointer to the buffer of the parent object. */ + Py_ssize_t length; /* Length of the buffer. */ + PyObject *parent; /* Parent object associated with this object. */ + PyObject *lock; /* Lock object for the surface. */ + +} PyBufferProxy; + #define PYGAMEAPI_BUFFERPROXY_FIRSTSLOT \ (PYGAMEAPI_RWOBJECT_FIRSTSLOT + PYGAMEAPI_RWOBJECT_NUMSLOTS) #define PYGAMEAPI_BUFFERPROXY_NUMSLOTS 2 Index: src/pygamedocs.h =================================================================== --- src/pygamedocs.h (Revision 1297) +++ src/pygamedocs.h (Arbeitskopie) @@ -717,6 +717,8 @@ #define DOC_SURFACEGETLOCKED "Surface.get_locked(): return bool\ntest if the Surface is current locked" +#define DOC_SURFACEGETLOCKS "Surface.get_locks(): return tuple\nGets the locks from the Surface" + #define DOC_SURFACEGETAT "Surface.get_at((x, y)): return Color\nget the color value at a single pixel" #define DOC_SURFACESETAT "Surface.set_at((x, y), Color): return None\nset the color value for a single pixel" Index: src/surface.doc =================================================================== --- src/surface.doc (Revision 1297) +++ src/surface.doc (Arbeitskopie) @@ -311,6 +311,14 @@ <END> +get_locks +Gets the locks from the Surface +Surface.get_locks(): return tuple + +Returns the currently existing locks for the Surface. +<END> + + get_at get the color value at a single pixel Surface.get_at((x, y)): return Color Index: src/surface.c =================================================================== --- src/surface.c (Revision 1297) +++ src/surface.c (Arbeitskopie) @@ -49,6 +49,7 @@ static PyObject *surf_unlock (PyObject *self); static PyObject *surf_mustlock (PyObject *self); static PyObject *surf_get_locked (PyObject *self); +static PyObject *surf_get_locks (PyObject *self); static PyObject *surf_get_palette (PyObject *self); static PyObject *surf_get_palette_at (PyObject *self, PyObject *args); static PyObject *surf_set_palette (PyObject *self, PyObject *args); @@ -106,6 +107,8 @@ DOC_SURFACEMUSTLOCK }, { "get_locked", (PyCFunction) surf_get_locked, METH_NOARGS, DOC_SURFACEGETLOCKED }, + { "get_locks", (PyCFunction) surf_get_locks, METH_NOARGS, + DOC_SURFACEGETLOCKS }, { "set_colorkey", surf_set_colorkey, METH_VARARGS, DOC_SURFACESETCOLORKEY }, { "get_colorkey", (PyCFunction) surf_get_colorkey, METH_NOARGS, @@ -239,6 +242,8 @@ self->surf = NULL; self->subsurface = NULL; self->weakreflist = NULL; + self->dependency = NULL; + self->locklist = NULL; } return (PyObject *) self; } @@ -269,6 +274,12 @@ Py_DECREF (self->dependency); self->dependency = NULL; } + + if (self->locklist) + { + Py_DECREF (self->locklist); + self->locklist = NULL; + } } static void @@ -667,8 +678,7 @@ static PyObject* surf_unlock (PyObject *self) { - if (!PySurface_Unlock (self)) - return NULL; + PySurface_Unlock (self); Py_RETURN_NONE; } @@ -677,19 +687,43 @@ { SDL_Surface *surf = PySurface_AsSurface (self); return PyInt_FromLong (SDL_MUSTLOCK (surf) || - ((PySurfaceObject *) self)->subsurface); + ((PySurfaceObject *) self)->subsurface); } static PyObject* surf_get_locked (PyObject *self) { PySurfaceObject *surf = (PySurfaceObject *) self; - if (surf->surf->pixels != NULL) + + if (surf->locklist && PyList_Size (surf->locklist) > 0) Py_RETURN_TRUE; Py_RETURN_FALSE; } static PyObject* +surf_get_locks (PyObject *self) +{ + PySurfaceObject *surf = (PySurfaceObject *) self; + Py_ssize_t len, i = 0; + PyObject *tuple, *tmp; + if (!surf->locklist) + return PyTuple_New (0); + + len = PyList_Size (surf->locklist); + tuple = PyTuple_New (len); + if (!tuple) + return NULL; + + for (i = 0; i < len; i++) + { + tmp = PyWeakref_GetObject (PyList_GetItem (surf->locklist, i)); + Py_INCREF (tmp); + PyTuple_SetItem (tuple, i, tmp); + } + return tuple; +} + +static PyObject* surf_get_palette (PyObject *self) { SDL_Surface *surf = PySurface_AsSurface (self); @@ -1797,23 +1831,26 @@ Py_ssize_t length; length = (Py_ssize_t) surface->pitch * surface->h; - lock = PySurface_LockLifetime (self); + + buffer = PyBufferProxy_New (self, NULL, length, NULL); + if (!buffer) + { + return RAISE (PyExc_SDLError, + "could not acquire a buffer for the surface"); + } + + lock = PySurface_LockLifetime (self, buffer); if (!lock) { + Py_DECREF (buffer); return RAISE (PyExc_SDLError, "could not lock surface"); } + ((PyBufferProxy *) buffer)->buffer = surface->pixels; + ((PyBufferProxy *) buffer)->lock = lock; - buffer = PyBufferProxy_New (self, surface->pixels, length, lock); - if (!buffer) - { - Py_DECREF (lock); - return RAISE (PyExc_SDLError, - "could not acquire a buffer for the surface"); - } return buffer; } - /*this internal blit function is accessable through the C api*/ int PySurface_Blit (PyObject * dstobj, PyObject * srcobj, SDL_Rect * dstrect, Index: src/surflock.c =================================================================== --- src/surflock.c (Revision 1297) +++ src/surflock.c (Arbeitskopie) @@ -1,6 +1,7 @@ /* pygame - Python Game Library Copyright (C) 2000-2001 Pete Shinners + Copyright (C) 2008 Marcus von Appen This library is free software; you can redistribute it and/or modify it under the terms of the GNU Library General Public @@ -28,7 +29,11 @@ static int PySurface_Lock (PyObject* surfobj); static int PySurface_Unlock (PyObject* surfobj); +static int PySurface_LockBy (PyObject* surfobj, PyObject* lockobj); +static int PySurface_UnlockBy (PyObject* surfobj, PyObject* lockobj); +static void _lifelock_dealloc (PyObject* self); + static void PySurface_Prep (PyObject* surfobj) { @@ -37,7 +42,7 @@ { SDL_Surface* surf = PySurface_AsSurface (surfobj); SDL_Surface* owner = PySurface_AsSurface (data->owner); - PySurface_Lock (data->owner); + PySurface_LockBy (data->owner, surfobj); surf->pixels = ((char*) owner->pixels) + data->pixeloffset; } } @@ -47,13 +52,43 @@ { struct SubSurface_Data* data = ((PySurfaceObject*) surfobj)->subsurface; if (data) - PySurface_Unlock (data->owner); + PySurface_UnlockBy (data->owner, surfobj); } static int PySurface_Lock (PyObject* surfobj) { + return PySurface_LockBy (surfobj, surfobj); +} + +static int +PySurface_Unlock (PyObject* surfobj) +{ + return PySurface_UnlockBy (surfobj, surfobj); +} + +static int +PySurface_LockBy (PyObject* surfobj, PyObject* lockobj) +{ + PyObject *ref; PySurfaceObject* surf = (PySurfaceObject*) surfobj; + + if (!surf->locklist) + { + surf->locklist = PyList_New (0); + if (!surf->locklist) + return 0; + } + ref = PyWeakref_NewRef (lockobj, NULL); + if (!ref) + return 0; + if (ref == Py_None) + { + Py_DECREF (ref); + return 0; + } + PyList_Append (surf->locklist, ref); + if (surf->subsurface) PySurface_Prep (surfobj); if (SDL_LockSurface (surf->surf) == -1) @@ -65,59 +100,143 @@ } static int -PySurface_Unlock (PyObject* surfobj) +PySurface_UnlockBy (PyObject* surfobj, PyObject* lockobj) { PySurfaceObject* surf = (PySurfaceObject*) surfobj; - if (surf->surf != NULL) - SDL_UnlockSurface (surf->surf); - if (surf->subsurface) - PySurface_Unprep (surfobj); - return 1; + int found = 0; + int noerror = 1; + + if (surf->locklist) + { + PyObject *item, *ref; + Py_ssize_t len = PyList_Size (surf->locklist); + while (--len >= 0 && !found) + { + item = PyList_GetItem (surf->locklist, len); + ref = PyWeakref_GetObject (item); + if (ref == lockobj) + { + if (PySequence_DelItem (surf->locklist, len) == -1) + return 0; + else + found = 1; + } + } + + /* Clear dead references */ + len = PyList_Size (surf->locklist); + while (--len >= 0) + { + item = PyList_GetItem (surf->locklist, len); + ref = PyWeakref_GetObject (item); + if (ref == Py_None) + { + if (PySequence_DelItem (surf->locklist, len) == -1) + noerror = 0; + else + found++; + } + } + } + + if (!found) + return noerror; + + /* Release all found locks. */ + while (found > 0) + { + if (surf->surf != NULL) + SDL_UnlockSurface (surf->surf); + if (surf->subsurface) + PySurface_Unprep (surfobj); + found--; + } + + return noerror; } -/* lifetimelock object internals */ -typedef struct + +static PyTypeObject PyLifetimeLock_Type = { - PyObject_HEAD - PyObject* surface; -} PyLifetimeLockObject; + PyObject_HEAD_INIT(NULL) + 0, /*size*/ + "SurfLifeLock", /*name*/ + sizeof(PyLifetimeLock), /*basic size*/ + 0, /* tp_itemsize */ + _lifelock_dealloc, /* tp_dealloc*/ + 0, /* tp_print */ + 0, /* tp_getattr */ + 0, /* tp_setattr */ + 0, /* tp_compare */ + 0, /* tp_repr */ + 0, /* tp_as_number */ + 0, /* tp_as_sequence */ + 0, /* tp_as_mapping */ + 0, /* tp_hash */ + 0, /* tp_call */ + 0, /* tp_str */ + 0, /* tp_getattro */ + 0, /* tp_setattro */ + 0, /* tp_as_buffer */ + Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_WEAKREFS, + 0, /* tp_doc */ + 0, /* tp_traverse */ + 0, /* tp_clear */ + 0, /* tp_richcompare */ + offsetof (PyLifetimeLock, weakrefs), /* tp_weaklistoffset */ + 0, /* tp_iter */ + 0, /* tp_iternext */ + 0, /* tp_methods */ + 0, /* tp_members */ + 0, /* tp_getset */ + 0, /* tp_base */ + 0, /* tp_dict */ + 0, /* tp_descr_get */ + 0, /* tp_descr_set */ + 0, /* tp_dictoffset */ + 0, /* tp_init */ + 0, /* tp_alloc */ + 0, /* tp_new */ + 0, /* tp_free */ + 0, /* tp_is_gc */ + 0, /* tp_bases */ + 0, /* tp_mro */ + 0, /* tp_cache */ + 0, /* tp_subclasses */ + 0, /* tp_weaklist */ + 0 /* tp_del */ +}; +/* lifetimelock object internals */ static void -lifelock_dealloc (PyObject* self) +_lifelock_dealloc (PyObject* self) { - PyLifetimeLockObject* lifelock = (PyLifetimeLockObject*) self; - - PySurface_Unlock (lifelock->surface); + PyLifetimeLock* lifelock = (PyLifetimeLock*) self; + + if (lifelock->weakrefs) + PyObject_ClearWeakRefs (self); + + PySurface_UnlockBy (lifelock->surface, lifelock->lockobj); Py_DECREF (lifelock->surface); - PyObject_DEL (self); } -static PyTypeObject PyLifetimeLock_Type = -{ - PyObject_HEAD_INIT(NULL) - 0, /*size*/ - "SurfLifeLock", /*name*/ - sizeof(PyLifetimeLockObject),/*basic size*/ - 0, /*itemsize*/ - lifelock_dealloc, /*dealloc*/ -}; - static PyObject* -PySurface_LockLifetime (PyObject* surf) +PySurface_LockLifetime (PyObject* surfobj, PyObject *lockobj) { - PyLifetimeLockObject* life; - if (!surf) + PyLifetimeLock* life; + if (!surfobj) return RAISE (PyExc_SDLError, SDL_GetError ()); - if (!PySurface_Lock (surf)) - return NULL; - - life = PyObject_NEW (PyLifetimeLockObject, &PyLifetimeLock_Type); + life = PyObject_NEW (PyLifetimeLock, &PyLifetimeLock_Type); if (life) { - life->surface = surf; - Py_INCREF (surf); + life->surface = surfobj; + life->lockobj = lockobj; + life->weakrefs = NULL; + Py_INCREF (surfobj); + if (!PySurface_LockBy (surfobj, lockobj)) + return NULL; } return (PyObject*) life; } @@ -141,11 +260,14 @@ dict = PyModule_GetDict (module); /* export the c api */ - c_api[0] = PySurface_Prep; - c_api[1] = PySurface_Unprep; - c_api[2] = PySurface_Lock; - c_api[3] = PySurface_Unlock; - c_api[4] = PySurface_LockLifetime; + c_api[0] = &PyLifetimeLock_Type; + c_api[1] = PySurface_Prep; + c_api[2] = PySurface_Unprep; + c_api[3] = PySurface_Lock; + c_api[4] = PySurface_Unlock; + c_api[5] = PySurface_LockBy; + c_api[6] = PySurface_UnlockBy; + c_api[7] = PySurface_LockLifetime; apiobj = PyCObject_FromVoidPtr (c_api, NULL); PyDict_SetItemString (dict, PYGAMEAPI_LOCAL_ENTRY, apiobj); Py_DECREF (apiobj); Index: src/_numericsurfarray.c =================================================================== --- src/_numericsurfarray.c (Revision 1297) +++ src/_numericsurfarray.c (Arbeitskopie) @@ -73,7 +73,7 @@ array = PyArray_FromDimsAndData (3, dim, PyArray_UBYTE, ""); if (array) { - lifelock = PySurface_LockLifetime (surfobj); + lifelock = PySurface_LockLifetime (surfobj, array); if (!lifelock) { Py_DECREF (array); @@ -122,7 +122,7 @@ array = PyArray_FromDimsAndData (2, dim, type, ""); if (array) { - lifelock = PySurface_LockLifetime (surfobj); + lifelock = PySurface_LockLifetime (surfobj, array); if (!lifelock) { Py_DECREF (array); @@ -170,7 +170,7 @@ array = PyArray_FromDimsAndData (2, dim, PyArray_UBYTE, ""); if(array) { - lifelock = PySurface_LockLifetime (surfobj); + lifelock = PySurface_LockLifetime (surfobj, array); if (!lifelock) { Py_DECREF (array); @@ -212,7 +212,7 @@ stridex = ((PyArrayObject*) array)->strides[0]; stridey = ((PyArrayObject*) array)->strides[1]; - if (!PySurface_Lock (surfobj)) + if (!PySurface_LockBy (surfobj, array)) { Py_DECREF (array); return NULL; @@ -279,7 +279,7 @@ break; } - if (!PySurface_Unlock (surfobj)) + if (!PySurface_UnlockBy (surfobj, array)) { Py_DECREF (array); return NULL; @@ -328,7 +328,7 @@ stridex = ((PyArrayObject*) array)->strides[0]; stridey = ((PyArrayObject*) array)->strides[1]; - if (!PySurface_Lock (surfobj)) + if (!PySurface_LockBy (surfobj, array)) { Py_DECREF (array); return NULL; @@ -340,7 +340,7 @@ if (!format->palette) { Py_DECREF (array); - if (!PySurface_Unlock (surfobj)) + if (!PySurface_UnlockBy (surfobj, array)) return NULL; return RAISE (PyExc_RuntimeError, "8bit surface has no palette"); } @@ -420,7 +420,7 @@ break; } - if (!PySurface_Unlock (surfobj)) + if (!PySurface_UnlockBy (surfobj, array)) { Py_DECREF (array); return NULL; @@ -467,7 +467,7 @@ stridex = ((PyArrayObject*) array)->strides[0]; stridey = ((PyArrayObject*) array)->strides[1]; - if (!PySurface_Lock (surfobj)) + if (!PySurface_LockBy (surfobj, array)) { Py_DECREF (array); return NULL; @@ -525,7 +525,7 @@ break; } - if (!PySurface_Unlock (surfobj)) + if (!PySurface_UnlockBy (surfobj, array)) { Py_DECREF (array); return NULL; @@ -569,7 +569,7 @@ stridex = ((PyArrayObject*) array)->strides[0]; stridey = ((PyArrayObject*) array)->strides[1]; - if (!PySurface_Lock (surfobj)) + if (!PySurface_LockBy (surfobj, array)) { Py_DECREF (array); return NULL; @@ -640,7 +640,7 @@ break; } - if (!PySurface_Unlock (surfobj)) + if (!PySurface_UnlockBy (surfobj, array)) { Py_DECREF (array); return NULL; @@ -870,7 +870,7 @@ if (sizex != surf->w || sizey != surf->h) return RAISE (PyExc_ValueError, "array must match surface dimensions"); - if (!PySurface_Lock (surfobj)) + if (!PySurface_LockBy (surfobj, (PyObject *) array)) return NULL; switch (surf->format->BytesPerPixel) @@ -893,7 +893,7 @@ COPYMACRO_2D(Uint8, Uint64); break; default: - if (!PySurface_Unlock (surfobj)) + if (!PySurface_UnlockBy (surfobj, (PyObject *) array)) return NULL; return RAISE (PyExc_ValueError, "unsupported datatype for array\n"); @@ -918,7 +918,7 @@ COPYMACRO_2D(Uint16, Uint64); break; default: - if (!PySurface_Unlock (surfobj)) + if (!PySurface_UnlockBy (surfobj, (PyObject *) array)) return NULL; return RAISE (PyExc_ValueError, "unsupported datatype for array\n"); @@ -941,7 +941,7 @@ COPYMACRO_3D(Uint16, Uint64); break; default: - if (!PySurface_Unlock (surfobj)) + if (!PySurface_UnlockBy (surfobj, (PyObject *) array)) return NULL; return RAISE (PyExc_ValueError, "unsupported datatype for array\n"); @@ -966,7 +966,7 @@ COPYMACRO_2D_24(Uint64); break; default: - if (!PySurface_Unlock (surfobj)) + if (!PySurface_UnlockBy (surfobj, (PyObject *) array)) return NULL; return RAISE (PyExc_ValueError, "unsupported datatype for array\n"); @@ -989,7 +989,7 @@ COPYMACRO_3D_24(Uint64); break; default: - if (!PySurface_Unlock (surfobj)) + if (!PySurface_UnlockBy (surfobj, (PyObject *) array)) return NULL; return RAISE (PyExc_ValueError, "unsupported datatype for array\n"); @@ -1014,7 +1014,7 @@ COPYMACRO_2D(Uint32, Uint64); break; default: - if (!PySurface_Unlock (surfobj)) + if (!PySurface_UnlockBy (surfobj, (PyObject *) array)) return NULL; return RAISE (PyExc_ValueError, "unsupported datatype for array\n"); @@ -1037,7 +1037,7 @@ COPYMACRO_3D(Uint32, Uint64); break; default: - if (!PySurface_Unlock (surfobj)) + if (!PySurface_UnlockBy (surfobj, (PyObject *) array)) return NULL; return RAISE (PyExc_ValueError, "unsupported datatype for array\n"); @@ -1045,12 +1045,12 @@ } break; default: - if (!PySurface_Unlock (surfobj)) + if (!PySurface_UnlockBy (surfobj, (PyObject *) array)) return NULL; return RAISE (PyExc_RuntimeError, "unsupported bit depth for image"); } - if (!PySurface_Unlock (surfobj)) + if (!PySurface_UnlockBy (surfobj, (PyObject *) array)) return NULL; Py_RETURN_NONE; } Index: src/bufferproxy.c =================================================================== --- src/bufferproxy.c (Revision 1297) +++ src/bufferproxy.c (Arbeitskopie) @@ -23,18 +23,6 @@ #include "pygame.h" #include "pygamedocs.h" -typedef struct -{ - PyObject_HEAD - PyObject *dict; /* dict for subclassing */ - PyObject *weakrefs; /* Weakrefs for subclassing */ - void *buffer; /* Pointer to the buffer of the parent object. */ - Py_ssize_t length; /* Length of the buffer. */ - PyObject *parent; /* Parent object associated with this object. */ - PyObject *lock; /* Lock object for the surface. */ - -} PyBufferProxy; - static PyObject* _bufferproxy_new (PyTypeObject *type, PyObject *args, PyObject *kwds); static void _bufferproxy_dealloc (PyBufferProxy *self); @@ -178,10 +166,8 @@ { if (self->weakrefs) PyObject_ClearWeakRefs ((PyObject *) self); - if (self->lock) - { - Py_DECREF (self->lock); - } + + Py_XDECREF (self->lock); Py_XDECREF (self->dict); self->ob_type->tp_free ((PyObject *) self); } Index: src/pixelarray.c =================================================================== --- src/pixelarray.c (Revision 1297) +++ src/pixelarray.c (Arbeitskopie) @@ -123,7 +123,7 @@ { { "__dict__", (getter) _pxarray_get_dict, NULL, NULL, NULL }, { "surface", (getter) _pxarray_get_surface, NULL, DOC_PIXELARRAYSURFACE, - NULL }, + NULL }, { NULL, NULL, NULL, NULL, NULL } }; @@ -226,7 +226,7 @@ /* Initial PixelArray */ if (surface) { - self->lock = PySurface_LockLifetime (surface); + self->lock = PySurface_LockLifetime (surface, (PyObject *) self); if (!self->lock) { Py_DECREF (surface); @@ -239,7 +239,7 @@ { self->parent = parent; Py_INCREF (parent); - self->lock = ((PyPixelArray*) parent)->lock; + self->lock = ((PyPixelArray *)parent)->lock; Py_INCREF (self->lock); } Index: docs/ref/surface.html =================================================================== --- docs/ref/surface.html (Revision 1297) +++ docs/ref/surface.html (Arbeitskopie) @@ -63,6 +63,7 @@ <tr><td><a href="surface.html#Surface.unlock">Surface.unlock</a> - <font size=-1>unlock the Surface memory from pixel access</font></td><td>unlock the Surface memory from pixel access</td></tr> <tr><td><a href="surface.html#Surface.mustlock">Surface.mustlock</a> - <font size=-1>test if the Surface requires locking</font></td><td>test if the Surface requires locking</td></tr> <tr><td><a href="surface.html#Surface.get_locked">Surface.get_locked</a> - <font size=-1>test if the Surface is current locked</font></td><td>test if the Surface is current locked</td></tr> + <tr><td><a href="surface.html#Surface.get_locks">Surface.get_locks</a> - <font size=-1>Gets the locks from the Surface</font></td><td>Gets the locks from the Surface</td></tr> <tr><td><a href="surface.html#Surface.get_at">Surface.get_at</a> - <font size=-1>get the color value at a single pixel</font></td><td>get the color value at a single pixel</td></tr> <tr><td><a href="surface.html#Surface.set_at">Surface.set_at</a> - <font size=-1>set the color value for a single pixel</font></td><td>set the color value for a single pixel</td></tr> <tr><td><a href="surface.html#Surface.get_palette">Surface.get_palette</a> - <font size=-1>get the color index palette for an 8bit Surface</font></td><td>get the color index palette for an 8bit Surface</td></tr> @@ -256,6 +257,15 @@ <br></ul> +<a name="Surface.get_locks"> +<big><b>Surface.get_locks</big></b><br><ul> + <i>Gets the locks from the Surface</i><br> + <tt>Surface.get_locks(): return tuple</tt><br> +<p>Returns the currently existing locks for the Surface. </p> +<!--COMMENTS:Surface.get_locks--> <br> +<br></ul> + + <a name="Surface.get_at"> <big><b>Surface.get_at</big></b><br><ul> <i>get the color value at a single pixel</i><br> Index: docs/ref/index.html =================================================================== --- docs/ref/index.html (Revision 1297) +++ docs/ref/index.html (Arbeitskopie) @@ -473,6 +473,7 @@ <li><a href="surface.html#Surface.get_flags">Surface.get_flags</a> - <font size=-1>get the additional flags used for the Surface</font></li> <li><a href="surface.html#Surface.get_height">Surface.get_height</a> - <font size=-1>get the height of the Surface</font></li> <li><a href="surface.html#Surface.get_locked">Surface.get_locked</a> - <font size=-1>test if the Surface is current locked</font></li> +<li><a href="surface.html#Surface.get_locks">Surface.get_locks</a> - <font size=-1>Gets the locks from the Surface</font></li> <li><a href="surface.html#Surface.get_losses">Surface.get_losses</a> - <font size=-1>the significant bits used to convert between a color and a mapped integer</font></li> <li><a href="surface.html#Surface.get_masks">Surface.get_masks</a> - <font size=-1>the bitmasks needed to convert between a color and a mapped integer</font></li> <li><a href="surface.html#Surface.get_offset">Surface.get_offset</a> - <font size=-1>find the position of a child subsurface inside a parent</font></li> Index: test/blit_test.py =================================================================== --- test/blit_test.py (Revision 1297) +++ test/blit_test.py (Arbeitskopie) @@ -6,7 +6,6 @@ def test_SRCALPHA( self ): """ SRCALPHA tests. """ - #blend(s, 0, d) = d s = pygame.Surface((1,1), SRCALPHA, 32) s.fill((255, 255,255, 0)) @@ -17,8 +16,6 @@ s.blit(d, (0,0)) self.assertEqual(s.get_at((0,0)), d.get_at((0,0)) ) - - #blend(s, 255, d) = s s = pygame.Surface((1,1), SRCALPHA, 32) s.fill((123, 0, 0, 255)) @@ -29,13 +26,11 @@ s.blit(d, (0,0)) self.assertEqual(s.get_at((0,0)), s1.get_at((0,0)) ) - #TODO: these should be true too. #blend(0, sA, 0) = 0 #blend(255, sA, 255) = 255 #blend(s, sA, d) <= 255 - def test_BLEND( self ): """ BLEND_ tests. """ Index: test/surflock_test.py =================================================================== --- test/surflock_test.py (Revision 1297) +++ test/surflock_test.py (Arbeitskopie) @@ -1,26 +1,26 @@ -import unittest +import unittest, sys import pygame class SurfaceLockTest (unittest.TestCase): -## def test_lock (self): -## sf = pygame.Surface ((5, 5)) + def test_lock (self): + sf = pygame.Surface ((5, 5)) -## sf.lock () -## self.assertEquals (sf.get_locked (), True) -## self.assertEquals (sf.get_locks (), (sf,)) + sf.lock () + self.assertEquals (sf.get_locked (), True) + self.assertEquals (sf.get_locks (), (sf,)) -## sf.lock () -## self.assertEquals (sf.get_locked (), True) -## self.assertEquals (sf.get_locks (), (sf,sf)) + sf.lock () + self.assertEquals (sf.get_locked (), True) + self.assertEquals (sf.get_locks (), (sf, sf)) -## sf.unlock () -## self.assertEquals (sf.get_locked (), True) -## self.assertEquals (sf.get_locks (), (sf,)) + sf.unlock () + self.assertEquals (sf.get_locked (), True) + self.assertEquals (sf.get_locks (), (sf,)) -## sf.unlock () -## self.assertEquals (sf.get_locked (), False) -## self.assertEquals (sf.get_locks (), ()) + sf.unlock () + self.assertEquals (sf.get_locked (), False) + self.assertEquals (sf.get_locks (), ()) def test_subsurface_lock (self): sf = pygame.Surface ((5, 5)) @@ -57,51 +57,102 @@ # Run a second unlock on the surface. This should ideally have # no effect as the subsurface is the locking reason! - # It has though and thus is wrong. sf.unlock () self.assertRaises (pygame.error, sf2.blit, sf, (0, 0)) - - # Now the complete surface is unlocked and we can do any - # nastiness on it, while the subsurface still keeps a (now - # unnecessary) lock around. The same applies to other reference - # types such as buffers. self.assertRaises (pygame.error, sf2.blit, subsf, (0, 0)) + subsf.unlock () + + sf.lock () + self.assertEquals (sf.get_locked (), True) + self.assertEquals (sf.get_locks (), (sf,)) + self.assertEquals (subsf.get_locked (), False) + self.assertEquals (subsf.get_locks (), ()) -## sf.lock () -## self.assertEquals (sf.get_locked (), True) -## self.assertEquals (sf.get_locks (), (sf,)) -## self.assertEquals (subsf.get_locked (), False) -## self.assertEquals (subsf.get_locks (), ()) + subsf.lock () + self.assertEquals (sf.get_locked (), True) + self.assertEquals (sf.get_locks (), (sf, subsf)) + self.assertEquals (subsf.get_locked (), True) + self.assertEquals (subsf.get_locks (), (subsf,)) -## subsf.lock () -## self.assertEquals (sf.get_locked (), True) -## self.assertEquals (sf.get_locks (), (sf, subsf)) -## self.assertEquals (subsf.get_locked (), True) -## self.assertEquals (subsf.get_locks (), (subsf,)) + sf.unlock () + self.assertEquals (sf.get_locked (), True) + self.assertEquals (sf.get_locks (), (subsf,)) + self.assertEquals (subsf.get_locked (), True) + self.assertEquals (subsf.get_locks (), (subsf,)) -## sf.unlock () -## self.assertEquals (sf.get_locked (), True) -## self.assertEquals (sf.get_locks (), (subsf,)) -## self.assertEquals (subsf.get_locked (), True) -## self.assertEquals (subsf.get_locks (), (subsf,)) + subsf.unlock () + self.assertEquals (sf.get_locked (), False) + self.assertEquals (sf.get_locks (), ()) + self.assertEquals (subsf.get_locked (), False) + self.assertEquals (subsf.get_locks (), ()) -## subsf.unlock () -## self.assertEquals (sf.get_locked (), False) -## self.assertEquals (sf.get_locks (), ()) -## self.assertEquals (subsf.get_locked (), False) -## self.assertEquals (subsf.get_locks (), ()) + subsf.lock () + self.assertEquals (sf.get_locked (), True) + self.assertEquals (sf.get_locks (), (subsf,)) + self.assertEquals (subsf.get_locked (), True) + self.assertEquals (subsf.get_locks (), (subsf,)) -## subsf.lock () -## self.assertEquals (sf.get_locked (), True) -## self.assertEquals (sf.get_locks (), (subsf,)) -## self.assertEquals (subsf.get_locked (), True) -## self.assertEquals (subsf.get_locks (), (subsf,)) + subsf.lock () + self.assertEquals (sf.get_locked (), True) + self.assertEquals (sf.get_locks (), (subsf, subsf)) + self.assertEquals (subsf.get_locked (), True) + self.assertEquals (subsf.get_locks (), (subsf, subsf)) -## subsf.lock () -## self.assertEquals (sf.get_locked (), True) -## self.assertEquals (sf.get_locks (), (subsf, subsf)) -## self.assertEquals (subsf.get_locked (), True) -## self.assertEquals (subsf.get_locks (), (subsf, subsf)) + def test_pxarray_ref (self): + sf = pygame.Surface ((5, 5)) + ar = pygame.PixelArray (sf) + ar2 = pygame.PixelArray (sf) + self.assertEquals (sf.get_locked (), True) + self.assertEquals (sf.get_locks (), (ar, ar2)) + + del ar + self.assertEquals (sf.get_locked (), True) + self.assertEquals (sf.get_locks (), (ar2,)) + + ar = ar2[:] + self.assertEquals (sf.get_locked (), True) + self.assertEquals (sf.get_locks (), (ar2,)) + + del ar + self.assertEquals (sf.get_locked (), True) + self.assertEquals (len (sf.get_locks ()), 1) + + def test_buffer (self): + sf = pygame.Surface ((5, 5)) + buf = sf.get_buffer () + + self.assertEquals (sf.get_locked (), True) + self.assertEquals (sf.get_locks (), (buf,)) + + sf.unlock () + self.assertEquals (sf.get_locked (), True) + self.assertEquals (sf.get_locks (), (buf,)) + + del buf + self.assertEquals (sf.get_locked (), False) + self.assertEquals (sf.get_locks (), ()) + + def test_surfarray_ref (self): + sf = pygame.Surface ((5, 5)) + for atype in pygame.surfarray.get_arraytypes (): + pygame.surfarray.use_arraytype (atype) + + ar = pygame.surfarray.pixels2d (sf) + self.assertEquals (sf.get_locked (), True) + + # Numpy uses the Surface's buffer. + if atype == "numeric": + self.assertEquals (sf.get_locks (), (ar,)) + + sf.unlock () + self.assertEquals (sf.get_locked (), True) + + del ar + self.assertEquals (sf.get_locked (), False) + self.assertEquals (sf.get_locks (), ()) + + #print "test_surfarray_ref - end" + if __name__ == '__main__': unittest.main()
Attachment:
pgpghdCb99uAQ.pgp
Description: PGP signature