Skip to content

Commit

Permalink
Colour4 stores floats rather than doubles
Browse files Browse the repository at this point in the history
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().
  • Loading branch information
Matthew Mott committed Mar 9, 2021
1 parent 07a10bb commit 1db3e73
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 23 deletions.
6 changes: 3 additions & 3 deletions libs/debugging/render.h
Expand Up @@ -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;
}
Expand All @@ -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;
}

Expand Down
17 changes: 9 additions & 8 deletions libs/render/Colour4.h
Expand Up @@ -11,28 +11,29 @@
* 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; }

public:

/// 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<typename U>
Colour4(const BasicVector3<U>& 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
Expand All @@ -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;
}
};
Expand Down
2 changes: 1 addition & 1 deletion radiant/xyview/tools/MeasurementTool.cpp
Expand Up @@ -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()));
}
Expand Down
6 changes: 3 additions & 3 deletions radiantcore/rendersystem/backend/OpenGLShader.cpp
Expand Up @@ -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);

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

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

Expand Down
4 changes: 2 additions & 2 deletions radiantcore/rendersystem/backend/OpenGLShaderPass.cpp
Expand Up @@ -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;
Expand Down Expand Up @@ -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();

Expand Down
Expand Up @@ -149,12 +149,7 @@ void GLSLBumpProgram::applyRenderParams(const Vector3& viewer,
static_cast<float>(localLight.y()),
static_cast<float>(localLight.z())
);
glUniform3f(
_locLightColour,
static_cast<float>(parms.lightColour.x()),
static_cast<float>(parms.lightColour.y()),
static_cast<float>(parms.lightColour.z())
);
glUniform3fv(_locLightColour, 1, parms.lightColour);
glUniform1f(_locLightScale, _lightScale);
glUniform1i(_locAmbientLight, parms.isAmbientLight);

Expand Down
28 changes: 28 additions & 0 deletions test/math/Vector3.cpp
Expand Up @@ -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);
}

}

0 comments on commit 1db3e73

Please sign in to comment.