Skip to content

Commit 7bfbd64

Browse files
committed
Make shader clip space consistent across APIs and projection matrices.
- All shaders are expected to output clip space values that are effectively y-up, with [-1, 1] z, in NDC. - The transform to a backend's expected clip space values from the above range now happens after user vertex shader code is run, instead of inside a projection matrix. - Projection matrices no longer need to be flipped when rendering to a canvas versus the main screen. This means custom projection matrices might need to be altered to account for the more consistent range.
1 parent 698d305 commit 7bfbd64

11 files changed

Lines changed: 93 additions & 68 deletions

File tree

src/modules/graphics/Graphics.cpp

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2831,29 +2831,14 @@ void Graphics::resetProjection()
28312831

28322832
state.useCustomProjection = false;
28332833

2834-
updateDeviceProjection(Matrix4::ortho(0.0f, w, 0.0f, h, -10.0f, 10.0f));
2834+
// NDC is y-up. The ortho() parameter names assume that as well. We want
2835+
// a y-down projection, so we set bottom to h and top to 0.
2836+
updateDeviceProjection(Matrix4::ortho(0.0f, w, h, 0.0f, -10.0f, 10.0f));
28352837
}
28362838

28372839
void Graphics::updateDeviceProjection(const Matrix4 &projection)
28382840
{
2839-
// Note: graphics implementations define computeDeviceProjection.
2840-
deviceProjectionMatrix = computeDeviceProjection(projection, isRenderTargetActive());
2841-
}
2842-
2843-
Matrix4 Graphics::calculateDeviceProjection(const Matrix4 &projection, uint32 flags) const
2844-
{
2845-
Matrix4 m = projection;
2846-
bool reverseZ = (flags & DEVICE_PROJECTION_REVERSE_Z) != 0;
2847-
2848-
if (flags & DEVICE_PROJECTION_FLIP_Y)
2849-
m.setRow(1, -m.getRow(1));
2850-
2851-
if (flags & DEVICE_PROJECTION_Z_01) // Go from Z [-1, 1] to Z [0, 1].
2852-
m.setRow(2, m.getRow(2) * (reverseZ ? -0.5f : 0.5f) + m.getRow(3));
2853-
else if (reverseZ)
2854-
m.setRow(2, -m.getRow(2));
2855-
2856-
return m;
2841+
deviceProjectionMatrix = projection;
28572842
}
28582843

28592844
STRINGMAP_CLASS_BEGIN(Graphics, Graphics::DrawMode, Graphics::DRAW_MAX_ENUM, drawMode)

src/modules/graphics/Graphics.h

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -627,6 +627,9 @@ class Graphics : public Module
627627
void setMeshCullMode(CullMode cull);
628628
CullMode getMeshCullMode() const;
629629

630+
// Note: These are meant to be relative to the y-down default projection,
631+
// which may be flipped compared to device NDC. Implementations may have
632+
// to flip the winding internally.
630633
virtual void setFrontFaceWinding(Winding winding) = 0;
631634
Winding getFrontFaceWinding() const;
632635

@@ -878,8 +881,6 @@ class Graphics : public Module
878881
void setCustomProjection(const Matrix4 &m);
879882
void resetProjection();
880883

881-
virtual Matrix4 computeDeviceProjection(const Matrix4 &projection, bool rendertotexture) const = 0;
882-
883884
virtual void draw(const DrawCommand &cmd) = 0;
884885
virtual void draw(const DrawIndexedCommand &cmd) = 0;
885886
virtual void drawQuads(int start, int count, const VertexAttributes &attributes, const BufferBindings &buffers, Texture *texture) = 0;
@@ -922,14 +923,6 @@ class Graphics : public Module
922923

923924
protected:
924925

