Page MenuHome

Shading: Truchet Texture Node
Needs ReviewPublic

Authored by Damien Smith (idomo) on Mon, Nov 25, 11:45 AM.
Tags
None
Tokens
"Love" token, awarded by devanshutak25."Love" token, awarded by EAW."Love" token, awarded by Kronk."Love" token, awarded by xrg."Love" token, awarded by franMarz."Love" token, awarded by amonpaike."Love" token, awarded by yebyte.

Details

Summary

Implements Truchet tiles as a texture node.

Width Parameter

Offset Parameter

Border Width Parameter

Flip Tiles Option

Diff Detail

Event Timeline

This looks nice. I will write a review as soon as I have the time.

Omar Emara (OmarSquircleArt) requested changes to this revision.Thu, Nov 28, 10:41 AM

General review:

  • It doesn't seem like you are using clang format. You should apply it to your files.
  • Vector operations should be utilized. For instance the d in truchet_arc can be computed using a distance function. gridify can simply be a single fract call.
  • Texture Mapping isn't implemented. All other textures implement it, so take a look at the code for the other textures as a reference.
This revision now requires changes to proceed.Thu, Nov 28, 10:41 AM
  • Applied clang formatting
  • Utilized vector operations within shaders
  • Implemented texture mapping
  • Adjusted smoothstep bounds to eliminate unnecessary operation
  • Removed unnecessary variables/commented lines
  • Specified TEXTURE_GENERATED link type for vector input
Omar Emara (OmarSquircleArt) requested changes to this revision.Fri, Nov 29, 9:55 AM

Texture mapping is still missing. Looking at other textures' OSL files, you will notice two extra inputs:

int use_mapping = 0,
matrix mapping = matrix(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0),

as well as a call to the transform function:

if (use_mapping)
    coord = transform(mapping, coord);

For EEVEE, you will notice a call to two functions:

node_shader_gpu_default_tex_coord(mat, node, &in[0].link);
node_shader_gpu_tex_mapping(mat, node, in, out);

Finally, notice that we can't use the custom variables in the nodes because texture mapping requires a dedicated DNA struct, which you already created. In this case, the struct needs to be initialized appropriately like so:

NodeTexTruchet *tex = MEM_callocN(sizeof(NodeTexTruchet), "NodeTexTruchet");
BKE_texture_mapping_default(&tex->base.tex_mapping, TEXMAP_TYPE_POINT);
BKE_texture_colormapping_default(&tex->base.color_mapping);

Also, I feel like some of the files are still not formatted using clang format. Make sure to format OSL and GLSL code as well.

This revision now requires changes to proceed.Fri, Nov 29, 9:55 AM
  • Properly implemented texture mapping
  • Updated node menu name to match other texture nodes
  • Applied formatting to OSL file