[Bf-committers] request for code review - Freestyle pilot Python API updates

[Tamito KAJIYAMA]

Hi Campbell,

There are 16 pairs of vector get/setters in 7 objects (including StrokeVertex.point).
So, only 7 Mathutils_Callback structs need to be implemented using subtype.
I guess consuming this number of Mathutils_Callback structs is acceptable.

With best regards,

-- 
KAJIYAMA, Tamito <rd6t-kjym at asahi-net.or.jp>


-----Original Message----- 
From: Campbell Barton 
Sent: Tuesday, January 08, 2013 1:22 AM 
To: bf-blender developers 
Subject: Re: [Bf-committers] request for code review - Freestyle pilot Python API updates 

On Tue, Jan 8, 2013 at 10:40 AM, Tamito KAJIYAMA
<rd6t-kjym at asahi-net.or.jp> wrote:
> Hi Campbell,
>
> Thank you for the prompt code review.  The reason for introducing VectorProxy was
> to use only one Mathutils_Callback struct in the entire Freestyle module, in order to
> be conservative and not increase too much the total number of Mathutils_Callback
> structs (it is recalled that the maximum has been set to 10, which could be increased
> but only up to 255, i.e., a limit because of unsigned char).
>
> Assuming that relaxing the Mathutils_Callback struct count limit is not a big deal,
> I have updated the patch set following your suggestion:
> http://www.pasteall.org/38629/diff (also pasted below)
>
> If this version is okay, I am going to proceed with updates of the Freestyle Python API.
>
> Thanks and with best regards,
>
> --
> KAJIYAMA, Tamito <rd6t-kjym at asahi-net.or.jp>

It depends a on how many callbacks you expect to have (if its less
then ~20 I wouldn't worry, otherwise maybe look into a different
method),

You can use the subtype within the one callback to get some extra control, eg:

KX_GameObject.cpp -> mathutils_kxgameob_vector_get()

In this case its used for getting different vectors from the one
KX_GameObject, but it could be used to treat  'self' as a different
type too, however Id only do that if you risk using many slots.


