[Bf-committers] request for code review - Freestyle pilot Python API updates
Tamito KAJIYAMA
rd6t-kjym at asahi-net.or.jp
Mon Jan 7 02:04:29 CET 2013
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 <rd6t-kjym at asahi-net.or.jp>
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 */
More information about the Bf-committers
mailing list