Implement ImageSurface.create_for_data(), using a modified version of a patch

from Paul Colomiets, bug #44935. pycairo will not be binary compatible with
earlier versions of pycairo.

5 files changed, 60 insertions, 97 deletions

diff --git a/doc/reference/surfaces.rst b/doc/reference/surfaces.rst
index bc57a6e..0ba51c9 100644
--- a/doc/reference/surfaces.rst
+++ b/doc/reference/surfaces.rst
@@ -257,11 +257,7 @@ those defined in :ref:`FORMAT attributes <constants_FORMAT>`.
 
    .. classmethod:: create_for_data(data, format, width, height[, stride])
 
-      Not yet available in Python 3
-
-   .. comment block - the old docs
-
-      :param data: a writable Python buffer object
+      :param data: an object which implements the read-write buffer interface
       :param format: the :ref:`FORMAT <constants_FORMAT>` of pixels in the
         buffer
       :param width: the width of the image to be stored in the buffer
diff --git a/src/py3cairo.h b/src/py3cairo.h
index 324ac37..853c1d8 100644
--- a/src/py3cairo.h
+++ b/src/py3cairo.h
@@ -75,6 +75,7 @@ typedef struct {
   PyObject_HEAD
   cairo_surface_t *surface;
   PyObject *base; /* base object used to create surface, or NULL */
+  Py_buffer buffer;
 } PycairoSurface;
 
 #define PycairoImageSurface         PycairoSurface
diff --git a/src/surface.c b/src/surface.c
index 7d00141..0578639 100644
--- a/src/surface.c
+++ b/src/surface.c
@@ -113,6 +113,8 @@ PycairoSurface_FromSurface (cairo_surface_t *surface, PyObject *base) {
     ((PycairoSurface *)o)->surface = surface;
     Py_XINCREF(base);
     ((PycairoSurface *)o)->base = base;
+
+    ((PycairoSurface *)o)->buffer.buf = NULL;
   }
   return o;
 }
