C++ OpenGL - GLFW Window Abstraction

Question

I made a GLFW window abstraction, as I find it very useful for me to make such abstractions while learning an API, and It actually pretty much eases my work while coding.

Anyways, share your thoughts. Is the code practice good? Is it bad If I am very explicit in my naming conventions? For example, every method is Window...something. Like DestroyWindow(). Should it be only Destroy();?

Window.h

#pragma once

#include "GLFW\glfw3.h"

enum class WindowMode
{
    FULLSCREEN = 0,
    WINDOWED = 1
};

struct WindowProps
{
    unsigned int    WindowWidth;
    unsigned int    WindowHeight;
    const char* WindowTitle;
    WindowMode  WM;

    WindowProps(unsigned int windowWidth = 1280,
            unsigned int windowHeight = 720,
            const char* windowTitle = "OpenGL",
            WindowMode wm = WindowMode::WINDOWED)
        : WindowWidth(windowWidth), WindowHeight(windowHeight), WindowTitle(windowTitle), WM(wm)
    {
    }
};

class Window
{
private:
    GLFWwindow* m_Window;

    unsigned int    m_WindowMinimumWidth;
    unsigned int    m_WindowMaximumWidth;
    unsigned int    m_WindowMinimumHeight;
    unsigned int    m_WindowMaximumHeight;

    unsigned int    m_MonitorWidth;
    unsigned int    m_MonitorHeight;

    struct WindowData
    {
        unsigned int    WindowWidth;
        unsigned int    WindowHeight;
        const char* WindowTitle;
        WindowMode  WM;
    } m_WindowData;

private:
    void InitWindow(const WindowProps& props = WindowProps());

public:
    Window(const unsigned int& windowWidth, const unsigned int& windowHeight, const char* windowTitle, WindowMode windowMode);
    Window();

    void MakeWindowContextCurrent();
    void DestroyWindow();
    void WindowOnFocus();

    void MaximizeWindow();
    void MinimizeWindow();
    void RestoreWindow();
    void CloseWindow();

    void SetWindowWidth(const unsigned int& windowWidth);
    void SetWindowHeight(const unsigned int& windowHeight);
    void SetWindowSizeLimit(const unsigned int& windowMinWidth, const unsigned int& windowMinHeight, const unsigned int& windowMaxWidth, const unsigned int& windowMaxHeight);
    void SetWindowTitle(const char* windowTitle);

    unsigned int GetMonitorWidth();
    unsigned int GetMonitorHeight();

    inline unsigned int GetWindowWidth() const { return m_WindowData.WindowWidth; }
    inline unsigned int GetWindowHeight() const { return m_WindowData.WindowHeight; }
    inline unsigned int GetWindowMinimumWidth() const { return m_WindowMinimumWidth; }
    inline unsigned int GetWindowMaximumWidth() const { return m_WindowMaximumWidth; }
    inline unsigned int GetWindowMinimumHeight() const { return m_WindowMinimumHeight; }
    inline unsigned int GetWindowMaximumHeight() const { return m_WindowMaximumHeight; }
    inline const char* GetWindowTitle() const { return m_WindowData.WindowTitle; }

    inline GLFWwindow* GetWindowInstance() const { return m_Window; }
};

Window.cpp

#include "Window.h"

Window::Window(const unsigned int& windowWidth, const unsigned int& windowHeight, const char* windowTitle, WindowMode windowMode)
{
    WindowProps windowProperties;

    windowProperties.WindowWidth = windowWidth;
    windowProperties.WindowHeight = windowHeight;
    windowProperties.WindowTitle = windowTitle;
    windowProperties.WM = windowMode;

    InitWindow(windowProperties);
}

Window::Window()
{
    InitWindow();
}

void Window::InitWindow(const WindowProps& windowProperties)
{
    m_WindowData.WindowWidth = windowProperties.WindowWidth;
    m_WindowData.WindowHeight = windowProperties.WindowHeight;
    m_WindowData.WindowTitle = windowProperties.WindowTitle;

    m_Window = glfwCreateWindow(
        windowProperties.WM == WindowMode::FULLSCREEN ? GetMonitorWidth() : windowProperties.WindowWidth,
        windowProperties.WM == WindowMode::FULLSCREEN ? GetMonitorHeight() : windowProperties.WindowHeight,
        windowProperties.WindowTitle,
        windowProperties.WM == WindowMode::FULLSCREEN ? glfwGetPrimaryMonitor() : nullptr, nullptr
    );
}

