Page MenuHome

Freestyle API changes and further scripting integration
Closed, ResolvedPublicDESIGN

Description

This is a continuation of the conversation in the blenderartists freestyle thread.

The goal is to update the freestyle python API now that some breakage of backwards compatibility is allowed with the upcoming 2.70 release. The current proposal is to let the updated freestyle module work more like blender's bpy module. The current freestyle module will internally be named _freestyle. this module will be extended by python with the python-defined objects (eg. shaders, predicates, chainingiterators).

so far, the following structure is somewhat agreed on

  • freestyle, base module, possibly container for objects that fit nowhere else
  • freestyle.iterators, iterators of all kinds
  • freestyle.predicates, all predicates for both 0D and 1D elements
  • freestyle.functions, all freestyle functions for both 0D and 1D elements
  • freestyle.shaders, all freestyle shaders
  • freestyle.utils, helper functions and generators

The goal of this project is to provide more structure to the freestyle module, whilst at the same time modernizing both code style and comments/descriptions. There are also some ideas already on how to integrate freestyle scripting with the parameter editor.

Revisions and Commits

Event Timeline

Folkert de Vries (flokkievids) raised the priority of this task from to Normal.
Folkert de Vries (flokkievids) updated the task description. (Show Details)
Folkert de Vries (flokkievids) edited a custom field.

for completeness:

TK said:
flokkievids,

(...) Now that we are in BCon1 for the upcoming 2.70 release, it is appropriate to do the module reorganization. I am thinking of the following structure.

freestyle - classes for internal data structures (e.g., viewmap components)
freestyle.iterators - iterators of all kinds
freestyle.predicates - unary and binary predicates for both 0D and 1D elements
freestyle.functions - functions for both 0D and 1D elements
freestyle.shaders - stroke shaders
freestyle.utils - helper generators

The reason why keeping predicates for 0D and 1D elements in one module is that all predicate classes have a suffix (e.g., U1D) indicating whether it is unary or binary and whether it works on 0D or 1D elements. There is no risk of name conflicts, so there is no much point to store *U0D, *U1D and *B1D in different modules. The same consideration applies to functions as well.

Comments on this restructuring plan are much appreciated.

flokkievids said:

TK

I agree on the structure idea. some comments:

you missed Nature and Noise. I would propose to keep freestyle.natures as a module and to get rid of freestyle.Noise in favour of mathutils.noise.
In your current proposal "dir(freestyle)" would still give quite a long list of objects. I would prefer to keep this list as small as possible. maybe adding freestyle.types for storing the core data structures would be an option?
what are your thoughts on integrating python- and c-defined objects (shaders, predicates, ect.)?

Continuing on my last comment, it would be very nice if style modules or rather modifiers could be registered like addons. I'm not quite sure whether this would be feasible (now), but in my opinion it would provide a real use for the freestyle API again: creating your own modifiers. using the restructured API they are quick to develop, yet flexible for the (non-programmer) artist.

Also, I would love to help with rewriting the shaders/functions/predicates in conform the new API.

finally, since this is becoming quite technical, we could continue this conversation on the new developer.blender.org freestyle project page.

TK said:

The idea is to rename the present 'freestyle' module to _freestyle, and classify its contents (classes) into sub-modules by importing them from the submodules. Python classes also go into the submodules. In the end the new 'freestyle' module will have 28 classes for core data structures in addition to the proposed 5 sub-modules.

I like the idea of having 'freestyle.types' to keep the core data classes. Technically speaking, the Freestyle Python API is a set of Python classes (also referred to as types), so if we have 'freestyle.types', basically everything should go into there. Maybe module names such as 'freestyle.types.iterators' and 'freestyle.types.predicates' are a bit redundant.

If we store the core data classes in the 'freestyle' module, then we will have freestyle.Nature.SILHOUETTE and so on. I guess a separate 'freestyle.natures' module may also appear redundant.

What do you think about these aspects?

