From f82ef715f0d7e1e7d0b93be4b7b89c8ce6bba40b Mon Sep 17 00:00:00 2001
From: Mikko Rasa <tdb@tdb.fi>
Date: Sun, 10 Oct 2021 17:08:42 +0300
Subject: [PATCH] Remove generic clipping state

Instead do things the Vulkan way and set the enabled clip planes based on
the declared size of the gl_ClipDistance array in the shader.
---
 shaderlib/common.glsl                         |  6 ---
 shaderlib/msp_interface.glsl                  | 12 ------
 .../backends/opengl/pipelinestate_backend.cpp | 36 ++++++++--------
 .../backends/opengl/pipelinestate_backend.h   |  1 +
 source/backends/opengl/program_backend.cpp    |  3 ++
 source/core/clipping.cpp                      | 42 -------------------
 source/core/clipping.h                        | 38 -----------------
 source/core/clipplane.cpp                     | 34 ---------------
 source/core/clipplane.h                       | 32 --------------
 source/core/pipelinestate.cpp                 |  5 ---
 source/core/pipelinestate.h                   |  3 --
 source/core/program.cpp                       | 14 +++++++
 source/core/program.h                         |  3 ++
 source/core/reflectdata.h                     |  1 +
 source/glsl/compiler.cpp                      |  8 ++++
 source/glsl/compiler.h                        |  2 +
 source/glsl/finalize.cpp                      |  4 ++
 source/glsl/syntax.h                          |  1 +
 source/render/renderer.cpp                    |  9 ----
 source/render/renderer.h                      |  3 --
 source/render/sequence.cpp                    |  7 ----
 source/render/sequence.h                      | 10 ++---
 22 files changed, 59 insertions(+), 215 deletions(-)
 delete mode 100644 source/core/clipping.cpp
 delete mode 100644 source/core/clipping.h
 delete mode 100644 source/core/clipplane.cpp
 delete mode 100644 source/core/clipplane.h

diff --git a/shaderlib/common.glsl b/shaderlib/common.glsl
index 03cfd5af..b83ff380 100644
--- a/shaderlib/common.glsl
+++ b/shaderlib/common.glsl
@@ -51,12 +51,6 @@ void standard_transform()
 	out vec3 world_look_dir = normalize(world_vertex.xyz-eye_pos);
 
 	out float fog_coord = eye_vertex.z;
-
-	if(use_clipping)
-	{
-		for(int i=0; i<max_clip_planes; ++i)
-			gl_ClipDistance[i] = dot(world_vertex, clip_planes[i].equation);
-	}
 }
 
 virtual void custom_transform()
diff --git a/shaderlib/msp_interface.glsl b/shaderlib/msp_interface.glsl
index 3fdd606b..13cd9ccf 100644
--- a/shaderlib/msp_interface.glsl
+++ b/shaderlib/msp_interface.glsl
@@ -6,11 +6,6 @@ struct LightSourceParameters
 	vec3 attenuation;
 };
 
-struct ClipPlane
-{
-	vec4 equation;
-};
-
 uniform mat4 world_obj_matrix;
 uniform mat3 world_obj_normal_matrix;
 uniform CameraTransform
@@ -30,13 +25,6 @@ uniform Lighting
 	float fog_density;
 };
 
-layout(constant_id=auto) const bool use_clipping = false;
-layout(constant_id=auto) const int max_clip_planes = 8;
-uniform Clipping
-{
-	ClipPlane clip_planes[max_clip_planes];
-};
-
 #pragma MSP stage(vertex)
 layout(location=0) in vec4 vertex;
 layout(location=1) in vec4 color;
diff --git a/source/backends/opengl/pipelinestate_backend.cpp b/source/backends/opengl/pipelinestate_backend.cpp
index 738de3ac..cda3439c 100644
--- a/source/backends/opengl/pipelinestate_backend.cpp
+++ b/source/backends/opengl/pipelinestate_backend.cpp
@@ -30,6 +30,7 @@ const OpenGLPipelineState *OpenGLPipelineState::last_applied = 0;
 vector<int> OpenGLPipelineState::bound_tex_targets;
 vector<char> OpenGLPipelineState::bound_uniform_blocks;
 unsigned OpenGLPipelineState::restart_index = 0;
