john/McRogueFace
1
0
Fork 0
Code Issues 35 Pull requests Projects Releases Wiki Activity

Fast path for hot property getters: stop importing mcrfpy per call; closes #331

Browse source 
Replace the PyImport_ImportModule("mcrfpy") + PyObject_GetAttrString +
full type-call ceremony with direct &mcrfpydef::PyXType references
(safe since all PyTypeObjects are inline C++17 definitions) in every
non-init call site:

- UIDrawable: get_pos/get_origin/get_global_pos now allocate via
  PyVector(...).pyObject(); set_pos/set_origin check the Vector type
  directly.
- GridLayers: ColorLayer at/subscript wrap via PyColor(...).pyObject()
  (also fixes a module refcount leak per read); set/fill/fill_rect,
  draw_fov/apply_perspective color parsing, Entity/Texture/HeightMap
  instance checks, and TileLayer texture get/set use direct type refs.
- UIGrid init layers=, add_layer/remove_layer/layer(), Grid.layers
  getter: direct ColorLayer/TileLayer type refs.
- PyTileSetFile.to_texture, PyLdtkProject.tileset: direct type refs.

Benchmark (tests/benchmarks/issue_331_property_read_bench.py, 200k
reads, release build):
  frame.pos              559 ns -> 65 ns/read  (8.6x)
  frame.origin           571 ns -> 64 ns/read  (8.9x)
  frame.global_position  589 ns -> 66 ns/read  (8.9x)
  ColorLayer.at          608 ns -> 262 ns/read (2.3x)

Test suite: 304/304 pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DK5DHgRDNGmpN54vnEHAot
This commit is contained in:
John McCardle 2026-07-08 21:58:50 -04:00
commit 182da6230d
8 changed files with 113 additions and 342 deletions
Download patch file Download diff file Expand all files Collapse all files

157
src/GridLayers.cpp
View file

