MatCap inverts if Viewport Display Metallic and Roughness are set below certain threshold #75938

New Issue

Marc Zbinden · 2020-04-20T17:11:27+02:00

Marc Zbinden commented

2020-04-20 17:11:27 +02:00

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: 23bb42a06e
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.

**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: `23bb42a06e` 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.

Marc Zbinden commented

2020-04-20 17:11:27 +02:00

Added subscriber: @MarcZbinden

Robert Guetzkow self-assigned this 2020-04-20 21:02:52 +02:00

Robert Guetzkow commented

2020-04-20 21:02:52 +02:00

Changed status from 'Needs Triage' to: 'Confirmed'

Robert Guetzkow commented

2020-04-21 12:20:39 +02:00

Added subscriber: @fclem

Robert Guetzkow commented

2020-04-21 12:20:39 +02:00

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:

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 ([ https:*developer.blender.org/diffusion/B/browse/master/source/blender/draw/engines/workbench/workbench_opaque.c | 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.
In [workbench_prepass_frag.glsl ]] the workbench_normal_encode(gl_FrontFacing, normal_interp) already includes backface/frontface information (see https:*developer.blender.org/diffusion/B/browse/master/source/blender/draw/engines/workbench/shaders/workbench_common_lib.glsl ). If I'm not mistaken that makes the (potential) normal inversion in the [ https://developer.blender.org/diffusion/B/browse/master/source/blender/draw/engines/workbench/shaders/workbench_composite_frag.glsl | 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.

@fclem Could you take a quick look and tell me which approach you would prefer?

The issue is the following part of the [workbench_composite_frag.glsl ](https://developer.blender.org/diffusion/B/browse/master/source/blender/draw/engines/workbench/shaders/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: - 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` ([[ https:*developer.blender.org/diffusion/B/browse/master/source/blender/draw/engines/workbench/workbench_opaque.c | workbench_opaque.c ](https:*developer.blender.org/diffusion/B/browse/master/source/blender/draw/engines/workbench/shaders/workbench_prepass_frag.glsl)). 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. - In [workbench_prepass_frag.glsl ]] the `workbench_normal_encode(gl_FrontFacing, normal_interp)` already includes backface/frontface information (see [[ https:*developer.blender.org/diffusion/B/browse/master/source/blender/draw/engines/workbench/shaders/workbench_common_lib.glsl | workbench_common_lib.glsl ]]). If I'm not mistaken that makes the (potential) normal inversion in the [[ https://developer.blender.org/diffusion/B/browse/master/source/blender/draw/engines/workbench/shaders/workbench_composite_frag.glsl | workbench_composite_frag.glsl ](https:*developer.blender.org/diffusion/B/browse/master/source/blender/draw/engines/workbench/shaders/workbench_prepass_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. @fclem Could you take a quick look and tell me which approach you would prefer?

Clément Foucault commented

2020-04-21 16:14:46 +02:00

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.

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.

Robert Guetzkow commented

2020-04-21 16:19:10 +02:00

@fclem Good to know. I will set useMatcap in the patch then.

@fclem Good to know. I will set `useMatcap` in the patch then.

blender-admin commented

2020-04-22 19:08:00 +02:00

This issue was referenced by 0dfe30502a

This issue was referenced by 0dfe30502a02c0567be50f980a8f1e00198cf93e

Robert Guetzkow commented

2020-04-22 19:19:26 +02:00

Changed status from 'Confirmed' to: 'Resolved'

Robert Guetzkow closed this issue

2020-04-22 19:19:26 +02:00

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

MatCap inverts if Viewport Display Metallic and Roughness are set below certain threshold #75938