925-
enum DeviceProjectionFlags
926-
{
927-
DEVICE_PROJECTION_DEFAULT = 0,
928-
DEVICE_PROJECTION_FLIP_Y = (1 << 0),
929-
DEVICE_PROJECTION_Z_01 = (1 << 1),
930-
DEVICE_PROJECTION_REVERSE_Z = (1 << 2),
931-
};
932-
933926
struct DisplayState
934927
{
935928
DisplayState();
@@ -1058,7 +1051,6 @@ class Graphics : public Module
10581051
void popTransform();
10591052

10601053
void updateDeviceProjection(const Matrix4 &projection);
1061-
Matrix4 calculateDeviceProjection(const Matrix4 &projection, uint32 flags) const;
10621054

10631055
int width;
10641056
int height;

src/modules/graphics/Shader.cpp

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,10 @@ static const char render_uniforms[] = R"(
9292
// but we can't guarantee that highp is always supported in fragment shaders...
9393
// We *really* don't want to use mediump for these in vertex shaders though.
9494
#ifdef LOVE_SPLIT_UNIFORMS_PER_DRAW
95-
uniform LOVE_HIGHP_OR_MEDIUMP vec4 love_UniformsPerDraw[12];
95+
uniform LOVE_HIGHP_OR_MEDIUMP vec4 love_UniformsPerDraw[13];
9696
uniform LOVE_HIGHP_OR_MEDIUMP vec4 love_UniformsPerDraw2[1];
9797
#else
98-
uniform LOVE_HIGHP_OR_MEDIUMP vec4 love_UniformsPerDraw[13];
98+
uniform LOVE_HIGHP_OR_MEDIUMP vec4 love_UniformsPerDraw[14];
9999
#endif
100100
101101
// Older GLSL doesn't support preprocessor line continuations...
@@ -107,12 +107,15 @@ uniform LOVE_HIGHP_OR_MEDIUMP vec4 love_UniformsPerDraw[13];
107107
108108
#define CurrentDPIScale (love_UniformsPerDraw[8].w)
109109
#define ConstantPointSize (love_UniformsPerDraw[9].w)
110-
#define ConstantColor (love_UniformsPerDraw[11])
110+
111+
#define love_ClipSpaceParams (love_UniformsPerDraw[11])
112+
113+
#define ConstantColor (love_UniformsPerDraw[12])
111114
112115
#ifdef LOVE_SPLIT_UNIFORMS_PER_DRAW
113116
#define love_ScreenSize (love_UniformsPerDraw2[0])
114117
#else
115-
#define love_ScreenSize (love_UniformsPerDraw[12])
118+
#define love_ScreenSize (love_UniformsPerDraw[13])
116119
#endif
117120
118121
// Alternate names
@@ -248,7 +251,13 @@ static const char vertex_header[] = R"(
248251
#endif
249252
)";
250253

251-
static const char vertex_functions[] = R"()";
254+
static const char vertex_functions[] = R"(
255+
vec4 love_clipSpaceTransform(vec4 clipPosition) {
256+
clipPosition.y *= love_ClipSpaceParams.x;
257+
clipPosition.z = (love_ClipSpaceParams.y * clipPosition.z + love_ClipSpaceParams.z * clipPosition.w) * love_ClipSpaceParams.w;
258+
return clipPosition;
259+
}
260+
)";
252261

253262
static const char vertex_main[] = R"(
254263
LOVE_IO_LOCATION(0) attribute vec4 VertexPosition;
@@ -264,6 +273,7 @@ void main() {
264273
VaryingTexCoord = VertexTexCoord;
265274
VaryingColor = gammaCorrectColor(VertexColor) * ConstantColor;
266275
love_Position = position(ClipSpaceFromLocal, VertexPosition);
276+
love_Position = love_clipSpaceTransform(love_Position);
267277
}
268278
)";
269279

@@ -272,6 +282,7 @@ void vertexmain();
272282
273283
void main() {
274284
vertexmain();
285+
love_Position = love_clipSpaceTransform(love_Position);
275286
}
276287
)";
277288

@@ -588,7 +599,7 @@ static Shader::EntryPoint getComputeEntryPoint(const std::string &src, const std
588599

589600
} // glsl
590601

591-
static_assert(sizeof(Shader::BuiltinUniformData) == sizeof(float) * 4 * 13, "Update the array in wrap_GraphicsShader.lua if this changes.");
602+
static_assert(sizeof(Shader::BuiltinUniformData) == sizeof(float) * 4 * 14, "Update the array in wrap_GraphicsShader.lua if this changes.");
592603

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

