Page MenuHome

World space switch for BI nodes.
ClosedPublic

Authored by Alexander Romanov (a.romanov) on Jun 1 2016, 11:52 AM.

Details

Summary

At the moment light shading in Blender is produced in viewspace. Apparently, that's why
shader nodes work with normals in camera space. But it is not convenient for artists.
The more convenient approach is implemented in Cycles where normals are represented in world space.
Blend4Web Team designed the engine keeping in mind shader parameters readability,
so normals are interpreted in world space as well. And now our users have to use some tweaks, like
empty node group with the name "Replace", which is replacing one input by another on the engine side
(replacing working configuration in Blender Viewport by the configuration that has the same behavior in the engine).

This patch adds the ability to switch to world space for normals and lamp vector in BI and Viewport.
This patch is very important to us and we crave to see this patch in Blender 2.7 because
it will significantly simplify Blend4Web material creation workflow.

Diff Detail

Repository
rB Blender

Event Timeline

Alexander Romanov (a.romanov) retitled this revision from World space outputs for node sockets related to the lighting in BI. to World space switch for BI nodes..Jun 1 2016, 1:13 PM
Brecht Van Lommel (brecht) requested changes to this revision.

These kinds of options aren't great UI but we don't have much choice for backwards compatibility reasons in 2.7. Obviously I think world space is the right convention.

source/blender/blenkernel/intern/scene.c
2199

It probably doesn't actually change the code behavior, but can this be:

return (type && ((type->flag & RE_USE_SHADING_NODES) || (scene->r.mode & R_USE_WS_LIGHT);

Both are world space so might as well return true in both cases.

source/blender/makesrna/intern/rna_scene.c
5571–5574

Should not use abbreviation in the property name. I suggest use_world_space_shading and "World Space Shading" for the names.

source/blender/nodes/shader/nodes/node_shader_normal_map.c
179–200

Can this be implemented as:

if (GPU_material_use_new_shading_nodes || GPU_material_use_world_space_light_params) {
   .. world space ..
}
else {
  .. camera space ..
}

Or is there a good reason for Cycles and Blender Internal to behave differently besides the camera/world space difference?

This revision now requires changes to proceed.Jun 2 2016, 12:29 AM
Alexander Romanov (a.romanov) marked 2 inline comments as done.
Alexander Romanov (a.romanov) updated this revision to Diff 6845.

Generally looks fine to me now, still some comments.

source/blender/nodes/shader/nodes/node_shader_normal_map.c
151

Not sure why this checks for GPU_material_use_new_shading_nodes.

The purpose of this feature was to support Blender-style normal map encoding and a more standard normal map encoding. I think that feature is useful for both Blender Internal and Cycles?

Though I guess that's not really an issue with this world space stuff specifically and doesn't need to be solved here.

186–192

This code should have been removed I think?

Alexander Romanov (a.romanov) marked 2 inline comments as done.Jun 6 2016, 10:38 AM
Alexander Romanov (a.romanov) added inline comments.
source/blender/nodes/shader/nodes/node_shader_normal_map.c
151

I agree. If necessary, this can be done in another patch.

180

Of cource

Brecht Van Lommel (brecht) edited edge metadata.EditedJun 6 2016, 9:46 PM
Brecht Van Lommel (brecht) accepted this revision.

Looks good to me.

This revision is now accepted and ready to land.Jun 6 2016, 9:46 PM
Alexander Romanov (a.romanov) marked 2 inline comments as done.Jun 7 2016, 9:48 AM
This revision was automatically updated to reflect the committed changes.