Page MenuHome

Cycles Strand Rendering
Closed, ResolvedPublic

Description

This is a first version of a strand rendering patch for cycles that I have been working on. It's only really here for debugging and to discuss implementation since there are plenty of aspects that require further work. Currently its features include :-

- Exporting of hair data (including child strands) into cycles. The `draw step' parameter gives control over the resolution.
- These are stored as a strand segment primitive, which are straight lines between two locations. These are stored in the mesh data.
- The bvh has been adjusted to include this strand primitive and intersections are calculated to model the segments as cylinders with a changing cross-sectional radius.
- A `Strand Info' node has been added to provide information in the material information. This includes a 0 - 1 floating point value that describes the location along the strand, the strand segments normalized direction, flag to say the ray has hit a strand, the radius of the strand and the tangent normal.
- The proper normals are calculated but instead a `tangent normal' can be used which causes the strand to behave as if it's a plane inclined towards the ray.
- For intersection testing I have provided various mode. One calculates an accurate intersection point for every strand. I expected this to be slower although it doesn't make a considerable difference. However, it does have accuracy issues that lead to a darkening of the results. There's an option provided to ignore the current strand for for the next bounce. This is perfectly suitable for non-glass materials. To speed up intersections I have also provided modes that ignore the strand width a points to avoid having to perform a square-root.
- The first UV map is given by the texture coordinates node.

In terms of problems and missing features. It requires :
- A full implementation of attributes. This would then enable vertex colours, the use of other UV maps, motion blur and generated coordinates.
- The strand storage is far from optimal. I initially used more data than is necessary and this needs rewriting.
- At various points I make a few approximations that need removing.
- The segments are currently straight and don't interpolate the tangent. For very thick strands with a high curvature, gaps can appear. I hope to add a tangent value into the data that would allow this issue to be solved and open the door to curve segments.
- There is a mode to export the strands as triangles in cycles. Either as a ribbon or geometry around the strands. Currently, this doesn't work well and isn't complete. I have disabled it in the current patch but it can easily be re-enabled by adjusting a line in the main sync_hair function.
- I've not even considered some functionality such as motion blur.
- It would be much better to enable child creation during rendering.
- No strand specific scattering functions have been added.
- Currently, all particle systems are rendered with the same parameters (such as shape). Ideally, this would be different for different systems. The size parameter in the particle system does allow variation of strand width though.

I'm certain I have missed loads of details and it's highly likely that there are memory errors. So, I only suggest that this is used for debugging and you use it at your own risk. Any help pointing out such errors would be greatly appreciated. Hopefully, there's some code in here that can be used. I am happy to maintain it and will shortly create a post on blender artist with a few examples.

Details

Type
Patch

Event Timeline

There are definitely bugs with emitting materials that use `sample as light'. So, I recommend not doing this for now.

"- The segments are currently straight and don't interpolate the tangent. For very thick strands with a high curvature, gaps can appear. I hope to add a tangent value into the data that would allow this issue to be solved and open the door to curve segments."

Maybe i can help you with that by writing down the theory, to quickly find the intersections with an curved cylinder segment. I just would need to know how you would define the curve segment and the corresponding radii.

@Tobias - yes, that would be really useful. I have tried to research this myself briefly. It will probably be awhile until I get around to implementing curved segments though. I intended to use either quadratic or cubic bezier curve segments with the strand volume being all points that are less than one radius from any point along this curve.

Yes that is what i thought. But one problem is the convention. If you use splines then they have to be continuous and the radius needs also to be interpolated in a continuous way. Thats means that i would need to know the tangent vector of the curve at start and end, together with the radius, and additional control points for the curvature. To be as precise as possible i would suggest you define a function header with all necessary data you can provide (including a description of that parameters) and what the function should return as the result(s).

I've added an updated version that removes three simple errors.

can you support scons as well?

Yousef, I will look into it. Tbh I have only ever used CMake.

