Page MenuHome

Freestyle Python API: new methods for Stroke and StrokeVertexIterator
ClosedPublic

Authored by Folkert de Vries (flokkievids) on May 23 2014, 7:52 PM.

Details

Summary

This patch adds some new functionality to Stroke and StrokeVertexIterator objects.

For strokes, the reversed method is implemented. it works the same as with other python sequence objects (returns an iterator starting from the end). this in in effect the same as Stroke.stroke_vertices_end().

For StrokeVertexIterator an incremented, decremented and reversed method has been added.

incremented and decremented return a new StrokeVertexIterator object that has been in- or decremented,
reversed returns a new StrokeVertexIterator object that will traverse in the opposite direction.

Diff Detail

Event Timeline

Folkert de Vries (flokkievids) updated this revision to Unknown Object (????).May 24 2014, 12:04 PM

The new BPy_StrokeVertexIterator(*self) used self->sv_it for copy->sv_it. incrementing one would increment the other, which is undesired.
This has now been fixed by copying the StrokeVertexIterator object itself.

Additionally, with the first version of this patch a for-loop and a reversed iterator object would be out of sync. this has now been fixed.
I also added some further comments to explain what is going on in iter_next.

Folkert de Vries (flokkievids) updated this revision to Unknown Object (????).May 24 2014, 9:03 PM

Added construction of iterators from Interface1D elements.

This provides a better solution for:
it = Interface0DIterator(iter(stroke))

the new method allows:
it = Interface0DIterator(stroke)
it = StrokeVertexIterator(stroke)

This means a cleaner syntax and prevents the creation of an extra iterator object.

Folkert de Vries (flokkievids) updated this revision to Unknown Object (????).May 24 2014, 9:05 PM
Tamito Kajiyama (kjym3) requested changes to this revision.Jun 2 2014, 4:44 AM

The patch looks mostly okay. The added constructor of iterators from Stroke instance is a good idea.

When you make a patch revision, please keep the original white space (indentation) to make code review easier.

source/blender/freestyle/intern/python/Interface1D/BPy_Stroke.cpp
313

I would suggest adding a docstring, e.g.:

PyDoc_STRVAR(Stroke_reversed_doc,
	".. method:: __reversed__()\n"
	"\n"
	"   Returns a StrokeVertexIterator iterating over the vertices of the Stroke\n"
	"   in the reversed order (from the last to the first).\n"
	"\n"
	"   :return: A StrokeVertexIterator pointing after the last StrokeVertex.\n"
	"   :rtype: :class:`StrokeVertexIterator`");
source/blender/freestyle/intern/python/Iterator/BPy_Interface0DIterator.cpp
74

The keyword argument name could be inter. Not really an issue but the camel case name seems unusual.

source/blender/freestyle/intern/python/Iterator/BPy_Interface0DIterator.h
29 ↗(On Diff #1757)

This #include should be done in BPy_Interface0DIterator.cpp because it is only necessary for the implementation.

source/blender/freestyle/intern/python/Iterator/BPy_StrokeVertexIterator.cpp
70

This first call of PyErr_Clear() is unnecessary.

source/blender/freestyle/intern/python/Iterator/BPy_StrokeVertexIterator.h
29 ↗(On Diff #1757)

This #include should be done in BPy_StrokeVertexIterator.cpp because it is only necessary for the implementation.

source/blender/freestyle/intern/view_map/Interface0D.h
203 ↗(On Diff #1757)

The added copy() method seems an over-generalization. For instance calling this method on a StrokeVertexIterator instance may result in an Interface0DIterator rather than a copy of the StrokeVertexIterator.

I suggest removing this method and using the usual new Interface0DIterator(it) syntax.

Folkert de Vries (flokkievids) updated this revision to Unknown Object (????).Jun 3 2014, 12:00 AM

Thanks TK for the review.

In line with your comments I have:

  • added your docstring to reversed
  • moved includes to the .cpp files
  • gave some proper variable names
  • removed the first PyErr_Clear in BPy_StrokeVertexIterator.cpp; I had just copied that blindly, which is obviously stupid.
  • used correct object duplication method
  • fixed problem with whitespace

I think this patch is complete now. I've implemented the new features from this patch into the python scripts locally already, and it all works nicely.

Thanks for the update of the patch. Now everything looks okay to me.
I will merge the patch to Git master for inclusion in the 2.72 release and close T37839.

source/blender/freestyle/intern/python/Iterator/BPy_Interface0DIterator.cpp
79

Looks like I forgot to point out this redundant call of PyErr_Clear().

Folkert de Vries (flokkievids) updated this revision to Unknown Object (????).Jun 14 2014, 10:45 PM

remove redundant call to PyErr_Clear()