+unsigned OpenGLPipelineState::n_clip_distances = 0;
 
 OpenGLPipelineState::OpenGLPipelineState()
 {
@@ -88,8 +89,23 @@ void OpenGLPipelineState::apply(unsigned mask) const
 	}
 
 	if(mask&PipelineState::SHPROG)
+	{
 		glUseProgram(self->shprog ? self->shprog->id : 0);
 
+		unsigned ncd = (self->shprog ? self->shprog->get_n_clip_distances() : 0);
+		if(ncd!=n_clip_distances)
+		{
+			for(unsigned i=0; (i<ncd || i<n_clip_distances); ++i)
+			{
+				if(i<ncd)
+					glEnable(GL_CLIP_PLANE0+i);
+				else
+					glDisable(GL_CLIP_PLANE0+i);
+			}
+			n_clip_distances = ncd;
+		}
+	}
+
 	if(mask&PipelineState::VERTEX_SETUP)
 	{
 		const VertexSetup *vertex_setup = self->vertex_setup;
@@ -123,18 +139,6 @@ void OpenGLPipelineState::apply(unsigned mask) const
 			glDisable(GL_CULL_FACE);
 	}
 
-	if(mask&PipelineState::CLIP_PLANES)
-	{
-		unsigned max_clip_planes = DeviceInfo::get_global().limits.max_clip_planes;
-		for(unsigned i=0; i<max_clip_planes; ++i)
-		{
-			if((self->enabled_clip_planes>>i)&1)
-				glEnable(GL_CLIP_PLANE0+i);
-			else
-				glDisable(GL_CLIP_PLANE0+i);
-		}
-	}
-
 	if(mask&PipelineState::TEXTURES)
 	{
 		for(const PipelineState::BoundTexture &t: self->textures)
@@ -243,11 +247,9 @@ void OpenGLPipelineState::clear()
 		glUseProgram(0);
 		glBindVertexArray(0);
 
-		unsigned max_clip_planes = DeviceInfo::get_global().limits.max_clip_planes;
-		unsigned enabled_clip_planes = static_cast<const PipelineState *>(last_applied)->enabled_clip_planes;
-		for(unsigned i=0; i<max_clip_planes; ++i)
-			if((enabled_clip_planes>>i)&1)
-				glDisable(GL_CLIP_PLANE0+i);
+		for(unsigned i=0; i<n_clip_distances; ++i)
+			glDisable(GL_CLIP_PLANE0+i);
+		n_clip_distances = 0;
 
 		for(unsigned i=0; i<bound_tex_targets.size(); ++i)
 			if(bound_tex_targets[i])
diff --git a/source/backends/opengl/pipelinestate_backend.h b/source/backends/opengl/pipelinestate_backend.h
index 24ef6989..0b1c4a87 100644
--- a/source/backends/opengl/pipelinestate_backend.h
+++ b/source/backends/opengl/pipelinestate_backend.h
@@ -16,6 +16,7 @@ protected:
 	static std::vector<int> bound_tex_targets;
 	static std::vector<char> bound_uniform_blocks;
 	static unsigned restart_index;
+	static unsigned n_clip_distances;
 
 	OpenGLPipelineState();
 	~OpenGLPipelineState();
diff --git a/source/backends/opengl/program_backend.cpp b/source/backends/opengl/program_backend.cpp
index 86235ebe..b902db5f 100644
--- a/source/backends/opengl/program_backend.cpp
+++ b/source/backends/opengl/program_backend.cpp
@@ -136,6 +136,9 @@ void OpenGLProgram::add_glsl_stages(const GlslModule &mod, const map<string, int
 
 	transient.textures = compiler.get_texture_bindings();
 	transient.blocks = compiler.get_uniform_block_bindings();
+
+	ReflectData &rd = static_cast<Program *>(this)->reflect_data;
+	rd.n_clip_distances = compiler.get_n_clip_distances();
 }
 
 void OpenGLProgram::compile_glsl_stage(const GlslModule &mod, unsigned stage_id)
diff --git a/source/core/clipping.cpp b/source/core/clipping.cpp
deleted file mode 100644
index 30d1f7c2..00000000
--- a/source/core/clipping.cpp
+++ /dev/null
@@ -1,42 +0,0 @@
-#include <msp/core/algorithm.h>
-#include "clipping.h"
-#include "clipplane.h"
-#include "deviceinfo.h"
-#include "error.h"
-
-using namespace std;
-
-namespace Msp {
-namespace GL {
-
-void Clipping::attach(const ClipPlane &p)
-{
-	if(find_member(planes, &p, &AttachedPlane::plane)!=planes.end())
-		return;
-	if(planes.size()>=DeviceInfo::get_global().limits.max_clip_planes)
-		throw invalid_operation("Clipping::attach");
-
-	planes.push_back(&p);
-}
-
-void Clipping::detach(const ClipPlane &p)
-{
-	auto i = find_member(planes, &p, &AttachedPlane::plane);
-	if(i!=planes.end())
-		planes.erase(i);
-}
-
-const ProgramData &Clipping::get_shader_data() const
-{
-	for(unsigned i=0; i<planes.size(); ++i)
-		if(planes[i].plane->get_generation()!=planes[i].generation)
-		{
-			planes[i].plane->update_shader_data(shdata, i);
-			planes[i].generation = planes[i].plane->get_generation();
-		}
-
-	return shdata;
-}
-
-} // namespace GL
-} // namespace Msp
diff --git a/source/core/clipping.h b/source/core/clipping.h
deleted file mode 100644
index f682dc64..00000000
--- a/source/core/clipping.h
+++ /dev/null
@@ -1,38 +0,0 @@
-#ifndef MSP_GL_CLIPPING_H_
-#define MSP_GL_CLIPPING_H_
-
-#include <vector>
-#include "programdata.h"
-
-namespace Msp {
-namespace GL {
-
-class ClipPlane;
-
-class Clipping
-{
-private:
-	struct AttachedPlane
-	{
-		const ClipPlane *plane;
-		mutable unsigned generation;
-
-		AttachedPlane(const ClipPlane *p): plane(p), generation(0) { }
-	};
-
-	std::vector<AttachedPlane> planes;
-	mutable ProgramData shdata;
-
-public:
-	void attach(const ClipPlane &);
-	void detach(const ClipPlane &);
-
-	unsigned get_n_planes() const { return planes.size(); }
-
-	const ProgramData &get_shader_data() const;
-};
-
-} // namespace GL
-} // namespace Msp
-
-#endif
diff --git a/source/core/clipplane.cpp b/source/core/clipplane.cpp
deleted file mode 100644
index 256f2bea..00000000
--- a/source/core/clipplane.cpp
+++ /dev/null
@@ -1,34 +0,0 @@
-#include <msp/strings/format.h>
-#include "clipplane.h"
-#include "programdata.h"
-
-namespace Msp {
-namespace GL {
-
-ClipPlane::ClipPlane(const Vector4 &e):
-	eq(e)
-{ }
-
-ClipPlane::ClipPlane(const Vector3 &p, const Vector3 &d):
-	eq(compose(d, -dot(p, d)))
-{ }
-
-void ClipPlane::set_equation(const Vector4 &e)
-{
-	eq = e;
-	++generation;
-}
-
-void ClipPlane::set_plane(const Vector3 &p, const Vector3 &d)
-{
-	Vector3 nd = normalize(d);
-	set_equation(compose(nd, -dot(p, nd)));
-}
-
-void ClipPlane::update_shader_data(ProgramData &shdata, unsigned i) const
-{
-	shdata.uniform(format("clip_planes[%d].equation", i), eq);
-}
-
-} // namespace GL
-} // namespace Msp
diff --git a/source/core/clipplane.h b/source/core/clipplane.h
deleted file mode 100644
index a2b56255..00000000
--- a/source/core/clipplane.h
+++ /dev/null
@@ -1,32 +0,0 @@
-#ifndef MSP_GL_CLIPPLANE_H_
-#define MSP_GL_CLIPPLANE_H_
-
-#include "vector.h"
-
-namespace Msp {
-namespace GL {
-
-class ProgramData;
-
-class ClipPlane
-{
-private:
-	Vector4 eq = { 0.0f, 0.0f, 0.0f, 1.0f };
-	unsigned generation = 0;
-
-public:
-	ClipPlane() = default;
-	ClipPlane(const Vector4 &);
-	ClipPlane(const Vector3 &, const Vector3 &);
-
-	void set_equation(const Vector4 &);
-	void set_plane(const Vector3 &, const Vector3 &);
-	void update_shader_data(ProgramData &, unsigned) const;
-
-	unsigned get_generation() const { return generation; }
-};
-
-} // namespace GL
-} // namespace Msp
-
-#endif
diff --git a/source/core/pipelinestate.cpp b/source/core/pipelinestate.cpp
index 23a2ed6a..933109e6 100644
--- a/source/core/pipelinestate.cpp
+++ b/source/core/pipelinestate.cpp
@@ -52,11 +52,6 @@ void PipelineState::set_face_cull(CullMode c)
 	set(face_cull, c, FACE_CULL);
 }
 
