Note for testers: Diff #1 (ID 2048) is against Git master rBf47360701436.

In view of inclusion in Blender 2.72 release I have updated the patch which is now considered ready for code review.

The patch does not touch the Cycles internal, so anyone who is familiar with the Blender internal in general is likely to find the code changes straightforward (application-specific implementation details may not appear that intuitive though).

A few notes on selected code changes of interest from reviewers' perspective:

• The only internal API change is in ntreeCopyTree_ex(), which now takes a struct Main pointer.

• Only textures are configured through shader nodes. Other line stylization parameters are specified in the GUI in the Render Layers context.

• Texture colors and alpha channels are output parameters of the shader nodes (in line with line style texture slots). These parameters are passed to Freestyle through the Line Style Output shader node.

• UV maps that allow texture mapping along strokes can be input parameters of the shader nodes by means of the UV Along Stroke shader node. The "Use Tips" option indicates that the lower half of a texture image is used as tips at both ends of a stroke.

• User-defined shader nodes (given as part of a line style) are transformed into an auto-generated node tree for the use in Freestyle stroke rendering.

• The auto-generated node tree can be obtained for interactive inspection by means of the SCENE_OT_freestyle_stroke_material_create operator (intended for debugging purposes, accessed through the spacebar search menu).

• An example.blend file
  d632-cycles-test.blend628 KBDownload
  and expected output image:
  

Could some of you guys please consider reviewing the patch? Thank you!

Hi Thomas,

Thanks for the quick response. I fully agree with the requested panel positioning and I was also concerned with that. Is there any chance that panel location within a property window can be specified in the code? My understanding is that panel location needs to be fixed manually and saved into the factory settings. I am aware of source/blender/blenloader/intern/versioning_defaults.c which has been provided to avoid error-prone manual updates of factory settings.

On the additional notes from Thomas:

Freestyle rendering for Cycles in the view port has not been implemented yet. I left it as a future to-do due to time constraints. If this is considered a bug, I would add this patch to 2.73 targets.

Removal of the "Line style settings are in the render layer tab" label appear fine to me. I will do it for inclusion in 2.72.

One more thing (sorry for message flood):

Also the Freestyle panels there are open per default, they should be closed here.

Freestyle has a global toggle button (in the Render tab) that is off by default. Unless the global switch is turned on, Freestyle panels in other tabs do not appear. Since Freestyle is an opt-in option, I think Freestyle panels should be open by default when they are requested to show up.

Thank you Thomas for the UI review. The removal of the context switch button is done in commit rB12720352a0e6.

Great, thank you. :)

Attached please find attachment

I rebuild the patch against master by doing the following:

1. create/checkout new branch based on f4484daed341, most recent merge-base
2. download D632#3 from this page
3. git apply -p0 --reject ../D632#3.patch
4. examine and fix .rej rejected patch portions
5. commit the patch
6. rebase origin/master

There were rejected patch changes in the following files
which needed to be resolved:

source/blender/blenkernel/BKE_linestyle.h
source/blender/blenkernel/intern/linestyle.c

And during the merge from origin/master had minor conflicts in these files:

release/scripts/freestyle/modules/parameter_editor.py
source/blender/blenkernel/intern/linestyle.c
source/blender/editors/space_node/node_edit.c
source/blender/freestyle/intern/stroke/StrokeRep.h

D632-cycles-test.blend renders properly.

Thank you @J Kauth (jakester488) for your effort to update the patch.
Diff #3 should apply cleanly to rBa798e01dc3ff. Sorry for the missing information.

Then the question would be: am i right this context is only used from within an operator, in terms all the code which uses this stored context is invoked ONLY from operator's exec/invoke? Or this code also might run when operator finished already (like, is it possible access to this context happens from a job?)

The stored context is used from within two operators: the rendering operator RENDER_OT_render, and the other is a debug operator SCENE_OT_freestyle_stroke_material_create (see source/blender/editors/render/render_shading.c in the patch). The same stored context is shared by the two operators, but won't be accessed after the operators have finished.

Then i think it's more clear than passing context as an argument all over the place.

Suggestion could be to set it to NULL once it's done being used, so nobody starts using it accidentally from a job or so. But that's something more like nice-to-have that something crucial.

Edit: tried to apply the patch and seems it needs an update for the current master. Mind doing this?

@Sergey Sharybin (sergey), I just rebased the patch to a very recent Git revision. Test and review of user experience are much appreciated.

@Sergey Sharybin (sergey),

I just noticed that the stored context might be used outside operator calls when view port preview is implemented (not yet implemented though). It should hence be avoided to store the context and reuse it. Also noticed that nodeAddStaticNode() does not refer to the passed pointer to the context when shader nodes are added to a node tree. Relying on this fact I removed the stored context from the Controller class, and just passed NULL to the node addition function. Please, consider review the changes. Thank you.

@Sergey Sharybin (sergey),
Concerning tests by artists, indeed calls for testers were published here and there several times, but I did not receive feedback. I am not really sure how many artists were actually testing the patch (except for @Thomas Dinges (dingto) who did patch review from a UI perspective). Hope the merge into Git master will facilitate testing by artists.

@Tamito Kajiyama (kjym3): i'm already testing that, but i have very little time because i'm on vacation. So sorry i can't help you soon.
But i think i'm not alone ^^

@Tamito Kajiyama (kjym3): in fact it will takes 10 days ^^.
So it will a little late :-P
But what i already tested (a super basic render of contour line) is perfectly working :)

Next time, please consider to squash the commits a bit, having 51 separate commits including irrelevant things like "Removed debug prints" is really not cool in my opinion. ;)

Agreed. I was afraid of making a mess in squashing commits, so went straight the safest way without history manipulation. Still, after the merge I found some not-that-nice commit logs a bit too much :( Will try to do better job next time.

My compliments to the chef!

I've been using the freestyle+cycles changes in master for several days and it's working GREAT.

My tests aren't complicated but as far as I've played with it it looks like feature parity
to the BI+freestyle renderer.

Thanks again.

Thank you for testing the patch, @J Kauth (jakester488)!

Hi!
This is my first time here in the development section of blender. I'm an artist and I downloaded the daily build in order to test freestyle for cycles. It works pretty well! Thank you!!!

There is one minor thing I noticed which could be improved:

I wanted to render the contours on a seperate layer, so I deleted the LineSet on the main RenderLayer.

If I hit render, Cycles first renders my main RenderLayer. Then it starts the freestyle process (Raytree building...) for this Layer even if there are no Line Sets! I think that is not necessary because there aren't going to be any lines at all... (In my case this wastes a lot of time, because there is an object on the main Layer which doesn't need outlines and which takes a while for freestyle...)

I hope that helps!

Greetings from Germany
Johannes

Me again! :)

Ok, I figured out, that the problem I described above only occures if I render just the main layer. If I render both layers then the freestyle process begins with the second layer and not at the end of the main Layer. Just how I expected it to work.

The main reason why I discovered this, is because I used an object which had some problems that dramatically slowed down freestyle. The object I used had some shaders applied which were originally for the Blender Renderer and not for Cycles. Those shaders also used some textures. When I switched to Cycles those shaders seemed to be diffuse shaders with one color applied - just as I wanted them to be. But in the Outliner I noticed that there is still some texture information. The result was that "Preparing Scene data" took about 5 min when freestyle started! Now that I removed those textures from the shaders it works fine and fast! ("Preparing Scene data" takes only one second now!)
Maybe this information is usefull. Thank you!