Skip to content

Surface multiphase init #3354

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Surface multiphase init #3354

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5e452f6
Merge branch 'main' of https://github.com/pygame-community/pygame-ce …
oddbookworm Feb 28, 2025
6478326
Surface is now multi-phase initialization
oddbookworm Feb 28, 2025
68bcd86
m_size actually zero now
oddbookworm Mar 2, 2025
fd2da99
Merge branch 'main' of https://github.com/pygame-community/pygame-ce …
oddbookworm Mar 2, 2025
f79f336
Merge branch 'main' of https://github.com/pygame-community/pygame-ce …
oddbookworm Mar 4, 2025
4434d28
Merge branch 'main' of https://github.com/pygame-community/pygame-ce …
oddbookworm Mar 9, 2025
5b61019
Removed unnecessary module decrefs and strong refs in exec_surface
oddbookworm Mar 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 55 additions & 50 deletions src_c/surface.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ typedef enum {
} SurfViewKind;

/* To avoid problems with non-const Py_buffer format field */
static char FormatUint8[] = "B";
static char FormatUint16[] = "=H";
static char FormatUint24[] = "3x";
static char FormatUint32[] = "=I";
#define FormatUint8 "B"
#define FormatUint16 "=H"
#define FormatUint24 "3x"
#define FormatUint32 "=I"

typedef struct pg_bufferinternal_s {
PyObject *consumer_ref; /* A weak reference to a bufferproxy object */
Expand Down Expand Up @@ -4275,17 +4275,63 @@ pgSurface_Blit(pgSurfaceObject *dstobj, pgSurfaceObject *srcobj,

static PyMethodDef _surface_methods[] = {{NULL, NULL, 0, NULL}};

MODINIT_DEFINE(surface)
int
exec_surface(PyObject *module)
{
PyObject *module, *apiobj;
PyObject *apiobj;
static void *c_api[PYGAMEAPI_SURFACE_NUMSLOTS];

if (pg_warn_simd_at_runtime_but_uncompiled() < 0) {
return -1;
}

if (PyModule_AddObjectRef(module, "SurfaceType",
(PyObject *)&pgSurface_Type)) {
return -1;
}

if (PyModule_AddObjectRef(module, "Surface",
(PyObject *)&pgSurface_Type)) {
return -1;
}

/* export the c api */
c_api[0] = &pgSurface_Type;
c_api[1] = pgSurface_New2;
c_api[2] = pgSurface_Blit;
c_api[3] = pgSurface_SetSurface;
apiobj = encapsulate_api(c_api, "surface");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a strong reference presumably, so my hunch is that it should be decref'd because now PyModule_AddObjectRef is not stealing a reference

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, as I said before it's probably not a big deal

if (PyModule_AddObjectRef(module, PYGAMEAPI_LOCAL_ENTRY, apiobj)) {
return -1;
}

if (PyModule_AddObjectRef(module, "_dict", pgSurface_Type.tp_dict)) {
return -1;
}

return 0;
}

MODINIT_DEFINE(surface)
{
static PyModuleDef_Slot surf_slots[] = {
{Py_mod_exec, &exec_surface},
#if PY_VERSION_HEX >= 0x030c0000
{Py_mod_multiple_interpreters,
Py_MOD_MULTIPLE_INTERPRETERS_NOT_SUPPORTED}, // TODO: see if this can
// be supported later
#endif
#if PY_VERSION_HEX >= 0x030d0000
{Py_mod_gil, Py_MOD_GIL_USED}, // TODO: support this later
#endif
{0, NULL}};

static struct PyModuleDef _module = {PyModuleDef_HEAD_INIT,
"surface",
DOC_SURFACE,
-1,
0,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change? for now I believe it should still be -1 right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ll double check, but it wouldn’t run for me with -1 because -1 is incompatible with subinterpreters, and multiphase implicitly assumes subinterpreter compat (and I actually forgot to figure out what the actual value should be)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's what I thought

❯ python -c "import pygame"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
    import pygame
  File "C:\Users\andre\Projects\pygame-ce\src_py\__init__.py", line 166, in <module>
    import pygame.display
SystemError: module pygame.surface: m_size may not be negative for multi-phase initialization

_surface_methods,
NULL,
surf_slots,
NULL,
NULL,
NULL};
Expand Down Expand Up @@ -4319,46 +4365,5 @@ MODINIT_DEFINE(surface)
return NULL;
}

/* create the module */
module = PyModule_Create(&_module);
if (module == NULL) {
return NULL;
}
if (pg_warn_simd_at_runtime_but_uncompiled() < 0) {
Py_DECREF(module);
return NULL;
}
Py_INCREF(&pgSurface_Type);
if (PyModule_AddObject(module, "SurfaceType",
(PyObject *)&pgSurface_Type)) {
Py_DECREF(&pgSurface_Type);
Py_DECREF(module);
return NULL;
}

Py_INCREF(&pgSurface_Type);
if (PyModule_AddObject(module, "Surface", (PyObject *)&pgSurface_Type)) {
Py_DECREF(&pgSurface_Type);
Py_DECREF(module);
return NULL;
}

/* export the c api */
c_api[0] = &pgSurface_Type;
c_api[1] = pgSurface_New2;
c_api[2] = pgSurface_Blit;
c_api[3] = pgSurface_SetSurface;
apiobj = encapsulate_api(c_api, "surface");
if (PyModule_AddObject(module, PYGAMEAPI_LOCAL_ENTRY, apiobj)) {
Py_XDECREF(apiobj);
Py_DECREF(module);
return NULL;
}
Py_XINCREF(pgSurface_Type.tp_dict);
if (PyModule_AddObject(module, "_dict", pgSurface_Type.tp_dict)) {
Py_XDECREF(pgSurface_Type.tp_dict);
Py_DECREF(module);
return NULL;
}
return module;
return PyModuleDef_Init(&_module);
}
Loading