3
\$\begingroup\$

I'm creating a window class in C++ to provide a bit of abstraction for a GLFW window.

window.h

#pragma once

#define GLFW_INCLUDE_VULKAN
#include <GLFW/glfw3.h>

class Window {
    GLFWwindow* m_Window;
    GLFWmonitor* m_Monitor;

    const char* m_Title;
    GLFWimage m_Icon[1];
    int m_Width, m_Height;
    int m_PosX, m_PosY;
    bool m_Fullscreen;

public:
    Window(int width, int height, const char* title, const char* iconPath);
    ~Window();

    GLFWwindow* getWindow();

    const char** getRequiredExtensions();

private:
    void queryVulkanSupport();

    void initGLFW();
    void createWindow();

    void setIcon(const char* path);
    void center();
    void setFullscreen();

    void setWindowSizeCallback();
    static void static_WindowSizeCallback(GLFWwindow* window, int width, int height);
    void windowSizeCallback(int width, int height);

    void setKeyCallback();
    static void static_KeyCallback(GLFWwindow* window, int key, int scancode, int action, int mods);
    void keyCallback(int key, int scancode, int action, int mods);
};

window.cpp

#include "window.h"

#define STB_IMAGE_IMPLEMENTATION
#include <stb/stb_image.h>

#include <stdexcept>
#include <string>

Window::Window(int width, int height, const char* title, const char* iconPath)
    : m_Fullscreen(false), m_Width(width), m_Height(height), m_Title(title) {
    initGLFW();
    queryVulkanSupport();
    m_Monitor = glfwGetPrimaryMonitor();
    createWindow();
    setIcon(iconPath);
    setWindowSizeCallback();
    setKeyCallback();
}

Window::~Window() {
    glfwDestroyWindow(m_Window);
    glfwTerminate();
}

GLFWwindow* Window::getWindow() {
    return m_Window;
}

const char** Window::getRequiredExtensions()
{
    uint32_t count;
    const char** extensions = glfwGetRequiredInstanceExtensions(&count);
    return extensions;
}

void Window::queryVulkanSupport() {
    if (!glfwVulkanSupported()) {
        throw std::runtime_error("Vulkan not supported!");
    }
}

void Window::initGLFW() {
    if (!glfwInit()) {
        throw std::runtime_error("Failed to initialize GLFW!");
    }
}

void Window::createWindow() {
    glfwWindowHint(GLFW_CLIENT_API, GLFW_NO_API);
    m_Window = glfwCreateWindow(m_Width, m_Height, m_Title, nullptr, nullptr);
    if (!m_Window) {
        throw std::runtime_error("Could not create GLFW window!");
    }
    glfwSetWindowUserPointer(m_Window, this);
    center();
}

void Window::setIcon(const char* path) {
    m_Icon[0].pixels = stbi_load(path, &m_Icon[0].width, &m_Icon[0].height, 0, 4);
    if (m_Icon[0].pixels)
        glfwSetWindowIcon(m_Window, 1, m_Icon);
}

void Window::center() {
    const GLFWvidmode* vidMode = glfwGetVideoMode(m_Monitor);
    glfwSetWindowPos(m_Window, (vidMode->width - m_Width) / 2, (vidMode->height - m_Height) / 2);
}

void Window::setFullscreen() {
    if (!m_Fullscreen) {
        const GLFWvidmode* vidMode = glfwGetVideoMode(m_Monitor);
        glfwGetWindowPos(m_Window, &m_PosX, &m_PosY);
        glfwGetWindowSize(m_Window, &m_Width, &m_Height);
        glfwSetWindowMonitor(m_Window, m_Monitor, 0, 0, vidMode->width, vidMode->height, vidMode->refreshRate);
        glfwSetWindowSize(m_Window, vidMode->width, vidMode->height);
        m_Fullscreen = !m_Fullscreen;
    }
    else {
        glfwSetWindowMonitor(m_Window, nullptr, m_PosX, m_PosY, m_Width, m_Height, 0);
        glfwSetWindowSize(m_Window, m_Width, m_Height);
        m_Fullscreen = !m_Fullscreen;
    }
}

void Window::setWindowSizeCallback() {
    glfwSetWindowSizeCallback(m_Window, static_WindowSizeCallback);
}

void Window::static_WindowSizeCallback(GLFWwindow* window, int width, int height) {
    Window* actualWindow = (Window*) glfwGetWindowUserPointer(window);
    actualWindow->windowSizeCallback(width, height);
}

void Window::windowSizeCallback(int width, int height) {
    glfwSetWindowSize(m_Window, width, height);
}

void Window::setKeyCallback() {
    glfwSetKeyCallback(m_Window, static_KeyCallback);
}