> -----Original Message-----
> From: Campbell Barton
> Sent: Monday, January 07, 2013 3:14 AM
> To: bf-blender developers
> Subject: Re: [Bf-committers] request for code review - Freestyle pilot Python API updates
>
> Regarding the mathutils wrapping, I'd suggest not having BPy_VectorProxy at all.
>
> Best remove 'BPy_VectorProxy' and return a mathutils.Vector that
> points directly to BPy_StrokeVertex object and has Mathutils_Callback
> struct defined that knows how to read/write the BPy_StrokeVertex
> values.
> If you need to wrap a different vector type, defining a new
> Mathutils_Callback struct is no problem.
>
> Otherwise LGTM from the python API side.
>
> --
> - Campbell
>
>
> 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_Interface0D.cpp
> ===================================================================
> --- source/blender/freestyle/intern/python/BPy_Interface0D.cpp    (revision 53595)
> +++ source/blender/freestyle/intern/python/BPy_Interface0D.cpp    (working copy)
> @@ -57,6 +57,8 @@
>      Py_INCREF( &TVertex_Type );
>      PyModule_AddObject(module, "TVertex", (PyObject *)&TVertex_Type);
>
> +    BPy_StrokeVertex_mathutils_register_callback();
> +
>      return 0;
> }
>
> 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"
> @@ -8,6 +9,8 @@
> extern "C" {
> #endif
>
> +#include "../../../python/mathutils/mathutils.h" /* for Vector callbacks */
> +
> ///////////////////////////////////////////////////////////////////////////////////////////
>
> //------------------------INSTANCE METHODS ----------------------------------
> @@ -351,6 +354,97 @@
>      {NULL, NULL, 0, NULL}
> };
>
> +/*----------------------StrokeVertex get/setters ----------------------------*/
> +
> +PyDoc_STRVAR(StrokeVertex_point_doc,
> +"2D point coordinates.\n"
> +"\n"
> +":type: mathutils.Vector"
> +);
> +
> +static int BPy_StrokeVertex_mathutils_check(BaseMathObject *bmo)
> +{
> +    if (!BPy_StrokeVertex_Check(bmo->cb_user))
> +        return -1;
> +    return 0;
> +}
> +
> +static int BPy_StrokeVertex_mathutils_get(BaseMathObject *bmo, int subtype)
> +{
> +    BPy_StrokeVertex *self = (BPy_StrokeVertex *)bmo->cb_user;
> +    bmo->data[0] = (float)self->sv->x();
> +    bmo->data[1] = (float)self->sv->y();
> +    return 0;
> +}
> +
> +static int BPy_StrokeVertex_mathutils_set(BaseMathObject *bmo, int subtype)
> +{
> +    BPy_StrokeVertex *self = (BPy_StrokeVertex *)bmo->cb_user;
> +    self->sv->setX((real)bmo->data[0]);
> +    self->sv->setY((real)bmo->data[1]);
> +    return 0;
> +}
> +
> +static int BPy_StrokeVertex_mathutils_get_index(BaseMathObject *bmo, int subtype, int index)
> +{
> +    BPy_StrokeVertex *self = (BPy_StrokeVertex *)bmo->cb_user;
> +    switch (index) {
> +    case 0: bmo->data[index] = (float)self->sv->x(); break;
> +    case 1: bmo->data[index] = (float)self->sv->y(); break;
> +    default:
> +        return -1;
> +    }
> +    return 0;
> +}
> +
> +static int BPy_StrokeVertex_mathutils_set_index(BaseMathObject *bmo, int subtype, int index)
> +{
> +    BPy_StrokeVertex *self = (BPy_StrokeVertex *)bmo->cb_user;
> +    switch (index) {
> +    case 0: self->sv->setX((real)bmo->data[index]); break;
> +    case 1: self->sv->setY((real)bmo->data[index]); break;
> +    default:
> +        return -1;
> +    }
> +    return 0;
> +}
> +
> +static Mathutils_Callback BPy_StrokeVertex_mathutils_cb = {
> +    BPy_StrokeVertex_mathutils_check,
> +    BPy_StrokeVertex_mathutils_get,
> +    BPy_StrokeVertex_mathutils_set,
> +    BPy_StrokeVertex_mathutils_get_index,
> +    BPy_StrokeVertex_mathutils_set_index
> +};
> +
> +static unsigned char BPy_StrokeVertex_mathutils_cb_index = -1;
> +
> +void BPy_StrokeVertex_mathutils_register_callback()
> +{
> +    BPy_StrokeVertex_mathutils_cb_index = Mathutils_RegisterCallback(&BPy_StrokeVertex_mathutils_cb);
> +}
> +
> +static PyObject *StrokeVertex_point_get(BPy_StrokeVertex *self, void *UNUSED(closure))
> +{
> +    return Vector_CreatePyObject_cb((PyObject *)self, 2, BPy_StrokeVertex_mathutils_cb_index, 0);
> +}
> +
> +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 +476,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 */
> Index: source/blender/freestyle/intern/python/Interface0D/CurvePoint/BPy_StrokeVertex.h
> ===================================================================
> --- source/blender/freestyle/intern/python/Interface0D/CurvePoint/BPy_StrokeVertex.h    (revision 53595)
> +++ source/blender/freestyle/intern/python/Interface0D/CurvePoint/BPy_StrokeVertex.h    (working copy)
> @@ -22,6 +22,10 @@
>      StrokeVertex *sv;
> } BPy_StrokeVertex;
>
> +/*---------------------------Python BPy_StrokeVertex visible prototypes-----------*/
> +
> +void BPy_StrokeVertex_mathutils_register_callback();
> +
> ///////////////////////////////////////////////////////////////////////////////////////////
>
> #ifdef __cplusplus
>
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers



-- 
- Campbell
_______________________________________________
Bf-committers mailing list
Bf-committers at blender.org
http://lists.blender.org/mailman/listinfo/bf-committers