-void PipelineState::set_enabled_clip_planes(unsigned p)
-{
-	set(enabled_clip_planes, p, CLIP_PLANES);
-}
-
 void PipelineState::set_texture(unsigned binding, const Texture *tex, const Sampler *samp)
 {
 	if((tex!=0)!=(samp!=0))
diff --git a/source/core/pipelinestate.h b/source/core/pipelinestate.h
index 0d0520a0..6d4d2f64 100644
--- a/source/core/pipelinestate.h
+++ b/source/core/pipelinestate.h
@@ -49,7 +49,6 @@ private:
 		SHPROG = 1,
 		VERTEX_SETUP = 2,
 		FACE_CULL = 4,
-		CLIP_PLANES = 8,
 		TEXTURES = 16,
 		UNIFORMS = 32,
 		DEPTH_TEST = 64,
@@ -67,7 +66,6 @@ private:
 	const VertexSetup *vertex_setup = 0;
 	FaceWinding front_face = COUNTERCLOCKWISE;
 	CullMode face_cull = NO_CULL;
-	unsigned enabled_clip_planes = 0;
 	std::vector<BoundTexture> textures;
 	std::vector<BoundUniformBlock> uniform_blocks;
 	const DepthTest *depth_test = 0;
@@ -85,7 +83,6 @@ public:
 	void set_vertex_setup(const VertexSetup *);
 	void set_front_face(FaceWinding);
 	void set_face_cull(CullMode);
-	void set_enabled_clip_planes(unsigned);
 	void set_texture(unsigned, const Texture *, const Sampler *);
 	void set_uniform_block(int, const UniformBlock *);
 	void set_depth_test(const DepthTest *);
diff --git a/source/core/program.cpp b/source/core/program.cpp
index 5c8257b2..4707f4c0 100644
--- a/source/core/program.cpp
+++ b/source/core/program.cpp
@@ -173,6 +173,20 @@ void Program::collect_attributes(const SpirVModule &mod)
 		}
 }
 
