diff --git a/src/ellipse.cpp b/src/ellipse.cpp index e453e65..14ac387 100644 --- a/src/ellipse.cpp +++ b/src/ellipse.cpp @@ -26,13 +26,12 @@ #include namespace { // anonymous namespace -nzl::Program create_program() { - const std::string vertex_shader_source = - nzl::slurp(nzl::get_env_var("SHADERS_PATH") + "/simple_shader.vert"); - - const std::string fragment_shader_source = - nzl::slurp(nzl::get_env_var("SHADERS_PATH") + "/simple_shader.frag"); +const std::string vertex_shader_source = + nzl::slurp(nzl::get_env_var("MXD_SHADER_ROOT") + "/simple_shader.vert"); +const std::string fragment_shader_source = + nzl::slurp(nzl::get_env_var("MXD_SHADER_ROOT") + "/simple_shader.frag"); +nzl::Program create_program() { nzl::Shader vert_shader(nzl::Shader::Stage::Vertex, vertex_shader_source); vert_shader.compile(); @@ -66,79 +65,90 @@ std::vector gen_points(float rX, float rY, float number_of_points) { namespace nzl { -struct Ellipse::IDContainer { - glm::vec3 m_color; - unsigned int m_vao_id; - unsigned int m_vbo_id; - int m_number_of_points{0}; - nzl::Program m_program; +struct Ellipse::EllipseImp { + EllipseImp(float s_ma, float s_mi, float number_of_points); - IDContainer(glm::vec3 color, float rX, float rY, float number_of_points) - : m_color{color}, m_program{create_program()} { - glGenVertexArrays(1, &m_vao_id); + ~EllipseImp(); + nzl::Program program; + glm::vec3 color; + unsigned int vao_id; + unsigned int vbo_id; + int number_of_points{0}; +}; - glBindVertexArray(m_vao_id); +nzl::Ellipse::EllipseImp::EllipseImp(float s_ma, float s_mi, + float number_of_points) + : program{create_program()} { + glGenVertexArrays(1, &vao_id); - glGenBuffers(1, &m_vbo_id); + glBindVertexArray(vao_id); - glBindBuffer(GL_ARRAY_BUFFER, m_vbo_id); + glGenBuffers(1, &vbo_id); - std::vector points{gen_points(rX, rY, number_of_points)}; + glBindBuffer(GL_ARRAY_BUFFER, vbo_id); - // Add the first point again, so the first point is connected to the last. - points.push_back(points.front()); + std::vector points{gen_points(s_ma, s_mi, number_of_points)}; - m_number_of_points = points.size(); + // Add the first point again, so the first point is connected to the last - glBufferData(GL_ARRAY_BUFFER, points.size() * 3 * sizeof(float), - points.data(), GL_STATIC_DRAW); + points.push_back(points.front()); - glEnableVertexAttribArray(0); + number_of_points = points.size(); - glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 3 * sizeof(float), - (void*)0); + glBufferData(GL_ARRAY_BUFFER, points.size() * 3 * sizeof(float), + points.data(), GL_STATIC_DRAW); - glDisableVertexAttribArray(0); + glEnableVertexAttribArray(0); - glBindBuffer(GL_ARRAY_BUFFER, 0); + glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 3 * sizeof(float), (void*)0); - glBindVertexArray(0); - } + glDisableVertexAttribArray(0); - ~IDContainer() { - glDeleteVertexArrays(1, &m_vao_id); - glDeleteBuffers(1, &m_vbo_id); - } -}; + glBindBuffer(GL_ARRAY_BUFFER, 0); -Ellipse::Ellipse(float rX, float rY, int number_of_points, - glm::vec3 color) noexcept - : m_id_container{ - std::make_shared(color, rX, rY, number_of_points)} {} + glBindVertexArray(0); + + nzl::check_gl_errors(); +} + +nzl::Ellipse::EllipseImp::~EllipseImp() { + glDeleteVertexArrays(1, &vao_id); + glDeleteBuffers(1, &vbo_id); + nzl::check_gl_errors(); +} -glm::vec3 Ellipse::color() const noexcept { return m_id_container->m_color; } +// ----------------------------------------------------------------------------- +// The section below forwards API calls to the implementation +// ----------------------------------------------------------------------------- -void Ellipse::set_color(glm::vec3 color) noexcept { - m_id_container->m_color = color; +Ellipse::Ellipse(float s_ma, float s_mi, int number_of_points, glm::vec3 color) + : m_pimpl{std::make_shared(s_ma, s_mi, number_of_points)} { + m_pimpl->color = color; } -nzl::Program Ellipse::get_program() const noexcept { - return m_id_container->m_program; +glm::vec3 Ellipse::color() const noexcept { return m_pimpl->color; } + +void Ellipse::set_color(glm::vec3 color) noexcept { m_pimpl->color = color; } + +const nzl::Program& Ellipse::get_program() const noexcept { + return m_pimpl->program; } void Ellipse::do_render(TimePoint t [[maybe_unused]]) { - m_id_container->m_program.use(); - m_id_container->m_program.set("color", m_id_container->m_color); + m_pimpl->program.use(); + m_pimpl->program.set("color", m_pimpl->color); - glBindVertexArray(m_id_container->m_vao_id); + glBindVertexArray(m_pimpl->vao_id); glEnableVertexAttribArray(0); - glDrawArrays(GL_LINE_STRIP, 0, m_id_container->m_number_of_points); + glDrawArrays(GL_LINE_STRIP, 0, m_pimpl->number_of_points); glDisableVertexAttribArray(0); glBindVertexArray(0); + + nzl::check_gl_errors(); } } // namespace nzl diff --git a/src/ellipse.hpp b/src/ellipse.hpp index 1427c37..1e37cc3 100644 --- a/src/ellipse.hpp +++ b/src/ellipse.hpp @@ -25,11 +25,11 @@ namespace nzl { class Ellipse : public Geometry { public: /// @brief Creates an ellipse. - /// @param rX Radius in the x direction. - /// @param rY Radius in the y direction. + /// @param s_ma Length of semi-major axis. + /// @param s_mi Length of semi-minor axis. /// @param number_of_points Number of points to be generated for the ellipse. /// @param color Color the ellipse will be drawn with. - Ellipse(float rX, float rY, int number_of_points, glm::vec3 color) noexcept; + Ellipse(float s_ma, float s_mi, int number_of_points, glm::vec3 color); /// @brief Return the ellipse's color. glm::vec3 color() const noexcept; @@ -40,11 +40,11 @@ class Ellipse : public Geometry { void set_color(glm::vec3 color) noexcept; /// @brief Returns the program used by the ellipse. - nzl::Program get_program() const noexcept; + const nzl::Program& get_program() const noexcept; private: - struct IDContainer; - std::shared_ptr m_id_container{nullptr}; + struct EllipseImp; + std::shared_ptr m_pimpl; void do_render(TimePoint t) override; }; diff --git a/src/ellipse.t.cpp b/src/ellipse.t.cpp index 2720e87..ca55b70 100644 --- a/src/ellipse.t.cpp +++ b/src/ellipse.t.cpp @@ -37,6 +37,12 @@ TEST(Ellipse, ConstructorAndParameterAccess) { EXPECT_FLOAT_EQ(ellipse.color().y, 0.0f); EXPECT_FLOAT_EQ(ellipse.color().z, 0.0f); + ellipse.set_color(glm::vec3(0.4f, 0.3f, 0.1f)); + + EXPECT_FLOAT_EQ(ellipse.color().x, 0.4f); + EXPECT_FLOAT_EQ(ellipse.color().y, 0.3f); + EXPECT_FLOAT_EQ(ellipse.color().z, 0.1f); + EXPECT_NE(ellipse.get_program().id(), 0); EXPECT_EQ(glGetError(), 0); diff --git a/src/geometry.t.cpp b/src/geometry.t.cpp index 93b2467..fb20ff9 100644 --- a/src/geometry.t.cpp +++ b/src/geometry.t.cpp @@ -16,9 +16,6 @@ // Google Test Framework #include -TEST(Geometry, Failing) { - ASSERT_TRUE(false) << "You must add unit tests for geometry.hpp"; -} int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); diff --git a/src/line.cpp b/src/line.cpp index 5e5d4b1..7b0f2a3 100644 --- a/src/line.cpp +++ b/src/line.cpp @@ -28,17 +28,12 @@ namespace { // anonymous namespace const std::string vertex_shader_source = - nzl::slurp(nzl::get_env_var("SHADERS_PATH") + "/simple_shader.vert"); + nzl::slurp(nzl::get_env_var("MXD_SHADER_ROOT") + "/simple_shader.vert"); const std::string fragment_shader_source = - nzl::slurp(nzl::get_env_var("SHADERS_PATH") + "/simple_shader.frag"); + nzl::slurp(nzl::get_env_var("MXD_SHADER_ROOT") + "/simple_shader.frag"); -/// @TODO This is too ugly. Program needs a full refactoring. auto make_program() { - /// @TODO The Shader/Program interface feels very awkward. For instance: why - /// do I need to pass a vector of shaders? Why do I need to compile the - /// shaders and then compile the program? Shouldn't the program compile the - /// shaders if it needs to? What happens if I forget to compile the shader? std::vector shaders; shaders.emplace_back(nzl::Shader::Stage::Vertex, vertex_shader_source); shaders.emplace_back(nzl::Shader::Stage::Fragment, fragment_shader_source); @@ -58,8 +53,8 @@ namespace nzl { /// Put all you need in the Implementation. struct nzl::Line::LineImp { LineImp(); - ~LineImp() noexcept; - nzl::Program program; /// @TODO Why not provide a default constructor? + ~LineImp(); + nzl::Program program; unsigned int vao_id; unsigned int vbo_id; int number_of_points{0}; @@ -69,8 +64,6 @@ struct nzl::Line::LineImp { }; nzl::Line::LineImp::LineImp() : program{make_program()} { - /// @TODO Add error checking! 10 minutes spent adding good error checking and - /// error messages will save you 10 hours debugging the program in the future. glGenVertexArrays(1, &vao_id); glBindVertexArray(vao_id); glGenBuffers(1, &vbo_id); @@ -81,12 +74,13 @@ nzl::Line::LineImp::LineImp() : program{make_program()} { glDisableVertexAttribArray(0); glBindBuffer(GL_ARRAY_BUFFER, 0); glBindVertexArray(0); -} -nzl::Line::LineImp::~LineImp() noexcept { - /// @TODO: Add error checking! + nzl::check_gl_errors(); +} +nzl::Line::LineImp::~LineImp() { glDeleteVertexArrays(1, &vao_id); glDeleteBuffers(1, &vbo_id); + nzl::check_gl_errors(); } void nzl::Line::LineImp::load_points(glm::vec3 points[], int size) { @@ -95,6 +89,7 @@ void nzl::Line::LineImp::load_points(glm::vec3 points[], int size) { glBufferData(GL_ARRAY_BUFFER, size * 3 * sizeof(float), points, GL_STATIC_DRAW); glBindBuffer(GL_ARRAY_BUFFER, 0); + nzl::check_gl_errors(); } // ----------------------------------------------------------------------------- @@ -111,11 +106,11 @@ Line::Line(glm::vec3 color, std::vector& points) : Line(color) { load_points(points); } -void Line::load_points(std::vector& points) noexcept { +void Line::load_points(std::vector& points) { m_pimpl->load_points(points.data(), points.size()); } -void Line::load_points(glm::vec3 points[], int size) noexcept { +void Line::load_points(glm::vec3 points[], int size) { m_pimpl->load_points(points, size); } @@ -134,12 +129,13 @@ void Line::do_render(TimePoint t [[maybe_unused]]) { program.use(); program.set("color", m_pimpl->color); - /// @TODO Add error checking! glBindVertexArray(m_pimpl->vao_id); glEnableVertexAttribArray(0); glDrawArrays(GL_LINE_STRIP, 0, m_pimpl->number_of_points); glDisableVertexAttribArray(0); glBindVertexArray(0); + + nzl::check_gl_errors(); } } // namespace nzl diff --git a/src/line.hpp b/src/line.hpp index abe9165..60097a1 100644 --- a/src/line.hpp +++ b/src/line.hpp @@ -27,27 +27,32 @@ namespace nzl { class Line : public Geometry { public: /// @brief Creates line with white color. + /// @throws std::runtime_error if an OpenGL error is found. Line(); /// @brief Creates line. /// @param color Line color. + /// @throws std::runtime_error if an OpenGL error is found. Line(glm::vec3 color); /// @brief Creates line and loads points onto line. /// @param color Line color. /// @param points Points to be loaded into the VBO. + /// @throws std::runtime_error if an OpenGL error is found. Line(glm::vec3 color, std::vector& points); /// @brief Loads points into the line's VBO. /// @param points Points to be loaded into the VBO. + /// @throws std::runtime_error if an OpenGL error is found. /// @note Affects all copies of this object. - void load_points(std::vector& points) noexcept; + void load_points(std::vector& points); /// @brief Loads points into the line's VBO. /// @param points Points to be loaded into the VBO. /// @param size Size of the array. + /// @throws std::runtime_error if an OpenGL error is found. /// @note Affects all copies of this object. - void load_points(glm::vec3 points[], int size) noexcept; + void load_points(glm::vec3 points[], int size); /// @brief Returns the line's color. glm::vec3 color() const noexcept; diff --git a/src/line.t.cpp b/src/line.t.cpp index 91bd536..fee85d5 100644 --- a/src/line.t.cpp +++ b/src/line.t.cpp @@ -37,6 +37,12 @@ TEST(Line, ConstructorAndParameterAccess) { EXPECT_FLOAT_EQ(line.color().y, 0.5f); EXPECT_FLOAT_EQ(line.color().z, 0.3f); + line.set_color(glm::vec3(1.0f, 0.3f, 0.1f)); + + EXPECT_FLOAT_EQ(line.color().x, 1.0f); + EXPECT_FLOAT_EQ(line.color().y, 0.3f); + EXPECT_FLOAT_EQ(line.color().z, 0.1f); + EXPECT_NE(line.get_program().id(), 0); EXPECT_EQ(glGetError(), 0); @@ -105,6 +111,34 @@ TEST(Line, DrawAndReplacePoints) { nzl::terminate(); } +TEST(Line, DrawFromArray) { + nzl::initialize(); + nzl::Window win(800, 600, "Test Window"); + win.hide(); + win.make_current(); + + nzl::Line line; + + glm::vec3 arr[3]; + + arr[0] = glm::vec3(-1.0f, 1.0f, 0.0f); + arr[1] = glm::vec3(1.0f, -1.0f, 0.0f); + arr[2] = glm::vec3(0.5f, 1.0f, 0.0f); + line.load_points(arr, 3); + + for (int i = 0; i < 3; i++) { + glClearColor(0.0f, 0.0f, 0.0f, 1.0f); + glClear(GL_COLOR_BUFFER_BIT); + + line.render(nzl::TimePoint()); + + win.swap_buffers(); + } + EXPECT_EQ(glGetError(), 0); + + nzl::terminate(); +} + int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); diff --git a/src/program.cpp b/src/program.cpp index 1372bdb..d05c3df 100644 --- a/src/program.cpp +++ b/src/program.cpp @@ -47,9 +47,14 @@ auto create_program() { nzl::requires_current_context(); if (auto id = glCreateProgram(); id == 0) { - /// @TODO: Add a more extensive error message. std::ostringstream oss; - oss << "Error creating Program object"; + oss << "Error creating Program object: "; + + if (glfwGetCurrentContext() == nullptr) { + oss << "OpenGL context non-existent"; + } else { + oss << "Unknown error"; + } throw std::runtime_error(oss.str()); } else { return id; @@ -119,6 +124,10 @@ void Program::use() const noexcept { glUseProgram(m_id_container->m_id); } void Program::add_shader(nzl::Shader& shader) { m_shaders.push_back(shader); } +void Program::emplace_shader(nzl::Shader::Stage stage, std::string source) { + m_shaders.emplace_back(stage, source); +} + void Program::set(const std::string& name, bool value) const { glUniform1i(m_id_container->find_uniform_location(name), (int)value); } diff --git a/src/program.hpp b/src/program.hpp index 4566cbc..8559e48 100644 --- a/src/program.hpp +++ b/src/program.hpp @@ -44,6 +44,12 @@ class Program { /// @param shader Shader to be added void add_shader(nzl::Shader& shader); + /// @brief Creates a @link Shader@endlink directly in the program's vector. + /// @param stage Stage in the rendering pipeline. + /// @param source Source code for the shader + void emplace_shader(nzl::Shader::Stage stage, std::string source); + + /// @brief Sets a boolean uniform within the Program. /// @throws std::runtime_error when uniform not found /// @param name Name of the uniform diff --git a/src/program.t.cpp b/src/program.t.cpp index 22d7d47..379b30a 100644 --- a/src/program.t.cpp +++ b/src/program.t.cpp @@ -113,6 +113,69 @@ TEST(Program, ParameterAccessAndCompilation) { nzl::terminate(); } +TEST(Program, CreateFromLooseShaders) { + nzl::initialize(); + nzl::Window win(800, 600, "Invisible Window"); + win.hide(); + win.make_current(); + + std::string vSource = + "#version 330\n" + "layout (location = 0) in vec3 aPos;\n" + "out vec4 vertexColor;\n" + "void main() {\n" + "gl_Position = vec4(aPos, 1.0);\n" + "vertexColor = vec4(0.5,0.0,0.0,1.0);\n}"; + + std::string fSource = + "#version 330\n" + "out vec4 FragColor;\n" + "" + "in vec4 vertexColor;\n" + "" + "void main(){\n" + "FragColor = vertexColor;}"; + + nzl::Shader shader1 = nzl::Shader(nzl::Shader::Stage::Vertex, vSource); + nzl::Shader shader2 = nzl::Shader(nzl::Shader::Stage::Fragment, fSource); + + ASSERT_NO_THROW(nzl::Program program; program.add_shader(shader1); + program.add_shader(shader2); program.compile()); + + nzl::terminate(); +} + +TEST(Program, EmplaceShaders) { + nzl::initialize(); + nzl::Window win(800, 600, "Invisible Window"); + win.hide(); + win.make_current(); + + std::string vSource = + "#version 330\n" + "layout (location = 0) in vec3 aPos;\n" + "out vec4 vertexColor;\n" + "void main() {\n" + "gl_Position = vec4(aPos, 1.0);\n" + "vertexColor = vec4(0.5,0.0,0.0,1.0);\n}"; + + std::string fSource = + "#version 330\n" + "out vec4 FragColor;\n" + "" + "in vec4 vertexColor;\n" + "" + "void main(){\n" + "FragColor = vertexColor;}"; + + ASSERT_NO_THROW(nzl::Program program; + program.emplace_shader(nzl::Shader::Stage::Vertex, vSource); + program.emplace_shader(nzl::Shader::Stage::Fragment, fSource); + program.compile()); + + nzl::terminate(); +} + TEST(Program, CopyConstructor) { nzl::initialize(); nzl::Window win(800, 600, "Invisible Window"); diff --git a/src/shader.cpp b/src/shader.cpp index 899dde2..60fa500 100644 --- a/src/shader.cpp +++ b/src/shader.cpp @@ -55,9 +55,13 @@ auto create_shader(nzl::Shader::Stage stage) { << ") passed as Shader::Stage"; throw std::runtime_error(oss.str()); } else if (id == 0) { - /// @TODO: Add a more extensive error message. std::ostringstream oss; - oss << "Error creating Shader object"; + oss << "Error creating Shader object: "; + if (glfwGetCurrentContext() == nullptr) { + oss << "OpenGL context non-existent"; + } else { + oss << "Unknown error"; + } throw std::runtime_error(oss.str()); } else { return id; diff --git a/src/shader.t.cpp b/src/shader.t.cpp index 623cdb2..96aee29 100644 --- a/src/shader.t.cpp +++ b/src/shader.t.cpp @@ -31,8 +31,10 @@ TEST(Shader, ParameterAccessAndCompilation) { EXPECT_EQ(shader.stage(), stage); EXPECT_EQ(shader.source(), source); + EXPECT_EQ(shader.is_compiled(), false); EXPECT_NO_THROW(shader.compile()); EXPECT_NE(shader.id(), 0u); + EXPECT_EQ(shader.is_compiled(), true); // Now try having an obiously wrong shader. In this case, the source will be // missing a semicolon after the `x`. diff --git a/src/utilities.cpp b/src/utilities.cpp index 0c3147e..ddd2a37 100644 --- a/src/utilities.cpp +++ b/src/utilities.cpp @@ -75,10 +75,15 @@ std::string slurp(const std::string& path) { std::string get_env_var(const std::string& key) { char* val; val = std::getenv(key.c_str()); - std::string ret_val = ""; - if (val != nullptr) { - ret_val = val; + + if (val == nullptr) { + std::ostringstream oss; + oss << "Unable to find environment variable: " << key; + throw std::runtime_error(oss.str()); } + + std::string ret_val = val; + return ret_val; } diff --git a/src/utilities.t.cpp b/src/utilities.t.cpp index 6d484af..4d77024 100644 --- a/src/utilities.t.cpp +++ b/src/utilities.t.cpp @@ -11,8 +11,10 @@ // C++ Standard Library #include +#include #include #include +#include #include // mxd library @@ -46,6 +48,15 @@ TEST(Utilities, SlurpFile) { std::remove(path.c_str()); } +TEST(Utilities, EnvVarShaderRootSet) { + ASSERT_NO_THROW(nzl::get_env_var("MXD_SHADER_ROOT")); +} + +TEST(Utilities, EnvVarNotFound) { + ASSERT_THROW(nzl::get_env_var("MXD_TEST_ENV_VAR_NOT_FOUND"), + std::runtime_error); +} + TEST(Utilities, CheckGLError) { nzl::initialize(); nzl::Window win(800, 600, "Hidden Window"); diff --git a/src/window.cpp b/src/window.cpp index d39e768..e889f9c 100644 --- a/src/window.cpp +++ b/src/window.cpp @@ -151,6 +151,7 @@ int Window::height() const noexcept { return m_pimpl->height; } const std::string& Window::title() const noexcept { return m_pimpl->title; } +// @TODO Implement close method void Window::close() {} void Window::swap_buffers() { return m_pimpl->swap_buffers(); } diff --git a/src/window.t.cpp b/src/window.t.cpp index c38a914..d4501c0 100644 --- a/src/window.t.cpp +++ b/src/window.t.cpp @@ -36,6 +36,20 @@ TEST(Window, InitializeWindow) { nzl::terminate(); } +TEST(Window, SwapBuffers) { + ASSERT_NO_THROW(nzl::initialize()); + + ASSERT_NO_THROW(nzl::Window win(800, 600, "Invisible Window")); + nzl::Window win(800, 600, "Invisible Window"); + + EXPECT_NO_THROW(win.hide()); + EXPECT_NO_THROW(win.make_current()); + + EXPECT_NO_THROW(win.swap_buffers()); + + nzl::terminate(); +} + int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS();