From 7bfbd647ba9b6a8f4861bf58a80f898fe2e8d518 Mon Sep 17 00:00:00 2001 From: Sasha Szpakowski Date: Sat, 23 Mar 2024 22:00:02 -0300 Subject: [PATCH] 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. --- src/modules/graphics/Graphics.cpp | 23 +++--------- src/modules/graphics/Graphics.h | 14 ++------ src/modules/graphics/Shader.cpp | 45 ++++++++++++++++++++---- src/modules/graphics/Shader.h | 21 +++++++++++ src/modules/graphics/metal/Graphics.h | 2 -- src/modules/graphics/metal/Graphics.mm | 9 ++--- src/modules/graphics/opengl/Graphics.cpp | 12 ------- src/modules/graphics/opengl/Graphics.h | 2 -- src/modules/graphics/opengl/Shader.cpp | 21 +++++++++-- src/modules/graphics/vulkan/Graphics.cpp | 11 +++--- src/modules/graphics/vulkan/Graphics.h | 1 - 11 files changed, 93 insertions(+), 68 deletions(-) diff --git a/src/modules/graphics/Graphics.cpp b/src/modules/graphics/Graphics.cpp index da42a8a50..7d58e4ed6 100644 --- a/src/modules/graphics/Graphics.cpp +++ b/src/modules/graphics/Graphics.cpp @@ -2831,29 +2831,14 @@ void Graphics::resetProjection() state.useCustomProjection = false; - updateDeviceProjection(Matrix4::ortho(0.0f, w, 0.0f, h, -10.0f, 10.0f)); + // NDC is y-up. The ortho() parameter names assume that as well. We want + // a y-down projection, so we set bottom to h and top to 0. + updateDeviceProjection(Matrix4::ortho(0.0f, w, h, 0.0f, -10.0f, 10.0f)); } void Graphics::updateDeviceProjection(const Matrix4 &projection) { - // Note: graphics implementations define computeDeviceProjection. - deviceProjectionMatrix = computeDeviceProjection(projection, isRenderTargetActive()); -} - -Matrix4 Graphics::calculateDeviceProjection(const Matrix4 &projection, uint32 flags) const -{ - Matrix4 m = projection; - bool reverseZ = (flags & DEVICE_PROJECTION_REVERSE_Z) != 0; - - if (flags & DEVICE_PROJECTION_FLIP_Y) - m.setRow(1, -m.getRow(1)); - - if (flags & DEVICE_PROJECTION_Z_01) // Go from Z [-1, 1] to Z [0, 1]. - m.setRow(2, m.getRow(2) * (reverseZ ? -0.5f : 0.5f) + m.getRow(3)); - else if (reverseZ) - m.setRow(2, -m.getRow(2)); - - return m; + deviceProjectionMatrix = projection; } STRINGMAP_CLASS_BEGIN(Graphics, Graphics::DrawMode, Graphics::DRAW_MAX_ENUM, drawMode) diff --git a/src/modules/graphics/Graphics.h b/src/modules/graphics/Graphics.h index 9fa1155a3..50bc8e65a 100644 --- a/src/modules/graphics/Graphics.h +++ b/src/modules/graphics/Graphics.h @@ -627,6 +627,9 @@ class Graphics : public Module void setMeshCullMode(CullMode cull); CullMode getMeshCullMode() const; + // Note: These are meant to be relative to the y-down default projection, + // which may be flipped compared to device NDC. Implementations may have + // to flip the winding internally. virtual void setFrontFaceWinding(Winding winding) = 0; Winding getFrontFaceWinding() const; @@ -878,8 +881,6 @@ class Graphics : public Module void setCustomProjection(const Matrix4 &m); void resetProjection(); - virtual Matrix4 computeDeviceProjection(const Matrix4 &projection, bool rendertotexture) const = 0; - virtual void draw(const DrawCommand &cmd) = 0; virtual void draw(const DrawIndexedCommand &cmd) = 0; virtual void drawQuads(int start, int count, const VertexAttributes &attributes, const BufferBindings &buffers, Texture *texture) = 0; @@ -922,14 +923,6 @@ class Graphics : public Module protected: - enum DeviceProjectionFlags - { - DEVICE_PROJECTION_DEFAULT = 0, - DEVICE_PROJECTION_FLIP_Y = (1 << 0), - DEVICE_PROJECTION_Z_01 = (1 << 1), - DEVICE_PROJECTION_REVERSE_Z = (1 << 2), - }; - struct DisplayState { DisplayState(); @@ -1058,7 +1051,6 @@ class Graphics : public Module void popTransform(); void updateDeviceProjection(const Matrix4 &projection); - Matrix4 calculateDeviceProjection(const Matrix4 &projection, uint32 flags) const; int width; int height; diff --git a/src/modules/graphics/Shader.cpp b/src/modules/graphics/Shader.cpp index 0f5f4480c..c3839a421 100644 --- a/src/modules/graphics/Shader.cpp +++ b/src/modules/graphics/Shader.cpp @@ -92,10 +92,10 @@ 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[12]; +uniform LOVE_HIGHP_OR_MEDIUMP vec4 love_UniformsPerDraw[13]; uniform LOVE_HIGHP_OR_MEDIUMP vec4 love_UniformsPerDraw2[1]; #else -uniform LOVE_HIGHP_OR_MEDIUMP vec4 love_UniformsPerDraw[13]; +uniform LOVE_HIGHP_OR_MEDIUMP vec4 love_UniformsPerDraw[14]; #endif // Older GLSL doesn't support preprocessor line continuations... @@ -107,12 +107,15 @@ uniform LOVE_HIGHP_OR_MEDIUMP vec4 love_UniformsPerDraw[13]; #define CurrentDPIScale (love_UniformsPerDraw[8].w) #define ConstantPointSize (love_UniformsPerDraw[9].w) -#define ConstantColor (love_UniformsPerDraw[11]) + +#define love_ClipSpaceParams (love_UniformsPerDraw[11]) + +#define ConstantColor (love_UniformsPerDraw[12]) #ifdef LOVE_SPLIT_UNIFORMS_PER_DRAW #define love_ScreenSize (love_UniformsPerDraw2[0]) #else -#define love_ScreenSize (love_UniformsPerDraw[12]) +#define love_ScreenSize (love_UniformsPerDraw[13]) #endif // Alternate names @@ -248,7 +251,13 @@ static const char vertex_header[] = R"( #endif )"; -static const char vertex_functions[] = R"()"; +static const char vertex_functions[] = R"( +vec4 love_clipSpaceTransform(vec4 clipPosition) { + clipPosition.y *= love_ClipSpaceParams.x; + clipPosition.z = (love_ClipSpaceParams.y * clipPosition.z + love_ClipSpaceParams.z * clipPosition.w) * love_ClipSpaceParams.w; + return clipPosition; +} +)"; static const char vertex_main[] = R"( LOVE_IO_LOCATION(0) attribute vec4 VertexPosition; @@ -264,6 +273,7 @@ void main() { VaryingTexCoord = VertexTexCoord; VaryingColor = gammaCorrectColor(VertexColor) * ConstantColor; love_Position = position(ClipSpaceFromLocal, VertexPosition); + love_Position = love_clipSpaceTransform(love_Position); } )"; @@ -272,6 +282,7 @@ void vertexmain(); void main() { vertexmain(); + love_Position = love_clipSpaceTransform(love_Position); } )"; @@ -588,7 +599,7 @@ static Shader::EntryPoint getComputeEntryPoint(const std::string &src, const std } // glsl -static_assert(sizeof(Shader::BuiltinUniformData) == sizeof(float) * 4 * 13, "Update the array in wrap_GraphicsShader.lua if this changes."); +static_assert(sizeof(Shader::BuiltinUniformData) == sizeof(float) * 4 * 14, "Update the array in wrap_GraphicsShader.lua if this changes."); love::Type Shader::type("Shader", &Object::type); @@ -793,6 +804,28 @@ bool Shader::isDefaultActive() return false; } +Vector4 Shader::computeClipSpaceParams(uint32 clipSpaceTransformFlags) +{ + // See the love_clipSpaceTransform vertex shader function. + Vector4 params(1.0f, 1.0f, 0.0f, 1.0f); + + if (clipSpaceTransformFlags & CLIP_TRANSFORM_FLIP_Y) + params.x = -1.0f; + + if (clipSpaceTransformFlags & CLIP_TRANSFORM_Z_NEG1_1_TO_0_1) + { + params.z = 1.0f; + params.w = 0.5f; + } + else if (clipSpaceTransformFlags & CLIP_TRANSFORM_Z_0_1_TO_NEG1_1) + { + params.y = 2.0f; + params.z = -1.0f; + } + + return params; +} + const Shader::UniformInfo *Shader::getUniformInfo(const std::string &name) const { const auto it = reflection.allUniforms.find(name); diff --git a/src/modules/graphics/Shader.h b/src/modules/graphics/Shader.h index 51b538fd7..abe57be38 100644 --- a/src/modules/graphics/Shader.h +++ b/src/modules/graphics/Shader.h @@ -108,6 +108,14 @@ class Shader : public Object, public Resource ACCESS_WRITE = (1 << 1), }; + enum ClipSpaceTransformFlags + { + CLIP_TRANSFORM_NONE = 0, + CLIP_TRANSFORM_FLIP_Y = 1 << 0, + CLIP_TRANSFORM_Z_NEG1_1_TO_0_1 = 1 << 1, + CLIP_TRANSFORM_Z_0_1_TO_NEG1_1 = 1 << 2, + }; + struct CompileOptions { std::map defines; @@ -177,6 +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 clipSpaceParams; Colorf constantColor; // Pixel shader-centric variables past this point. @@ -212,6 +221,18 @@ class Shader : public Object, public Resource **/ static bool isDefaultActive(); + /** + * Used for transforming standardized post-projection clip space positions + * into the backend's current clip space. + * Right now, the standard is: + * NDC y is [-1, 1] starting at the bottom (y-up). + * NDC z is [-1, 1]. + * Pixel coordinates are y-down. + * Pixel (0, 0) in a texture is the top-left. + * Aside from NDC z, this matches Metal and D3D12. + */ + static Vector4 computeClipSpaceParams(uint32 clipSpaceTransformFlags); + /** * Returns any warnings this Shader may have generated. **/ diff --git a/src/modules/graphics/metal/Graphics.h b/src/modules/graphics/metal/Graphics.h index 41aa71be9..c23b5b090 100644 --- a/src/modules/graphics/metal/Graphics.h +++ b/src/modules/graphics/metal/Graphics.h @@ -64,8 +64,6 @@ class Graphics final : public love::graphics::Graphics love::graphics::Texture *newTextureView(love::graphics::Texture *base, const Texture::ViewSettings &viewsettings) override; love::graphics::Buffer *newBuffer(const Buffer::Settings &settings, const std::vector &format, const void *data, size_t size, size_t arraylength) override; - Matrix4 computeDeviceProjection(const Matrix4 &projection, bool rendertotexture) const override; - void backbufferChanged(int width, int height, int pixelwidth, int pixelheight, bool backbufferstencil, bool backbufferdepth, int msaa) override; bool setMode(void *context, int width, int height, int pixelwidth, int pixelheight, bool backbufferstencil, bool backbufferdepth, int msaa) override; void unSetMode() override; diff --git a/src/modules/graphics/metal/Graphics.mm b/src/modules/graphics/metal/Graphics.mm index 5319f7dc4..6fbe01a69 100644 --- a/src/modules/graphics/metal/Graphics.mm +++ b/src/modules/graphics/metal/Graphics.mm @@ -465,12 +465,6 @@ static inline void setSampler(id encoder, Graphics::Re return new GraphicsReadback(this, method, texture, slice, mipmap, rect, dest, destx, desty); } -Matrix4 Graphics::computeDeviceProjection(const Matrix4 &projection, bool /*rendertotexture*/) const -{ - uint32 flags = DEVICE_PROJECTION_FLIP_Y; - return calculateDeviceProjection(projection, flags); -} - void Graphics::backbufferChanged(int width, int height, int pixelwidth, int pixelheight, bool backbufferstencil, bool backbufferdepth, int msaa) { bool sizechanged = width != this->width || height != this->height @@ -1133,6 +1127,9 @@ static bool isClampOne(SamplerState::WrapMode w) // Same with point size. builtins->normalMatrix[1].w = getPointSize(); + uint32 flags = Shader::CLIP_TRANSFORM_Z_NEG1_1_TO_0_1; + builtins->clipSpaceParams = Shader::computeClipSpaceParams(flags); + builtins->screenSizeParams = Vector4(getPixelWidth(), getPixelHeight(), 1.0f, 0.0f); auto rt = states.back().renderTargets.getFirstTarget(); if (rt.texture.get()) diff --git a/src/modules/graphics/opengl/Graphics.cpp b/src/modules/graphics/opengl/Graphics.cpp index a7a3b3ab0..f0450dfbf 100644 --- a/src/modules/graphics/opengl/Graphics.cpp +++ b/src/modules/graphics/opengl/Graphics.cpp @@ -188,18 +188,6 @@ love::graphics::GraphicsReadback *Graphics::newReadbackInternal(ReadbackMethod m return new GraphicsReadback(this, method, texture, slice, mipmap, rect, dest, destx, desty); } -Matrix4 Graphics::computeDeviceProjection(const Matrix4 &projection, bool rendertotexture) const -{ - uint32 flags = DEVICE_PROJECTION_DEFAULT; - - // The projection matrix is flipped compared to rendering to a texture, due - // to OpenGL considering (0,0) bottom-left instead of top-left. - if (!rendertotexture) - flags |= DEVICE_PROJECTION_FLIP_Y; - - return calculateDeviceProjection(projection, flags); -} - void Graphics::backbufferChanged(int width, int height, int pixelwidth, int pixelheight, bool backbufferstencil, bool backbufferdepth, int msaa) { bool changed = width != this->width || height != this->height diff --git a/src/modules/graphics/opengl/Graphics.h b/src/modules/graphics/opengl/Graphics.h index 7736731f8..351532b39 100644 --- a/src/modules/graphics/opengl/Graphics.h +++ b/src/modules/graphics/opengl/Graphics.h @@ -60,8 +60,6 @@ class Graphics final : public love::graphics::Graphics love::graphics::Texture *newTextureView(love::graphics::Texture *base, const Texture::ViewSettings &viewsettings) override; love::graphics::Buffer *newBuffer(const Buffer::Settings &settings, const std::vector &format, const void *data, size_t size, size_t arraylength) override; - Matrix4 computeDeviceProjection(const Matrix4 &projection, bool rendertotexture) const override; - void backbufferChanged(int width, int height, int pixelwidth, int pixelheight, bool backbufferstencil, bool backbufferdepth, int msaa) override; bool setMode(void *context, int width, int height, int pixelwidth, int pixelheight, bool backbufferstencil, bool backbufferdepth, int msaa) override; void unSetMode() override; diff --git a/src/modules/graphics/opengl/Shader.cpp b/src/modules/graphics/opengl/Shader.cpp index 21e26fb0f..3885b7d8a 100644 --- a/src/modules/graphics/opengl/Shader.cpp +++ b/src/modules/graphics/opengl/Shader.cpp @@ -767,6 +767,8 @@ void Shader::updateBuiltinUniforms(love::graphics::Graphics *gfx, int viewportW, if (current != this) return; + bool rt = gfx->isRenderTargetActive(); + BuiltinUniformData data; data.transformMatrix = gfx->getTransform(); @@ -792,13 +794,26 @@ void Shader::updateBuiltinUniforms(love::graphics::Graphics *gfx, int viewportW, // Same with point size. data.normalMatrix[1].w = gfx->getPointSize(); + // Users expect to work with y-up NDC, y-down pixel coordinates and textures + // (see graphics/Shader.h). + // OpenGL has y-up NDC and y-up pixel coordinates and textures. If we just flip + // NDC y when rendering to a texture, it's enough to make (0, 0) on the texture + // match what we expect when sampling from it - so it's the same as if textures + // are y-down with y-up NDC. + // Windowing systems treat (0, 0) on the backbuffer texture as the bottom left, + // so we don't need to do that there. + uint32 clipflags = 0; + if (rt) + clipflags |= CLIP_TRANSFORM_FLIP_Y; + data.clipSpaceParams = computeClipSpaceParams(clipflags); + data.screenSizeParams.x = viewportW; data.screenSizeParams.y = viewportH; // The shader does pixcoord.y = gl_FragCoord.y * params.z + params.w. // This lets us flip pixcoord.y when needed, to be consistent (drawing // with no RT active makes the pixel coordinates y-flipped.) - if (gfx->isRenderTargetActive()) + if (rt) { // No flipping: pixcoord.y = gl_FragCoord.y * 1.0 + 0.0. data.screenSizeParams.z = 1.0f; @@ -825,7 +840,7 @@ void Shader::updateBuiltinUniforms(love::graphics::Graphics *gfx, int viewportW, { GLint location = builtinUniforms[BUILTIN_UNIFORMS_PER_DRAW]; if (location >= 0) - glUniform4fv(location, 12, (const GLfloat *) &data); + glUniform4fv(location, 13, (const GLfloat *) &data); GLint location2 = builtinUniforms[BUILTIN_UNIFORMS_PER_DRAW_2]; if (location2 >= 0) glUniform4fv(location2, 1, (const GLfloat *) &data.screenSizeParams); @@ -834,7 +849,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, 14, (const GLfloat *) &data); } } diff --git a/src/modules/graphics/vulkan/Graphics.cpp b/src/modules/graphics/vulkan/Graphics.cpp index 3cdfc145e..b4d98a0ce 100644 --- a/src/modules/graphics/vulkan/Graphics.cpp +++ b/src/modules/graphics/vulkan/Graphics.cpp @@ -1239,12 +1239,6 @@ bool Graphics::dispatch(love::graphics::Shader *shader, love::graphics::Buffer * return true; } -Matrix4 Graphics::computeDeviceProjection(const Matrix4 &projection, bool rendertotexture) const -{ - uint32 flags = DEVICE_PROJECTION_DEFAULT; - return calculateDeviceProjection(projection, flags); -} - void Graphics::setRenderTargetsInternal(const RenderTargets &rts, int pixelw, int pixelh, bool hasSRGBtexture) { if (renderPassState.active) @@ -1430,6 +1424,11 @@ graphics::Shader::BuiltinUniformData Graphics::getCurrentBuiltinUniformData() // Same with point size. data.normalMatrix[1].w = 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]. + uint32 flags = Shader::CLIP_TRANSFORM_FLIP_Y | Shader::CLIP_TRANSFORM_Z_NEG1_1_TO_0_1; + data.clipSpaceParams = Shader::computeClipSpaceParams(flags); + const auto &rt = states.back().renderTargets.getFirstTarget(); if (rt.texture != nullptr) { diff --git a/src/modules/graphics/vulkan/Graphics.h b/src/modules/graphics/vulkan/Graphics.h index ca8b8e3a5..ffaff9399 100644 --- a/src/modules/graphics/vulkan/Graphics.h +++ b/src/modules/graphics/vulkan/Graphics.h @@ -271,7 +271,6 @@ class Graphics final : public love::graphics::Graphics graphics::GraphicsReadback *newReadbackInternal(ReadbackMethod method, love::graphics::Texture *texture, int slice, int mipmap, const Rect &rect, image::ImageData *dest, int destx, int desty) override; void clear(OptionalColorD color, OptionalInt stencil, OptionalDouble depth) override; void clear(const std::vector &colors, OptionalInt stencil, OptionalDouble depth) override; - Matrix4 computeDeviceProjection(const Matrix4 &projection, bool rendertotexture) const override; void discard(const std::vector& colorbuffers, bool depthstencil) override; void present(void *screenshotCallbackdata) override; void backbufferChanged(int width, int height, int pixelwidth, int pixelheight, bool backbufferstencil, bool backbufferdepth, int msaa) override;