+void Program::collect_builtins(const SpirVModule &mod)
+{
+	for(const SpirVModule::Variable &v: mod.get_variables())
+		if(v.storage==SpirVModule::OUTPUT && v.struct_type)
+			collect_builtins(*v.struct_type);
+}
+
+void Program::collect_builtins(const SpirVModule::Structure &strct)
+{
+	for(const SpirVModule::StructMember &m: strct.members)
+		if(m.builtin==SpirVModule::CLIP_DISTANCE)
+			reflect_data.n_clip_distances = m.array_size;
+}
+
 const ReflectData::UniformBlockInfo &Program::get_uniform_block_info(const string &name) const
 {
 	auto i = find_member(reflect_data.uniform_blocks, name, &ReflectData::UniformBlockInfo::name);
diff --git a/source/core/program.h b/source/core/program.h
index fd1c1ae0..770c6bad 100644
--- a/source/core/program.h
+++ b/source/core/program.h
@@ -62,6 +62,8 @@ private:
 	void collect_uniforms(const SpirVModule &, const std::map<unsigned, int> &);
 	void collect_block_uniforms(const SpirVModule::Structure &, const std::string &, unsigned, const std::map<unsigned, int> &, std::vector<std::string> &);
 	void collect_attributes(const SpirVModule &);
+	void collect_builtins(const SpirVModule &);
+	void collect_builtins(const SpirVModule::Structure &);
 
 public:
 	ReflectData::LayoutHash get_uniform_layout_hash() const { return reflect_data.layout_hash; }
@@ -76,6 +78,7 @@ public:
 	const std::vector<ReflectData::AttributeInfo> &get_attributes() const { return reflect_data.attributes; }
 	const ReflectData::AttributeInfo &get_attribute_info(const std::string &) const;
 	int get_attribute_location(const std::string &) const;
+	unsigned get_n_clip_distances() const { return reflect_data.n_clip_distances; }
 
 	using ProgramBackend::set_debug_name;
 };
