Vertex shaders can write to gl_PointSize (fixes #1603).

If a shader uses gl_PointSize then it can only be used when drawing points. If a shader doesn't use gl_PointSize then it can't be used when drawing points.

Added 'ConstantPointSize' and 'CurrentDPIScale' built-in shader uniforms.

ConstantPointSize is in DPI-scaled points, whereas gl_PointSize is in pixels, so to get the correct default behaviour a shader needs to do gl_PointSize = ConstantPointSize * CurrentDPIScale

src/modules/graphics/Graphics.cpp

@@ -1138,11 +1138,14 @@ Graphics::BatchedVertexData Graphics::requestBatchedDraw(const BatchedDrawComman
 		state.standardShaderType = cmd.standardShaderType;
 	}
 
-	if (state.vertexCount == 0 && Shader::isDefaultActive())
-		Shader::attachDefault(state.standardShaderType);
+	if (state.vertexCount == 0)
+	{
+		if (Shader::isDefaultActive())
+			Shader::attachDefault(state.standardShaderType);
 
-	if (state.vertexCount == 0 && Shader::current != nullptr && cmd.texture != nullptr)
-		Shader::current->checkMainTexture(cmd.texture);
+		if (Shader::current != nullptr)
+			Shader::current->validateDrawState(cmd.primitiveMode, cmd.texture);
+	}
 
 	if (shouldresize)
 	{
@@ -1350,6 +1353,7 @@ void Graphics::points(const Vector2 *positions, const Colorf *colors, size_t num
 	cmd.formats[0] = getSinglePositionFormat(is2D);
 	cmd.formats[1] = CommonFormat::RGBAub;
 	cmd.vertexCount = (int) numpoints;
+	cmd.standardShaderType = Shader::STANDARD_POINTS;
 
 	BatchedVertexData data = requestBatchedDraw(cmd);
 

src/modules/graphics/Mesh.cpp

@@ -519,8 +519,8 @@ void Mesh::drawInstanced(Graphics *gfx, const Matrix4 &m, int instancecount)
 	if (Shader::isDefaultActive())
 		Shader::attachDefault(Shader::STANDARD_DEFAULT);
 
-	if (Shader::current && texture.get())
-		Shader::current->checkMainTexture(texture);
+	if (Shader::current)
+		Shader::current->validateDrawState(primitiveType, texture);
 
 	VertexAttributes attributes;
 	BufferBindings buffers;

src/modules/graphics/ParticleSystem.cpp

@@ -1037,8 +1037,8 @@ void ParticleSystem::draw(Graphics *gfx, const Matrix4 &m)
 	if (Shader::isDefaultActive())
 		Shader::attachDefault(Shader::STANDARD_DEFAULT);
 
-	if (Shader::current && texture.get())
-		Shader::current->checkMainTexture(texture);
+	if (Shader::current)
+		Shader::current->validateDrawState(PRIMITIVE_TRIANGLES, texture);
 
 	const Vector2 *positions = texture->getQuad()->getVertexPositions();
 	const Vector2 *texcoords = texture->getQuad()->getVertexTexCoords();

src/modules/graphics/Shader.cpp

@@ -28,6 +28,7 @@
 
 // Needed for reflection information.
 #include "libraries/glslang/glslang/Include/Types.h"
+#include "libraries/glslang/glslang/MachineIndependent/localintermediate.h"
 
 // C++
 #include <string>
@@ -94,6 +95,10 @@ LOVE_HIGHP_OR_MEDIUMP mat3 NormalMatrix;
 LOVE_HIGHP_OR_MEDIUMP vec4 love_ScreenSize;
 LOVE_HIGHP_OR_MEDIUMP vec4 ConstantColor;
 
+LOVE_HIGHP_OR_MEDIUMP float CurrentDPIScale;
+
+LOVE_HIGHP_OR_MEDIUMP float ConstantPointSize;
+
 #define TransformProjectionMatrix (ProjectionMatrix * TransformMatrix)
 
 // Alternate names
@@ -123,6 +128,8 @@ void love_initializeBuiltinUniforms() {
 	   love_UniformsPerDraw[10].xyz
 	);
 
+	CurrentDPIScale = love_UniformsPerDraw[8].w;
+	ConstantPointSize = love_UniformsPerDraw[9].w;
 	love_ScreenSize = love_UniformsPerDraw[11];
 	ConstantColor = love_UniformsPerDraw[12];
 }
@@ -237,6 +244,7 @@ mediump vec4 linearToGammaFast(mediump vec4 c) { return vec4(linearToGammaFast(c
 
 static const char vertex_header[] = R"(
 #define love_Position gl_Position
+#define love_PointSize gl_PointSize
 
 #if __VERSION__ >= 130
 	#define attribute in
@@ -246,19 +254,9 @@ static const char vertex_header[] = R"(
 		#define love_InstanceID gl_InstanceID
 	#endif
 #endif
-
-#ifdef GL_ES
-	uniform mediump float love_PointSize;
-#endif
 )";
 
-static const char vertex_functions[] = R"(
-void setPointSize() {
-#ifdef GL_ES
-	gl_PointSize = love_PointSize;
-#endif
-}
-)";
+static const char vertex_functions[] = R"()";
 
 static const char vertex_main[] = R"(
 attribute vec4 VertexPosition;
@@ -274,7 +272,6 @@ void main() {
 	love_initializeBuiltinUniforms();
 	VaryingTexCoord = VertexTexCoord;
 	VaryingColor = gammaCorrectColor(VertexColor) * ConstantColor;
-	setPointSize();
 	love_Position = position(ClipSpaceFromLocal, VertexPosition);
 }
 )";
@@ -284,7 +281,6 @@ void vertexmain();
 
 void main() {
 	love_initializeBuiltinUniforms();
-	setPointSize();
 	vertexmain();
 }
 )";
@@ -599,13 +595,29 @@ TextureType Shader::getMainTextureType() const
 	return info != nullptr ? info->textureType : TEXTURE_MAX_ENUM;
 }
 
