Differential D2011

Enable correct GLSL output for cycles normalmap node
ClosedPublic
Actions

Authored by Ralf Hölzemer (cheleb) on May 19 2016, 6:53 PM.

Tags

Subscribers

None

Details

Reviewers

Sergey Sharybin (sergey)
Alexander Romanov (a.romanov)

Commits

rB50376529ee11: Enable correct GLSL output for cycles normalmap node
rBSa6b218af2e62: Enable correct GLSL output for cycles normalmap node
rBa6b218af2e62: Enable correct GLSL output for cycles normalmap node

Summary

See T48453 for details and test scenes

Diff Detail

Repository: rB Blender

Event Timeline

Ralf Hölzemer (cheleb) updated this revision to Diff 6704.May 19 2016, 6:53 PM

Ralf Hölzemer (cheleb) retitled this revision from to Enable correct GLSL output for cycles normalmap node.

Ralf Hölzemer (cheleb) updated this object.

Ralf Hölzemer (cheleb) set the repository for this revision to rB Blender.

Ralf Hölzemer (cheleb) added projects: GPU / Viewport, Nodes.

Alexander Romanov (a.romanov) added reviewers: Alexander Romanov (a.romanov), Sergey Sharybin (sergey).May 19 2016, 7:03 PM

The main drawback that I can see is two switches for nm->space that are confusing. And I think that convertion can be simplified.

source/blender/gpu/shaders/gpu_shader_material.glsl
145	These glsl functions came from Сycles logic? Or it's just heuristics?
source/blender/nodes/shader/nodes/node_shader_normal_map.c
158	Broken indentation

Address comments and restructure logic

Hi @Alexander Romanov (a.romanov),

thanks for the review!
I tried to address your comments by rearranging the logic into a more readable form. Now there is a clear separation by shading type and I also got rid of the second switch by nm->space.

source/blender/gpu/shaders/gpu_shader_material.glsl
145	This function exists because in Cycles, the y component of the normal is not flipped like in BI.
152	This is based on the OSL version of the normal_map_node here and here, where the y and the z components of the normal are flipped for BI variants (SHD_NORMAL_MAP_BLENDER_OBJECT and SHD_NORMAL_MAP_BLENDER_WORLD).
source/blender/nodes/shader/nodes/node_shader_normal_map.c
158	Rearranged and fixed, thanks!

It's ok for me now. BI behavior is correct. @Sergey Sharybin (sergey), would you make a final review?

This revision is now accepted and ready to land.May 23 2016, 2:29 PM

LGTM

@Alexander Romanov (a.romanov), will you commit the patch?

Closed by commit rBa6b218af2e62: Enable correct GLSL output for cycles normalmap node (authored by Ralf Hölzemer (cheleb), committed by Alexander Romanov (a.romanov)). · Explain WhyMay 23 2016, 3:29 PM

This revision was automatically updated to reflect the committed changes.

Thanks for the patch!

Revision Contents
Changeset List

Path

Size

source/

blender/

gpu/

shaders/

gpu_shader_material.glsl

14 lines

nodes/

shader/

nodes/

node_shader_normal_map.c

61 lines

Diff 6746

source/blender/gpu/shaders/gpu_shader_material.glsl

Loading...

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

Loading...