User-defined modifiers would be nice to have. I also plan to include in the 2.70 release an extended scripting support in the Parameter Editor mode. The idea is to allow users to write Python code segments in such a way that they can be integral part of style settings manipulated through the GUI.

I agree to move on to developer.blender.org Freestyle project to further discuss these and other future updates.

TK

I think i'm getting kind of stuck in the naming conventions here.
since freestyle.Nature holds objects (real classes), I think it would be a submodule, whilst StrokeVertex only holds its own methods and therefore would be a normal object. is that incorrect?

secondly, my proposal to add a freestyle.types (or something similar) was meant to let freestyle hold less objects (which would mean "dir(freestyle)" would return a relatively short list). iterators will be stored in .iterators, shaders in .shaders, and the core data structures (StrokeVertex for example) could be stored in freestyle.types, -.structures or -.containers.

thirdly, do you agree on dropping freestyle.Noise in favour of mathutils.noise? freestyle.Noise is at the moment (as far as i'm aware) only used in shaders.py and could be easily replaced with mathutils.noise (optionally, freestyle's perlin noise in 1 and 2 dimensions could be put into mathutils as well)

finally, as I mentioned in the top post, to me it would seem logical that whilst we're at it, the documentation/docstrings/comments are also updated. some are quite technical or not up to date. Also, could you give some examples of your scripting integration idea?

flokkievids,

Many thanks for the creation of the task!

The freestyle.Nature class has a set of its instances (such as Nature.SILHOUETTE) as class members. The class object has been used as a container to organize the instances that are used as pre-defined constants. Another example of classes used as containers is found in freestyle.IntegrationType (storing integrationType.MEAN etc. as class members).

I see the point of having the 'freestyle.types' submodule to store the classes for core data structures. Two questions in this regard:

  1. The number of objects that will go into 'freestyle.types' is about 30. Do you think they are too many to stay in the top level 'freestyle' module?
  2. If we have 'freestyle.types', where would you store those classes such as UnaryPredicate1D and ChainingIterator from which users define their own stylization rules by subclassing? One option is obviously freestyle.types, and another is to store them in other submodules such as freestyle.predicates and freestyle.iterators.

Concerning the removal of freestyle.Noise, I feel like I need to look into the code more closely. There are several places in the C code base where the Freestyle-specific noise class is used, and they need to be rewritten properly using the corresponding C API for mathutils.noise. A major difference between freestyle.Noise and mathutils.noise is that the latter requires a global random number generator seed used by all of the noise generation functions. In contrast, freestyle.Noise takes a seed when instanciated, and the present implementation of randomness-based geometry modifiers relies on this API design. Which to select seems to have impacts on user control. The present per-modifier seeds may provide users with a finer control on visual results.

Fully agreed on documentation updates.

For what concerns a better scripting support in the Parameter Editor mode, the basic idea is to provide users with additional GUI elements for specifying a Python script in a text data block to control stroke stylization. For instance, in addition to the present fixed set of options for feature edge selection (based on edge types, visibility and so on), a new feature edge selection option based on Python scripting is introduced, where a text data block ID is specified by the user (just like style modules in the Python Scripting mode). The text data block is supposed to define a module variable (say, 'predicate') whose value is a 1D unary predicate instance used as edge selection criteria, as illustrated below:

__predicate__ = OrUP1D(AndUP1D(NotUP1D(FaceMarkOneUP1D()), pyNatureUP1D(Nature.SILHOUETTE)), ContourUP1D())

This predicate selects silhouettes not masked by face marks, as well as all contours regardless of face masks (see [[ https://twitter.com/blenderyard/status/400754659930816513

this tweet ]] for more detail). At the moment this kind of complex selection conditions is difficult or impossible to specify through combinations of available edge selection options. The fine-grain scripting support will allow users to take advantages of both the interactive stylization through the Parameter Editor and in-depth exploitation of Freestyle programability.

I would create a separate task for discussing the new scripting component since this topic is quite independent of the present discussions on the reorganization of Freestyle Python modules.

TK

