Skip to content

Commit

Permalink
Remove NormalMatrix shader uniform.
Browse files Browse the repository at this point in the history
It doesn't provide much value given its costs. It also didn't work properly with auto-batched draws.
  • Loading branch information
slime73 committed Jun 17, 2024
1 parent e180b25 commit bfc4212
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 71 deletions.
19 changes: 8 additions & 11 deletions src/modules/graphics/Shader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,37 +92,34 @@ static const char render_uniforms[] = R"(
// but we can't guarantee that highp is always supported in fragment shaders...
// We *really* don't want to use mediump for these in vertex shaders though.
#ifdef LOVE_SPLIT_UNIFORMS_PER_DRAW
uniform LOVE_HIGHP_OR_MEDIUMP vec4 love_UniformsPerDraw[13];
uniform LOVE_HIGHP_OR_MEDIUMP vec4 love_UniformsPerDraw[11];
uniform LOVE_HIGHP_OR_MEDIUMP vec4 love_UniformsPerDraw2[1];
#else
uniform LOVE_HIGHP_OR_MEDIUMP vec4 love_UniformsPerDraw[14];
uniform LOVE_HIGHP_OR_MEDIUMP vec4 love_UniformsPerDraw[12];
#endif
// Older GLSL doesn't support preprocessor line continuations...
#define TransformMatrix mat4(love_UniformsPerDraw[0], love_UniformsPerDraw[1], love_UniformsPerDraw[2], love_UniformsPerDraw[3])
#define ProjectionMatrix mat4(love_UniformsPerDraw[4], love_UniformsPerDraw[5], love_UniformsPerDraw[6], love_UniformsPerDraw[7])
#define TransformProjectionMatrix (ProjectionMatrix * TransformMatrix)
#define NormalMatrix mat3(love_UniformsPerDraw[8].xyz, love_UniformsPerDraw[9].xyz, love_UniformsPerDraw[10].xyz)
#define CurrentDPIScale (love_UniformsPerDraw[8].x)
#define ConstantPointSize (love_UniformsPerDraw[8].y)
#define CurrentDPIScale (love_UniformsPerDraw[8].w)
#define ConstantPointSize (love_UniformsPerDraw[9].w)
#define love_ClipSpaceParams (love_UniformsPerDraw[9])
#define love_ClipSpaceParams (love_UniformsPerDraw[11])
#define ConstantColor (love_UniformsPerDraw[12])
#define ConstantColor (love_UniformsPerDraw[10])
#ifdef LOVE_SPLIT_UNIFORMS_PER_DRAW
#define love_ScreenSize (love_UniformsPerDraw2[0])
#else
#define love_ScreenSize (love_UniformsPerDraw[13])
#define love_ScreenSize (love_UniformsPerDraw[11])
#endif
// Alternate names
#define ViewSpaceFromLocal TransformMatrix
#define ClipSpaceFromView ProjectionMatrix
#define ClipSpaceFromLocal TransformProjectionMatrix
#define ViewNormalFromLocal NormalMatrix
)";

static const char global_functions[] = R"(
Expand Down Expand Up @@ -599,7 +596,7 @@ static Shader::EntryPoint getComputeEntryPoint(const std::string &src, const std

} // glsl

static_assert(sizeof(Shader::BuiltinUniformData) == sizeof(float) * 4 * 14, "Update the array in wrap_GraphicsShader.lua if this changes.");
static_assert(sizeof(Shader::BuiltinUniformData) == sizeof(float) * 4 * 12, "Update the array in wrap_GraphicsShader.lua if this changes.");

love::Type Shader::type("Shader", &Object::type);

