Applies usages of the introduced #include directive. Significantly reduces code duplication for fog and shadows.
Details
- Reviewers
- None
- Group Reviewers
Restricted Owners Package (Owns No Changed Paths) - Commits
- rP24598: Removes GLSL shaders code duplication for fog and shadows
- Apply patch
- Make sure that objects with changed shaders look the same (w/ and w/o fog/shadows): water, terrain, models, particles, models with water, waterfalls.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
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.
binaries/data/mods/public/shaders/glsl/model_common.fs | ||
---|---|---|
3 ↗ | (On Diff #15209) | Missing the common folder? |
228 ↗ | (On Diff #15209) | Why nuke macro? Is it unused? |
binaries/data/mods/public/shaders/glsl/model_common.vs | ||
176 ↗ | (On Diff #15209) | Same |
binaries/data/mods/public/shaders/glsl/model_water.vs | ||
63 ↗ | (On Diff #15209) | ^ |
binaries/data/mods/public/shaders/glsl/model_waterfall.vs | ||
46 ↗ | (On Diff #15209) | ^ |
Fixes notes.
binaries/data/mods/public/shaders/glsl/model_common.fs | ||
---|---|---|
228 ↗ | (On Diff #15209) | 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. |
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 ↗ | (On Diff #15220) | Why idented below, why don't do this elsewhere? |
binaries/data/mods/public/shaders/glsl/model_common.fs | ||
228 ↗ | (On Diff #15209) | 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.
I'm not sure, shaders don't have copyright headers.
binaries/data/mods/public/shaders/glsl/common/shadows_fragment.h | ||
---|---|---|
4 ↗ | (On Diff #15220) | Just copy, we have no standardized way to format nested macros. |
binaries/data/mods/public/shaders/glsl/model_common.fs | ||
228 ↗ | (On Diff #15209) | 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. |