Skip to content

Commit

Permalink
Batch and error check allocations (#3224)
Browse files Browse the repository at this point in the history
* Batch PyMem_New-s in draw routines

* Batch and error check mallocs in blur routines

* Move PyErr_NoMemory out of GIL-less block

* Remove misplaced PyMem_Free
  • Loading branch information
Starbuck5 authored Nov 19, 2024
1 parent 28db8df commit 816bd77
Showing 2 changed files with 53 additions and 48 deletions.
48 changes: 16 additions & 32 deletions src_c/draw.c
Original file line number Diff line number Diff line change
@@ -279,7 +279,6 @@ aalines(PyObject *self, PyObject *arg, PyObject *kwargs)
Uint32 color;
float pts[4];
float pts_prev[4];
float *xlist, *ylist;
float x, y;
int l, t;
int extra_px;
@@ -333,16 +332,12 @@ aalines(PyObject *self, PyObject *arg, PyObject *kwargs)
"points argument must contain 2 or more points");
}

xlist = PyMem_New(float, length);
ylist = PyMem_New(float, length);
// Allocate bytes for the xlist and ylist at once to reduce allocations.
float *points_buf = PyMem_New(float, length * 2);
float *xlist = points_buf;
float *ylist = points_buf + length;

if (NULL == xlist || NULL == ylist) {
if (xlist) {
PyMem_Free(xlist);
}
if (ylist) {
PyMem_Free(ylist);
}
if (points_buf == NULL) {
return RAISE(PyExc_MemoryError,
"cannot allocate memory to draw aalines");
}
@@ -357,8 +352,7 @@ aalines(PyObject *self, PyObject *arg, PyObject *kwargs)
Py_DECREF(item);

if (!result) {
PyMem_Free(xlist);
PyMem_Free(ylist);
PyMem_Free(points_buf);
return RAISE(PyExc_TypeError, "points must be number pairs");
}

@@ -367,8 +361,7 @@ aalines(PyObject *self, PyObject *arg, PyObject *kwargs)
}

if (!pgSurface_Lock(surfobj)) {
PyMem_Free(xlist);
PyMem_Free(ylist);
PyMem_Free(points_buf);
return RAISE(PyExc_RuntimeError, "error locking surface");
}

@@ -460,8 +453,7 @@ aalines(PyObject *self, PyObject *arg, PyObject *kwargs)
disable_endpoints, disable_endpoints, extra_px);
}

PyMem_Free(xlist);
PyMem_Free(ylist);
PyMem_Free(points_buf);

if (!pgSurface_Unlock(surfobj)) {
return RAISE(PyExc_RuntimeError, "error unlocking surface");
@@ -970,7 +962,6 @@ polygon(PyObject *self, PyObject *arg, PyObject *kwargs)
PyObject *colorobj, *points, *item = NULL;
SDL_Surface *surf = NULL;
Uint32 color;
int *xlist = NULL, *ylist = NULL;
int width = 0; /* Default width. */
int x, y, result, l, t;
int drawn_area[4] = {INT_MAX, INT_MAX, INT_MIN,
@@ -1021,16 +1012,12 @@ polygon(PyObject *self, PyObject *arg, PyObject *kwargs)
"points argument must contain more than 2 points");
}

xlist = PyMem_New(int, length);
ylist = PyMem_New(int, length);
// Allocate bytes for the xlist and ylist at once to reduce allocations.
int *points_buf = PyMem_New(int, length * 2);
int *xlist = points_buf;
int *ylist = points_buf + length;

if (NULL == xlist || NULL == ylist) {
if (xlist) {
PyMem_Free(xlist);
}
if (ylist) {
PyMem_Free(ylist);
}
if (points_buf == NULL) {
return RAISE(PyExc_MemoryError,
"cannot allocate memory to draw polygon");
}
@@ -1045,8 +1032,7 @@ polygon(PyObject *self, PyObject *arg, PyObject *kwargs)
Py_DECREF(item);

if (!result) {
PyMem_Free(xlist);
PyMem_Free(ylist);
PyMem_Free(points_buf);
return RAISE(PyExc_TypeError, "points must be number pairs");
}

@@ -1055,8 +1041,7 @@ polygon(PyObject *self, PyObject *arg, PyObject *kwargs)
}

if (!pgSurface_Lock(surfobj)) {
PyMem_Free(xlist);
PyMem_Free(ylist);
PyMem_Free(points_buf);
return RAISE(PyExc_RuntimeError, "error locking surface");
}

@@ -1066,8 +1051,7 @@ polygon(PyObject *self, PyObject *arg, PyObject *kwargs)
else {
draw_filltri(surf, xlist, ylist, color, drawn_area);
}
PyMem_Free(xlist);
PyMem_Free(ylist);
PyMem_Free(points_buf);

