Page MenuHome

[Cycles] Add rounded corners to brick texture.
AbandonedPublic

Authored by Ray molenkamp (LazyDodo) on Jan 2 2017, 8:53 PM.

Details

Summary

This diff adds rounded corners to the cycles brick texture, the brick texture is nice and useful, but it just looks too 'perfect' also i really needed an easy way to make some cobble stones.

this replaces pretty much the core of the function with a distance field ,idea taken from here but then adjusted to account for the mortar size which made the math slightly less obvious.

Tested and working: CPU / OSL / CUDA / opencl mega+split on nvidia sm_30 card, gsgl shader

Some examples on why this is useful:

standard brick, with rounded corners, looks a more natural

But you can go fully round if you want.

or combine it with some squash and make a nice little street pattern

by overdoing the radius you end up with diamond shaped patters, great for some sheet metal

A little more creative use:

used random noise for the corner radius, makes a nice irregular stones.

used a texture * noise for the corner radius and used it for displacement

Diff Detail

Repository
rB Blender

Event Timeline

Ray molenkamp (LazyDodo) retitled this revision from to [Cycles] Add rounded corners to brick texture. .
Ray molenkamp (LazyDodo) updated this object.
Ray molenkamp (LazyDodo) set the repository for this revision to rB Blender.

Nice addition!

intern/cycles/kernel/shaders/node_brick_texture.osl
60

Code style, no gap between statement and the brace. if() would be correct. Should be fixed in the other files as well.

source/blender/gpu/shaders/gpu_shader_material.glsl
2877

Try putting corner_radius after row_height variable. That should fix it. The ones afterwards are not actually sockets but RNA values. Sockets come first, then RNA, then output.

intern/cycles/kernel/shaders/node_brick_texture.osl
60

I don't understand, this is the exact same code style as everywhere else? and the one outlined in the code style docs here

/* Do: */
    if (a == b) {
        d = 1;
    }

campbell updated all osl scripts to this formatting in rBaeda5142ef41ce69264166a5d7e31f2c110c0096

Aaron Carlisle (Blendify) added inline comments.
intern/cycles/kernel/shaders/node_brick_texture.osl
33

Missing space

source/blender/gpu/shaders/gpu_shader_material.glsl
2876

White space

Ray molenkamp (LazyDodo) updated this object.

Updated codesyle with feedback and fixed the gsgl shader with dingto's tip

intern/cycles/kernel/shaders/node_brick_texture.osl
60

Okay, you're correct for OSL it seems, but for the rest of Cycles we use if() without gap. Just check existing code for SVM and other Cycles modules.

Ray molenkamp (LazyDodo) marked 2 inline comments as done.

changed fmax to fmaxf some compilers did not have the float overload for fmax and caused a compile error.

Ray molenkamp (LazyDodo) marked an inline comment as done.Jan 3 2017, 6:09 PM
Ray molenkamp (LazyDodo) marked an inline comment as done.Jan 3 2017, 6:17 PM
Ray molenkamp (LazyDodo) added inline comments.
intern/cycles/kernel/shaders/node_brick_texture.osl
60

Fixed it, should be allright now, however I'm all for following code style, but if it changes depending on where one is in the code base, ...... yikes.....

Thomas Dinges (dingto) edited edge metadata.

I tested the patch quickly, works nicely. LGTM from my side. Thanks for the addition!

intern/cycles/kernel/shaders/node_brick_texture.osl
60

Honestly I have no idea, why OSL is an exception to the other Cycles areas, which are (or should be ) consistent among each other.

This revision is now accepted and ready to land.Jan 3 2017, 6:45 PM

I'm starting to think we REALLY need some formal rules about what textures to accept and which not to. This is something what stresses registers A LOT on GPU and it's something what was supposed to be addressed by OSL so we dn't have to support all weird and wonderful textures as a core feature. Or maybe decide to drop OSL?

intern/cycles/kernel/shaders/node_brick_texture.osl
66

Here and in other places: would really prefer parenthesis to be added ALWAYS. Not only such style looks weird it's also known to give bugs due to coders mistake in the future and makes future patches more noisy.

Ray molenkamp (LazyDodo) marked an inline comment as done.Jan 3 2017, 8:03 PM

This is something what stresses registers A LOT on GPU

I can't speak for all architectures but my sm_30 card both spilling stores and loads are down, not up. (I doubled checked I didn't mix up the pre/post outputs)

Metric		Change 	Number_of_functions
Stack		+8	4
Stack		-8	6
Spills stores	-8	1
Spills stores	-12	2
Spills stores	-16	1
Spills stores	-24	1
Spills stores	-32	1
Spills loads	-8	1
Spills loads	-16	1
Spills loads	-20	2
Spills loads	-24	2
Spills loads	-64	1

I'm all for guidelines of what is and isn't allowed ,I for one hate to spend time on things that'll never get accepted.

Ray molenkamp (LazyDodo) edited edge metadata.

added brackets to one-liners that needed it.

Ray molenkamp (LazyDodo) marked an inline comment as done.Jan 3 2017, 8:31 PM

The problem is, nobody uses OSL because it doesn't run on GPU and is frequently much slower even on CPU. As long as SVM is faster even on CPU, no one will use OSL except as an absolute last resort. And maybe not even then, since OSL doesn't run on GPU and there's no reasonable way to change that anytime soon. There will always be user pressure to add this or that texture to SVM, just to have it on the "fast side".

I don't see this as a new texture or special case, more as a reasonable improvement to an already implemented procedural texture.

Managed to do it in a node group, no longer need this.

Will this ever get back to being developed? It would improve Brick Texture a LOT! It's really needed for environment creation.