-void Shader::checkMainTextureType(TextureType textype, bool isDepthSampler) const
+void Shader::validateDrawState(PrimitiveType primtype, Texture *maintex) const
 {
+	if ((primtype == PRIMITIVE_POINTS) != validationReflection.usesPointSize)
+	{
+		if (validationReflection.usesPointSize)
+			throw love::Exception("The active shader can only be used to draw points.");
+		else
+			throw love::Exception("The gl_PointSize variable must be set in a vertex shader when drawing points.");
+	}
+
+	if (maintex == nullptr)
+		return;
+
 	const UniformInfo *info = getUniformInfo(BUILTIN_TEXTURE_MAIN);
 
 	if (info == nullptr)
 		return;
 
+	if (!maintex->isReadable())
+		throw love::Exception("Textures with non-readable formats cannot be sampled from in a shader.");
+
+	auto textype = maintex->getTextureType();
+
 	if (info->textureType != TEXTURE_MAX_ENUM && info->textureType != textype)
 	{
 		const char *textypestr = "unknown";
@@ -615,7 +627,7 @@ void Shader::checkMainTextureType(TextureType textype, bool isDepthSampler) cons
 		throw love::Exception("Texture's type (%s) must match the type of the shader's main texture type (%s).", textypestr, shadertextypestr);
 	}
 
-	if (info->isDepthSampler != isDepthSampler)
+	if (info->isDepthSampler != maintex->getSamplerState().depthSampleMode.hasValue)
 	{
 		if (info->isDepthSampler)
 			throw love::Exception("Depth comparison samplers in shaders can only be used with depth textures which have depth comparison set.");
@@ -624,14 +636,6 @@ void Shader::checkMainTextureType(TextureType textype, bool isDepthSampler) cons
 	}
 }
 
-void Shader::checkMainTexture(Texture *tex) const
-{
-	if (!tex->isReadable())
-		throw love::Exception("Textures with non-readable formats cannot be sampled from in a shader.");
-
-	checkMainTextureType(tex->getTextureType(), tex->getSamplerState().depthSampleMode.hasValue);
-}
-
 bool Shader::validate(ShaderStage* vertex, ShaderStage* pixel, std::string& err)
 {
 	ValidationReflection reflection;
@@ -660,6 +664,13 @@ bool Shader::validateInternal(ShaderStage *vertex, ShaderStage *pixel, std::stri
 		return false;
 	}
 
+	const auto *vertintermediate = program.getIntermediate(EShLangVertex);
+	if (vertintermediate != nullptr)
+	{
+		// NOTE: this doesn't check whether the use affects final output...
+		reflection.usesPointSize = vertintermediate->inIoAccessed("gl_PointSize");
+	}
+
 	for (int i = 0; i < program.getNumBufferBlocks(); i++)
 	{
 		const glslang::TObjectReflection &info = program.getBufferBlock(i);
@@ -727,6 +738,14 @@ vec4 position(mat4 clipSpaceFromLocal, vec4 localPosition)
 }
 )";
 
