GPU: Split gpu_shader_material into multiple files.
ClosedPublic
Actions

Authored by Omar Emara (OmarSquircleArt) on Aug 28 2019, 6:52 PM.

Details

Reviewers

Brecht Van Lommel (brecht)
Clément Foucault (fclem)

Commits

rB8cd0da88e55a: GPU: Split gpu_shader_material into multiple files.

Summary

This patch continue the efforts to split the gpu_shader_material file
started in D5569.

Dependency resolution has been refactored to be recursive, so it is
sufficient to add the direct dependencies only. Each shading node
gets its own file. Additionally, some utility files are added to be
shared between files, like math_util, color_util, and hash.
Some files are always included because they are used in the execution
function, like world_normals.

Some glsl functions appears to be unused, so they were removed, like
output_node, bits_to_01, and exp_blender. Other functions have
been renamed to be more general and get used as utils, like texco_norm
which became vector_normalize.

A lot of the opengl tests fails, but those same tests also fail in
master, so this is probably unrelated to this patch.

Diff Detail

Repository

rB Blender

Branch

split-shader-material-glsl (branched from master)

Build Status

Buildable 4654
Build 4654: arc lint + arc unit

Event Timeline

Omar Emara (OmarSquircleArt) created this revision.Aug 28 2019, 6:52 PM

Harbormaster completed remote builds in B4637: Diff 17610.Aug 28 2019, 6:52 PM

Rename color_alpha utils to be more general.

Harbormaster completed remote builds in B4638: Diff 17614.Aug 28 2019, 7:24 PM

Update cell noise dependencies.

Harbormaster completed remote builds in B4654: Diff 17642.Aug 28 2019, 10:50 PM

Fix anisotropic stubbing for volumetrics.

Harbormaster completed remote builds in B4656: Diff 17644.Aug 28 2019, 11:10 PM

Brecht Van Lommel (brecht) added a reviewer: Clément Foucault (fclem).Aug 29 2019, 11:26 AM

Thanks for doing this.

source/blender/gpu/intern/gpu_codegen.c
1785	This is too slow, we should not loop over the entire array for every dependency. If looking up dependencies becomes this slow it can be done in `code_generate_material_library` instead of for every node added. Even then it would be good to avoid `O(n^2)` looping.
source/blender/gpu/intern/gpu_material_library.h
129	The dependencies array must be ended with a `, NULL` terminator. Otherwise if we reach the array size limit we'll read past the end.

This revision now requires changes to proceed.Aug 29 2019, 11:41 AM

Omar Emara (OmarSquircleArt) added inline comments.Aug 29 2019, 12:00 PM

source/blender/gpu/intern/gpu_codegen.c
1785	To avoid that, can we edit the `GPUMaterialLibrary` structure such that the dependencies array is an array of pointers to `GPUMaterialLibrary`? Would that work? To clarify, do you want me to add a call to a function in `code_generate_material_library` to resolve the dependencies only one time? Or do you want to revert back to non recursive dependency resolution?
source/blender/gpu/intern/gpu_material_library.h
129	The C standard guarantees the other elements of the array to be zero initialized, see §6.7.9 section 21 and §6.7.9 section 10 of the C11 standard. Or is this not what you meant?

Brecht Van Lommel (brecht) added inline comments.Aug 29 2019, 1:07 PM

source/blender/gpu/intern/gpu_codegen.c
1785	I'm fine with recursive dependency resolution if it's needed. Using an array of `GPUMaterialLibrary` pointers sounds good to me. At that point it doesn't really matter if it's done here or in `code_generate_material_library`, so may as well keep it here.
source/blender/gpu/intern/gpu_material_library.h
129	This array has a fixed size. If you add 8 dependencies, it will silently not be `NULL` terminated. If you add `, NULL`, it will give a compile error.