Page MenuHome

Cycles: Vector Rotate Node using Axis and Angle method
ClosedPublic

Authored by Charlie Jolly (charlie) on Oct 13 2018, 6:37 PM.

Details

Summary

This node provides the ability to rotate a vector around a center point using either Axis Angle , Single Axis or Euler methods.

This node/diff has gone through a few iterations after the Mapping node was updated in D5541. This is why it has changed since the first diff.

--Old description--
This provides ability to rotate a vector around an origin point and also provide access to the XYZ values via sockets.
Together with D3786 Vector Multipy, this provides sockets and an alternative to the mapping node.
Like the mapping node it uses XYZ order and this could be expanded in future to support other rotation modes.

Diff Detail

Repository
rB Blender

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

This wouldn't work with the mapping node as this exposes rotation values as socket inputs.

This wouldn't work with the mapping node as this exposes rotation values as socket inputs.

I was talking about using the dynamic mapping node from my branch. It will probably land in master this week, so we can utilize it.

Charlie Jolly (charlie) marked 10 inline comments as done.Aug 14 2019, 6:35 PM
Charlie Jolly (charlie) added inline comments.
intern/cycles/kernel/shaders/node_vector_rotate.osl
21

Function copied from math_rotation.c.
Renamed to eul_to_mat4.

25

This is taken from eul_to_mat4 in math_rotation.c

51

It works. Matches the names in nodes.cpp.

intern/cycles/kernel/svm/svm_vector_rotate.h
44

this is different because transform_point doesn't work the same in cycles as it does in glsl

intern/cycles/render/nodes.cpp
5570

This was copied from the Combine nodes that use the same code. Not sure I understand this.

5588

Crashes if this is changed to stack_assign_if_linked

