Page MenuHome

Workbench: Specular Highlighting for MatCaps
Needs ReviewPublic

Authored by Jeroen Bakker (jbakker) on Wed, Jul 24, 4:18 PM.
Tokens
"Love" token, awarded by pablodp606."Love" token, awarded by JulienKaspar."Love" token, awarded by xrg."Like" token, awarded by ace_dragon."Love" token, awarded by SecuoyaEx.

Details

Summary

With Blender 2.80 we introduced a more flexible matcap system. One
change we did was to multiply the matcap with the base color that was
shaded. As matcaps contains diffuse and specular lighting in a single
texture this lead to rendering artifacts. Artists were complaining that
everything looked to metalic.

We now have the option to have a separate diffuse and specular
texture for matcaps.

shaded_color = diffuse_light * base_color + specular_light

For matcaps to support this feature they need to be multilayer openexr
files with 2 renderpasses (diffuse and specular). In the future we can
change this to first pass/second pass in stead of this naming
convention.

Example with separate specular highlights rendered

Example matcaps for reference/testing

File used for creating these matcap files

Diff Detail

Repository
rB Blender
Branch
arcpatch-D5335 (branched from master)
Build Status
Buildable 4304
Build 4304: arc lint + arc unit

Event Timeline

Jeroen Bakker (jbakker) planned changes to this revision.EditedWed, Jul 24, 4:27 PM
  • Add support in forward rendering
  • Disable the Specular Highlight option when a single pass matcap has been selected.
  • Clean up matcap code
  • Support 4 specular only matcaps
Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Wed, Jul 24, 4:55 PM
Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Wed, Jul 24, 5:00 PM
  • Workbench forward rendering support
  • Disable specular highlight option in UI when no specular highlight pass available
  • Support for multilayer matcaps with only a specular highlight pass
Jeroen Bakker (jbakker) added inline comments.
source/blender/draw/engines/workbench/workbench_private.h
323

should be a utility function... is not always valid to only check ibuf.

Comparison between the current gray matcap and a white matcap with a separate specular pass. It is already looking much better!

Brecht Van Lommel (brecht) requested changes to this revision.Thu, Jul 25, 5:46 PM
Brecht Van Lommel (brecht) added inline comments.
source/blender/blenkernel/intern/studiolight.c
449–452

Create a new image buffer here rather than editing the existing one, it's not safe to assume these values can just be overwritten for multilayer EXR image buffers.

462–463

Don't set flag and mall manually, IMB_allocFromBuffer already does this.

580–582

This code assumes there are always 4 channels, which I don't think is a valid assumption.

593

error is an optional argument, if you don't use it pass NULL so that any errors get printed to the console still.

1380

This one also assumes 4 channels it seems.

source/blender/makesrna/intern/rna_userdef.c
3710

Hightlighting -> Highlight

3711

I would be more specific here:

Studio light image file has separate "diffuse" and "specular" passes
This revision now requires changes to proceed.Thu, Jul 25, 5:46 PM
source/blender/blenkernel/intern/studiolight.c
462–463

I think it would also be better to make channels a parameter to IMB_allocFromBuffer.

Currently it does not try to read or reallocate the float buffer that is passed, but it's not really safe to assume that.

@Jeroen Bakker (jbakker) Just curious, is there any performance difference when sculpting using this type of matcaps, compared to the regular ones?

The performance difference Is not noticeable for users. In the end iT is a texture read and addition per visible pixel.

Jeroen Bakker (jbakker) marked an inline comment as done.Fri, Jul 26, 8:47 AM
Jeroen Bakker (jbakker) updated this revision to Diff 16609.

Some tasks from code review (WIP)

Jeroen Bakker (jbakker) planned changes to this revision.Fri, Jul 26, 8:48 AM

Still need to solve the alloc buffer and buffer size conversions

Jeroen Bakker (jbakker) marked 6 inline comments as done.Fri, Jul 26, 12:04 PM
Jeroen Bakker (jbakker) updated this revision to Diff 16613.

Code review

Jeroen Bakker (jbakker) marked an inline comment as done.Fri, Jul 26, 12:06 PM
Brecht Van Lommel (brecht) requested changes to this revision.Fri, Jul 26, 12:48 PM
Brecht Van Lommel (brecht) added inline comments.
source/blender/blenkernel/intern/studiolight.c
438

specular_buffer leaks here.

453

num_specular_channels -> num_diffuse_channels

501–502

The logic in this function is more complicated than it needs to be, suggest to reorganize it like this: P1051

530

It would be simpler to always convert to 4 channel float with IMB_buffer_float_from_float when the image buffer is loaded.

This revision now requires changes to proceed.Fri, Jul 26, 12:48 PM

Implementation looks fine.

I'm just nitpicking about the functionality :)