Thanks for explaining. I would propose to keep freestyle.Nature and -.IntegrationType at the module level. I don't think a user would ever subclass those (and even if so, that is still possible) and really there is no other logical place for them to go.

I indeed think that 30+ object at module level is a lot. if you look at bpy, using "dir(bpy)" is a relatively short list containing clear names.

If we follow bpy, the base classes (StrokeShader, UnaryPredicate1D, ect.) should indeed go into freestyle.types (just like for instance bpy.types.Operator). this will also mean that freestyle.shaders only contains shaders that actually work, and not the base class. I think that is quite a clear structure.

Using scripting for selection seems very useful. as you said, the ease of a GUI with the control of scripting.

EDIT

currently doing some tests on 'utils.iterator', which brings up a problem from some time ago: at the moment I see no possibility to clone a user-defined chaining iterator.

this is my current prototype

def iter_adjacency(it:AdjacencyIterator):
		# make local copy
		it = AdjacencyIterator(it)
		while not it.is_end:
			yield it.object, it
			it.increment()

using this function both the iterator element and the iterator object are available.

working with a local copy of the iterator seems the safest way, though copying a user-defined chaining iterator seems not really possible at the moment. maybe we could also think about a generic Iterator.copy and generalize above function to work on all freestyle iterators.

what are your thoughts on this?

flokkievids,

In what context do you need to clone user-defined chaining iterators? Chaining iterators are usually used through the chaining operators such as Operators.bidirectional_chain(), and you don't have to clone them in typical application scenarios...

Here is a patch set for reorganizing Freestyle API modules in a hierarchical package structure. The patch set consists of two patch files, which need to be applied one after another in the order of file name serial numbers.

The package structure is a bit updated as follows:

  • freestyle - top-level package.
  • freestyle.types - classes for core data structures, and base classes for user-defined stylization rules (i.e., chaining iterators, predicates, functions, and shaders).
  • freestyle.chainingiterators - predefined chaining iterators.
  • freestyle.predicates - predefined 0D and 1D predicates.
  • freestyle.functions - predefined 0D and 1D functions.
  • freestyle.shaders - predefined stroke shaders.
  • freestyle.utils - utility functions for style module writing.

The top-level 'freestyle' package is mostly just a container of the 6 submodules. The only exception is that it also contains the Operators class. To me the top-level module is the most logical place to store it. Since this class is required by all Python style files, it deserves a special location. Another reasonable location to store it would be the 'freestyle.types' submodule. An advantage of this latter option is that users may have one less import statement when they import something from 'freestyle.types', saving a bit of typing during style module writing. Anyway, the present implementation is tentative and is a matter of discussion.

For the ease of further discussions, below I show freestyle/styles/qi0.py as of this writing:

from freestyle import Operators
from freestyle.chainingiterators import ChainSilhouetteIterator
from freestyle.predicates import (
    NotUP1D,
    QuantitativeInvisibilityUP1D,
    TrueUP1D,
    )
from freestyle.shaders import (
    ConstantColorShader,
    ConstantThicknessShader,
    SamplingShader,
    )

Operators.select(QuantitativeInvisibilityUP1D(0))
Operators.bidirectional_chain(ChainSilhouetteIterator(), NotUP1D(QuantitativeInvisibilityUP1D(0)))
shaders_list = [
    SamplingShader(5.0),
    ConstantThicknessShader(4.0),
    ConstantColorShader(0.0, 0.0, 0.0)
    ]
Operators.create(TrueUP1D(), shaders_list)

Any comments on design and implementation detail are very welcome.

TK

The patch seems really nice. the importing, although quite chaotic still, is a whole lot more structured.

first of all, I guess this is a small mistake, the shaders sub-module is not imported in the init-file. Also, the freestyle module sits in the /freestyle folder, and not in the /modules folder. what's the rationale behind this?

Furthermore, it would propose to either put Operators into freestyle.types, or to store Nature and MediumType in the top-level package as well.