if (!pgSurface_Unlock(surfobj)) {
return RAISE(PyExc_RuntimeError, "error unlocking surface");
53 changes: 37 additions & 16 deletions src_c/transform.c
Original file line number Diff line number Diff line change
@@ -3562,7 +3562,7 @@ surf_average_color(PyObject *self, PyObject *args, PyObject *kwargs)
return Py_BuildValue("(bbbb)", r, g, b, a);
}

static void
static int
box_blur(SDL_Surface *src, SDL_Surface *dst, int radius, SDL_bool repeat)
{
// Reference : https://blog.csdn.net/blogshinelee/article/details/80997324
@@ -3574,9 +3574,16 @@ box_blur(SDL_Surface *src, SDL_Surface *dst, int radius, SDL_bool repeat)
int dst_pitch = dst->pitch;
int src_pitch = src->pitch;
int i, x, y, color;
Uint32 *buf = malloc(dst_pitch * sizeof(Uint32));
Uint32 *sum_v = malloc(dst_pitch * sizeof(Uint32));
Uint32 *sum_h = malloc(nb * sizeof(Uint32));

// Allocate bytes for buf, sum_v, and sum_h at once to reduce allocations.
Uint32 *overall_buf = malloc(sizeof(Uint32) * (dst_pitch * 2 + nb));
Uint32 *buf = overall_buf;
Uint32 *sum_v = overall_buf + dst_pitch;
Uint32 *sum_h = overall_buf + dst_pitch * 2;

if (overall_buf == NULL) {
return -1;
}

memset(sum_v, 0, dst_pitch * sizeof(Uint32));
for (y = 0; y <= radius; y++) { // y-pre
@@ -3641,12 +3648,11 @@ box_blur(SDL_Surface *src, SDL_Surface *dst, int radius, SDL_bool repeat)
}
}

free(buf);
free(sum_v);
free(sum_h);
free(overall_buf);
return 0;
}

static void
static int
gaussian_blur(SDL_Surface *src, SDL_Surface *dst, int sigma, SDL_bool repeat)
{
Uint8 *srcpx = (Uint8 *)src->pixels;
@@ -3657,11 +3663,19 @@ gaussian_blur(SDL_Surface *src, SDL_Surface *dst, int sigma, SDL_bool repeat)
int src_pitch = src->pitch;
int i, j, x, y, color;
int kernel_radius = sigma * 2;
float *buf = malloc(dst_pitch * sizeof(float));
float *buf2 = malloc(dst_pitch * sizeof(float));
float *lut = malloc((kernel_radius + 1) * sizeof(float));
float lut_sum = 0.0;

// Allocate bytes for buf, buf2, and lut at once to reduce allocations.
float *overall_buf =
malloc(sizeof(float) * (dst_pitch * 2 + kernel_radius + 1));
float *buf = overall_buf;
float *buf2 = overall_buf + dst_pitch;
float *lut = overall_buf + dst_pitch * 2;

if (overall_buf == NULL) {
return -1;
}

for (i = 0; i <= kernel_radius; i++) { // init gaussian lut
// Gaussian function
lut[i] =
@@ -3723,9 +3737,8 @@ gaussian_blur(SDL_Surface *src, SDL_Surface *dst, int sigma, SDL_bool repeat)
}
}

free(buf);
free(buf2);
free(lut);
free(overall_buf);
return 0;
}

static SDL_Surface *
@@ -3734,6 +3747,7 @@ blur(pgSurfaceObject *srcobj, pgSurfaceObject *dstobj, int radius,
{
SDL_Surface *src = NULL;
SDL_Surface *retsurf = NULL;
int result = 0;

if (radius < 0) {
return RAISE(PyExc_ValueError,
@@ -3793,17 +3807,24 @@ blur(pgSurfaceObject *srcobj, pgSurfaceObject *dstobj, int radius,
Py_BEGIN_ALLOW_THREADS;

if (algorithm == 'b') {
box_blur(src, retsurf, radius, repeat);
result = box_blur(src, retsurf, radius, repeat);
}
else if (algorithm == 'g') {
gaussian_blur(src, retsurf, radius, repeat);
result = gaussian_blur(src, retsurf, radius, repeat);
}

Py_END_ALLOW_THREADS;

pgSurface_Unlock(srcobj);
SDL_UnlockSurface(retsurf);

// Routines only set error flag if memory allocation failed
// Setting Python exception here outside of Py_ THREADS block to be safe
if (result) {
PyErr_NoMemory();
return NULL;
}

return retsurf;
}

0 comments on commit 816bd77

Please sign in to comment.