Page MenuHome

Freestyle line rendering in Cycles
ClosedPublic

Authored by Tamito Kajiyama (kjym3) on Jul 6 2014, 4:15 AM.

Details

Summary

This patch provides Freestyle with basic support for the Cycles rendering engine.

Freestyle option panels in the Render Layers tab are now available when the current rendering engine is set to Cycles.

The patch is still work in progress and not finished. Testing from both users' and developers' view point is much appreciated.

User-visible limitations:

  • Material color/alpha/thickness modifiers won't work properly, except for the default diffuse color when Cycles shader nodes are not used.
  • Textures are not available (actually they are, but through the texture option panels accessible only when the rendering engine is set back to the Blender Internal).
  • Preview rendering in the 3D view port is not supported yet.

Notes for developers:

  • The Blender Internal is used for stroke rendering by default. Cycles can be used for that purpose by defining USE_CYCLES_FOR_STROKE_RENDERING in source/blender/freestyle/intern/blender_interface/BlenderStrokeRenderer.cpp at compile time.
  • For testing purposes, the material used for stroke rendering with Cycles can be created manually by the "Create Freestyle Stroke Material" operator. Just invoke it through the spacebar search menu in the 3D view port, and you will be able to choose the stroke material in the pull-down material selector. This way you can browse the shader node setup used for stroke rendering. Updates of the stroke material must be done in BlenderStrokeRenderer::GetStrokeMaterial().

Major to-do items:

  • Devise a way to make Cycles materials and textures accessible from within Freestyle line rendering options.

Diff Detail

Event Timeline

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

Tamito Kajiyama (kjym3) updated this revision to Unknown Object (????).Jul 9 2014, 3:52 AM

Rebased the patch on Git master rBf4484daed341. Per-material Freestyle line colors can be defined through the new Freestyle Line panel of the Material properties context (rB7915d7277ac8). Use the Material color/alpha/thickness modifiers to use the per-material line colors for stroke rendering.

Proper support for node-based textures has been added.

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 and expected output image:
Thomas Dinges (dingto) edited edge metadata.EditedJul 21 2014, 9:57 PM
Thomas Dinges (dingto) requested changes to this revision.

Screenshot: http://www.pasteall.org/pic/74285

Tested it quickly, and I am unhappy about panel placement.

First of all though, I know that the UI panel order is a bit random, thats not a direct issue of the patch.
However, I feel like generic panels should always be above specific ones per default.

Properties Editor:

  1. Material tab: "Freestyle Line" should be put beneath all other ones.
  2. Layer Tab: The 3 Freestyle panels should be beneath "Layer" and "Passes". Also the Freestyle panels there are open per default, they should be closed here. Otherwise the user has to scroll a lot until he gets to the generic ones.
  • Also on a related note, I really like to see that "Line style settings are in the render layer tab" label gone. We also have Motion blur settings in the render and object tab for example, and no label. That is a matter of user documentation of the workflow. We had this discussion already when Freestyle was added, and if I remember correct we agreed on removing it after a while? :)
  • Freestyle does not work in the Cycles Viewport. It works with BI. Is that on purpose or a bug?
This revision now requires changes to proceed.Jul 21 2014, 9:57 PM

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.

Thomas Dinges (dingto) edited edge metadata.EditedJul 22 2014, 9:15 AM
Thomas Dinges (dingto) accepted this revision.

Hi Tamito, thanks for the response.

  1. You are correct, as the user first needs to enable Freestyle, the panels then can stay open in the Layers Tab.
  1. Panel placement: I am not really sure how to fix this. Maybe @Campbell Barton (campbellbarton) has an idea? In the early 2.5x days, we manually specified the order by having a list with panel IDs. Today it's simply a matter of the declaration. The panel that is declared first, gets shown first. As Cycles comes *after* all the BI / Freestyle panels, they are shown afterwards.

A solution here would be nice, but I don't consider this a stopper on second thought, as indeed the user first needs to enable Freestyle (same as point 1), so it's not a problem.

So from a UI perspective, I am fine with it (once the label gets removed). Other code can be reviewed by Campbell and Sergey. :)

This revision is now accepted and ready to land.Jul 22 2014, 9:15 AM

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

Attached please find attachment

which is D632#3 rebased to work against current master.
(It is a gzipped patch)

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.

source/blender/freestyle/intern/application/Controller.h
220 ↗(On Diff #2171)

I don't really think storing context is safe here. What is it for?

source/blender/freestyle/intern/blender_interface/BlenderStrokeRenderer.h
64

Same as above.

source/blender/freestyle/intern/application/Controller.h
220 ↗(On Diff #2171)

The stored context is used to create a shader node during Freestyle line rendering.

It is recalled that Freestyle generates a temporary scene populated by mesh objects that represent stylized strokes. The auto-generated scene is then rendered using the same rendering engine with the input scene (i.e., Cycles). When textures are applied to stroke meshes, a shader node tree is also generated as part of the meshes. The stored context is eventually passed to nodeAddStaticNode() called within BlenderStrokeRenderer::GetStrokeShader().

It would be possible to avoid storing the context and instead to pass the context through from the rendering operator to Freestyle every time the rendering process is started. As far as I tested, however, struct bContext is a singleton and only one instance of it is created and used within the entire Blender code at run time.

The context has been stored also in the PythonInterpreter class (through the PythonInterpreter::setContext() method called at line 212 of Controller.cpp below) in this case to get access to the ReportList by CTX_wm_reports(). This has been since almost the beginning of the Freestyle integration project.

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?

Tamito Kajiyama (kjym3) updated this revision to Diff 2248.

Rebased the patch on Git master rBc85265b4551acbc7adf41f47f612373e42e0176d. Also fixed some merge glitches.

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

Made revisions according to the review comments on the stored bContext. Also fixed a couple of issues and rebased the patch with Git master rB3e607f61b580.

  • Freestyle: Removed the stored bContext from the Controller class.

The stored context object was used for creation of shade nodes, but actually the context passed to nodeAddStaticNode() is not used when adding shader nodes to a node tree. Relying on this fact, now a NULL pointer is passed to nodeAddStaticNode() instead.

  • Added support for Feestyle edge/face marks in the 3D view window in Cycles.
  • Fix for wrong idname of the "Create Freestyle Stroke Material" operator.

@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.

source/blender/freestyle/intern/blender_interface/BlenderStrokeRenderer.cpp
448

Maybe either uncomment or remove?

496

Remove dead code.

source/blender/freestyle/intern/blender_interface/BlenderStrokeRenderer.cpp
448

Yes, I will remove the #if 0 block.

496

This will be removed as well.

Sergey Sharybin (sergey) accepted this revision.

LGTM, but consider applying this http://www.pasteall.org/53380/diff

Can't really test functionality-wise because not so much familiar with this thing, but guess you already had artists who tested this?

Tamito Kajiyama (kjym3) updated this revision to Diff 2412.

Just to get the patch prepared for merge into Git master.

@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 ^^

Patch merged into Git master. Thank you @Thomas Dinges (dingto) and @Sergey Sharybin (sergey) for the patch review!

@Lapineige,
Glad to know you are interested in testing the code. Any feedback is much appreciated, but please feel free to take your time.

@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.

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!