void Window::MakeWindowContextCurrent()
{
    glfwMakeContextCurrent(m_Window);
}

void Window::DestroyWindow()
{
    glfwDestroyWindow(m_Window);
}

void Window::WindowOnFocus()
{
    glfwFocusWindow(m_Window);
}

void Window::MaximizeWindow()
{
    glfwMaximizeWindow(m_Window);
}

void Window::MinimizeWindow()
{
    glfwIconifyWindow(m_Window);
}

void Window::RestoreWindow()
{
    glfwRestoreWindow(m_Window);
}

void Window::CloseWindow()
{
    glfwSetWindowShouldClose(m_Window, GL_TRUE);
}

void Window::SetWindowWidth(const unsigned int& windowWidth)
{
    m_WindowData.WindowWidth = windowWidth;

    glfwSetWindowSize(m_Window, m_WindowData.WindowWidth, m_WindowData.WindowHeight);
}

void Window::SetWindowHeight(const unsigned int& windowHeight)
{
    m_WindowData.WindowHeight = windowHeight;

    glfwSetWindowSize(m_Window, m_WindowData.WindowWidth, m_WindowData.WindowHeight);
}

void Window::SetWindowSizeLimit(const unsigned int& windowMinWidth, const unsigned int& windowMinHeight, const unsigned int& windowMaxWidth,
    const unsigned int& windowMaxHeight)
{
    m_WindowMinimumWidth = windowMinWidth;
    m_WindowMinimumHeight = windowMinHeight;
    m_WindowMaximumWidth = windowMaxWidth;
    m_WindowMaximumHeight = windowMaxHeight;

    glfwSetWindowSizeLimits(m_Window, m_WindowMinimumWidth, m_WindowMinimumHeight, m_WindowMaximumWidth, m_WindowMaximumHeight);
}

void Window::SetWindowTitle(const char* windowTitle)
{
    m_WindowData.WindowTitle = windowTitle;

    glfwSetWindowTitle(m_Window, m_WindowData.WindowTitle);
}

unsigned int Window::GetMonitorWidth()
{
    const GLFWvidmode* VidMode = glfwGetVideoMode(glfwGetPrimaryMonitor());
    m_MonitorWidth = VidMode->width;

    return m_MonitorWidth;
}

unsigned int Window::GetMonitorHeight()
{
    const GLFWvidmode* VidMode = glfwGetVideoMode(glfwGetPrimaryMonitor());
    m_MonitorHeight = VidMode->height;

    return m_MonitorHeight;
}

Usage Example:

Window myWindow(960, 540, "OpenGL", WindowMode::WINDOWED);
myWindow.MakeWindowContextCurrent();

std::cout << myWindow.GetWindowTitle() << "\n";
std::cout << myWindow.GetWindowWidth() << "\n";
std::cout << myWindow.GetWindowHeight() << "\n";
std::cout << myWindow.GetMonitorWidth() << "\n";
std::cout << myWindow.GetMonitorHeight() << "\n";
myWindow.DestroyWindow();

Answer 1

• WindowProps and WindowData are effectively identical. We could just use WindowProps for both.

• We can query the window width and height from GLFW (with glfwGetWindowSize). We should do this instead of storing the width and height ourselves because:

  • If the window is resized, the width and height stored in the class will be incorrect. To update these values, we'd have to use glfwSetWindowSizeCallback.

  • I suspect that glfwSetWindowSize may not set the window size to the requested values if the window has a size limit (so the values stored in SetWindowWidth and SetWindowHeight may be wrong).

• Similarly, GetMonitorWidth() may return unexpected values if a window is moved to a monitor other than the primary monitor.

• Note that m_MonitorWidth and m_MonitorHeight are set, and never referred to again. These members could be removed.

• We should initialize m_Window to nullptr in the constructor(s).

• The window title should be stored in a std::string. We should not require an externally owned char* to be kept alive for the duration of the Window class lifetime.

