[Bf-committers] Freestyle branch status report May 2012
rd6t-kjym at asahi-net.or.jp
Thu May 24 01:54:27 CEST 2012
Dan and Campbell,
Thanks for your review of the Python part. I am going to address all the
suggested changes and improvements.
The current naming convention of the Freestyle Python API is largely historical
and based on the underlying C++ class hierarchy. Previously the Python API was
auto-generated by SWIG. Then we rewrote the Python API by manual coding in
order to get rid of the C++ syntax from the Python API.
For what concerns an automatic generation of the Python API from the C++ code,
there is an application requirement. The Freestyle Python API is a wrapper of the
underlying C++ class system, and the Python and C++ layers have been connected
by a specific mechanism that allows you to define a Python subclass of a C++ class.
The present API implementation heavily relies on this mechanism, and indeed
it works very well. Implementing a similar (automated) functionality in makerna
would be a major task.
KAJIYAMA, Tamito <rd6t-kjym at asahi-net.or.jp>
From: Campbell Barton
Sent: Tuesday, May 22, 2012 6:57 AM
To: bf-blender developers
Subject: Re: [Bf-committers] Freestyle branch status report May 2012
On Tue, May 22, 2012 at 4:05 AM, Dan Eicher <dan at trollwerks.org> wrote:
> Quickly looking over the python part of the patch, there's lots of
> places where you should be using PyGetSetDef for attributes instead of
> function getters/setters -- for eg StrokeShader.name instead of
> Don't know if makesrna can do C++ but that would make it a lot easier
> than maintaining a bunch of hand written python bindings. Probably
> really depends on if the generated code is compiled by C++ or not
> because the access functions would be a breeze.
Yep, if you can use PyGetSetDef this would make a lot more sense for
BPy_FrsMaterial_methods for eg,
Now mathutils defines mathutils.Color too which could be used instead
of r/g/b accessors.
if the data isnt in DNA then wrapping like this is needed but think it
could be improved.
Would be better to do full codereview but noticed some other things.
- suffers from api clogging we had in 2.4x - eg:
-- StrokeVertex_setPoint(x, y)
... all have docstrings too - 12 definitions which could be replaced
with one attribute ".point" ? and one docstring
- PyTuple_SET_ITEM can be used when you create the tuple.
- keywords are ignored in some initializers.
- api doesnt use pep8/blender api naming conventions (lowercase +
underscores for methods and attributes, CamelCase for classes)
- scripts use import * - bad practice.
- defines "__getitem__" method rather then implementing Mapping or
Sequence protocols - PyMappingMethods / PySequenceMethods.
- bool_from_PyBool accepts any object type and doesn't typecheck -
could lead to confusing errors if "foo.bar = None" is equivalent to
"foo.bar = False" and "foo.bar = 2.0" is equivalent to "foo.bar =
True" rather then raising an error.
More information about the Bf-committers