Skip to content

Commit 79858d7

Browse files
committed
Fix bad material SSBO uniform layout for multiple of 16
If the uniform size is divisible by 16 it wrongly thought that 16 bytes of padding (instead of 0) were needed. Add a unit test demonstrating the case that was broken.
1 parent 8a64d1c commit 79858d7

File tree

4 files changed

+83
-3
lines changed

4 files changed

+83
-3
lines changed

CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -952,7 +952,7 @@ if (BUILD_CLIENT)
952952
Flags ${WARNINGS}
953953
Files ${WIN_RC} ${BUILDINFOLIST} ${QCOMMONLIST} ${SERVERLIST} ${CLIENTBASELIST} ${CLIENTLIST}
954954
Libs ${LIBS_CLIENT} ${LIBS_CLIENTBASE} ${LIBS_ENGINE}
955-
Tests ${ENGINETESTLIST}
955+
Tests ${CLIENTTESTLIST}
956956
)
957957

958958
# generate glsl include files

src.cmake

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,10 @@ set(CLIENTLIST
356356
${RENDERERLIST}
357357
)
358358

359+
set(CLIENTTESTLIST ${ENGINETESTLIST}
360+
${ENGINE_DIR}/renderer/gl_shader_test.cpp
361+
)
362+
359363
set(TTYCLIENTLIST
360364
${ENGINE_DIR}/null/NullAudio.cpp
361365
${ENGINE_DIR}/null/NullKeyboard.cpp

src/engine/renderer/gl_shader.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2128,14 +2128,14 @@ void GLShader::PostProcessUniforms() {
21282128
GLuint size = _materialSystemUniforms[0]->_std430Size;
21292129
GLuint components = _materialSystemUniforms[0]->_components;
21302130
size = components ? PAD( size, 4 ) * components : size;
2131-
GLuint alignmentConsume = 4 - size % 4;
2131+
GLuint alignmentConsume = PAD( size, 4 ) - size;
21322132

21332133
GLUniform* tmpUniform = _materialSystemUniforms[0];
21342134
tmp.emplace_back( _materialSystemUniforms[0] );
21352135
_materialSystemUniforms.erase( _materialSystemUniforms.begin() );
21362136

21372137
int uniform;
2138-
while ( ( alignmentConsume & 3 ) && _materialSystemUniforms.size()
2138+
while ( alignmentConsume && _materialSystemUniforms.size()
21392139
&& ( uniform = FindUniformForAlignment( _materialSystemUniforms, alignmentConsume ) ) != -1 ) {
21402140
alignmentConsume -= _materialSystemUniforms[uniform]->_std430Size;
21412141

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/*
2+
===========================================================================
3+
Daemon BSD Source Code
4+
Copyright (c) 2025, Daemon Developers
5+
All rights reserved.
6+
7+
Redistribution and use in source and binary forms, with or without
8+
modification, are permitted provided that the following conditions are met:
9+
* Redistributions of source code must retain the above copyright
10+
notice, this list of conditions and the following disclaimer.
11+
* Redistributions in binary form must reproduce the above copyright
12+
notice, this list of conditions and the following disclaimer in the
13+
documentation and/or other materials provided with the distribution.
14+
* Neither the name of the Daemon developers nor the
15+
names of its contributors may be used to endorse or promote products
16+
derived from this software without specific prior written permission.
17+
18+
THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
19+
ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
20+
WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
21+
DISCLAIMED. IN NO EVENT SHALL DAEMON DEVELOPERS BE LIABLE FOR ANY
22+
DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
23+
(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
24+
LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
25+
ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
26+
(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
27+
SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
28+
===========================================================================
29+
*/
30+
31+
#include <gtest/gtest.h>
32+
33+
#include "common/Common.h"
34+
35+
#include "engine/renderer/gl_shader.h"
36+
37+
namespace {
38+
39+
class MaterialUniformPackingTestShaderBase : public GLShader
40+
{
41+
public:
42+
MaterialUniformPackingTestShaderBase()
43+
: GLShader("MaterialUniformPackingTestShaderBase", 0, true, "", "") {}
44+
45+
std::vector<GLUniform*> GetUniforms()
46+
{
47+
PostProcessUniforms();
48+
return _materialSystemUniforms;
49+
}
50+
};
51+
52+
template<typename U, typename ShaderT>
53+
GLUniform* Get(ShaderT& shader)
54+
{
55+
// assume U has a single base class of GLUniform
56+
return reinterpret_cast<GLUniform*>(static_cast<U*>(&shader));
57+
}
58+
59+
TEST(MaterialUniformPackingTest, OneMatrix)
60+
{
61+
class Shader1 : public MaterialUniformPackingTestShaderBase,
62+
public u_ModelViewMatrix //mat4
63+
{
64+
public:
65+
Shader1() : u_ModelViewMatrix(this) {}
66+
};
67+
68+
Shader1 shader1;
69+
std::vector<GLUniform*> uniforms = shader1.GetUniforms();
70+
EXPECT_EQ(shader1.GetSTD430Size(), 16u);
71+
ASSERT_EQ(uniforms.size(), 1);
72+
EXPECT_EQ(uniforms[0], Get<u_ModelViewMatrix>(shader1));
73+
EXPECT_EQ(uniforms[0]->_std430Size, 16u);
74+
}
75+
76+
} // namespace

0 commit comments

Comments
 (0)