void Window::static_KeyCallback(GLFWwindow* window, int key, int scancode, int action, int mods) {
    Window* actualWindow = (Window*) glfwGetWindowUserPointer(window);
    actualWindow->keyCallback(key, scancode, action, mods);
}

void Window::keyCallback(int key, int scancode, int action, int mods) {
    if (key == GLFW_KEY_ESCAPE && action == GLFW_PRESS)
        glfwSetWindowShouldClose(m_Window, true);
    if (key == GLFW_KEY_F11 && action == GLFW_RELEASE)
        setFullscreen();
}

main.cpp

#include "window.h"

int main() {
    Window window(600, 300, "Vulkan engine!", "include/Ressources/VulkanIcon.png");

    while (!glfwWindowShouldClose(window.getWindow())) {
        glfwPollEvents();
    }
}

The code works as intended. I'm really new to c++ though and I assume there are tons I could improve about this class, not just in terms of speed, but also structure.

I'm especially concerned with the way I get around the fact that GLFW callbacks are static. In my callbacks I want to access methods in my class and so I create a pointer to the actual 'window user' using glfwGetWindowUserPointer, to access the actual class methods an member variables.

Still I'm just looking for any thoughts on how to improve this code.

Links to GLFW and STB.

\$\endgroup\$
0

1 Answer 1

3
\$\begingroup\$

Use default member initialization where appropriate

You always want to set m_Fullscreen to false when creating a new Window. Instead of setting this variable in the constructor, just initialize it where you declare it:

class Window {
    ...
    bool m_Fullscreen = false;

It doesn't seem a big deal here, but once you have more member variables or more constructors, you will see that this is the preferred way to do this.

Avoid creating trivial single-line private member functions

Functions like the following are rather pointless to have:

void Window::setWindowSizeCallback()
{
    glfwSetWindowSizeCallback(m_Window, static_WindowSizeCallback);
}

You are writing a lot of boiler plate here (5 lines in windows.cpp and windows.h combined), for no gain at all: this function is only used once, and calling glfwSetWindowSizeCallback() is roughly just as readable as setWindowSizeCallback().

Don't center the window after creating it

The desktop environment's window manager will take care of placing new windows on the screen, and will usually take care of ensuring there is minimum overlap with existing windows, and might allow users to configure this behaviour. By forcing the window to be centered, you might cause undesirable results.

Don't make getWindow() public

The whole point of this class is to abstract away the details of GLFW's C API. So don't expose m_Window to users of your class. If a user of this class would need this pointer, it probably means you should add a new member function instead that performs the desired action on the window.

Put class Window inside namespace GLFW

This makes it clear that you are providing a GLFW window, and it avoids conflicts with other libraries that might expose a class with the same name. It will also provide a nice home for any other classes that you might need to add, if your goal is to provide full coverage of GLFW's functions.

Avoid declaring multiple variables on a single line

Instead of:

int m_Width, m_Height;

Just write:

int m_Width;
int m_Height;

It generally is easier to read and to modify. Also, if things belong together, such as:

int m_PosX, m_PosY;

Maybe they should be put into their own class or struct, like:

struct {
    int x;
    int y;
} m_Pos;

Or if you use things like coordinates often, consider using a library that provides you with classes that do this. If this is going to be used for an application using Vulkan or OpenGL, I recommend using the GLM library, which provides a class vec2 that is perfect for 2D coordinates.

Consider alternatives to GLFW

I would seriously recommend that you look to alternatives for GLFW. It has issues. You already encountered one: toggling between fullscreen and windowed mode. Almost every other library just has a function that looks like setFullScreen(Window *w, bool fullscreen), and they take care of everything, including choosing the right monitor, remembering the original size, and so on. I strongly suggest you look at the SDL2 library, which has a better, friendlier API, can do much more than GLFW, and is very well supported on many platforms.

\$\endgroup\$
4
  • \$\begingroup\$ is there a way to not include GLFW files in the window.h file, so that the glfw api doesnt polute the user's code. But if i do that, then in the window.h, GLFWwindow* is unavailable \$\endgroup\$
    – ihsan
    Dec 26, 2022 at 9:08
  • \$\begingroup\$ @ihsan If you really want to avoid that, the PImpl pattern could be used. However, considering that everything in the GLFW library is prefixed with glfw or GLFW, there is little chance that it would conflict with anything in the user code. \$\endgroup\$
    – G. Sliepen
    Dec 26, 2022 at 16:30
  • \$\begingroup\$ is it a good idea to use void* instead of glfwwindow* and cast it later \$\endgroup\$
    – ihsan
    Dec 26, 2022 at 16:33
  • \$\begingroup\$ Casting to void* and back is unsafe as the compiler cannot do its type checking. \$\endgroup\$
    – G. Sliepen
    Dec 26, 2022 at 20:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.