Tobias, yes I agree. The function would likely be in the format of `bvh_curve_intersect(... ray direction vector, ray initial point, curve start and end, curve start and end radius, curve start and end tangents+scaling parameters for curve control points). It would need to calculate the distance along the ray to the closest intersection point. If there are some simiplifications for speed or extra data required then it should be possible to supply it.

I have also added a newer version that deals with many issues people mention including a strand circle producing precision issue. Currently, there's also a large slow down with CUDA.

well actually i tried to support it myself, but i found that it doesn't need any mods, as by default everything in (intern/cycles/blender..nodes) is included to build, i tried it today again and it did not give any errors and the patch is working as expected, thanks anyway.

i will try the new patch as well.

Regarding the curve intersection, here's a piece of code that evaluates position and tangent the same as the strand curves that are used for blender internal. Cardinal is the most used one, which is a cubic bezier curve. There are no special tangent vectors supplied, they are derived automatically from the curve position points, it's just the derivative w.r.t the t value that is used to compute them.
http://www.pasteall.org/38117/cpp

Quadratic bezier curves or some other curve type may be easier to intersect though, it's difficult to give the spec for that in advance, it depends on how efficient the ray intersection code can be made.

Cheers Brecht! I am just starting to look at curves. This will be very helpful.

I have also just posted a new updated version that fixes a few small issues and cleans up some of the code.

Here's an initial review of the code, overall I think it's fine but in a big patch like this I can always find a bunch of stuff to comment on. I didn't review the RNA / blender export code yet.


Naming
* This is a bit pedantic, but the name "strand" is not commonly used in other render engines for this. It's called this way in blender internal, and I regret naming it that way a bit, maybe it would be good to rename "hair" and "strand" to "curve" in this patch?

Strand Settings
* As discussed these should become particle settings instead of scene properties. I guess this requires changes all the way to the kernel code.
* I can help figuring out the animation keying crash, would not consider that a major issue though.
* The exact properties that need to be here I'm not sure about yet, probably after more experimenting this can become more clear.
* Some of the property names could be change, I think the strand_ prefix is unnecessary in most cases because they are already in CyclesStrandRenderSettings.
* Generally abbreviations should not be used in property names, "trimesh", "linemeth", "interp" could be changed.
* The smooth property should have a "use_" prefix like other booleans.

Strand Info node:
* Strand? should be renamed to "Is Strand" (like the light path node)
* Intercept could be renamed to something easier to understand, but I don't have a better suggestion yet.
* Radius: name is probably ok, but Thickness might be easier to understand? Ideally the particle setting and node socket names would match.
so maybe Size could be renamed in the particle UI.
* Tangent & tangent normal: can the geometry node tangent and normal outputs give these instead of having them here? Though I'm not really sure what the tangent normal is?

Storage
* I'm not sure yet if strands should be stored in Mesh or their own dataype. For now it's ok, but maybe I'll refactor it later so we have a Geometry base class from which a Mesh and Curves class is derived and change MeshManager to GeometryManager.

Code Style
* Convention is to put { brackets on the same line as if(), some code doesn't follow this.
* Some code in kernel_shader.h is not properly indented
* Rename Strandkey => StrandKey, strandkeys => strand_keys, Strand_Data => StrandData, strandinfo => strand_info

BVH
* prim_type: since this only takes one bit it would be good if we could stuff that in somewhere.
** we can leave this the way it is for now, just some ideas on how to do this:
** maybe we could enforce putting only strands or triangles in leaf bvh nodes, and then it can be stored in the leaf node
** the kernel_bvh check can then become if(primAddr >= 0) { triangles } else if(..) { hair } else { object }
** light sampling still needs some way to store it too then

Kernel
* Can the new hair strand code in the kernel be put behind #ifdef __HAIR__? Probably it's best to keep it disabled for CUDA still.
* kernel_types.h: #ifndef __KERNEL_CUDA__ for motion blur should not be there?
* strand_light_sample could use some more comments about what it's doing
* OSL nodes need to be updated too (I can do this too)
* in light.cpp, why is LIGHT_STRAND put in light_data, I don't think it's every read from there? probably some left over testing code?
* attributes: this is WIP anyway, but eventually the triangle_attribute_* functions should get equivalent strand_attribute_* ones, and the entry point for this from SVM or OSL should then be just attribute_* functions that internally call either the triangle or strand version. In various places the (sd->strand != -1) check could then be removed.

I didn't review the very latest patch, so if anything there is already fixed please ignore it :)

Thanks for the comments Brecht. The only ones I addressed in the update were the __KERNEL_CUDA__ and __HAIR__ ones. I will have a quick go through them tomorrow, fix some and try to come up with a few suggestions. They all seem very reasonable.

Note: I have just uploaded another version as there was an issue compiling on mingw.

Hi Brecht, I have gone through most of your comments and posted an updated patch. In regards to each one

Naming
- I renamed most of the properties and variables so that they are hopefully clearer. I've also renamed the internal variable so that they refer to curves rather than strands or hair. The UI still refers to hair rendering though.

Settings
- I re-introduced the particle settings code and it actually seems to work fine without the animation keying crash that previously occurred. I will see if any other problems occur with more testing.
- The strand prefix has been removed.
- I have removed the abbreviations.
- All of the boolean properties now use the "use_" prefix.

Strand Info node
- I have actually renamed this as Hair Info as I thought this was slightly clearer.
- "Strand?" has been changed to "Is Strand".
- Radius has been changed to "Thickness" and now returns the diameter of the strand.
- I have removed the Tangent output. The Tangent normal is still there though as this isn't returned by the geometry node. We can remove it later if you think it's un-necessary? The tangent normal simply refers to the vector obtained by removing the component along the tangent from the reversed incoming ray vector. When used the shading behaves as though the strand is a plane that is always tilted towards the incoming ray.
- I don't really know of any good suggestions for "intersect" yet. One suggestion I received was `uvW'? Another possibility could also be to add a "strand" texture coordinate to the texcoord node? If the "Is strand" and tangent normal were moved to the geometry node, the Hair info node could also be completely removed. There wouldn't be any obvious place for "Thickness" though.

