Skip to content

Commit

Permalink
Make shader clip space consistent across APIs and projection matrices.
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
slime73 committed Mar 24, 2024
1 parent 698d305 commit 7bfbd64
Show file tree
Hide file tree
Showing 11 changed files with 93 additions and 68 deletions.
23 changes: 4 additions & 19 deletions src/modules/graphics/Graphics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 3 additions & 11 deletions src/modules/graphics/Graphics.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down
45 changes: 39 additions & 6 deletions src/modules/graphics/Shader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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...
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -264,6 +273,7 @@ void main() {
VaryingTexCoord = VertexTexCoord;
VaryingColor = gammaCorrectColor(VertexColor) * ConstantColor;
love_Position = position(ClipSpaceFromLocal, VertexPosition);
love_Position = love_clipSpaceTransform(love_Position);
}
)";

Expand All @@ -272,6 +282,7 @@ void vertexmain();
void main() {
vertexmain();
love_Position = love_clipSpaceTransform(love_Position);
}
)";

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand Down
21 changes: 21 additions & 0 deletions src/modules/graphics/Shader.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string, std::string> defines;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
**/
Expand Down
2 changes: 0 additions & 2 deletions src/modules/graphics/metal/Graphics.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<Buffer::DataDeclaration> &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;
Expand Down
9 changes: 3 additions & 6 deletions src/modules/graphics/metal/Graphics.mm
Original file line number Diff line number Diff line change
Expand Up @@ -465,12 +465,6 @@ static inline void setSampler(id<MTLComputeCommandEncoder> 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
Expand Down Expand Up @@ -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())
Expand Down
12 changes: 0 additions & 12 deletions src/modules/graphics/opengl/Graphics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions src/modules/graphics/opengl/Graphics.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<Buffer::DataDeclaration> &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;
Expand Down
21 changes: 18 additions & 3 deletions src/modules/graphics/opengl/Shader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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);
}
}

Expand Down
11 changes: 5 additions & 6 deletions src/modules/graphics/vulkan/Graphics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
{
Expand Down
1 change: 0 additions & 1 deletion src/modules/graphics/vulkan/Graphics.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<OptionalColorD> &colors, OptionalInt stencil, OptionalDouble depth) override;
Matrix4 computeDeviceProjection(const Matrix4 &projection, bool rendertotexture) const override;
void discard(const std::vector<bool>& 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;
Expand Down

0 comments on commit 7bfbd64

Please sign in to comment.