@@ -124,8 +126,6 @@ PycairoSurface_FromSurface (cairo_surface_t *surface, PyObject *base) {
 static cairo_status_t
 _write_func (void *closure, const unsigned char *data, unsigned int length) {
   PyGILState_STATE gstate = PyGILState_Ensure();
-  //  PyObject *res = PyObject_CallMethod ((PyObject *)closure, "write", "(s#)",
-  //				       data, (Py_ssize_t)length);
   PyObject *res = PyObject_CallMethod ((PyObject *)closure, "write", "(y#)",
 				       data, (Py_ssize_t)length);
   if (res == NULL) {
@@ -147,8 +147,8 @@ surface_dealloc (PycairoSurface *o) {
     o->surface = NULL;
   }
   Py_CLEAR(o->base);
+  PyBuffer_Release(&o->buffer);
 
-  //o->ob_type->tp_free((PyObject *)o);
   Py_TYPE(o)->tp_free(o);
 }
 
@@ -416,23 +416,12 @@ image_surface_new (PyTypeObject *type, PyObject *args, PyObject *kwds) {
 
 static PyObject *
 image_surface_create_for_data (PyTypeObject *type, PyObject *args) {
-  cairo_surface_t *surface;
   cairo_format_t format;
-  unsigned char *buffer;
-  int width, height, stride = -1, res;
-  Py_ssize_t buffer_len;
-  PyObject *obj;
-
-  // buffer function disabled
-  PyErr_SetString(PyExc_NotImplementedError, "Surface.create_for_data: Not Implemented yet.");
-  return NULL;
+  int width, height, stride = -1;
+  Py_buffer buffer;
 
-  if (!PyArg_ParseTuple(args, "Oiii|i:Surface.create_for_data",
-			&obj, &format, &width, &height, &stride))
-    return NULL;
-
-  res = PyObject_AsWriteBuffer (obj, (void **)&buffer, &buffer_len);
-  if (res == -1)
+  if (!PyArg_ParseTuple(args, "w*iii|i:Surface.create_for_data",
+			&buffer, &format, &width, &height, &stride))
     return NULL;
 
   if (width <= 0) {
@@ -452,15 +441,20 @@ image_surface_create_for_data (PyTypeObject *type, PyObject *args) {
       return NULL;
     }
   }
-  if (height * stride > buffer_len) {
+  if (height * stride > buffer.len) {
     PyErr_SetString(PyExc_TypeError, "buffer is not long enough");
     return NULL;
   }
+  cairo_surface_t *surface;
+  PyObject *o;
   Py_BEGIN_ALLOW_THREADS;
-  surface = cairo_image_surface_create_for_data (buffer, format, width,
+  surface = cairo_image_surface_create_for_data (buffer.buf, format, width,
 						 height, stride);
   Py_END_ALLOW_THREADS;
-  return PycairoSurface_FromSurface(surface, obj);
+
+  o = PycairoSurface_FromSurface(surface, NULL);
+  ((PycairoSurface *)o)->buffer = buffer;
+  return o;
 }
 
 
@@ -570,7 +564,11 @@ image_surface_get_width (PycairoImageSurface *o) {
 }
 
 
-/* Buffer interface functions, used by ImageSurface.get_data() */
+/* Buffer interface functions
+used by ImageSurface.get_data()
+PEP 3118 -- Revising the buffer protocol
+http://www.python.org/dev/peps/pep-3118/
+*/
 static int
 image_surface_buffer_getbufferproc (PycairoImageSurface *o, Py_buffer *view,
 				                    int flags) {
@@ -578,74 +576,19 @@ image_surface_buffer_getbufferproc (PycairoImageSurface *o, Py_buffer *view,
   int height, stride;
   void *data;
 
+  /* buffer does not give ndim, shape or stride information
+   */
+
   height = cairo_image_surface_get_height (surface);
   stride = cairo_image_surface_get_stride (surface);
   data   = cairo_image_surface_get_data   (surface);
 
   if(!PyBuffer_FillInfo(view, (PyObject *)o, data,
-        height * stride, 0, PyBUF_CONTIG))
+			height * stride, 0, PyBUF_CONTIG))
     return 0;
   return -1;
 }
 
-/* Buffer interface functions, used by ImageSurface.get_data() */
-/*
-static int
-image_surface_buffer_getreadbuf (PycairoImageSurface *o, int segment,
-				 const void **ptr) {
-  cairo_surface_t *surface = o->surface;
-  int height, stride;
-
-  if (segment != 0) {
-    PyErr_SetString(PyExc_SystemError,
-		    "accessing non-existent ImageSurface segment");
-    return -1;
-  }
-  height = cairo_image_surface_get_height (surface);
-  stride = cairo_image_surface_get_stride (surface);
-  *ptr = (void *) cairo_image_surface_get_data (surface);
-  return height * stride;
-}
-
-static int
-image_surface_buffer_getwritebuf (PycairoImageSurface *o, int segment,
-				  const void **ptr) {
-  cairo_surface_t *surface = o->surface;
-  int height, stride;
-
-  if (segment != 0) {
-    PyErr_SetString(PyExc_SystemError,
-		    "accessing non-existent ImageSurface segment");
-    return -1;
-  }
-  height = cairo_image_surface_get_height (surface);
-  stride = cairo_image_surface_get_stride (surface);
-  *ptr = (void *) cairo_image_surface_get_data (surface);
-  return height * stride;
-}
-
-static int
-image_surface_buffer_getsegcount (PycairoImageSurface *o, int *lenp) {
-  if (lenp) {
-  // report the sum of the sizes (in bytes) of all segments
-    cairo_surface_t *surface = o->surface;
-    int height = cairo_image_surface_get_height (surface);
-    int stride = cairo_image_surface_get_stride (surface);
-    *lenp = height * stride;
-  }
-  return 1;  // surface data is all in one segment
-}
-*/
-
-/* See Python C API Manual 10.7 */
-/*
-static PyBufferProcs image_surface_as_buffer = {
-  (readbufferproc) image_surface_buffer_getreadbuf,
-  (writebufferproc)image_surface_buffer_getwritebuf,
-  (segcountproc)   image_surface_buffer_getsegcount,
-  (charbufferproc) NULL,
-};
-*/
 static PyBufferProcs image_surface_as_buffer = {
   (getbufferproc) image_surface_buffer_getbufferproc,
   (releasebufferproc) NULL,
diff --git a/test/isurface_create_for_data_memleak.py b/test/isurface_create_for_data_memleak.py
new file mode 100755
index 0000000..d07287f
--- /dev/null
+++ b/test/isurface_create_for_data_memleak.py
@@ -0,0 +1,28 @@
+#!/usr/bin/env python
+"""test cairo.ImageSurface.create_for_data() for memory leaks
+"""
+
+import array
+import resource
+
+import cairo
+
+
+pagesize = resource.getpagesize()
+
+if not (cairo.HAS_IMAGE_SURFACE and cairo.HAS_PNG_FUNCTIONS):
+  raise SystemExit ('cairo was not compiled with ImageSurface and PNG support')
+
+width, height = 255, 255
+lst = [0] * width * height * 4
+
+c = 1
+while True:
+  for i in range(50):
+    data = array.array('B', lst)
+    surface = cairo.ImageSurface.create_for_data(data, cairo.FORMAT_ARGB32,
+                                                 width, height)
+    ctx = cairo.Context(surface)
+
+  print(c, resource.getrusage(resource.RUSAGE_SELF).ru_maxrss * pagesize)
+  c += 1
diff --git a/test/isurface_get_data_memleak.py b/test/isurface_get_data_memleak.py
index 5a30c1e..f8368bc 100755
--- a/test/isurface_get_data_memleak.py
+++ b/test/isurface_get_data_memleak.py
@@ -2,9 +2,7 @@
 """test cairo.ImageSurface.get_data() for a memory leak
 """
 
-import array
 import resource
-import tempfile
 
 import cairo
 
@@ -16,18 +14,15 @@ if not (cairo.HAS_IMAGE_SURFACE and cairo.HAS_PNG_FUNCTIONS):
 
 width, height = 32, 32
 
+c = 1
 while True:
   for i in range(100000):
     surface = cairo.ImageSurface(cairo.FORMAT_ARGB32, width, height)
     ctx = cairo.Context(surface)
-    data = surface.get_data()
-    b = memoryview(memoryview(data))
-    del surface
-    del ctx
-    b = bytes(data)
 
-    surface = cairo.ImageSurface(cairo.FORMAT_ARGB32, width, height)
-    #ctx = cairo.Context(surface)
-    b = bytes(surface.get_data())
+    buf1 = surface.get_data()
+    buf2 = memoryview(surface)
+    buf3 = bytes(surface)
 
-  print(resource.getrusage(resource.RUSAGE_SELF).ru_maxrss * pagesize)
+  print(c, resource.getrusage(resource.RUSAGE_SELF).ru_maxrss * pagesize)
+  c += 1