Page MenuHome

Workbench Simplification Refactor
ClosedPublic

Authored by Clément Foucault (fclem) on Mar 8 2020, 2:08 AM.
Tags
None
Tokens
"100" token, awarded by shader."Love" token, awarded by franMarz."Love" token, awarded by brilliant_ape."Like" token, awarded by Tetone."Burninate" token, awarded by amonpaike.

Details

Summary

This patch is (almost) a complete rewrite of workbench engine. The features remain unchanged but the code quality is greatly improved.
This also introduce the concept of DRWShaderLibrary to make a simple include system inside the GLSL files.
I did not see any performance change at first glance but this needs more testing.

What it fixes

  • Infront with transparent object is now possible.
  • No more trick with the stencil buffer for wireframe masking.
  • Lower memory usage: Buffers are the same for the transparent and opaque passes. All compositing happens on main framebuffer
  • Replace WORKBENCH_Material allocations/setup/hash with UBOs containing materials properties in chunks of 4096 materials.
  • Remove a lot of shader variations.
  • Unify the opaque and transparent pipelines.
  • Offscreen rendering broken under some circumstances.

Remaining Regression:

  • Bug with TAA when operators setting overlay values not tagging view_updated. (needs to be fixed ASAP) Done
  • This patch seems to affect roughness a bit. This seems to be caused by the new encoding inside the material. Yes new encoding changes the value as it is rounded before quatization.
  • Hair rendering seems to be a bit brighter. Might be caused by the roughness change. Was intentional.
  • In front objects don't recieve SSAO (cavity effect) anymore. (design decision)
  • In front objects don't behave correctly with Depth Of Field effect. (same design decision as SSAO but could be fixed if enough demand for it)
  • All shader variations are compiled when opening blender. (optimization task) This does not seems to be a concern in my tests.

Diff Detail

Repository
rB Blender
Branch
tmp-workbench-rewrite
Build Status
Buildable 7078
Build 7078: arc lint + arc unit

Event Timeline

  • Workbench: Fix Ghosting when toggling Xray
  • Workbench: Optimize out uneeded passes.
  • Workbench: Fix performance regression caused by TAA samples init

I did 50% of the review so far. There are a lot of stuff I like, especially how the materials are now handled. Much better than the previous implementation. I do want to do some more indepth testing by opening old tickets are try to reproduce them. But so far it seems all ok.

source/blender/draw/engines/workbench/shaders/workbench_matcap_lib.glsl
22

This uniform can be moved to the world_block. It is constant for the whole frame.

source/blender/draw/engines/workbench/shaders/workbench_transparent_accum_frag.glsl
60

Could be moved in the
#ifdef V3D_LIGHTING_STUDIO as it is only used there

source/blender/draw/engines/workbench/workbench_data.c
79

* 8 is not clear to me.

183

use DRW_state_is_playback It does the same, but is nowwhere used :-)

source/blender/draw/engines/workbench/workbench_opaque.c
149

Can be removed

From running the workbench tests, the specular seems quite significantly different, especially for hair. Maybe worth looking at before committing?

Clément Foucault (fclem) marked 5 inline comments as done.
  • Put specular boolean inside world_ubo
  • Remove material data encoding from transparent shaders
  • Use DRW_state_is_playback instead of copy pasted code
  • Disable shadowing pass if not necessary
  • Merge branch 'master' into tmp-workbench-rewrite

I found a crash when dealling with lot of instances. I'm going to fix it.
Maybe it's related but AMD pro driver+ linux seems to have issues with instances now.

source/blender/draw/engines/workbench/workbench_data.c
79

2 for material_ubo_data is to avoid an assert.
8 is arbitrary to avoid too much memory fragmentation.

Code-wise I couldn't detect any issues. There are many new stuff in the patch that we also can utilize in other areas! Just some small notes:

  • The outline is now DPI dependent. We should use a different kernel as it now makes the lines very blurry.
  • Some aspects of the workbench we should write in a doc as parts are hard to describe in a comment. For example where the roughness/metallic are defined in the shader (OPAQUE_MATERIAL). I could spend some time on it during the code quality day next month
source/blender/draw/engines/workbench/workbench_materials.c
47

We should use eV3dShadingColorType. I think I missed this one during the clean up last week

source/blender/draw/engines/workbench/workbench_private.h
444

Use eV3dShadingColorType

source/blender/draw/engines/workbench/workbench_shader.c
120

Add a comment that these needs to be ordered?

source/blender/draw/intern/DRW_render.h
439

com_mask vs compare_mask? At least be consistent.

source/blender/draw/intern/draw_manager_data.c
565

should we remove the parameter?

  • Fix wrong materialId inside material setup resulting in buffer overflow.
Clément Foucault (fclem) marked 2 inline comments as done.
  • Merge branch 'master' into tmp-workbench-rewrite
  • Address review comments

@Brecht Van Lommel (brecht) I did change the hair shading (I just forgot I did). Previously the random shading wasn't applied correctly and made the hair color change too much compared to what it should be.

See this code snippet:

void workbench_hair_random_material(float rand,
                                    inout vec3 color,
                                    inout float roughness,
                                    inout float metallic)
{
  /* Center noise around 0. */
  rand -= 0.5;
  rand *= 0.1;
  /* Add some variation to the hairs to avoid uniform look. */
  metallic = clamp(metallic + rand, 0.0, 1.0);
  roughness = clamp(roughness + rand, 0.0, 1.0);
  /* Modulate by color intensity to reduce very high contrast when color is dark. */
  color = clamp(color + rand * (color + 0.05), 0.0, 1.0);
}

Previous code

#ifdef HAIR_SHADER
  /* Add some variation to the hairs to avoid uniform look. */
  float hair_variation = hair_rand * 0.1;
  color = clamp(color - hair_variation, 0.0, 1.0);
  metallic = clamp(materialColorAndMetal.a - hair_variation, 0.0, 1.0);
  roughness = clamp(materialRoughness - hair_variation, 0.0, 1.0);
#endif

Tests should be updated.

source/blender/draw/intern/draw_manager_data.c
565

This is mostly for API consistency. moreover, it's not excluded we might store the id inside the object in the future.

This revision is now accepted and ready to land.Mar 11 2020, 11:39 AM
This revision was automatically updated to reflect the committed changes.