+static const std::string defaultPointsVertex = R"(
+vec4 position(mat4 clipSpaceFromLocal, vec4 localPosition)
+{
+	love_PointSize = ConstantPointSize * CurrentDPIScale;
+	return clipSpaceFromLocal * localPosition;
+}
+)";
+
 static const std::string defaultStandardPixel = R"(
 vec4 effect(vec4 vcolor, Image tex, vec2 texcoord, vec2 pixcoord)
 {
@@ -752,7 +771,12 @@ void effect()
 const std::string &Shader::getDefaultCode(StandardShader shader, ShaderStage::StageType stage)
 {
 	if (stage == ShaderStage::STAGE_VERTEX)
-		return defaultVertex;
+	{
+		if (shader == STANDARD_POINTS)
+			return defaultPointsVertex;
+		else
+			return defaultVertex;
+	}
 
 	static std::string nocode = "";
 
@@ -761,6 +785,7 @@ const std::string &Shader::getDefaultCode(StandardShader shader, ShaderStage::St
 		case STANDARD_DEFAULT: return defaultStandardPixel;
 		case STANDARD_VIDEO: return defaultVideoPixel;
 		case STANDARD_ARRAY: return defaultArrayPixel;
+		case STANDARD_POINTS: return defaultStandardPixel;
 		case STANDARD_MAX_ENUM: return nocode;
 	}
 
@@ -783,7 +808,6 @@ static StringMap<Shader::BuiltinUniform, Shader::BUILTIN_MAX_ENUM>::Entry builti
 	{ "love_VideoCbChannel",  Shader::BUILTIN_TEXTURE_VIDEO_CB  },
 	{ "love_VideoCrChannel",  Shader::BUILTIN_TEXTURE_VIDEO_CR  },
 	{ "love_UniformsPerDraw", Shader::BUILTIN_UNIFORMS_PER_DRAW },
-	{ "love_PointSize",       Shader::BUILTIN_POINT_SIZE        },
 };
 
 static StringMap<Shader::BuiltinUniform, Shader::BUILTIN_MAX_ENUM> builtinNames(builtinNameEntries, sizeof(builtinNameEntries));

src/modules/graphics/Shader.h

@@ -64,7 +64,6 @@ public:
 		BUILTIN_TEXTURE_VIDEO_CB,
 		BUILTIN_TEXTURE_VIDEO_CR,
 		BUILTIN_UNIFORMS_PER_DRAW,
-		BUILTIN_POINT_SIZE,
 		BUILTIN_MAX_ENUM
 	};
 
@@ -88,6 +87,7 @@ public:
 		STANDARD_DEFAULT,
 		STANDARD_VIDEO,
 		STANDARD_ARRAY,
+		STANDARD_POINTS,
 		STANDARD_MAX_ENUM
 	};
 
@@ -209,8 +209,7 @@ public:
 	virtual void setVideoTextures(Texture *ytexture, Texture *cbtexture, Texture *crtexture) = 0;
 
 	TextureType getMainTextureType() const;
-	void checkMainTextureType(TextureType textype, bool isDepthSampler) const;
-	void checkMainTexture(Texture *texture) const;
+	void validateDrawState(PrimitiveType primtype, Texture *maintexture) const;
 
 	static SourceInfo getSourceInfo(const std::string &src);
 	static std::string createShaderStageCode(Graphics *gfx, ShaderStage::StageType stage, const std::string &code, const SourceInfo &info);
@@ -239,6 +238,7 @@ protected:
 	struct ValidationReflection
 	{
 		std::map<std::string, BufferReflection> storageBuffers;
+		bool usesPointSize;
 	};
 
 	static bool validateInternal(ShaderStage* vertex, ShaderStage* pixel, std::string& err, ValidationReflection &reflection);

src/modules/graphics/SpriteBatch.cpp

@@ -329,11 +329,11 @@ void SpriteBatch::draw(Graphics *gfx, const Matrix4 &m)
 
 			Shader::attachDefault(defaultshader);
 		}
-
-		if (Shader::current)
-			Shader::current->checkMainTexture(texture);
 	}
 
+	if (Shader::current)
+		Shader::current->validateDrawState(PRIMITIVE_TRIANGLES, texture);
+
 	flush(); // Upload any modified sprite data to the GPU.
 
 	VertexAttributes attributes;

src/modules/graphics/Text.cpp

@@ -262,8 +262,12 @@ void Text::draw(Graphics *gfx, const Matrix4 &m)
 	if (Shader::isDefaultActive())
 		Shader::attachDefault(Shader::STANDARD_DEFAULT);
 
+	Texture *firsttex = nullptr;
+	if (!drawCommands.empty())
+		firsttex = drawCommands[0].texture;
+
 	if (Shader::current)
-		Shader::current->checkMainTextureType(TEXTURE_2D, false);
+		Shader::current->validateDrawState(PRIMITIVE_TRIANGLES, firsttex);
 
 	// Re-generate the text if the Font's texture cache was invalidated.
 	if (font->getTextureCacheID() != textureCacheID)

src/modules/graphics/opengl/Graphics.cpp

@@ -304,6 +304,9 @@ bool Graphics::setMode(int width, int height, int pixelwidth, int pixelheight, b
 		glEnable(GL_TEXTURE_2D);
 	}
 
