From 1db3e73542bbe14a07f85e76390640b198e6a435 Mon Sep 17 00:00:00 2001 From: Matthew Mott Date: Tue, 9 Mar 2021 20:10:35 +0000 Subject: [PATCH] Colour4 stores floats rather than doubles Using doubles in vectors may be necessary for accuracy when storing brush and patch geometry, but using doubles just to store OpenGL colours is a waste of bytes. Also add some tests for Vector3 and Vector3f packing, ensuring that the values are contiguous and can be passed to functions like glUniform3fv(). --- libs/debugging/render.h | 6 ++-- libs/render/Colour4.h | 17 +++++------ radiant/xyview/tools/MeasurementTool.cpp | 2 +- .../rendersystem/backend/OpenGLShader.cpp | 6 ++-- .../rendersystem/backend/OpenGLShaderPass.cpp | 4 +-- .../backend/glprogram/GLSLBumpProgram.cpp | 7 +---- test/math/Vector3.cpp | 28 +++++++++++++++++++ 7 files changed, 47 insertions(+), 23 deletions(-) diff --git a/libs/debugging/render.h b/libs/debugging/render.h index 375cbea2e9..e29092714d 100644 --- a/libs/debugging/render.h +++ b/libs/debugging/render.h @@ -58,9 +58,9 @@ namespace debug GLboolean vals[4]; glGetBooleanv(GL_COLOR_WRITEMASK, &vals[0]); - os << "{ R = " << boolFromGLBool(vals[0]) + os << "{ R = " << boolFromGLBool(vals[0]) << ", G = " << boolFromGLBool(vals[1]) - << ", B = " << boolFromGLBool(vals[2]) + << ", B = " << boolFromGLBool(vals[2]) << ", A = " << boolFromGLBool(vals[3]) << " }"; return os; } @@ -82,7 +82,7 @@ namespace debug inline Colour4 getGLColor() { Colour4 result; - glGetDoublev(GL_CURRENT_COLOR, &result[0]); + glGetFloatv(GL_CURRENT_COLOR, &result[0]); return result; } diff --git a/libs/render/Colour4.h b/libs/render/Colour4.h index 1ef0932fea..a2284f9bec 100644 --- a/libs/render/Colour4.h +++ b/libs/render/Colour4.h @@ -11,7 +11,7 @@ * vector as an RGBA colour, for example, ensuring that the colour values lie * within the [0.0, 1.0] range. */ -class Colour4: public Vector4 +class Colour4: public Vector4f { bool channelValid(double c) const { return c >= 0.0 && c <= 1.0; } @@ -19,20 +19,21 @@ class Colour4: public Vector4 /// Default-construct an invalid colour Colour4() - : Vector4(-1, -1, -1, -1) + : Vector4f(-1, -1, -1, -1) { assert(!isValid()); } /// Initialise a colour with individual components Colour4(float r, float g, float b, float a) - : Vector4(r, g, b, a) - { } + : Vector4f(r, g, b, a) + {} /// Construct a Colour4 from a Vector3 and optional alpha - Colour4(const Vector3& vec, float alpha = 1.0f) - : Vector4(vec, alpha) - { } + template + Colour4(const BasicVector3& vec, float alpha = 1.0f) + : Vector4f(vec.x(), vec.y(), vec.z(), alpha) + {} /// Return true if this colour contains valid component values bool isValid() const @@ -52,7 +53,7 @@ class Colour4: public Vector4 static const Colour4& WHITE() { - static Colour4 white(1.0f, 1.0f, 1.0f, 1.0f); + static Colour4 white(1.0f, 1.0f, 1.0f, 1.0f); return white; } }; diff --git a/radiant/xyview/tools/MeasurementTool.cpp b/radiant/xyview/tools/MeasurementTool.cpp index 2722a44709..bda1f7cfa5 100644 --- a/radiant/xyview/tools/MeasurementTool.cpp +++ b/radiant/xyview/tools/MeasurementTool.cpp @@ -178,7 +178,7 @@ void MeasurementTool::render(RenderSystem& renderSystem, RenderableCollector& co const Vector3& a = _points[i-1].vertex; const Vector3& b = _points[i].vertex; - glColor4dv(_colour); + glColor4fv(_colour); glRasterPos3dv((a+b)*0.5); GlobalOpenGL().drawString(string::to_string((a-b).getLength())); } diff --git a/radiantcore/rendersystem/backend/OpenGLShader.cpp b/radiantcore/rendersystem/backend/OpenGLShader.cpp index bfff45ef39..fa62f1203c 100644 --- a/radiantcore/rendersystem/backend/OpenGLShader.cpp +++ b/radiantcore/rendersystem/backend/OpenGLShader.cpp @@ -638,7 +638,7 @@ void OpenGLShader::construct(const std::string& name) state.setName(name); Colour4 colour; - sscanf(name.c_str(), "(%lf %lf %lf)", &colour[0], &colour[1], &colour[2]); + sscanf(name.c_str(), "(%f %f %f)", &colour[0], &colour[1], &colour[2]); colour[3] = 1.0f; state.setColour(colour); @@ -657,7 +657,7 @@ void OpenGLShader::construct(const std::string& name) state.setName(name); Colour4 colour; - sscanf(name.c_str(), "[%lf %lf %lf]", &colour[0], &colour[1], &colour[2]); + sscanf(name.c_str(), "[%f %f %f]", &colour[0], &colour[1], &colour[2]); colour[3] = 0.5f; state.setColour(colour); @@ -677,7 +677,7 @@ void OpenGLShader::construct(const std::string& name) state.setName(name); Colour4 colour; - sscanf(name.c_str(), "<%lf %lf %lf>", &colour[0], &colour[1], &colour[2]); + sscanf(name.c_str(), "<%f %f %f>", &colour[0], &colour[1], &colour[2]); colour[3] = 1; state.setColour(colour); diff --git a/radiantcore/rendersystem/backend/OpenGLShaderPass.cpp b/radiantcore/rendersystem/backend/OpenGLShaderPass.cpp index 1a7351c3bd..13f93ff912 100644 --- a/radiantcore/rendersystem/backend/OpenGLShaderPass.cpp +++ b/radiantcore/rendersystem/backend/OpenGLShaderPass.cpp @@ -347,7 +347,7 @@ void OpenGLShaderPass::applyState(OpenGLState& current, if (current.glProgram != 0) { current.glProgram->disable(); - glColor4dv(current.getColour()); + glColor4fv(current.getColour()); } current.glProgram = program; @@ -530,7 +530,7 @@ void OpenGLShaderPass::applyState(OpenGLState& current, { _glState.setColour(_glState.stage0->getColour()); } - glColor4dv(_glState.getColour()); + glColor4fv(_glState.getColour()); current.setColour(_glState.getColour()); debug::assertNoGlErrors(); diff --git a/radiantcore/rendersystem/backend/glprogram/GLSLBumpProgram.cpp b/radiantcore/rendersystem/backend/glprogram/GLSLBumpProgram.cpp index 809e748497..a9fbbe32fd 100644 --- a/radiantcore/rendersystem/backend/glprogram/GLSLBumpProgram.cpp +++ b/radiantcore/rendersystem/backend/glprogram/GLSLBumpProgram.cpp @@ -149,12 +149,7 @@ void GLSLBumpProgram::applyRenderParams(const Vector3& viewer, static_cast(localLight.y()), static_cast(localLight.z()) ); - glUniform3f( - _locLightColour, - static_cast(parms.lightColour.x()), - static_cast(parms.lightColour.y()), - static_cast(parms.lightColour.z()) - ); + glUniform3fv(_locLightColour, 1, parms.lightColour); glUniform1f(_locLightScale, _lightScale); glUniform1i(_locAmbientLight, parms.isAmbientLight); diff --git a/test/math/Vector3.cpp b/test/math/Vector3.cpp index 316a716a50..f52f720063 100644 --- a/test/math/Vector3.cpp +++ b/test/math/Vector3.cpp @@ -14,4 +14,32 @@ TEST(MathTest, ConstructVector3) EXPECT_EQ(vec.z(), 3.5); } +TEST(MathTest, Vector3IsPacked) +{ + Vector3 vec(256, -10, 10000); + + // Ensure that X, Y and Z are at consecutive memory locations and can be + // treated as a 3-element array. + EXPECT_EQ(&vec.y(), &vec.x() + 1); + EXPECT_EQ(&vec.z(), &vec.y() + 1); + + double* array = vec; + EXPECT_EQ(array[0], 256); + EXPECT_EQ(array[1], -10); + EXPECT_EQ(array[2], 10000); +} + +TEST(MathTest, Vector3fIsPacked) +{ + Vector3f vec(-0.5, 485, 0); + + EXPECT_EQ(&vec.y(), &vec.x() + 1); + EXPECT_EQ(&vec.z(), &vec.y() + 1); + + float* array = vec; + EXPECT_EQ(array[0], -0.5); + EXPECT_EQ(array[1], 485); + EXPECT_EQ(array[2], 0); +} + }