Page MenuHome

Fix T89684: GPU builtin uniforms refactor.
AbandonedPublic

Authored by Jeroen Bakker (jbakker) on Jul 7 2021, 4:52 PM.

Details

Summary

For the vulkan port we are looking for a way to handle default uniforms.
Vulkan doesn't support default uniforms and GL_EXT_vulkan_glsl_relaxed isn't widely supported (yet).

The idea of this batch is to have builtin uniform block (named shaderBlock) that wraps the needed default uniforms in a define struct.

When constructing a shader the internal data structure of the shaderBlock can be selected (see GPU_shader_create_ex#shader_block.
This creates a uniform buffer managed by the shader containing. When setting an default uniform it is checked if it is inside the
shaderBlock. If that is the case the uniform buffer is updated and marked dirty.

Before each draw call the uniform buffer is uploaded (if dirty) and bound to the shaderBlock.

This patch also migrates GPU_SHADER_TEXT to the new structure. After reviewing and making sure that this is the right direction
we can migrate shaders per internal dependancy.

NOTE: During implementation many builtin shaders used non-builtin uniforms.

TODO

  • Add support for none builtin uniforms. Most builtin shaders don't have their uniforms defined as builtin. We could add them as buitin uniforms or relax our implementation to support non-buitin uniforms.

Diff Detail

Repository
rB Blender
Branch
temp-gpu-uniform-builtin-structs
Build Status
Buildable 15725
Build 15725: arc lint + arc unit

Event Timeline

Jeroen Bakker (jbakker) requested review of this revision.Jul 7 2021, 4:52 PM
Jeroen Bakker (jbakker) created this revision.
  • Fix crash: access unallocated memory.
  • Fix crash: incorrect uniform location for builtins.
  • Rename GPUUniformBuiltinStructType to GPUShaderBlockType.
  • Rename to ShaderBlocks
  • More renaming.
  • Test multiple paths to uniform retrieval.
  • Updating and binding of shader block.
  • Migrated GPU_SHADER_TEXT.
Jeroen Bakker (jbakker) retitled this revision from [WIP] Fix T89684: GPU builtin uniforms refactor. to Fix T89684: GPU builtin uniforms refactor..Jul 9 2021, 2:05 PM
Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)
  • Cleanup: use shader_block.
  • Cleanup: removed unused file.
  • Cleanup: removed empty line.

What I don't really like:

  • Builtin Block definitions are not in the shader itself: Having it declared in the compilation call makes it hard to follow. For this I would prefer to have a sort of include system.
  • Depending on the complexity, the struct declaration could be shared just like in eevee-rewrite branch. Not sure if it is a problem if the structs are only a handfull.
  • Builtin Blocks are monolithic. Impossible to reuse. For instance I would keep a ViewBlock and a ModelBlock separated and even not managed by the shader, only by the GPUMatrix module. I would put the srgbTransform and color uniforms into the same block.

I do think MVP matrix should be removed and just do the transform in 2 steps in the shaders.

Note: I still haven't reviewed everything properly.

source/blender/gpu/intern/gpu_uniform_buffer_private.hh
39

Not sure about forward declaration of classes when not even used in the header. Why not just include gpu_shader_interface.hh?

Replaced by GPUShaderCreateInfo