diff --git a/source/core/reflectdata.h b/source/core/reflectdata.h
index fff190a5..5041b579 100644
--- a/source/core/reflectdata.h
+++ b/source/core/reflectdata.h
@@ -58,6 +58,7 @@ struct ReflectData
 	std::vector<UniformInfo> uniforms;
 	LayoutHash layout_hash;
 	std::vector<AttributeInfo> attributes;
+	unsigned n_clip_distances = 0;
 
 	void update_layout_hash();
 };
diff --git a/source/glsl/compiler.cpp b/source/glsl/compiler.cpp
index 4b1d4edf..e658a79e 100644
--- a/source/glsl/compiler.cpp
+++ b/source/glsl/compiler.cpp
@@ -195,6 +195,14 @@ const map<string, unsigned> &Compiler::get_uniform_block_bindings() const
 	return module->shared.uniform_block_bindings;
 }
 
+unsigned Compiler::get_n_clip_distances() const
+{
+	if(!compiled)
+		throw invalid_operation("Compiler::get_n_clip_distances");
+	auto i = find_member(module->stages, Stage::VERTEX, &Stage::type);
+	return (i!=module->stages.end() ? i->n_clip_distances : 0);
+}
+
 const SourceMap &Compiler::get_source_map() const
 {
 	return module->source_map;
diff --git a/source/glsl/compiler.h b/source/glsl/compiler.h
index 1c07f325..9d839745 100644
--- a/source/glsl/compiler.h
+++ b/source/glsl/compiler.h
@@ -112,6 +112,8 @@ public:
 	source). */
 	const std::map<std::string, unsigned> &get_uniform_block_bindings() const;
 
+	unsigned get_n_clip_distances() const;
+
 	/** Returns the mapping of source indices to filenames.  Can be used to
 	translate error messages. */
 	const SourceMap &get_source_map() const;
diff --git a/source/glsl/finalize.cpp b/source/glsl/finalize.cpp
index 4068037e..27c35209 100644
--- a/source/glsl/finalize.cpp
+++ b/source/glsl/finalize.cpp
@@ -574,6 +574,10 @@ void LegacyConverter::visit(VariableDeclaration &var)
 		}
 	}
 
+	if(var.name=="gl_ClipDistance")
+		if(const Literal *literal_size = dynamic_cast<const Literal *>(var.array_size.get()))
+			stage->n_clip_distances = literal_size->value.value<int>();
+
 	TraversingVisitor::visit(var);
 }
 
diff --git a/source/glsl/syntax.h b/source/glsl/syntax.h
index a3b217d4..d3797ac4 100644
--- a/source/glsl/syntax.h
+++ b/source/glsl/syntax.h
@@ -533,6 +533,7 @@ struct Stage
 	std::map<std::string, unsigned> locations;
 	std::map<std::string, unsigned> texture_bindings;
 	std::map<std::string, unsigned> uniform_block_bindings;
+	unsigned n_clip_distances = 0;
 	Features required_features;
 	std::vector<Diagnostic> diagnostics;
 
diff --git a/source/render/renderer.cpp b/source/render/renderer.cpp
index ad5f87d5..5e155f2e 100644
--- a/source/render/renderer.cpp
+++ b/source/render/renderer.cpp
@@ -1,7 +1,6 @@
 #include "batch.h"
 #include "buffer.h"
 #include "camera.h"
-#include "clipping.h"
 #include "error.h"
 #include "framebuffer.h"
 #include "lighting.h"
@@ -124,13 +123,6 @@ void Renderer::set_lighting(const Lighting *l)
 		add_shader_data(l->get_shader_data());
 }
 