@@ -793,6 +804,28 @@ bool Shader::isDefaultActive()
793804
return false;
794805
}
795806

807+
Vector4 Shader::computeClipSpaceParams(uint32 clipSpaceTransformFlags)
808+
{
809+
// See the love_clipSpaceTransform vertex shader function.
810+
Vector4 params(1.0f, 1.0f, 0.0f, 1.0f);
811+
812+
if (clipSpaceTransformFlags & CLIP_TRANSFORM_FLIP_Y)
813+
params.x = -1.0f;
814+
815+
if (clipSpaceTransformFlags & CLIP_TRANSFORM_Z_NEG1_1_TO_0_1)
816+
{
817+
params.z = 1.0f;
818+
params.w = 0.5f;
819+
}
820+
else if (clipSpaceTransformFlags & CLIP_TRANSFORM_Z_0_1_TO_NEG1_1)
821+
{
822+
params.y = 2.0f;
823+
params.z = -1.0f;
824+
}
825+
826+
return params;
827+
}
828+
796829
const Shader::UniformInfo *Shader::getUniformInfo(const std::string &name) const
797830
{
798831
const auto it = reflection.allUniforms.find(name);

src/modules/graphics/Shader.h

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,14 @@ class Shader : public Object, public Resource
108108
ACCESS_WRITE = (1 << 1),
109109
};
110110

111+
enum ClipSpaceTransformFlags
112+
{
113+
CLIP_TRANSFORM_NONE = 0,
114+
CLIP_TRANSFORM_FLIP_Y = 1 << 0,
115+
CLIP_TRANSFORM_Z_NEG1_1_TO_0_1 = 1 << 1,
116+
CLIP_TRANSFORM_Z_0_1_TO_NEG1_1 = 1 << 2,
117+
};
118+
111119
struct CompileOptions
112120
{
113121
std::map<std::string, std::string> defines;
@@ -177,6 +185,7 @@ class Shader : public Object, public Resource
177185
Matrix4 transformMatrix;
178186
Matrix4 projectionMatrix;
179187
Vector4 normalMatrix[3]; // 3x3 matrix padded to an array of 3 vector4s.
188+
Vector4 clipSpaceParams;
180189
Colorf constantColor;
181190

182191
// Pixel shader-centric variables past this point.
@@ -212,6 +221,18 @@ class Shader : public Object, public Resource
212221
**/
213222
static bool isDefaultActive();
214223

224+
/**
225+
* Used for transforming standardized post-projection clip space positions
226+
* into the backend's current clip space.
227+
* Right now, the standard is:
228+
* NDC y is [-1, 1] starting at the bottom (y-up).
229+
* NDC z is [-1, 1].
230+
* Pixel coordinates are y-down.
231+
* Pixel (0, 0) in a texture is the top-left.
232+
* Aside from NDC z, this matches Metal and D3D12.
233+
*/
234+
static Vector4 computeClipSpaceParams(uint32 clipSpaceTransformFlags);
235+
215236
/**
216237
* Returns any warnings this Shader may have generated.
217238
**/

src/modules/graphics/metal/Graphics.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,6 @@ class Graphics final : public love::graphics::Graphics
6464
love::graphics::Texture *newTextureView(love::graphics::Texture *base, const Texture::ViewSettings &viewsettings) override;
6565
love::graphics::Buffer *newBuffer(const Buffer::Settings &settings, const std::vector<Buffer::DataDeclaration> &format, const void *data, size_t size, size_t arraylength) override;
6666

67-
Matrix4 computeDeviceProjection(const Matrix4 &projection, bool rendertotexture) const override;
68-
6967
void backbufferChanged(int width, int height, int pixelwidth, int pixelheight, bool backbufferstencil, bool backbufferdepth, int msaa) override;
7068
bool setMode(void *context, int width, int height, int pixelwidth, int pixelheight, bool backbufferstencil, bool backbufferdepth, int msaa) override;
7169
void unSetMode() override;

src/modules/graphics/metal/Graphics.mm

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -465,12 +465,6 @@ static inline void setSampler(id<MTLComputeCommandEncoder> encoder, Graphics::Re
465465
return new GraphicsReadback(this, method, texture, slice, mipmap, rect, dest, destx, desty);
466466
}
467467

468-
Matrix4 Graphics::computeDeviceProjection(const Matrix4 &projection, bool /*rendertotexture*/) const
469-
{
470-
uint32 flags = DEVICE_PROJECTION_FLIP_Y;
471-
return calculateDeviceProjection(projection, flags);
472-
}
473-
474468
void Graphics::backbufferChanged(int width, int height, int pixelwidth, int pixelheight, bool backbufferstencil, bool backbufferdepth, int msaa)
475469
{
476470
bool sizechanged = width != this->width || height != this->height
@@ -1133,6 +1127,9 @@ static bool isClampOne(SamplerState::WrapMode w)
11331127
// Same with point size.
11341128
builtins->normalMatrix[1].w = getPointSize();
11351129

1130+
uint32 flags = Shader::CLIP_TRANSFORM_Z_NEG1_1_TO_0_1;
1131+
builtins->clipSpaceParams = Shader::computeClipSpaceParams(flags);
1132+
11361133
builtins->screenSizeParams = Vector4(getPixelWidth(), getPixelHeight(), 1.0f, 0.0f);
11371134
auto rt = states.back().renderTargets.getFirstTarget();
11381135
if (rt.texture.get())

src/modules/graphics/opengl/Graphics.cpp

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -188,18 +188,6 @@ love::graphics::GraphicsReadback *Graphics::newReadbackInternal(ReadbackMethod m
188188
return new GraphicsReadback(this, method, texture, slice, mipmap, rect, dest, destx, desty);
189189
}
190190

191-
Matrix4 Graphics::computeDeviceProjection(const Matrix4 &projection, bool rendertotexture) const
192-
{
193-
uint32 flags = DEVICE_PROJECTION_DEFAULT;
194-
195-
// The projection matrix is flipped compared to rendering to a texture, due
196-
// to OpenGL considering (0,0) bottom-left instead of top-left.
197-
if (!rendertotexture)
198-
flags |= DEVICE_PROJECTION_FLIP_Y;
199-
200-
return calculateDeviceProjection(projection, flags);
201-
}
202-
203191
void Graphics::backbufferChanged(int width, int height, int pixelwidth, int pixelheight, bool backbufferstencil, bool backbufferdepth, int msaa)
204192
{
205193
bool changed = width != this->width || height != this->height

src/modules/graphics/opengl/Graphics.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,6 @@ class Graphics final : public love::graphics::Graphics
6060
love::graphics::Texture *newTextureView(love::graphics::Texture *base, const Texture::ViewSettings &viewsettings) override;
6161
love::graphics::Buffer *newBuffer(const Buffer::Settings &settings, const std::vector<Buffer::DataDeclaration> &format, const void *data, size_t size, size_t arraylength) override;
6262

63-
Matrix4 computeDeviceProjection(const Matrix4 &projection, bool rendertotexture) const override;
64-
6563
void backbufferChanged(int width, int height, int pixelwidth, int pixelheight, bool backbufferstencil, bool backbufferdepth, int msaa) override;
6664
bool setMode(void *context, int width, int height, int pixelwidth, int pixelheight, bool backbufferstencil, bool backbufferdepth, int msaa) override;
6765
void unSetMode() override;

src/modules/graphics/opengl/Shader.cpp

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -767,6 +767,8 @@ void Shader::updateBuiltinUniforms(love::graphics::Graphics *gfx, int viewportW,
767767
if (current != this)
768768
return;
769769

770+
bool rt = gfx->isRenderTargetActive();
771+
770772
BuiltinUniformData data;
771773

772774
data.transformMatrix = gfx->getTransform();
@@ -792,13 +794,26 @@ void Shader::updateBuiltinUniforms(love::graphics::Graphics *gfx, int viewportW,
792794
// Same with point size.
793795
data.normalMatrix[1].w = gfx->getPointSize();
794796

797+
// Users expect to work with y-up NDC, y-down pixel coordinates and textures
798+
// (see graphics/Shader.h).
799+
// OpenGL has y-up NDC and y-up pixel coordinates and textures. If we just flip
800+
// NDC y when rendering to a texture, it's enough to make (0, 0) on the texture
801+
// match what we expect when sampling from it - so it's the same as if textures
802+
// are y-down with y-up NDC.
803+
// Windowing systems treat (0, 0) on the backbuffer texture as the bottom left,
804+
// so we don't need to do that there.
805+
uint32 clipflags = 0;
806+
if (rt)
807+
clipflags |= CLIP_TRANSFORM_FLIP_Y;
808+
data.clipSpaceParams = computeClipSpaceParams(clipflags);
809+
795810
data.screenSizeParams.x = viewportW;
796811
data.screenSizeParams.y = viewportH;
797812

798813
// The shader does pixcoord.y = gl_FragCoord.y * params.z + params.w.
799814
// This lets us flip pixcoord.y when needed, to be consistent (drawing
800815
// with no RT active makes the pixel coordinates y-flipped.)
801-
if (gfx->isRenderTargetActive())
816+
if (rt)
802817
{
803818
// No flipping: pixcoord.y = gl_FragCoord.y * 1.0 + 0.0.
804819
data.screenSizeParams.z = 1.0f;
@@ -825,7 +840,7 @@ void Shader::updateBuiltinUniforms(love::graphics::Graphics *gfx, int viewportW,
825840
{
826841
GLint location = builtinUniforms[BUILTIN_UNIFORMS_PER_DRAW];
827842
if (location >= 0)
828-
glUniform4fv(location, 12, (const GLfloat *) &data);
843+
glUniform4fv(location, 13, (const GLfloat *) &data);
829844
GLint location2 = builtinUniforms[BUILTIN_UNIFORMS_PER_DRAW_2];
830845
if (location2 >= 0)
831846
glUniform4fv(location2, 1, (const GLfloat *) &data.screenSizeParams);
@@ -834,7 +849,7 @@ void Shader::updateBuiltinUniforms(love::graphics::Graphics *gfx, int viewportW,
834849
{
835850
GLint location = builtinUniforms[BUILTIN_UNIFORMS_PER_DRAW];
836851
if (location >= 0)
837-
glUniform4fv(location, 13, (const GLfloat *) &data);
852+
glUniform4fv(location, 14, (const GLfloat *) &data);
838853
}
839854
}
840855

src/modules/graphics/vulkan/Graphics.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1239,12 +1239,6 @@ bool Graphics::dispatch(love::graphics::Shader *shader, love::graphics::Buffer *
12391239
return true;
12401240
}
12411241

1242-
Matrix4 Graphics::computeDeviceProjection(const Matrix4 &projection, bool rendertotexture) const
1243-
{
1244-
uint32 flags = DEVICE_PROJECTION_DEFAULT;
1245-
return calculateDeviceProjection(projection, flags);
1246-
}
1247-
12481242
void Graphics::setRenderTargetsInternal(const RenderTargets &rts, int pixelw, int pixelh, bool hasSRGBtexture)
12491243
{
12501244
if (renderPassState.active)
@@ -1430,6 +1424,11 @@ graphics::Shader::BuiltinUniformData Graphics::getCurrentBuiltinUniformData()
14301424
// Same with point size.
14311425
data.normalMatrix[1].w = getPointSize();
14321426

1427+
// Flip y to convert input y-up [-1, 1] to vulkan's y-down [-1, 1].
1428+
// Convert input z [-1, 1] to vulkan [0, 1].
1429+
uint32 flags = Shader::CLIP_TRANSFORM_FLIP_Y | Shader::CLIP_TRANSFORM_Z_NEG1_1_TO_0_1;
1430+
data.clipSpaceParams = Shader::computeClipSpaceParams(flags);
1431+
14331432
const auto &rt = states.back().renderTargets.getFirstTarget();
14341433
if (rt.texture != nullptr)
14351434
{

0 commit comments

Comments
 (0)