about cloning user-defined iterators, it seemed nice as a concept (to generalize all freestyle iterators), but in practice it has little use. EDIT: cloning user-defined iterators is/was also necessary for implementing it.incremented() correctly.

I've created the following generator for iterating over the iterator object and the iterator's current object.

def iter_obj_iterator(it):
    """
    Yields the current object of the iterator
    and the iterator object itself
    """
    try:
        it = it.__class__(it)
    except TypeError:
        # user-defined object
        pass
    while not it.is_end:
        yield it.object, it
        it.increment()

this along with a helper function yielding only the current object are all the basic helper generators we need I believe. of course there are still some other, more specific helper functions, among others the helper functions present in parameter_editer.py.

I'm not quite sure yet whether the cloning here is right/useful enough. sometimes it'll safe some extra typing (avoiding long lines), often it will make no difference...

As a next step I think having a core developer check on the proposal would be nice. Also we can establish what helper functions we want to include in utils.py (I have some suggestions/prototypes already), which will be the foundation for further rewriting.

EDIT
disregard what was here previously, it was based on a dated test file.

I did find out however that using a try/except code block a for-loop can be made functional (like a while loop). Preferably the both methods of an AdjacencyIterator would raise StopIteration if the object is invalid (at its end), but the StopIteration exception doesn't seem to have the correct scope and is raised and not catched by the for-loop.

Thanks again flokkievids for code review and updates. Here I have attached a new patch set that incorporates your suggestions and code changes (the latter with minor modifications). The Operators class has been moved to the 'freestyle.types' submodule from the top-level 'freestyle' module. I believe the API updates are ready for a code review by other core developers (especially by Campbell) to evaluate if the patch set can go into the trunk (likely in 2.71 instead of the upcoming 2.7 release).

Code review requested through D163.

to get some more experience with the new api and have a look at the implementation of freestyle modifiers, I made a generalization of the blueprint modifier, that bundles all its features into one shader. this shader allows the creation of a polygon with a specified number of vertices, specify the backbone length, draw several rounds and randomize the points. in combination with the 2d-transform shader all features of the current blueprint modifier are present.

an idea for a util function also came up:

def stroke_extremes(s:Stroke):
    p_min, p_max = s[0].point.copy(), s[0].point.copy()
    for svert in iter(s):
        p = svert.point
        p_min.x = min(p_min.x, p.x)
        p_max.x = max(p_max.x, p.x)
        p_min.y = min(p_min.y, p.y)
        p_max.y = max(p_max.y, p.y)
    return (p_min, p_max)

Addition

I'm currently trying to rewrite (read: bring back to life) the TVertexOrientationShader.
After some reshuffeling I am really doubting this ever really worked and am still unsure of its use (why would I want to make alterations to a StrokeVertex' attribute based on the orientation of its TVertex?)
To me it seems that the 'normal' orientation and a check for the TVertex nature would do as well.

also, I see that there is a difference between '++it' and 'it++'. could this be a solution for the problem with for-loops?

@Bastien Montagne (mont29)

It sort of depends on how broad you interpret the title. I've still got D319 waiting, which won't go in before 2.71. It is an update mainly to the code style of the freestyle API (with very little functional changes).
IMO when D319 gets in the current api project is really finished. @Tamito Kajiyama (kjym3) also planned on working on extending the api, integrating it with the parameter editor, so there are still some things in the pipeline concerning this task.

guess it's up to @Tamito Kajiyama (kjym3) to decide whether it is useful to keep this task open.

I agree with @Folkert de Vries (flokkievids) and propose to keep this task open until patch D319 is merged into Git master. The patch has already been partly reviewed, but I still need to complete the review (since the patch is quite big). Plus, we are in BCon4 now. I will work on D319 for inclusion in 2.72.

Tamito Kajiyama (kjym3) edited this Maniphest Task.Jun 9 2014, 5:29 AM
Tamito Kajiyama (kjym3) changed the task status from Unknown Status to Resolved.Jun 24 2014, 6:54 AM

Closed by commit rBce729677db3e.