Hi all, I would like to ask for quick code review of pilot Freestyle branch updates for using PyGetSetDef in conjunction with mathutils Vector.
Scope of the pilot updates is to make an attempt to use PyGetSetDef for defining get/setter functions (instead of individually defining them as ordinary methods) in order to improve the Freestyle Python API syntax. In Freestyle many API functions return a vector, so the present updates also make use of mathutils Vector for wrapping underlying C++ vectors. A patch set of the updates (against Freestyle branch r53595) has been posted on http://www.pasteall.org/38592/diff and also attached below just for record. As a pilot case, a pair of getter/setter functions StrokeVertex.point was defined using PyGetSetDef in view of replacing .getPoint() and .setPoint() methods. These API functions manipulate 2D coordinates of StrokeVertex. The 2D point is internally represented by a specific C++ vector class defined in the Freestyle code base, so that the new .point getter function employs mathutils Vector to wrap the C++ vector object. Code examples below illustrate differences between the old and new API functions, where sv is a StrokeVertex instance. OLD: p = sv.getPoint() p[0] += 10 sv.setPoint(p) NEW: sv.point[0] += 10 If the proposed updates are considered okay, we will proceed with major Python API updates using get/setters and mathutils Vector. This will be the last major component that needs to be finished before another round of code review is asked for the branch merge into the trunk. Any comments and suggestions are duly acknowledged. I take this opportunity to thank Bastien Montagne for his help and discussions concerning the present pilot updates, as well as for his huge efforts to clean the code style and address many issues in the Freestyle branch. With best regards, -- KAJIYAMA, Tamito <[email protected]> Index: release/scripts/freestyle/style_modules/parameter_editor.py =================================================================== --- release/scripts/freestyle/style_modules/parameter_editor.py (revision 53595) +++ release/scripts/freestyle/style_modules/parameter_editor.py (working copy) @@ -171,7 +171,7 @@ distance = 0.0 it = stroke.strokeVerticesBegin() while not it.isEnd(): - p = it.getObject().getPoint() + p = it.getObject().point if not it.isBegin(): distance += (prev - p).length prev = p Index: source/blender/freestyle/intern/python/BPy_Freestyle.cpp =================================================================== --- source/blender/freestyle/intern/python/BPy_Freestyle.cpp (revision 53595) +++ source/blender/freestyle/intern/python/BPy_Freestyle.cpp (working copy) @@ -25,6 +25,7 @@ #include "BPy_ViewMap.h" #include "BPy_ViewShape.h" +#include "../../../python/mathutils/mathutils.h" /* for Vector callbacks */ #ifdef __cplusplus extern "C" { @@ -460,6 +461,141 @@ module_functions }; +/*-----------------------MATHUTILS VECTOR WRAPPER FUNCTIONS----------------------------*/ + +extern PyTypeObject VectorProxy_Type; + +#define BPy_VectorProxy_Check(v) (PyObject_IsInstance((PyObject *)v, (PyObject *)&VectorProxy_Type)) + +typedef struct { + PyObject_HEAD + PyObject *obj; /* borrowed reference to a wrapped PyObject */ + int size; /* vector size */ + VectorProxy_getter getter; + VectorProxy_setter setter; +} BPy_VectorProxy; + +static void VectorProxy___dealloc__(BPy_VectorProxy* self) +{ + Py_DECREF(self->obj); + Py_TYPE(self)->tp_free((PyObject*)self); +} + +PyTypeObject VectorProxy_Type = { + PyVarObject_HEAD_INIT(NULL, 0) + "VectorProxy", /* tp_name */ + sizeof(BPy_VectorProxy), /* tp_basicsize */ + 0, /* tp_itemsize */ + (destructor)VectorProxy___dealloc__, /* tp_dealloc */ + 0, /* tp_print */ + 0, /* tp_getattr */ + 0, /* tp_setattr */ + 0, /* tp_reserved */ + 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, /* tp_flags */ + 0, /* tp_doc */ + 0, /* tp_traverse */ + 0, /* tp_clear */ + 0, /* tp_richcompare */ + 0, /* 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 */ + PyType_GenericNew, /* tp_new */ +}; + +static int Freestyle_VectorProxy_check(BaseMathObject *bmo) +{ + if (!BPy_VectorProxy_Check(bmo->cb_user)) + return -1; + return 0; +} + +static int Freestyle_VectorProxy_get(BaseMathObject *bmo, int subtype) +{ + BPy_VectorProxy *proxy; + + if (!BPy_VectorProxy_Check(bmo->cb_user)) + return -1; + proxy = (BPy_VectorProxy *)bmo->cb_user; + for (int i = 0; i < proxy->size; i++) + bmo->data[i] = proxy->getter(proxy->obj, i); + return 0; +} + +static int Freestyle_VectorProxy_set(BaseMathObject *bmo, int subtype) +{ + BPy_VectorProxy *proxy; + + if (!BPy_VectorProxy_Check(bmo->cb_user)) + return -1; + proxy = (BPy_VectorProxy *)bmo->cb_user; + for (int i = 0; i < proxy->size; i++) + proxy->setter(proxy->obj, i, bmo->data[i]); + return 0; +} + +static int Freestyle_VectorProxy_get_index(BaseMathObject *bmo, int subtype, int index) +{ + BPy_VectorProxy *proxy; + + if (!BPy_VectorProxy_Check(bmo->cb_user)) + return -1; + proxy = (BPy_VectorProxy *)bmo->cb_user; + bmo->data[index] = proxy->getter(proxy->obj, index); + return 0; +} + +static int Freestyle_VectorProxy_set_index(BaseMathObject *bmo, int subtype, int index) +{ + BPy_VectorProxy *proxy; + + if (!BPy_VectorProxy_Check(bmo->cb_user)) + return -1; + proxy = (BPy_VectorProxy *)bmo->cb_user; + proxy->setter(proxy->obj, index, bmo->data[index]); + return 0; +} + +static Mathutils_Callback Freestyle_VectorProxy_cb = { + Freestyle_VectorProxy_check, + Freestyle_VectorProxy_get, + Freestyle_VectorProxy_set, + Freestyle_VectorProxy_get_index, + Freestyle_VectorProxy_set_index +}; + +static unsigned char Freestyle_VectorProxy_cb_index = -1; + +PyObject *Freestyle_VectorProxy_new(PyObject *obj, int size, VectorProxy_getter getter, VectorProxy_setter setter) +{ + BPy_VectorProxy *proxy = PyObject_New(BPy_VectorProxy, &VectorProxy_Type); + Py_INCREF(obj); + proxy->obj = obj; + proxy->size = size; + proxy->getter = getter; + proxy->setter = setter; + return Vector_CreatePyObject_cb((PyObject *)proxy, size, Freestyle_VectorProxy_cb_index, 0); +} + //-------------------MODULE INITIALIZATION-------------------------------- PyObject *Freestyle_Init( void ) { @@ -499,6 +635,9 @@ ViewMap_Init( module ); ViewShape_Init( module ); + // register mathutils Vector callbacks + Freestyle_VectorProxy_cb_index = Mathutils_RegisterCallback(&Freestyle_VectorProxy_cb); + return module; } Index: source/blender/freestyle/intern/python/BPy_Freestyle.h =================================================================== --- source/blender/freestyle/intern/python/BPy_Freestyle.h (revision 53595) +++ source/blender/freestyle/intern/python/BPy_Freestyle.h (working copy) @@ -9,8 +9,13 @@ /////////////////////////////////////////////////////////////////////////////////////////// +typedef float (*VectorProxy_getter)(void *self, int index); +typedef void (*VectorProxy_setter)(void *self, int index, float value); + /*---------------------------Python BPy_Freestyle visible prototypes-----------*/ +PyObject *Freestyle_VectorProxy_new(PyObject *obj, int size, VectorProxy_getter getter, VectorProxy_setter setter); + PyObject *Freestyle_Init( void ); /////////////////////////////////////////////////////////////////////////////////////////// Index: source/blender/freestyle/intern/python/Interface0D/CurvePoint/BPy_StrokeVertex.cpp =================================================================== --- source/blender/freestyle/intern/python/Interface0D/CurvePoint/BPy_StrokeVertex.cpp (revision 53595) +++ source/blender/freestyle/intern/python/Interface0D/CurvePoint/BPy_StrokeVertex.cpp (working copy) @@ -1,5 +1,6 @@ #include "BPy_StrokeVertex.h" +#include "../../BPy_Freestyle.h" #include "../../BPy_Convert.h" #include "../../BPy_StrokeAttribute.h" #include "../../Interface0D/BPy_SVertex.h" @@ -351,6 +352,52 @@ {NULL, NULL, 0, NULL} }; +/*----------------------StrokeVertex get/setters ----------------------------*/ + +PyDoc_STRVAR(StrokeVertex_point_doc, +"2D point coordinates.\n" +"\n" +":type: mathutils.Vector" +); + +static float VectorProxy_point_get_cb(void *self, int index) +{ + switch (index) { + case 0: return (float)((BPy_StrokeVertex *)self)->sv->x(); + case 1: return (float)((BPy_StrokeVertex *)self)->sv->y(); + } + return 0.f; // should not reach here +} + +static void VectorProxy_point_set_cb(void *self, int index, float value) +{ + switch (index) { + case 0: ((BPy_StrokeVertex *)self)->sv->setX((real)value); break; + case 1: ((BPy_StrokeVertex *)self)->sv->setY((real)value); break; + } +} + +static PyObject *StrokeVertex_point_get(BPy_StrokeVertex *self, void *UNUSED(closure)) +{ + return Freestyle_VectorProxy_new((PyObject *)self, 2, VectorProxy_point_get_cb, VectorProxy_point_set_cb); +} + +static int StrokeVertex_point_set(BPy_StrokeVertex *self, PyObject *value, void *UNUSED(closure)) +{ + if (!VectorObject_Check(value) || ((VectorObject *)value)->size != 2) { + PyErr_SetString(PyExc_ValueError, "value must be a 2-dimensional Vector"); + return -1; + } + self->sv->setX(((VectorObject *)value)->vec[0]); + self->sv->setY(((VectorObject *)value)->vec[1]); + return 0; +} + +static PyGetSetDef BPy_StrokeVertex_getseters[] = { + {(char *)"point", (getter)StrokeVertex_point_get, (setter)StrokeVertex_point_set, StrokeVertex_point_doc, NULL}, + {NULL, NULL, NULL, NULL, NULL} /* Sentinel */ +}; + /*-----------------------BPy_StrokeVertex type definition ------------------------------*/ PyTypeObject StrokeVertex_Type = { PyVarObject_HEAD_INIT(NULL, 0) @@ -382,7 +429,7 @@ 0, /* tp_iternext */ BPy_StrokeVertex_methods, /* tp_methods */ 0, /* tp_members */ - 0, /* tp_getset */ + BPy_StrokeVertex_getseters, /* tp_getset */ &CurvePoint_Type, /* tp_base */ 0, /* tp_dict */ 0, /* tp_descr_get */ _______________________________________________ Bf-committers mailing list [email protected] http://lists.blender.org/mailman/listinfo/bf-committers