-void Renderer::set_clipping(const Clipping *c)
-{
-	state->clipping = c;
-	if(c)
-		add_shader_data(c->get_shader_data());
-}
-
 void Renderer::set_shader_program(const Program *p, const ProgramData *d)
 {
 	state->shprog = p;
@@ -324,7 +316,6 @@ void Renderer::apply_state()
 
 	pipeline_state.set_front_face(state->front_face);
 	pipeline_state.set_face_cull(state->face_cull);
-	pipeline_state.set_enabled_clip_planes(state->clipping ? (1<<state->clipping->get_n_planes())-1 : 0);
 
 	if(state->texture_count<texture_stack.size())
 		flush_textures();
diff --git a/source/render/renderer.h b/source/render/renderer.h
index fa7de736..42515e82 100644
--- a/source/render/renderer.h
+++ b/source/render/renderer.h
@@ -16,7 +16,6 @@ class Batch;
 class Buffer;
 class Camera;
 union ClearValue;
-class Clipping;
 class Material;
 class Mesh;
 class Lighting;
@@ -91,7 +90,6 @@ private:
 		const Rect *viewport = 0;
 		const Rect *scissor = 0;
 		unsigned texture_count = 0;
-		const Clipping *clipping = 0;
 		const Program *shprog = 0;
 		unsigned shdata_count = 0;
 		const VertexSetup *vertex_setup = 0;
@@ -150,7 +148,6 @@ public:
 	DEPRECATED void set_material(const Material *);
 
 	DEPRECATED void set_lighting(const Lighting *);
-	void set_clipping(const Clipping *);
 
 	/** Sets the shader program to use.  An initial set of data can be set as
 	well, with the same semantics as add_shader_data. */
diff --git a/source/render/sequence.cpp b/source/render/sequence.cpp
index f2a88a51..e0fb408f 100644
--- a/source/render/sequence.cpp
+++ b/source/render/sequence.cpp
@@ -150,7 +150,6 @@ void Sequence::render(Renderer &renderer, Tag tag) const
 
 		if (const Lighting *lighting = s.get_lighting())
 			renderer.add_shader_data(lighting->get_shader_data());
-		renderer.set_clipping(s.get_clipping());
 
 		if(const Renderable *renderable = s.get_renderable())
 			renderer.render(*renderable, s.get_tag());
@@ -193,7 +192,6 @@ void Sequence::set_debug_name(const string &name)
 Sequence::Step::Step(Tag t, Renderable *r):
 	tag(t),
 	lighting(0),
-	clipping(0),
 	renderable(r)
 { }
 
@@ -217,10 +215,5 @@ void Sequence::Step::set_blend(const Blend &b)
 	blend = b;
 }
 
-void Sequence::Step::set_clipping(const Clipping *c)
-{
-	clipping = c;
-}
-
 } // namespace GL
 } // namespace Msp
diff --git a/source/render/sequence.h b/source/render/sequence.h
index cc4b6963..9a3d0207 100644
--- a/source/render/sequence.h
+++ b/source/render/sequence.h
@@ -11,7 +11,6 @@
 namespace Msp {
 namespace GL {
 
-class Clipping;
 class Lighting;
 class PostProcessor;
 class RenderTarget;
@@ -21,9 +20,9 @@ Top-level content class.  Typically a Sequence is used as the content
 Renderable for a View or effects such as ShadowMap or EnvironmentMap.
 
 A Sequence consists of a number of steps.  Each step is defined with a
-Renderable and a tag to render it with and may also have Lighting, Clipping,
-DepthTest and Blend states.  Scenes can be used to further organize Renderables
-within a step.
+Renderable and a tag to render it with and may also have Lighting, DepthTest
+and Blend states.  Scenes can be used to further organize Renderables within a
+step.
 
 PostProcessors can be applied after all of the steps in the Sequence have been
 processed.  Framebuffer objects are automatically used to pass render results
@@ -41,7 +40,6 @@ public:
 		DepthTest depth_test;
 		StencilTest stencil_test;
 		Blend blend;
-		const Clipping *clipping;
 		Renderable *renderable;
 
 	public:
@@ -53,12 +51,10 @@ public:
 		void set_depth_test(const DepthTest &);
 		void set_stencil_test(const StencilTest &);
 		void set_blend(const Blend &);
-		void set_clipping(const Clipping *);
 		const Lighting *get_lighting() const { return lighting; }
 		const DepthTest &get_depth_test() const { return depth_test; }
 		const StencilTest &get_stencil_test() const { return stencil_test; }
 		const Blend &get_blend() const { return blend; }
-		const Clipping *get_clipping() const { return clipping; }
 		Renderable *get_renderable() const { return renderable; }
 	};
 
-- 
2.43.0