Expand Down
2 changes: 1 addition & 1 deletion src/modules/graphics/Shader.h
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ class Shader : public Object, public Resource
{
Matrix4 transformMatrix;
Matrix4 projectionMatrix;
Vector4 normalMatrix[3]; // 3x3 matrix padded to an array of 3 vector4s.
Vector4 scaleParams;
Vector4 clipSpaceParams;
Colorf constantColor;

Expand Down
21 changes: 2 additions & 19 deletions src/modules/graphics/metal/Graphics.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1116,25 +1116,8 @@ static bool isClampOne(SamplerState::WrapMode w)
builtins->transformMatrix = getTransform();
builtins->projectionMatrix = getDeviceProjection();

// The normal matrix is the transpose of the inverse of the rotation portion
// (top-left 3x3) of the transform matrix.
{
Matrix3 normalmatrix = Matrix3(builtins->transformMatrix).transposedInverse();
const float *e = normalmatrix.getElements();
for (int i = 0; i < 3; i++)
{
builtins->normalMatrix[i].x = e[i * 3 + 0];
builtins->normalMatrix[i].y = e[i * 3 + 1];
builtins->normalMatrix[i].z = e[i * 3 + 2];
builtins->normalMatrix[i].w = 0.0f;
}
}

// Store DPI scale in an unused component of another vector.
builtins->normalMatrix[0].w = (float) getCurrentDPIScale();

// Same with point size.
builtins->normalMatrix[1].w = getPointSize();
builtins->scaleParams.x = = (float) getCurrentDPIScale();
builtins->scaleParams.y = getPointSize();

uint32 flags = Shader::CLIP_TRANSFORM_Z_NEG1_1_TO_0_1;
builtins->clipSpaceParams = Shader::computeClipSpaceParams(flags);
Expand Down
25 changes: 4 additions & 21 deletions src/modules/graphics/opengl/Shader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -774,25 +774,8 @@ void Shader::updateBuiltinUniforms(love::graphics::Graphics *gfx, int viewportW,
data.transformMatrix = gfx->getTransform();
data.projectionMatrix = gfx->getDeviceProjection();

// The normal matrix is the transpose of the inverse of the rotation portion
// (top-left 3x3) of the transform matrix.
{
Matrix3 normalmatrix = Matrix3(data.transformMatrix).transposedInverse();
const float *e = normalmatrix.getElements();
for (int i = 0; i < 3; i++)
{
data.normalMatrix[i].x = e[i * 3 + 0];
data.normalMatrix[i].y = e[i * 3 + 1];
data.normalMatrix[i].z = e[i * 3 + 2];
data.normalMatrix[i].w = 0.0f;
}
}

// Store DPI scale in an unused component of another vector.
data.normalMatrix[0].w = (float) gfx->getCurrentDPIScale();

// Same with point size.
data.normalMatrix[1].w = gfx->getPointSize();
data.scaleParams.x = (float) gfx->getCurrentDPIScale();
data.scaleParams.y = gfx->getPointSize();

// Users expect to work with y-up NDC, y-down pixel coordinates and textures
// (see graphics/Shader.h).
Expand Down Expand Up @@ -840,7 +823,7 @@ void Shader::updateBuiltinUniforms(love::graphics::Graphics *gfx, int viewportW,
{
GLint location = builtinUniforms[BUILTIN_UNIFORMS_PER_DRAW];
if (location >= 0)
glUniform4fv(location, 13, (const GLfloat *) &data);
glUniform4fv(location, 11, (const GLfloat *) &data);
GLint location2 = builtinUniforms[BUILTIN_UNIFORMS_PER_DRAW_2];
if (location2 >= 0)
glUniform4fv(location2, 1, (const GLfloat *) &data.screenSizeParams);
Expand All @@ -849,7 +832,7 @@ void Shader::updateBuiltinUniforms(love::graphics::Graphics *gfx, int viewportW,
{
GLint location = builtinUniforms[BUILTIN_UNIFORMS_PER_DRAW];
if (location >= 0)
glUniform4fv(location, 14, (const GLfloat *) &data);
glUniform4fv(location, 12, (const GLfloat *) &data);
}
}

Expand Down
21 changes: 2 additions & 19 deletions src/modules/graphics/vulkan/Graphics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1409,25 +1409,8 @@ graphics::Shader::BuiltinUniformData Graphics::getCurrentBuiltinUniformData()
data.transformMatrix = getTransform();
data.projectionMatrix = getDeviceProjection();

// The normal matrix is the transpose of the inverse of the rotation portion
// (top-left 3x3) of the transform matrix.
{
Matrix3 normalmatrix = Matrix3(data.transformMatrix).transposedInverse();
const float *e = normalmatrix.getElements();
for (int i = 0; i < 3; i++)
{
data.normalMatrix[i].x = e[i * 3 + 0];
data.normalMatrix[i].y = e[i * 3 + 1];
data.normalMatrix[i].z = e[i * 3 + 2];
data.normalMatrix[i].w = 0.0f;
}
}

// Store DPI scale in an unused component of another vector.
data.normalMatrix[0].w = (float)getCurrentDPIScale();

// Same with point size.
data.normalMatrix[1].w = getPointSize();
data.scaleParams.x = (float) getCurrentDPIScale();
data.scaleParams.y = getPointSize();;

// Flip y to convert input y-up [-1, 1] to vulkan's y-down [-1, 1].
// Convert input z [-1, 1] to vulkan [0, 1].
Expand Down

0 comments on commit bfc4212

Please sign in to comment.