[Bf-committers] Freestyle branch status report May 2012

[Campbell Barton]

@Tamito, I figured auto generation wasn't really an option here so
didn't mention it - of course if it was practical it would be a nice
time saver.

We already have gone to great lengths to get more consistent API's for
blender so Its nice if additional APIs can follow.
Though I realise we could consider freestyle an external application
and not attempt consistency.
However from looking at the code I think its worth doing to remove
duplication alone.

- You  could post link to bf-python --- api docs & some example
scripts to help others get an overview of how this works

- Review existing blender python api's in blender --- bmesh_py_*.c are
good examples of a pythonic api which closely follows internal C API.

If this is near being accepted I can help out a bit with an api update
before merging.

On Thu, May 24, 2012 at 1:54 AM, Tamito KAJIYAMA
<rd6t-kjym at asahi-net.or.jp> wrote:
> 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>
>
>
> -----Original Message-----
> 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
>> StrokeShader.getName().
>>
>> 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.
>>
>> Dan
>
> 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_setX
> -- StrokeVertex_setY
> -- StrokeVertex_setPoint(x, y)
> -- StrokeVertex_x
> -- StrokeVertex_y
> -- StrokeVertex_getPoint
> ... 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.
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers



-- 
- Campbell