Storage
- I agree it would be good to have the strands stored in a separate datatype. This would probably simplify the hiding of the emitter and updates.

Code Style
- I that that I have put all `if()' and `for' code into the conventional form.
- The kernel_shader.h code should now be correctly indented.
- I have renamed the variables as instructed (although they have undergone further changes with "strand" in all these cases changed to "curve" and strand_info => curve_attrib)

BVH
- I completely agree that prim_type should go somewhere. I thought a short term fix could be to put it somewhere in prim_visibility? Not sure if this is possible though and it's not particularly tidy.

Kernel
- I have put #ifdef __HAIR__ around all of the new non-essential code within the kernel.
- I removed the #ifndef __KERNEL_CUDA__.
- I haven't looked at the light sampling issues yet.

I was also curious about your thoughts on the ideal final format to store the strands in. Currently, I tried to mirror the triangle storage but this is not optimal. I also initially stored more information in the triwoop structure that doesn't seem to be necessary now. It's probably better to wait until attributes are working and more features are implemented before changing this though.

It would be great if you could update the OSL nodes. I'm not familiar with this at all. Let me know what you think and what plans you have.

Cheers,
Stuart

Again, it compiled fine on MSVC without showing an error... but not gcc. I have posted an update that now also compiles with gcc.

Changes look good, some more comments:

Experimental
* Could you put this feature behind the experimental option? Mainly that means hiding the UI panel, and disabling export of hair in blender_object.cpp in case experimental is false.

Kernel
* I'd enable __HAIR__ only for the CPU now, so I'd move that #define just below __NON_PROGRESSIVE__ which is also CPU only.
* It seems the #ifdef __KERNEL_CUDA__ sneaked back in in the latest patch?
* I'd expand some of the abbreviations in enum CurveFlag, CURVE_TGNORMCORR is pretty cryptic.
* enum CurveFlag needs to use typedef like other enums, OpenCL needs this.
* I prefer to not indent preprocessor macros, e.g. #ifdef __INSTANCING__ is indented