• There is no advantage to passing built-in types by const& (e.g. void SetWindowWidth(const unsigned int& windowWidth);, and others).

• We should not have a publicly accessible DestroyWindow function. If this is called, it leaves us with an invalid Window object, for which none of the functions (setting size, etc.) make sense or work. We should destroy the window in a class destructor instead.

• Functions defined inside the body of the class do not need the inline keyword (e.g. inline unsigned int GetWindowWidth() const).

• As noted by programmer in the comments, the use of Window in the function names is unnecessary.

• I'd suggest that the default values in WindowProps are unnecessary. Asking the user to supply these every time is not a significant burden. We can do this by deleteing the default constructor:

  struct WindowProperties
  {
      unsigned int Width, Height;
      std::string Title;
      WindowMode Mode;
  
      WindowProperties() = delete;
  };
  

  We then need only one Window constructor: Window(WindowProperties const& properties); which can be called like so: Window window({ 1024, 768, "blah", WindowMode::FULLSCREEN });

• We need to think about copying and moving the Window class. It's probably easiest to prevent both:

  Window(Window const&) = delete;
  Window& operator=(Window const&) = delete;
  Window(Window&&) = delete;
  Window& operator=(Window&&) = delete;
  

  However, there might be a case for allowing move construction and assignment.

---

Comment reply - passing by value:

In C++ all types are value-types. If we want a reference we have to explicitly request it. So if we define a function like this:

void foo(ValueType value);

The ValueType is always copied. If ValueType is defined by typedef int ValueType, this is very cheap. If ValueType is a struct ValueType { int[10000]; }; it isn't.

Where copying is expensive, we can pass a reference to the object in the outer scope instead:

void foo(const ValueType& value);

We can think of this as passing (a safer version of) a pointer to an object somewhere outside the foo function.

For types where copying is cheap (e.g. built-in types like int, unsigned int etc.) this may introduce overhead, and prevents compiler optimizations. So for types like unsigned int it's best to do the copy. So we should have:

void SetWidth(unsigned int width);
void SetHeight(unsigned int height);
void SetSizeLimit(unsigned int minWidth, unsigned int minHeight, unsigned int maxWidth, unsigned int maxHeight);

---

Comment reply - destructor:

We should call DestroyWindow from the destructor:

Window::~Window() { DestroyWindow(); }

However, DestroyWindow should not be a public function, otherwise we can do this:

myWindow.DestroyWindow();
myWindow.SetWidth(480); // crash? error? we shouldn't be able to do this here...

Many things in C++ are based on the idea of RAII (resource acquisition is initialization). This attempts to ensure that the lifetime of a resource (the glfw window in this case) has the same lifetime as the object that "owns" it (the Window class). So when the user creates an instance of the Window class, the window is created, and when the user destroys that instance (or it goes out of scope), the window is destroyed.

{
    Window window(...); <--- window resource created
    ... ok to do stuff with window
} <--- window destructor called automatically at end of scope (window destroyed)

... window doesn't exist any more

---

Comment reply - copying:

We need to think about what happens if the user does this:

Window window1(1280, 720, "title", WindowMode::WINDOWED);
Window window2(window1); // copy construction (create new window by copying an existing one)
Window window3 = window1; // also copy construction

Or if the user does this:

Window window1(1280, 720, "title", WindowMode::WINDOWED);
Window window2(920, 53, "blah", WindowMode::WINDOWED);
window2 = window3; // copy assignment (assign one existing window to another)

These correspond to calling Window member functions with the following signature:

Window(Window const& rhs); // copy construction
Window& operator=(Window const& rhs); // copy assignment

The compiler may automatically generate these for you.

For a class representing a data structure, we can just copy the data inside the class. For a Window class, we don't really want to have multiple Window objects representing the same glfw window. We could open a new glfw window, copying the settings from the old one. However, it's much easier to just prevent copying altogether by deleting these operators (preventing the compiler from defining them).

Window(Window const&) = delete;
Window& operator=(Window const&) = delete;

Moving is similar. It allows an object instance to adopt / steal the internal resources from another (which is much quicker than copying them). This is getting a bit long, so I'm not going to go into detail here.