Page MenuHome

MatCap inverts if Viewport Display Metallic and Roughness are set below certain threshold
Closed, ResolvedPublicBUG

Description

System Information
Operating system: Windows-10-10.0.18362-SP0 64 Bits
Graphics card: GeForce RTX 2080 Ti/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 445.87

Blender Version
Broken: version: 2.83 (sub 13), branch: master, commit date: 2020-04-20 02:11, hash: rB23bb42a06e28
Worked: 2.82a

Short description of error
MatCap inverts if Viewport Display Metallic and Roughness are set below certain threshold.

Exact steps for others to reproduce the error
Change Viewport Shading to MatCap, select Cube, go to Material Properties to Viewport Display, set Metallic and Roughness to 0. Metallic can be as low as 0.144.

Event Timeline

Robert Guetzkow (rjg) changed the task status from Needs Triage to Confirmed.Apr 20 2020, 9:02 PM
Robert Guetzkow (rjg) claimed this task.
Robert Guetzkow (rjg) triaged this task as Low priority.

The issue is the following part of the workbench_composite_frag.glsl

#ifdef V3D_LIGHTING_MATCAP
  /* When using matcaps, mat_data.a is the backface sign. */
  N = (mat_data.a > 0.0) ? N : -N;

  fragColor.rgb = get_matcap_lighting(base_color, N, I);
#endif

It changes the normal direction based on the data stored in the alpha channel. This appears to be incorrect for two reasons:

  1. In workbench_prepass_frag.glsl the materialData.a = float(gl_FrontFacing); is only set if useMatcap is true. Despite its name, the variable is not always set to true when matcaps are used. It is only set to true for hair certain shading groups in workbench_opaque_cache_init (workbench_opaque.c). As a result the alpha channel instead contains the packed_rough_metal, which is why the values for roughness and metal impact the direction of the normal.
  2. In workbench_prepass_frag.glsl the workbench_normal_encode(gl_FrontFacing, normal_interp) already includes backface/frontface information (see workbench_common_lib.glsl). If I'm not mistaken that makes the (potential) normal inversion in the workbench_composite_frag.glsl redundant.

If the N = (mat_data.a > 0.0) ? N : -N; is indeed redundant, then it could simply be removed along with the materialData.a = float(gl_FrontFacing);. If it isn't redundant, then if (useMatcap) should be replaced with a check that is always true when matcaps are used. This means either a different check/macro or useMatcap needs to be set to true for more cases than just hair the current shading groups.

@Clément Foucault (fclem) Could you take a quick look and tell me which approach you would prefer?

We used the double inversion because users asked for it. It is indeed a strange thing but it's by design.

I think it's just a matter of adding DRW_shgroup_uniform_bool_copy(grp, "useMatcap", use_matcap);somewhere.

@Clément Foucault (fclem) Good to know. I will set useMatcap in the patch then.

Robert Guetzkow (rjg) changed the subtype of this task from "Report" to "Bug".May 1 2020, 11:21 AM