source/blender/draw/engines/workbench/shaders/workbench_deferred_composite_frag.glsl
58–59

One complain we had is that this uv projection is only nice for specular and remove flat shading from diffuse surfaces.

Maybe using the "dumb" projection for diffuse (only when specular is separated) maybe nice.

67

Now that we have separated specular and diffuse, we could use the metallic parameter to give a material hint.

Without going into fresnel approximation, we could just mix based on metallic parameter. (or maybe we want fresnel approx here ! 😂)

The roughness is still not used by the matcap shading so we can encode the backface bit in it.

Jeroen Bakker (jbakker) marked 3 inline comments as done.Fri, Jul 26, 2:34 PM
Jeroen Bakker (jbakker) updated this revision to Diff 16617.

Comments from code review

Brecht Van Lommel (brecht) requested changes to this revision.Fri, Jul 26, 3:35 PM

Just one more thing.

source/blender/blenkernel/intern/studiolight.c
369–370

The code that calls this frees rect, not the return value of this function?

This revision now requires changes to proceed.Fri, Jul 26, 3:35 PM
Jeroen Bakker (jbakker) marked 2 inline comments as done.Tue, Aug 6, 8:54 AM
Jeroen Bakker (jbakker) updated this revision to Diff 16873.

Fixed memory leak introduced by freeing the incorrect float-buffer after converting passes.

source/blender/blenkernel/intern/studiolight.c
501–502

I did a tiny adjustment (magenta when no diffuse and specular buffer). but that patch is much more readable.

530

Can do!

Point of attention Seems like IMB_buffer_float_from_float isn't consistent with premul alpha. When channels_from = 1 it is pre multiplied, otherwise it is straight. It isn't a problem for this case as we ignore alpha. Makes me wonder if this is the reason why single channel PNG files are created not according the specification.

source/blender/draw/engines/workbench/shaders/workbench_deferred_composite_frag.glsl
67

I did some tests, we could add it, but I would propose to do it not part of this patch.

1diff --git a/source/blender/draw/engines/workbench/shaders/workbench_deferred_composite_frag.glsl b/source/blender/draw/engines/workbench/shaders/workbench_deferred_composite_frag.glsl
2index fd4cea4279a..8036b4babed 100644
3--- a/source/blender/draw/engines/workbench/shaders/workbench_deferred_composite_frag.glsl
4+++ b/source/blender/draw/engines/workbench/shaders/workbench_deferred_composite_frag.glsl
5@@ -32,7 +32,11 @@ void main()
6
7 #ifndef MATDATA_PASS_ENABLED
8 base_color = materialSingleColor;
9+# ifdef V3D_SHADING_SPECULAR_HIGHLIGHT
10+ metallic = 1.0;
11+# else
12 metallic = 0.0;
13+# endif
14 roughness = 0.5;
15 #else
16 vec4 material_data = texelFetch(materialBuffer, texel, 0);
17@@ -53,7 +57,7 @@ void main()
18
19 #elif defined(V3D_LIGHTING_MATCAP)
20 /* When using matcaps, the metallic is the backface sign. */
21- normal_viewport = (metallic > 0.0) ? normal_viewport : -normal_viewport;
22+ normal_viewport = (roughness > 0.0) ? normal_viewport : -normal_viewport;
23 bool flipped = world_data.matcap_orientation != 0;
24 vec2 matcap_uv = matcap_uv_compute(I_vs, normal_viewport, flipped);
25 vec3 matcap_diffuse = textureLod(matcapDiffuseImage, matcap_uv, 0.0).rgb;
26@@ -64,7 +68,7 @@ void main()
27 vec3 matcap_specular = vec3(0.0);
28 # endif
29
30- vec3 shaded_color = matcap_diffuse * base_color + matcap_specular;
31+ vec3 shaded_color = matcap_diffuse * base_color + (metallic * matcap_specular);
32
33 #elif defined(V3D_LIGHTING_STUDIO)
34
35diff --git a/source/blender/draw/engines/workbench/shaders/workbench_prepass_frag.glsl b/source/blender/draw/engines/workbench/shaders/workbench_prepass_frag.glsl
36index c673b2484de..b629a8ef709 100644
37--- a/source/blender/draw/engines/workbench/shaders/workbench_prepass_frag.glsl
38+++ b/source/blender/draw/engines/workbench/shaders/workbench_prepass_frag.glsl
39@@ -52,9 +52,9 @@ void main()
40 # endif
41
42 # ifdef V3D_LIGHTING_MATCAP
43- /* Encode front facing in metallic channel. */
44- metallic = float(gl_FrontFacing);
45- roughness = 0.0;
46+ /* Encode front facing in roughness channel. */
47+ metallic = materialMetallic;
48+ roughness = float(gl_FrontFacing);
49 # else
50 metallic = materialMetallic;
51 roughness = materialRoughness;