@ -63,17 +63,9 @@ static bool ValidateHeightMapSize(PyHeightMapObject* hmap, int grid_x, int grid_
return true;
}
// Helper to check if an object is a HeightMap (runtime lookup to avoid static type issues)
// Helper to check if an object is a HeightMap
static bool IsHeightMapObject(PyObject* obj, PyHeightMapObject** out_hmap) {
auto* mcrfpy_module = PyImport_ImportModule("mcrfpy");
if (!mcrfpy_module) return false;
auto* heightmap_type = PyObject_GetAttrString(mcrfpy_module, "HeightMap");
Py_DECREF(mcrfpy_module);
if (!heightmap_type) return false;
bool result = PyObject_IsInstance(obj, heightmap_type);
Py_DECREF(heightmap_type);
bool result = PyObject_IsInstance(obj, (PyObject*)&mcrfpydef::PyHeightMapType);
if (result && out_hmap) {
*out_hmap = (PyHeightMapObject*)obj;
@ -88,19 +80,10 @@ static bool ParseColorArg(PyObject* obj, sf::Color& out_color, const char* arg_n
return false;
}
auto* mcrfpy_module = PyImport_ImportModule("mcrfpy");
if (!mcrfpy_module) return false;
auto* color_type = PyObject_GetAttrString(mcrfpy_module, "Color");
Py_DECREF(mcrfpy_module);
if (!color_type) return false;
if (PyObject_IsInstance(obj, color_type)) {
if (PyObject_IsInstance(obj, (PyObject*)&mcrfpydef::PyColorType)) {
out_color = ((PyColorObject*)obj)->data;
Py_DECREF(color_type);
return true;
}
Py_DECREF(color_type);
if (PyTuple_Check(obj) || PyList_Check(obj)) {
PyObject* seq = PySequence_Fast(obj, "color must be sequence");
@ -894,19 +877,8 @@ PyObject* PyGridLayerAPI::ColorLayer_at(PyColorLayerObject* self, PyObject* args
return NULL;
}
const sf::Color& color = self->data->at(x, y);
// Return as mcrfpy.Color
auto* color_type = (PyTypeObject*)PyObject_GetAttrString(
PyImport_ImportModule("mcrfpy"), "Color");
if (!color_type) return NULL;
PyColorObject* color_obj = (PyColorObject*)color_type->tp_alloc(color_type, 0);
Py_DECREF(color_type);
if (!color_obj) return NULL;
color_obj->data = color;
return (PyObject*)color_obj;
// Return as mcrfpy.Color (#331: direct allocation, no module import per read)
return PyColor(self->data->at(x, y)).pyObject();
}
PyObject* PyGridLayerAPI::ColorLayer_set(PyColorLayerObject* self, PyObject* args) {
@ -933,24 +905,15 @@ PyObject* PyGridLayerAPI::ColorLayer_set(PyColorLayerObject* self, PyObject* arg
// Parse color
sf::Color color;
auto* mcrfpy_module = PyImport_ImportModule("mcrfpy");
if (!mcrfpy_module) return NULL;
auto* color_type = PyObject_GetAttrString(mcrfpy_module, "Color");
Py_DECREF(mcrfpy_module);
if (!color_type) return NULL;
if (PyObject_IsInstance(color_obj, color_type)) {
if (PyObject_IsInstance(color_obj, (PyObject*)&mcrfpydef::PyColorType)) {
color = ((PyColorObject*)color_obj)->data;
} else if (PyTuple_Check(color_obj)) {
int r, g, b, a = 255;
if (!PyArg_ParseTuple(color_obj, "iii|i", &r, &g, &b, &a)) {
Py_DECREF(color_type);
return NULL;
}
// #213 - Validate color component range
if (r < 0 || r > 255 || g < 0 || g > 255 || b < 0 || b > 255 || a < 0 || a > 255) {
Py_DECREF(color_type);
PyErr_Format(PyExc_ValueError,
"color components must be in range 0-255, got (%d, %d, %d, %d)",
r, g, b, a);
@ -958,11 +921,9 @@ PyObject* PyGridLayerAPI::ColorLayer_set(PyColorLayerObject* self, PyObject* arg
}
color = sf::Color(r, g, b, a);
} else {
Py_DECREF(color_type);
PyErr_SetString(PyExc_TypeError, "color must be a Color object or (r, g, b[, a]) tuple");
return NULL;
}
Py_DECREF(color_type);
self->data->at(x, y) = color;
self->data->markDirty(x, y); // Mark only the affected chunk
@ -1006,17 +967,8 @@ PyObject* PyGridLayerAPI::ColorLayer_subscript(PyColorLayerObject* self, PyObjec
return NULL;
}
const sf::Color& color = self->data->at(x, y);
// Wrap as mcrfpy.Color
auto* color_type = (PyTypeObject*)PyObject_GetAttrString(
PyImport_ImportModule("mcrfpy"), "Color");
if (!color_type) return NULL;
PyColorObject* color_obj = (PyColorObject*)color_type->tp_alloc(color_type, 0);
Py_DECREF(color_type);
if (!color_obj) return NULL;
color_obj->data = color;
return (PyObject*)color_obj;
// Wrap as mcrfpy.Color (#331: direct allocation, no module import per read)
return PyColor(self->data->at(x, y)).pyObject();
}
int PyGridLayerAPI::ColorLayer_subscript_assign(PyColorLayerObject* self, PyObject* key, PyObject* value) {
@ -1066,24 +1018,15 @@ PyObject* PyGridLayerAPI::ColorLayer_fill(PyColorLayerObject* self, PyObject* ar
// Parse color
sf::Color color;
auto* mcrfpy_module = PyImport_ImportModule("mcrfpy");
if (!mcrfpy_module) return NULL;
auto* color_type = PyObject_GetAttrString(mcrfpy_module, "Color");
Py_DECREF(mcrfpy_module);
if (!color_type) return NULL;
if (PyObject_IsInstance(color_obj, color_type)) {
if (PyObject_IsInstance(color_obj, (PyObject*)&mcrfpydef::PyColorType)) {
color = ((PyColorObject*)color_obj)->data;
} else if (PyTuple_Check(color_obj)) {
int r, g, b, a = 255;
if (!PyArg_ParseTuple(color_obj, "iii|i", &r, &g, &b, &a)) {
Py_DECREF(color_type);
return NULL;
}
// #213 - Validate color component range
if (r < 0 || r > 255 || g < 0 || g > 255 || b < 0 || b > 255 || a < 0 || a > 255) {
Py_DECREF(color_type);
PyErr_Format(PyExc_ValueError,
"color components must be in range 0-255, got (%d, %d, %d, %d)",
r, g, b, a);
@ -1091,11 +1034,9 @@ PyObject* PyGridLayerAPI::ColorLayer_fill(PyColorLayerObject* self, PyObject* ar
}
color = sf::Color(r, g, b, a);
} else {
Py_DECREF(color_type);
PyErr_SetString(PyExc_TypeError, "color must be a Color object or (r, g, b[, a]) tuple");
return NULL;
}
Py_DECREF(color_type);
self->data->fill(color);
Py_RETURN_NONE;
@ -1141,24 +1082,15 @@ PyObject* PyGridLayerAPI::ColorLayer_fill_rect(PyColorLayerObject* self, PyObjec
// Parse color
sf::Color color;
auto* mcrfpy_module = PyImport_ImportModule("mcrfpy");
if (!mcrfpy_module) return NULL;
auto* color_type = PyObject_GetAttrString(mcrfpy_module, "Color");
Py_DECREF(mcrfpy_module);
if (!color_type) return NULL;
if (PyObject_IsInstance(color_obj, color_type)) {
if (PyObject_IsInstance(color_obj, (PyObject*)&mcrfpydef::PyColorType)) {
color = ((PyColorObject*)color_obj)->data;
} else if (PyTuple_Check(color_obj)) {
int r, g, b, a = 255;
if (!PyArg_ParseTuple(color_obj, "iii|i", &r, &g, &b, &a)) {
Py_DECREF(color_type);
return NULL;
}
// #213 - Validate color component range
if (r < 0 || r > 255 || g < 0 || g > 255 || b < 0 || b > 255 || a < 0 || a > 255) {
Py_DECREF(color_type);
PyErr_Format(PyExc_ValueError,
"color components must be in range 0-255, got (%d, %d, %d, %d)",
r, g, b, a);
@ -1166,11 +1098,9 @@ PyObject* PyGridLayerAPI::ColorLayer_fill_rect(PyColorLayerObject* self, PyObjec
}
color = sf::Color(r, g, b, a);
} else {
Py_DECREF(color_type);
PyErr_SetString(PyExc_TypeError, "color must be a Color object or (r, g, b[, a]) tuple");
return NULL;
}
Py_DECREF(color_type);
self->data->fillRect(x, y, width, height, color);
Py_RETURN_NONE;
@ -1233,29 +1163,18 @@ PyObject* PyGridLayerAPI::ColorLayer_draw_fov(PyColorLayerObject* self, PyObject
return true;
}
auto* mcrfpy_module = PyImport_ImportModule("mcrfpy");
if (!mcrfpy_module) return false;
auto* color_type = PyObject_GetAttrString(mcrfpy_module, "Color");
Py_DECREF(mcrfpy_module);
if (!color_type) return false;
if (PyObject_IsInstance(obj, color_type)) {
if (PyObject_IsInstance(obj, (PyObject*)&mcrfpydef::PyColorType)) {
out = ((PyColorObject*)obj)->data;
Py_DECREF(color_type);
return true;
} else if (PyTuple_Check(obj)) {
int r, g, b, a = 255;
if (!PyArg_ParseTuple(obj, "iii|i", &r, &g, &b, &a)) {
Py_DECREF(color_type);
return false;
}
out = sf::Color(r, g, b, a);
Py_DECREF(color_type);
return true;
}
Py_DECREF(color_type);
PyErr_Format(PyExc_TypeError, "%s must be a Color object or (r, g, b[, a]) tuple", name);
return false;
};
@ -1295,20 +1214,10 @@ PyObject* PyGridLayerAPI::ColorLayer_apply_perspective(PyColorLayerObject* self,
return NULL;
}
// Get the Entity type
auto* mcrfpy_module = PyImport_ImportModule("mcrfpy");
if (!mcrfpy_module) return NULL;
auto* entity_type = PyObject_GetAttrString(mcrfpy_module, "Entity");
Py_DECREF(mcrfpy_module);
if (!entity_type) return NULL;
if (!PyObject_IsInstance(entity_obj, entity_type)) {
Py_DECREF(entity_type);
if (!PyObject_IsInstance(entity_obj, (PyObject*)&mcrfpydef::PyUIEntityType)) {
PyErr_SetString(PyExc_TypeError, "entity must be an Entity object");
return NULL;
}
Py_DECREF(entity_type);
// Get the shared_ptr to the entity
PyUIEntityObject* py_entity = (PyUIEntityObject*)entity_obj;
@ -1324,29 +1233,18 @@ PyObject* PyGridLayerAPI::ColorLayer_apply_perspective(PyColorLayerObject* self,
return true;
}
auto* mcrfpy_module = PyImport_ImportModule("mcrfpy");
if (!mcrfpy_module) return false;
auto* color_type = PyObject_GetAttrString(mcrfpy_module, "Color");
Py_DECREF(mcrfpy_module);
if (!color_type) return false;
if (PyObject_IsInstance(obj, color_type)) {
if (PyObject_IsInstance(obj, (PyObject*)&mcrfpydef::PyColorType)) {
out = ((PyColorObject*)obj)->data;
Py_DECREF(color_type);
return true;
} else if (PyTuple_Check(obj)) {
int r, g, b, a = 255;
if (!PyArg_ParseTuple(obj, "iii|i", &r, &g, &b, &a)) {
Py_DECREF(color_type);
return false;
}
out = sf::Color(r, g, b, a);
Py_DECREF(color_type);
return true;
}
Py_DECREF(color_type);
PyErr_Format(PyExc_TypeError, "%s must be a Color object or (r, g, b[, a]) tuple", name);
return false;
};
@ -1982,22 +1880,12 @@ int PyGridLayerAPI::TileLayer_init(PyTileLayerObject* self, PyObject* args, PyOb
// Parse texture
std::shared_ptr<PyTexture> texture;
if (texture_obj && texture_obj != Py_None) {
// Check if it's a PyTexture
auto* mcrfpy_module = PyImport_ImportModule("mcrfpy");
if (!mcrfpy_module) return -1;
auto* texture_type = PyObject_GetAttrString(mcrfpy_module, "Texture");
Py_DECREF(mcrfpy_module);
if (!texture_type) return -1;
if (PyObject_IsInstance(texture_obj, texture_type)) {
if (PyObject_IsInstance(texture_obj, (PyObject*)&mcrfpydef::PyTextureType)) {
texture = ((PyTextureObject*)texture_obj)->data;
} else {
Py_DECREF(texture_type);
PyErr_SetString(PyExc_TypeError, "texture must be a Texture object");
return -1;
}
Py_DECREF(texture_type);
}
// Parse grid_size if provided
@ -2403,12 +2291,8 @@ PyObject* PyGridLayerAPI::TileLayer_get_texture(PyTileLayerObject* self, void* c
Py_RETURN_NONE;
}
auto* texture_type = (PyTypeObject*)PyObject_GetAttrString(
PyImport_ImportModule("mcrfpy"), "Texture");
if (!texture_type) return NULL;
auto* texture_type = &mcrfpydef::PyTextureType;
PyTextureObject* tex_obj = (PyTextureObject*)texture_type->tp_alloc(texture_type, 0);
Py_DECREF(texture_type);
if (!tex_obj) return NULL;
tex_obj->data = self->data->texture;
@ -2427,19 +2311,10 @@ int PyGridLayerAPI::TileLayer_set_texture(PyTileLayerObject* self, PyObject* val
return 0;
}
auto* mcrfpy_module = PyImport_ImportModule("mcrfpy");
if (!mcrfpy_module) return -1;
auto* texture_type = PyObject_GetAttrString(mcrfpy_module, "Texture");
Py_DECREF(mcrfpy_module);
if (!texture_type) return -1;
if (!PyObject_IsInstance(value, texture_type)) {
Py_DECREF(texture_type);
if (!PyObject_IsInstance(value, (PyObject*)&mcrfpydef::PyTextureType)) {
PyErr_SetString(PyExc_TypeError, "texture must be a Texture object or None");
return -1;
}
Py_DECREF(texture_type);
self->data->texture = ((PyTextureObject*)value)->data;
self->data->markDirty(); // Mark ALL chunks for re-render (texture change affects all)

74
src/UIDrawable.cpp
View file

@ -577,20 +577,8 @@ PyObject* UIDrawable::get_pos(PyObject* self, void* closure) {
UIDrawable* drawable = extractDrawable(self, objtype);
if (!drawable) return NULL;
// Create a Python Vector object from position
PyObject* module = PyImport_ImportModule("mcrfpy");
if (!module) return NULL;
PyObject* vector_type = PyObject_GetAttrString(module, "Vector");
Py_DECREF(module);
if (!vector_type) return NULL;
PyObject* args = Py_BuildValue("(ff)", drawable->position.x, drawable->position.y);
PyObject* result = PyObject_CallObject(vector_type, args);
Py_DECREF(vector_type);
Py_DECREF(args);
return result;
// #331: direct allocation fast path — no module import or type call per read
return PyVector(drawable->position).pyObject();
}
int UIDrawable::set_pos(PyObject* self, PyObject* value, void* closure) {
@ -619,17 +607,7 @@ int UIDrawable::set_pos(PyObject* self, PyObject* value, void* closure) {
}
} else {
// Try to get as Vector
PyObject* module = PyImport_ImportModule("mcrfpy");
if (!module) return -1;
PyObject* vector_type = PyObject_GetAttrString(module, "Vector");
Py_DECREF(module);
if (!vector_type) return -1;
int is_vector = PyObject_IsInstance(value, vector_type);
Py_DECREF(vector_type);
if (is_vector) {
if (PyObject_IsInstance(value, (PyObject*)&mcrfpydef::PyVectorType)) {
PyVectorObject* vec = (PyVectorObject*)value;
x = vec->data.x;
y = vec->data.y;
@ -680,20 +658,8 @@ PyObject* UIDrawable::get_origin(PyObject* self, void* closure) {
UIDrawable* drawable = extractDrawable(self, objtype);
if (!drawable) return NULL;
// Create a Python Vector object from origin
PyObject* module = PyImport_ImportModule("mcrfpy");
if (!module) return NULL;
PyObject* vector_type = PyObject_GetAttrString(module, "Vector");
Py_DECREF(module);
if (!vector_type) return NULL;
PyObject* args = Py_BuildValue("(ff)", drawable->origin.x, drawable->origin.y);
PyObject* result = PyObject_CallObject(vector_type, args);
Py_DECREF(vector_type);
Py_DECREF(args);
return result;
// #331: direct allocation fast path — no module import or type call per read
return PyVector(drawable->origin).pyObject();
}
int UIDrawable::set_origin(PyObject* self, PyObject* value, void* closure) {
@ -722,17 +688,7 @@ int UIDrawable::set_origin(PyObject* self, PyObject* value, void* closure) {
}
} else {
// Try to get as Vector
PyObject* module = PyImport_ImportModule("mcrfpy");
if (!module) return -1;
PyObject* vector_type = PyObject_GetAttrString(module, "Vector");
Py_DECREF(module);
if (!vector_type) return -1;
int is_vector = PyObject_IsInstance(value, vector_type);
Py_DECREF(vector_type);
if (is_vector) {
if (PyObject_IsInstance(value, (PyObject*)&mcrfpydef::PyVectorType)) {
PyVectorObject* vec = (PyVectorObject*)value;
x = vec->data.x;
y = vec->data.y;
@ -1550,22 +1506,8 @@ PyObject* UIDrawable::get_global_pos(PyObject* self, void* closure) {
UIDrawable* drawable = extractDrawable(self, objtype);
if (!drawable) return NULL;
sf::Vector2f global_pos = drawable->get_global_position();
// Create a Python Vector object
PyObject* module = PyImport_ImportModule("mcrfpy");
if (!module) return NULL;
PyObject* vector_type = PyObject_GetAttrString(module, "Vector");
Py_DECREF(module);
if (!vector_type) return NULL;
PyObject* args = Py_BuildValue("(ff)", global_pos.x, global_pos.y);
PyObject* result = PyObject_CallObject(vector_type, args);
Py_DECREF(vector_type);
Py_DECREF(args);
return result;
// #331: direct allocation fast path — no module import or type call per read
return PyVector(drawable->get_global_position()).pyObject();
}
// #138, #188 - Python API for bounds property - returns (pos, size) as pair of Vectors

38
src/UIGrid.cpp
View file

@ -828,22 +828,8 @@ int UIGrid::init(PyUIGridObject* self, PyObject* args, PyObject* kwds) {
return -1;
}
auto* mcrfpy_module = PyImport_ImportModule("mcrfpy");
if (!mcrfpy_module) {
Py_DECREF(iterator);
return -1;
}
auto* color_layer_type = PyObject_GetAttrString(mcrfpy_module, "ColorLayer");
auto* tile_layer_type = PyObject_GetAttrString(mcrfpy_module, "TileLayer");
Py_DECREF(mcrfpy_module);
if (!color_layer_type || !tile_layer_type) {
if (color_layer_type) Py_DECREF(color_layer_type);
if (tile_layer_type) Py_DECREF(tile_layer_type);
Py_DECREF(iterator);
return -1;
}
auto* color_layer_type = (PyObject*)&mcrfpydef::PyColorLayerType;
auto* tile_layer_type = (PyObject*)&mcrfpydef::PyTileLayerType;
PyObject* item;
while ((item = PyIter_Next(iterator)) != NULL) {
@ -854,8 +840,6 @@ int UIGrid::init(PyUIGridObject* self, PyObject* args, PyObject* kwds) {
if (!py_layer->data) {
Py_DECREF(item);
Py_DECREF(iterator);
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
PyErr_SetString(PyExc_RuntimeError, "Layer has no data");
return -1;
}
@ -864,8 +848,6 @@ int UIGrid::init(PyUIGridObject* self, PyObject* args, PyObject* kwds) {
if (py_layer->grid) {
Py_DECREF(item);
Py_DECREF(iterator);
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
PyErr_SetString(PyExc_ValueError, "Layer is already attached to another Grid");
return -1;
}
@ -876,8 +858,6 @@ int UIGrid::init(PyUIGridObject* self, PyObject* args, PyObject* kwds) {
if (!layer->name.empty() && UIGrid::isProtectedLayerName(layer->name)) {
Py_DECREF(item);
Py_DECREF(iterator);
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
PyErr_Format(PyExc_ValueError, "Layer name '%s' is reserved", layer->name.c_str());
return -1;
}
@ -897,8 +877,6 @@ int UIGrid::init(PyUIGridObject* self, PyObject* args, PyObject* kwds) {
} else if (layer->grid_x != self->data->grid_w || layer->grid_y != self->data->grid_h) {
Py_DECREF(item);
Py_DECREF(iterator);
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
PyErr_Format(PyExc_ValueError,
"Layer size (%d, %d) does not match Grid size (%d, %d)",
layer->grid_x, layer->grid_y, self->data->grid_w, self->data->grid_h);
@ -915,8 +893,6 @@ int UIGrid::init(PyUIGridObject* self, PyObject* args, PyObject* kwds) {
if (!py_layer->data) {
Py_DECREF(item);
Py_DECREF(iterator);
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
PyErr_SetString(PyExc_RuntimeError, "Layer has no data");
return -1;
}
@ -925,8 +901,6 @@ int UIGrid::init(PyUIGridObject* self, PyObject* args, PyObject* kwds) {
if (py_layer->grid) {
Py_DECREF(item);
Py_DECREF(iterator);
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
PyErr_SetString(PyExc_ValueError, "Layer is already attached to another Grid");
return -1;
}
@ -937,8 +911,6 @@ int UIGrid::init(PyUIGridObject* self, PyObject* args, PyObject* kwds) {
if (!layer->name.empty() && UIGrid::isProtectedLayerName(layer->name)) {
Py_DECREF(item);
Py_DECREF(iterator);
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
PyErr_Format(PyExc_ValueError, "Layer name '%s' is reserved", layer->name.c_str());
return -1;
}
@ -958,8 +930,6 @@ int UIGrid::init(PyUIGridObject* self, PyObject* args, PyObject* kwds) {
} else if (layer->grid_x != self->data->grid_w || layer->grid_y != self->data->grid_h) {
Py_DECREF(item);
Py_DECREF(iterator);
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
PyErr_Format(PyExc_ValueError,
"Layer size (%d, %d) does not match Grid size (%d, %d)",
layer->grid_x, layer->grid_y, self->data->grid_w, self->data->grid_h);
@ -980,8 +950,6 @@ int UIGrid::init(PyUIGridObject* self, PyObject* args, PyObject* kwds) {
} else {
Py_DECREF(item);
Py_DECREF(iterator);
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
PyErr_SetString(PyExc_TypeError, "layers must contain only ColorLayer or TileLayer objects");
return -1;
}
@ -990,8 +958,6 @@ int UIGrid::init(PyUIGridObject* self, PyObject* args, PyObject* kwds) {
}
Py_DECREF(iterator);
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
if (PyErr_Occurred()) {
return -1;

66
src/UIGridPyMethods.cpp
View file

@ -156,18 +156,8 @@ PyObject* UIGrid::py_add_layer(PyUIGridObject* self, PyObject* args) {
return NULL;
}
auto* mcrfpy_module = PyImport_ImportModule("mcrfpy");
if (!mcrfpy_module) return NULL;
auto* color_layer_type = PyObject_GetAttrString(mcrfpy_module, "ColorLayer");
auto* tile_layer_type = PyObject_GetAttrString(mcrfpy_module, "TileLayer");
Py_DECREF(mcrfpy_module);
if (!color_layer_type || !tile_layer_type) {
if (color_layer_type) Py_DECREF(color_layer_type);
if (tile_layer_type) Py_DECREF(tile_layer_type);
return NULL;
}
auto* color_layer_type = (PyObject*)&mcrfpydef::PyColorLayerType;
auto* tile_layer_type = (PyObject*)&mcrfpydef::PyTileLayerType;
std::shared_ptr<GridLayer> layer;
PyObject* py_layer_ref = nullptr;
@ -175,15 +165,11 @@ PyObject* UIGrid::py_add_layer(PyUIGridObject* self, PyObject* args) {
if (PyObject_IsInstance(layer_obj, color_layer_type)) {
PyColorLayerObject* py_layer = (PyColorLayerObject*)layer_obj;
if (!py_layer->data) {
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
PyErr_SetString(PyExc_RuntimeError, "Layer has no data");
return NULL;
}
if (py_layer->grid && py_layer->grid.get() != self->data.get()) {
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
PyErr_SetString(PyExc_ValueError, "Layer is already attached to another Grid");
return NULL;
}
@ -192,8 +178,6 @@ PyObject* UIGrid::py_add_layer(PyUIGridObject* self, PyObject* args) {
py_layer_ref = layer_obj;
if (!layer->name.empty() && UIGrid::isProtectedLayerName(layer->name)) {
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
PyErr_Format(PyExc_ValueError, "Layer name '%s' is reserved", layer->name.c_str());
return NULL;
}
@ -209,8 +193,6 @@ PyObject* UIGrid::py_add_layer(PyUIGridObject* self, PyObject* args) {
if (layer->grid_x == 0 && layer->grid_y == 0) {
layer->resize(self->data->grid_w, self->data->grid_h);
} else if (layer->grid_x != self->data->grid_w || layer->grid_y != self->data->grid_h) {
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
PyErr_Format(PyExc_ValueError,
"Layer size (%d, %d) does not match Grid size (%d, %d)",
layer->grid_x, layer->grid_y, self->data->grid_w, self->data->grid_h);
@ -225,15 +207,11 @@ PyObject* UIGrid::py_add_layer(PyUIGridObject* self, PyObject* args) {
} else if (PyObject_IsInstance(layer_obj, tile_layer_type)) {
PyTileLayerObject* py_layer = (PyTileLayerObject*)layer_obj;
if (!py_layer->data) {
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
PyErr_SetString(PyExc_RuntimeError, "Layer has no data");
return NULL;
}
if (py_layer->grid && py_layer->grid.get() != self->data.get()) {
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
PyErr_SetString(PyExc_ValueError, "Layer is already attached to another Grid");
return NULL;
}
@ -242,8 +220,6 @@ PyObject* UIGrid::py_add_layer(PyUIGridObject* self, PyObject* args) {
py_layer_ref = layer_obj;
if (!layer->name.empty() && UIGrid::isProtectedLayerName(layer->name)) {
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
PyErr_Format(PyExc_ValueError, "Layer name '%s' is reserved", layer->name.c_str());
return NULL;
}
@ -259,8 +235,6 @@ PyObject* UIGrid::py_add_layer(PyUIGridObject* self, PyObject* args) {
if (layer->grid_x == 0 && layer->grid_y == 0) {
layer->resize(self->data->grid_w, self->data->grid_h);
} else if (layer->grid_x != self->data->grid_w || layer->grid_y != self->data->grid_h) {
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
PyErr_Format(PyExc_ValueError,
"Layer size (%d, %d) does not match Grid size (%d, %d)",
layer->grid_x, layer->grid_y, self->data->grid_w, self->data->grid_h);
@ -278,15 +252,10 @@ PyObject* UIGrid::py_add_layer(PyUIGridObject* self, PyObject* args) {
}
} else {
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
PyErr_SetString(PyExc_TypeError, "layer must be a ColorLayer or TileLayer");
return NULL;
}
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
Py_INCREF(py_layer_ref);
return py_layer_ref;
}
@ -312,13 +281,7 @@ PyObject* UIGrid::py_remove_layer(PyUIGridObject* self, PyObject* args) {
Py_RETURN_NONE;
}
auto* mcrfpy_module = PyImport_ImportModule("mcrfpy");
if (!mcrfpy_module) return NULL;
auto* color_layer_type = PyObject_GetAttrString(mcrfpy_module, "ColorLayer");
if (color_layer_type && PyObject_IsInstance(layer_obj, color_layer_type)) {
Py_DECREF(color_layer_type);
Py_DECREF(mcrfpy_module);
if (PyObject_IsInstance(layer_obj, (PyObject*)&mcrfpydef::PyColorLayerType)) {
auto* py_layer = (PyColorLayerObject*)layer_obj;
if (py_layer->data) {
py_layer->data->parent_grid = nullptr;
@ -327,12 +290,8 @@ PyObject* UIGrid::py_remove_layer(PyUIGridObject* self, PyObject* args) {
}
Py_RETURN_NONE;
}
if (color_layer_type) Py_DECREF(color_layer_type);
auto* tile_layer_type = PyObject_GetAttrString(mcrfpy_module, "TileLayer");
if (tile_layer_type && PyObject_IsInstance(layer_obj, tile_layer_type)) {
Py_DECREF(tile_layer_type);
Py_DECREF(mcrfpy_module);
if (PyObject_IsInstance(layer_obj, (PyObject*)&mcrfpydef::PyTileLayerType)) {
auto* py_layer = (PyTileLayerObject*)layer_obj;
if (py_layer->data) {
py_layer->data->parent_grid = nullptr;
@ -341,9 +300,7 @@ PyObject* UIGrid::py_remove_layer(PyUIGridObject* self, PyObject* args) {
}
Py_RETURN_NONE;
}
if (tile_layer_type) Py_DECREF(tile_layer_type);
Py_DECREF(mcrfpy_module);
PyErr_SetString(PyExc_TypeError, "layer must be a string (layer name), ColorLayer, or TileLayer");
return NULL;
}
@ -359,28 +316,17 @@ PyObject* UIGrid::py_layer(PyUIGridObject* self, PyObject* args) {
Py_RETURN_NONE;
}
auto* mcrfpy_module = PyImport_ImportModule("mcrfpy");
if (!mcrfpy_module) return NULL;
if (layer->type == GridLayerType::Color) {
auto* type = (PyTypeObject*)PyObject_GetAttrString(mcrfpy_module, "ColorLayer");
Py_DECREF(mcrfpy_module);
if (!type) return NULL;
auto* type = &mcrfpydef::PyColorLayerType;
PyColorLayerObject* obj = (PyColorLayerObject*)type->tp_alloc(type, 0);
Py_DECREF(type);
if (!obj) return NULL;
obj->data = std::static_pointer_cast<ColorLayer>(layer);
obj->grid = self->data;
return (PyObject*)obj;
} else {
auto* type = (PyTypeObject*)PyObject_GetAttrString(mcrfpy_module, "TileLayer");
Py_DECREF(mcrfpy_module);
if (!type) return NULL;
auto* type = &mcrfpydef::PyTileLayerType;
PyTileLayerObject* obj = (PyTileLayerObject*)type->tp_alloc(type, 0);
Py_DECREF(type);
if (!obj) return NULL;
obj->data = std::static_pointer_cast<TileLayer>(layer);

22
src/UIGridPyProperties.cpp
View file

@ -385,22 +385,8 @@ PyObject* UIGrid::get_layers(PyUIGridObject* self, void* closure) {
PyObject* tuple = PyTuple_New(self->data->layers.size());
if (!tuple) return NULL;
auto* mcrfpy_module = PyImport_ImportModule("mcrfpy");
if (!mcrfpy_module) {
Py_DECREF(tuple);
return NULL;
}
auto* color_layer_type = (PyTypeObject*)PyObject_GetAttrString(mcrfpy_module, "ColorLayer");
auto* tile_layer_type = (PyTypeObject*)PyObject_GetAttrString(mcrfpy_module, "TileLayer");
Py_DECREF(mcrfpy_module);
if (!color_layer_type || !tile_layer_type) {
if (color_layer_type) Py_DECREF(color_layer_type);
if (tile_layer_type) Py_DECREF(tile_layer_type);
Py_DECREF(tuple);
return NULL;
}
auto* color_layer_type = &mcrfpydef::PyColorLayerType;
auto* tile_layer_type = &mcrfpydef::PyTileLayerType;
for (size_t i = 0; i < self->data->layers.size(); ++i) {
auto& layer = self->data->layers[i];
@ -423,8 +409,6 @@ PyObject* UIGrid::get_layers(PyUIGridObject* self, void* closure) {
}
if (!py_layer) {
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
Py_DECREF(tuple);
return NULL;
}
@ -432,8 +416,6 @@ PyObject* UIGrid::get_layers(PyUIGridObject* self, void* closure) {
PyTuple_SET_ITEM(tuple, i, py_layer);
}
Py_DECREF(color_layer_type);
Py_DECREF(tile_layer_type);
return tuple;
}

10
src/ldtk/PyLdtkProject.cpp
View file

@ -106,16 +106,10 @@ PyObject* PyLdtkProject::tileset(PyLdtkProjectObject* self, PyObject* args) {
if (self->data->tilesets[i]->name == name) {
// Return a TileSetFile-compatible object
// We create a PyTileSetFileObject wrapping our existing TileSetData
PyObject* mcrfpy_module = PyImport_ImportModule("mcrfpy");
if (!mcrfpy_module) return NULL;
PyObject* tsf_type = PyObject_GetAttrString(mcrfpy_module, "TileSetFile");
Py_DECREF(mcrfpy_module);
if (!tsf_type) return NULL;
auto* tsf_type = &mcrfpydef::PyTileSetFileType;
// Allocate without calling init (we set data directly)
PyObject* tsf_obj = ((PyTypeObject*)tsf_type)->tp_alloc((PyTypeObject*)tsf_type, 0);
Py_DECREF(tsf_type);
PyObject* tsf_obj = tsf_type->tp_alloc(tsf_type, 0);
if (!tsf_obj) return NULL;
// Construct the shared_ptr in place using the proper type

14
src/tiled/PyTileSetFile.cpp
View file

@ -1,6 +1,7 @@
#include "PyTileSetFile.h"
#include "TiledParse.h"
#include "PyWangSet.h"
#include "PyTexture.h"
#include "McRFPy_Doc.h"
using namespace mcrf::tiled;
@ -109,22 +110,13 @@ PyObject* PyTileSetFile::get_wang_sets(PyTileSetFileObject* self, void*) {
PyObject* PyTileSetFile::to_texture(PyTileSetFileObject* self, PyObject* args) {
// Create a PyTexture using the image source and tile dimensions
// Get the Texture type from the mcrfpy module (safe cross-compilation-unit access)
PyObject* mcrfpy_module = PyImport_ImportModule("mcrfpy");
if (!mcrfpy_module) return NULL;
PyObject* tex_type = PyObject_GetAttrString(mcrfpy_module, "Texture");
Py_DECREF(mcrfpy_module);
if (!tex_type) return NULL;
PyObject* tex_args = Py_BuildValue("(sii)",
self->data->image_source.c_str(),
self->data->tile_width,
self->data->tile_height);
if (!tex_args) { Py_DECREF(tex_type); return NULL; }
if (!tex_args) return NULL;
PyObject* tex = PyObject_Call(tex_type, tex_args, NULL);
Py_DECREF(tex_type);
PyObject* tex = PyObject_Call((PyObject*)&mcrfpydef::PyTextureType, tex_args, NULL);
Py_DECREF(tex_args);
return tex;
}

74
tests/benchmarks/issue_331_property_read_bench.py Normal file
View file

@ -0,0 +1,74 @@
#!/usr/bin/env python3
"""
Issue #331: micro-benchmark for hot property getters.
Times tight-loop reads of properties that (pre-fix) executed
PyImport_ImportModule("mcrfpy") + PyObject_GetAttrString + full type call
per read, versus the direct tp_alloc fast path (post-fix).
Direct-execution benchmark: no game loop needed, property reads work
immediately after object construction.
Usage:
./mcrogueface --headless --exec tests/benchmarks/issue_331_property_read_bench.py
"""
import mcrfpy
import sys
import time
N = 200_000
def bench(label, fn):
start = time.perf_counter()
fn()
elapsed = time.perf_counter() - start
per_read_ns = elapsed / N * 1e9
print(f"{label:24s} {elapsed*1000:8.1f} ms total {per_read_ns:8.0f} ns/read")
return per_read_ns
def main():
frame = mcrfpy.Frame(pos=(10, 20), size=(100, 100))
layer = mcrfpy.ColorLayer(name="bench", z_index=0)
grid = mcrfpy.Grid(grid_size=(8, 8), layers=[layer])
layer.set((3, 3), (10, 20, 30, 255))
def read_pos():
for _ in range(N):
frame.pos
def read_origin():
for _ in range(N):
frame.origin
def read_global_pos():
for _ in range(N):
frame.global_position
def read_layer_at():
for _ in range(N):
layer.at((3, 3))
print(f"issue #331 property-read benchmark, N={N}")
results = {
"pos": bench("frame.pos", read_pos),
"origin": bench("frame.origin", read_origin),
"global_pos": bench("frame.global_position", read_global_pos),
"layer_at": bench("ColorLayer.at", read_layer_at),
}
# Sanity: values must be correct regardless of construction path
p = frame.pos
assert (p.x, p.y) == (10.0, 20.0), f"pos wrong: {p}"
c = layer.at((3, 3))
assert (c.r, c.g, c.b) == (10, 20, 30), f"color wrong: {c}"
print("PASS")
return results
if __name__ == "__main__":
main()
sys.exit(0)