+	if (!GLAD_ES_VERSION_2_0)
+		glEnable(GL_VERTEX_PROGRAM_POINT_SIZE);
+
 	gl.setTextureUnit(0);
 
 	// Set pixel row alignment - code that calls glTexSubImage and glReadPixels
@@ -1444,10 +1447,9 @@ void Graphics::setBlendState(const BlendState &blend)
 
 void Graphics::setPointSize(float size)
 {
-	if (batchedDrawState.primitiveMode == PRIMITIVE_POINTS)
+	if (size != states.back().pointSize)
 		flushBatchedDraws();
 
-	gl.setPointSize(size * getCurrentDPIScale());
 	states.back().pointSize = size;
 }
 

src/modules/graphics/opengl/OpenGL.cpp

@@ -943,19 +943,6 @@ void OpenGL::setScissor(const Rect &v, bool rtActive)
 	state.scissor = v;
 }
 
-void OpenGL::setPointSize(float size)
-{
-	if (GLAD_VERSION_1_0)
-		glPointSize(size);
-
-	state.pointSize = size;
-}
-
-float OpenGL::getPointSize() const
-{
-	return state.pointSize;
-}
-
 void OpenGL::setEnableState(EnableState enablestate, bool enable)
 {
 	GLenum glstate = GL_NONE;

src/modules/graphics/opengl/OpenGL.h

@@ -270,12 +270,6 @@ public:
 	 **/
 	void setScissor(const Rect &v, bool rtActive);
 
-	/**
-	 * Sets the global point size.
-	 **/
-	void setPointSize(float size);
-	float getPointSize() const;
-
 	/**
 	 * State-tracked version of glEnable.
 	 **/

src/modules/graphics/opengl/Shader.cpp

@@ -48,7 +48,6 @@ Shader::Shader(love::graphics::ShaderStage *vertex, love::graphics::ShaderStage
 	, builtinUniforms()
 	, builtinUniformInfo()
 	, builtinAttributes()
-	, lastPointSize(0.0f)
 {
 	// load shader source and create program object
 	loadVolatile();
@@ -430,8 +429,6 @@ bool Shader::loadVolatile()
 {
 	OpenGL::TempDebugGroup debuggroup("Shader load");
 
-	lastPointSize = -1.0f;
-
 	// zero out active texture list
 	textureUnits.clear();
 	textureUnits.push_back(TextureUnit());
@@ -971,26 +968,11 @@ void Shader::setVideoTextures(love::graphics::Texture *ytexture, love::graphics:
 	}
 }
 
-void Shader::updatePointSize(float size)
-{
-	if (size == lastPointSize || current != this)
-		return;
-
-	GLint location = builtinUniforms[BUILTIN_POINT_SIZE];
-	if (location >= 0)
-		glUniform1f(location, size);
-
-	lastPointSize = size;
-}
-
 void Shader::updateBuiltinUniforms(love::graphics::Graphics *gfx, int viewportW, int viewportH)
 {
 	if (current != this)
 		return;
 
-	if (GLAD_ES_VERSION_2_0)
-		updatePointSize(gl.getPointSize());
-
 	BuiltinUniformData data;
 
 	data.transformMatrix = gfx->getTransform();
@@ -1010,6 +992,12 @@ void Shader::updateBuiltinUniforms(love::graphics::Graphics *gfx, int viewportW,
 		}
 	}
 
+	// 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.screenSizeParams.x = viewportW;
 	data.screenSizeParams.y = viewportH;
 

src/modules/graphics/opengl/Shader.h

@@ -43,10 +43,6 @@ class Shader final : public love::graphics::Shader, public Volatile
 {
 public:
 
-	/**
-	 * Creates a new Shader using a list of source codes.
-	 * Source must contain either vertex or pixel shader code, or both.
-	 **/
 	Shader(love::graphics::ShaderStage *vertex, love::graphics::ShaderStage *pixel);
 	virtual ~Shader();
 
@@ -67,7 +63,6 @@ public:
 	ptrdiff_t getHandle() const override;
 	void setVideoTextures(love::graphics::Texture *ytexture, love::graphics::Texture *cbtexture, love::graphics::Texture *crtexture) override;
 
-	void updatePointSize(float size);
 	void updateBuiltinUniforms(love::graphics::Graphics *gfx, int viewportW, int viewportH);
 
 private:
@@ -128,8 +123,6 @@ private:
 
 	std::vector<std::pair<const UniformInfo *, int>> pendingUniformUpdates;
 
-	float lastPointSize;
-
 }; // Shader
 
 } // opengl

src/modules/graphics/wrap_Graphics.cpp

@@ -3103,7 +3103,7 @@ int w_rectangle(lua_State *L)
 
 	if (lua_isnoneornil(L, 6))
 	{
-		instance()->rectangle(mode, x, y, w, h);
+		luax_catchexcept(L, [&](){ instance()->rectangle(mode, x, y, w, h); });
 		return 0;
 	}