Page MenuHomeWildfire Games

Removes GLSL shaders code duplication for fog and shadows
ClosedPublic

Authored by vladislavbelov on Jan 12 2021, 8:43 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP24598: Removes GLSL shaders code duplication for fog and shadows
Summary

Applies usages of the introduced #include directive. Significantly reduces code duplication for fog and shadows.

Test Plan
  1. Apply patch
  2. Make sure that objects with changed shaders look the same (w/ and w/o fog/shadows): water, terrain, models, particles, models with water, waterfalls.

Event Timeline

vladislavbelov created this revision.Jan 12 2021, 8:43 PM

Build is green

builderr-debug-macos.txt
../../../source/graphics/ShaderProgram.cpp:86:15: warning: 'Reload' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
        virtual void Reload()
                     ^
../../../source/graphics/ShaderProgram.h:124:15: note: overridden virtual function is here
        virtual void Reload() = 0;
                     ^
../../../source/graphics/ShaderProgram.cpp:118:15: warning: 'Bind' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
        virtual void Bind()
                     ^
../../../source/graphics/ShaderProgram.h:135:15: note: overridden virtual function is here
        virtual void Bind() = 0;
                     ^
../../../source/graphics/ShaderProgram.cpp:128:15: warning: 'Unbind' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
        virtual void Unbind()
                     ^
../../../source/graphics/ShaderProgram.h:140:

See https://jenkins.wildfiregames.com/job/macos-differential/2809/display/redirect for more details.

vladislavbelov requested review of this revision.Jan 12 2021, 9:10 PM
Stan added a subscriber: Stan.Jan 13 2021, 12:21 AM
Stan added inline comments.
binaries/data/mods/public/shaders/glsl/model_common.fs
3

Missing the common folder?

228

Why nuke macro? Is it unused?

binaries/data/mods/public/shaders/glsl/model_common.vs
176

Same

binaries/data/mods/public/shaders/glsl/model_water.vs
63

^

binaries/data/mods/public/shaders/glsl/model_waterfall.vs
46

^

vladislavbelov marked an inline comment as done.

Fixes notes.

binaries/data/mods/public/shaders/glsl/model_common.fs
228

No, it's moved in the function. It reduces the code duplication. Also we should think as a complete renderer as much possible to make it simpler to understand.

Stan added a comment.Jan 13 2021, 12:44 AM

Should h files get copyright headers?
This patch makes the code much cleaner at least.

binaries/data/mods/public/shaders/glsl/common/shadows_fragment.h
4

Why idented below, why don't do this elsewhere?

binaries/data/mods/public/shaders/glsl/model_common.fs
228

Function call inexpensive?

Build is green

builderr-debug-macos.txt
../../../source/graphics/ShaderProgram.cpp:86:15: warning: 'Reload' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
        virtual void Reload()
                     ^
../../../source/graphics/ShaderProgram.h:124:15: note: overridden virtual function is here
        virtual void Reload() = 0;
                     ^
../../../source/graphics/ShaderProgram.cpp:118:15: warning: 'Bind' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
        virtual void Bind()
                     ^
../../../source/graphics/ShaderProgram.h:135:15: note: overridden virtual function is here
        virtual void Bind() = 0;
                     ^
../../../source/graphics/ShaderProgram.cpp:128:15: warning: 'Unbind' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
        virtual void Unbind()
                     ^
../../../source/graphics/ShaderProgram.h:140:

See https://jenkins.wildfiregames.com/job/macos-differential/2817/display/redirect for more details.

In D3340#148586, @Stan wrote:

Should h files get copyright headers?

I'm not sure, shaders don't have copyright headers.

binaries/data/mods/public/shaders/glsl/common/shadows_fragment.h
4

Just copy, we have no standardized way to format nested macros.

binaries/data/mods/public/shaders/glsl/model_common.fs
228

There is no function calling in core GLSL (without extensions), the whole code will be inlined, as well as no recursion, because of performance reasons.

This revision was not accepted when it landed; it landed in state Needs Review.Jan 13 2021, 9:44 PM
This revision was automatically updated to reflect the committed changes.