source/blender/gpu/shaders/gpu_shader_material.glsl
2247 ↗(On Diff #17102)

Function copied from math_rotation.c.
Renamed to eul_to_mat4.

Charlie Jolly (charlie) marked 7 inline comments as done.

Address comments from @Omar Emara (OmarSquircleArt)

@Omar Emara (OmarSquircleArt) I've kept the eul_to_mat4 the same as in maths_rotation.c. OSL uses mat4 so I've tried to keep all the code as consistent as possible.

intern/cycles/kernel/shaders/node_vector_rotate.osl
21

C code is different from OSL and GLSL code, I think we should just return the matrix directly, no need to use output parameters.

25

I don't think there is a code convention here, so just do what you think is more readable. You don't have to stick to ijh.

intern/cycles/kernel/svm/svm_vector_rotate.h
44

Sure, but that still doesn't mean it should be done this way.

intern/cycles/render/nodes.cpp
5570

This line basically says that if all inputs are constants, that is, not linked to other sockets, then we can reduce the node to a constant value. You compute this value as make_float3(x, y, z), which is obviously not correct. To correct this, simply rotate the point just as you did in the SVM code.

5588

If you don't assign if linked then the second node will be useless since the inputs will always be stack assigned. Find out why Blender crashes and try to fix it.

source/blender/gpu/shaders/gpu_shader_material.glsl
2247 ↗(On Diff #17102)

Well there is also a eul_to_mat3 function in the same file, not sure what the reasoning here is.

source/blender/nodes/shader/nodes/node_shader_vectRotate.c
54 ↗(On Diff #17126)

node_type_storage is not needed in this case.

Charlie Jolly (charlie) marked 7 inline comments as done.Aug 15 2019, 2:24 PM
Charlie Jolly (charlie) added inline comments.
intern/cycles/kernel/shaders/node_vector_rotate.osl
21

I've made this a return function and changed name to eul_to_matrix

25

Because the code is copied I think it's better to keep the variable names as this helps if the codebase is searched.

intern/cycles/kernel/svm/svm_vector_rotate.h
44

Simplified to match OSL and GLSL code. Remove call to transform_translate.

intern/cycles/render/nodes.cpp
5570

I've removed the constant folding. I've noticed a lot of nodes do not have this.

5588

This is unclear to me why this crashes. Take a look at the CombineXYZNode and VectorTransformNode, they are coded the same way.

source/blender/gpu/shaders/gpu_shader_material.glsl
2247 ↗(On Diff #17102)

Simplified glsl to use mat3 and eul_to_mat3 which is identical to eul_to_mat4 but without the additional two lines at the end. Also used return function.

Charlie Jolly (charlie) marked 6 inline comments as done.

Cleanup and address comments from @Omar Emara (OmarSquircleArt) - thanks

Charlie Jolly (charlie) updated this revision to Diff 17274.EditedAug 19 2019, 4:57 PM

Change XYZ sockets to a single vector socket.

This comment has been deleted.
intern/cycles/kernel/svm/svm.h
524

I like passing the uints of the node directly like:
svm_node_vector_rotate(kg, sd, stack, node.y, node.z, &offset);
The reason I like this is because we can use explicit names in the function parameters like:

ccl_device void svm_node_vector_rotate(KernelGlobals *kg,
                                       ShaderData *sd,
                                       float *stack,
                                       uint inputs_stack_offsets,
                                       uint result_stack_offset,
                                       int *offset)
intern/cycles/kernel/svm/svm_vector_rotate.h
25

A naming convention I like to use is *_stack_offset, for instance, vector_stack_offset, origin_stack_offset, rotation_stack_offset, and result_stack_offset. This helps distinguishing sockets, stack offsets, and global offsets.

36

I think you should create a new variable instead of overwriting the input this many times:
float3 result = transform_point(&rmat, vector - origin) + origin;
The same goes for other backends.

intern/cycles/render/nodes.cpp
6163

No need to encode the output using compiler.encode_uchar4, just pass it directly.

source/blender/nodes/shader/nodes/node_shader_vectRotate.c
30 ↗(On Diff #17274)

I think it is better to call this Rotation, like in the rest of Blender.

Charlie Jolly (charlie) marked 5 inline comments as done.Aug 19 2019, 6:42 PM

Rename and tidy up with comments from @Omar Emara (OmarSquircleArt)

Variable renaming for better readability

Missed one function name in last changes

Keep patch up to date

Change rotation method to axis angle

Add previous method as an option to new mapping node

Charlie Jolly (charlie) retitled this revision from Cycles & Eevee Vector Rotate Node: Standalone node with access to XYZ sockets. to Cycles: Vector Rotate Node using Axis and Angle method.Sep 7 2019, 5:16 PM
Charlie Jolly (charlie) edited the summary of this revision. (Show Details)
Charlie Jolly (charlie) edited the summary of this revision. (Show Details)

Keep patch updated with master

Sergey Sharybin (sergey) requested changes to this revision.Jan 2 2020, 2:32 PM

This node is something we've been always missing here in the studio, and we were using a dedicated node group in assets for this. So to me it seems something very useful to have.

I can't quite understand how Rotate in mapping works, and what is the purpose of having it there? Think it's more clear to just have dedicated rotation node.
Also name Origin was a bit confusing at first. How about Center?

intern/cycles/kernel/shaders/node_vector_rotate.osl
3

Copyright 2011-2020 Blender Foundation

20

You should be able to replace this function with rotate from stdosl.h.

intern/cycles/kernel/svm/svm_vector_rotate.h
3

Copyright 2011-2020 Blender Foundation

This revision now requires changes to proceed.Jan 2 2020, 2:32 PM
Charlie Jolly (charlie) updated this revision to Diff 20588.EditedJan 2 2020, 5:54 PM
Charlie Jolly (charlie) marked 3 inline comments as done.

@Sergey Sharybin (sergey) address comments and test for zero length axis

The Rotate mapping option in the mapping node uses the location as the center of rotation. By including this option, it provides users with two new methods to rotate vectors. One using this new Axis/Angle (one angle) node and one using the Rotate option (three angles) in the mapping node.

Do you want me to remove this code and submit as a separate patch?

Do you want me to remove this code and submit as a separate patch?

Currently i am just trying to understand why there is a need to have two ways of doing the same thing.

@Sergey Sharybin (sergey) this provides two different rotation methods.

Left: Axis/Angle Rotation
Right: Euler Rotation

Here is an example of rotating a texture using an axis defined by two empties.

I think it should then be a rotation model menu in the Vector Rotate node.

Charlie Jolly (charlie) updated this revision to Diff 20648.EditedJan 6 2020, 4:54 PM

@Sergey Sharybin (sergey)

  • Removed rotate option from Mapping node
  • Added Euler options to the Rotate node. Default is Axis Angle.
  • Tidy and move shared OSL functions to math.h

Charlie Jolly (charlie) edited the summary of this revision. (Show Details)Jan 6 2020, 5:09 PM

Fix after gpu refactor in master

Charlie Jolly (charlie) edited the summary of this revision. (Show Details)Sun, Feb 16, 7:07 PM

Can you add tests for these new shader nodes?

Something similar to tests/render/shader/tex_voronoi.blend, a single test that renders with the node in a few different configurations.

This revision is now accepted and ready to land.Mon, Feb 17, 3:58 PM

Can you add tests for these new shader nodes?
Something similar to tests/render/shader/tex_voronoi.blend, a single test that renders with the node in a few different configurations.

Yes, I can do that. Assume it's just the blend file I need to create?