RNA
* abbreviations should not be used for function parameters, i.e. ob => object, psmd => modifier, pa => particle
* parameters generally go from broad to specific, so instead of "particle, modifier, object" the order should be "object, modifier, particle"
* rna_ParticleHairKey_co_plus: I'd rename this to co_object, to indicate this is the coordinate in object space.
* rna_Particle_UV, rna_ParticleSystem_UV: maybe rename this function to uv_on_emitter (uv also should be lowercase for consistency). The function also defines "float n_uv[3]" but only gives array length 2 for the parameter later on.
* rna_ParticleSystem_co_cache: maybe rename to co_hair
* rna_ParticleSystem_drawing: this flag doesn't seem to actually do anything. It's set in blender code but not read anywhere, so that can be removed.
* there some indentation inconsistencies in rna_ParticleSystem_UV, and some code style "a=b" should be "a = b".

Other
* curves.h still uses __HAIRS_H__
* the enums in curves.h should either get a CURVE_ prefix or be put inside the namespace of a class. 'TRIANGLES' or 'CAMERA' could clash with other defines.

Regarding strand storage in the BVH, I'm not sure yet. I wanted to change the triwoop stuff as well at some point, it's fast but also memory intensive, ideally I could find a fast ray intersection that can work with just triangle vertex indexes and vertex coordinates.

That's about it at the moment, mainly it's important to put this behind experimental, disable hair on the GPU to avoid breaking performance there, and get the RNA properties and functions names consistent so we hopefully don't have to change them later. Once that is done I think it's ready for trunk and work can continue there.

I've also added commit rights on the bf-blender project for you now, and there should be a mail from Ton in your inbox about that. If you keep working this quick I guess early next week things could be ready to commit.

Hi Brecht,
I have made those changes you mentioned. I have put most of the kernel code behind ifdef__hair__ so it shouldn't make any difference when compiled for the CUDA. One place where the code is still slightly reordered is in shader_setup_from_ray in kernel_shader.h. I can't see that this would make any difference but can you just check? I could always just use an #else with the original code.
The only feature that I'm not sure how to disable for cpu only builds and experimental mode are the hair info node. Does this need to be done?
In regards to LIGHT_STRANDS, I was using this as means to indicate that the sample is a strand without having to add any further data. This was still causing an error when sample as light was used in conjunction with intersect so I decided to disable `sample as light' for all strands. The code to 'sample as light' is still included although I could revert these files?
Hopefully all the changes are ok? I'm not sure how much time I will have over the coming days but hopefully it can be ready to commit by the end of the week?

Merry Christmas!

There were some mistakes in the previous version I just corrected. These include:
1. The hair was being added to the mesh even in the experimental GPU mode. I have corrected this by adding a `is_cpu' variable to Blender_sync and using this, with `experimental', to prevent sync_curves running.
2. The UI was also appearing in experimental GPU mode and this shouldn't be the case now.

Hopefully this works.

Regarding the Hair Info, indeed there is no simple way to disable it. Just leave it always enabled.

In blender_mesh.cpp, change
if(!hide_tris) {
to
if(!(hide_tris && experimental && is_cpu)) {

It's ok to keep the disabled sample as light code. But there's one piece of code in light.cpp that I don't think is ever needed, where it checks:
else if(light->type == LIGHT_STRAND) {
And then stores some stuff in light_data. This is only used for lamps, if strands work similar to triangles then it will not read from this array.

Further did not find other issues. I forgot that it's christmas now so indeed next few days are not ideal to merge. From thursday I have time to be standby and fix issues that might come up in committing this. So when you plan to do it just give me a heads up the day before.

Thanks! Just made those changes and uploaded it. I also fixed a bug. I will probably commit the patch on Friday but will let you know closer to the time.

The patch is committed now, seems to be working fine here, thanks! I'll close the report here then.

The svn commit mail didn't get through, probably because you're not subscribed to bf-blender-cvs with the mail address of you projects.blender.org account.

Also one thing I forgot to mention, much developer discussion happens in the #blendercoders irc channel freenode.net. That's usually a good place to discuss things in addition to the bf-cycles mailing list.

I actually just sent you an email enquiring about the cvs mailing list. I also sent a message to the committers mailing list that didn't get through. I think this is just because I only registered on these lists yesterday.