2
\$\begingroup\$

When I am writing OpenGL applications with the QOpenGLWidget class, deciding what code goes into the initializeGL function has always been tricky. One example is when I have a setter that requires an initialized OpenGL context, I have to make sure the OpenGL context is initialized before calling the setter, which is a huge pain.

The following snippet allows me to put GL functions anywhere in the class without worrying whether the OpenGL context is initialized. This is done by wrapping the GL functions with the delayForInit function. It checks whether the OpenGL context is already initialized. If it is, then run the GL functions immediately. If not, then delay the GL functions until initializeGL is called.

A simple usage example is shown at the bottom.

#ifndef OPENGLWIDGET_H
#define OPENGLWIDGET_H

#include <QOpenGLWidget>
#include <functional>

/**
 * @brief The OpenGLWidget class
 */
class OpenGLWidget : public QOpenGLWidget {
    Q_OBJECT
public:
    explicit OpenGLWidget(QWidget* parent = 0)
      : QOpenGLWidget(parent)
      , _isInitialized(false) {}

protected:
    virtual void initializeGL() override {
        for (auto& initFunc : _initFuncs)
            initFunc();
        _initFuncs.clear();
        _isInitialized = true;
    }
    void delayForInit(std::function<void()> func) {
        if (_isInitialized) {
            makeCurrent();
            func();
            doneCurrent();
        } else
            _initFuncs.push_back(func);
    }

private:
    bool _isInitialized;
    std::vector<std::function<void()>> _initFuncs;
};

#endif // OPENGLWIDGET_H

// usage example
class SomeWidget : public OpenGLWidget {
    Q_OBJECT
public:
    SomeWidget() {
        delayForInit([this]() {
            glClearColor(0.f, 0.f, 0.f, 0.f);
            glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
        });
    }
};
\$\endgroup\$
  • \$\begingroup\$ I'm trying to understand why this problem exists in the first place. I've never been in a position where I wanted to call OpenGL functions before I had created a context and made it current. But it's also not clear to me what a "GLWidget" would be, so I'm probably missing something. Can you elaborate on that? Is that something like a single view or pane in a window that displays OpenGL drawing? \$\endgroup\$ – user1118321 Sep 27 '17 at 5:34
  • \$\begingroup\$ @user1118321 In Qt's terminology, a widget is basic element of a UI. Widgets can be used to compose complex UIs, including windows. For example, the following classes all inherit from QWidget: QPushButton, QListWidget, QTabWidget. A QOpenGLWidget is basically what you said, a view/pane/widget that displays OpenGL drawing. Normally, a QOpenGLWidget maintains a OpenGL context. Function initializeGL is called after the context is initialized, and other OpenGL events are handled accordingly by resizeGL, paintGL, etc. \$\endgroup\$ – Snowfish Sep 27 '17 at 6:32
  • \$\begingroup\$ @user1118321 I would agree that the problem doesn't reveal itself very often. Let's say the class has a setter for the background color, which is effectively calling glClearColor. Usually this isn't a problem but when someone is trying to call this setter in the constructor, which is before the OpenGL context is initialized, it will cause problems. Of course we can use a background color variable to store the background color and only call glClearColor in paintGL, but it's just inconvenient in my opinion. \$\endgroup\$ – Snowfish Sep 27 '17 at 6:42
  • \$\begingroup\$ Not a full review, but the override of initializeGL() probably ought to first call QOpenGLWidget::initializeGL() before executing the stored functions. \$\endgroup\$ – Toby Speight Oct 6 '17 at 7:55
  • \$\begingroup\$ @TobySpeight From the examples provided by Qt's documentation, it doesn't seem to be the case, but I might be wrong. \$\endgroup\$ – Snowfish Oct 6 '17 at 17:43
1
+50
\$\begingroup\$

Small nitpicks:

  • The default value of the parameter QWidget* parent of the constructor is 0. This should be replaced with nullptr.

  • Either OpenGLWidget or SomeWidget would need to inherit from QOpenGLFunctions to get access to the OpenGL functions used in the example.

  • The implementation as-is is absolutely not thread-safe. This might not be a concern if your application is guaranteed to be single threaded, but more often than not, applications using advanced graphics end up using multiple threads.

  • Since the context() member function is publicly accessible, it might be appropriate to make delayForInit adhere to the same level of accessibility.

  • While delayForInit isn't the worst name, it doesn't really convey what it's doing. deferGLCall or withGLContext might be more fitting.

Design

It's simplicity of usage is great. However, you have no way of enforcing that it will be used at all possible call sites (where appropriate).
Another possibility that comes to mind would be to use the State pattern to provide access to the OpenGL functions in form of a member, with 2 states: One that records the OpenGL calls and one that simply forwards them. The call to initializeGL then switches state from recording to forwarding, executing the recorded calls.
Though also not foolproof, but which is harder to forget: "I have to use OpenGL calls in a lambda passed to this function." or "I have to use this member to get access to the OpenGL functions (as they aren't provided directly by default anyways)."?

Example (rough outline, as I currently have no Qt install at hand):

class GLState {
protected:
    QOpenGLWidget& widget;
public:
    explicit GLState(QOpenGLWidget& w) : widget(w) {}
    virtual ~GLState() {}

    // define gl functions as needed
    void glClearColor(GLclampf red, GLclampf green, GLclampf blue, GLclampf alpha) = 0;
    void glClear(GLbitfield mask) = 0;
    // ...
};

class ForwardingGLState : public GLState, public QOpenGLFunctions {
    Q_OBJECT // not sure if needed, but probably is
public:
    ForwardingGLState(QOpenGLWidget& w) : GLState(w), QOpenGLFunctions(w.context()) {}

    // gl calls implemented by QOpenGLFunctions, nothing to do here
};

class RecordingGLState : public GLState {
    std::vector<std::function<void()>> records;
public:
    RecordingGLState(QOpenGLWidget& w) : GLState(w), records() {}

    // either do execution of recorded calls during destructor (might
    // be tricky with exceptions) or implement another virtual function
    // in GLState
    virtual ~RecordingGLState() {
        if(widget.context() == nullptr) return;

        widget.makeCurrent();
        for(auto& record : records) {
            record();
        }
        widget.doneCurrent();
    }

    // record gl calls
    // might be able to simplify lambda implementation
    void glClearColor(GLclampf red, GLclampf green, GLclampf blue, GLclampf alpha) {
        records.push_back([&]() {
            widget.context()->functions()->glClearColor(red, green, blue, alpha);
        });
    }

    void glClear(GLbitfield mask) {
        records.push_back([&]() {
            widget.context()->functions()->glClear(mask);
        });
    }
};

Usage in OpenGLWidget:

class OpenGLWidget : public QOpenGLWidget {
    Q_OBJECT
protected:
    std::unique_ptr<GLState> gl;

public:
    OpenGLWidget(QWidget *parent = nullptr) : QOpenGLWidget(parent), gl(std::make_unique<RecordingGLState>(*this)) {}

protected:
    virtual void initializeGL() override {
        // if not (ab-)using the destructor, execute recorded calls here
        gl = std::make_unique<ForwardingGLState>(*this);
    }
};

class SomeWidget : public OpenGLWidget {
    Q_OBJECT
public:
    SomeWidget(QWidget *parent = nullptr) : OpenGLWidget(parent) {
        gl->glClearColor(0.f, 0.f, 0.f, 0.f);
        gl->glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
    }
};

Yes, it's a lot of boiler plate code inside the GLStates, but after that its mostly done (though some of it could probably be replaced with macros if it gets too bad). Plus side: you can't really get the final usage wrong!

\$\endgroup\$
  • \$\begingroup\$ All the UI operations will be performed on the UI thread, so unless somebody does something weird (and contrary to Qt documentation), how would it be thread-unsafe? I like the idea of using a State pattern - that makes a lot of sense. \$\endgroup\$ – Toby Speight Oct 9 '17 at 8:43
  • \$\begingroup\$ @TobySpeight: Can you guarantee no call to delayForInit will be made while initializeGL is running? Other than that, if I take your word that no concurrent calls to delayForInit will be made, I guess there might be no issues... But in the given example, this isn't obvious, and somebody else trying to do multithreaded stuff using your code base might be still be surprised \$\endgroup\$ – hoffmale Oct 9 '17 at 12:05
  • \$\begingroup\$ @hoffmale I really like the renaming to withGLContext. I am trying to imagine how the implementation of the state pattern would look like. When you say recording/forwarding OpenGL calls, do you mean implementing all OpenGL functions in the two state classes? That sounds like tedious work to me. Would you mind to explain in more detail about the state pattern implementation? \$\endgroup\$ – Snowfish Oct 9 '17 at 21:56
  • \$\begingroup\$ @Snowfish: added a rough outline example of what I think the state pattern implementation could look like (I mostly didn't bother looking for const and noexcept correctness).. \$\endgroup\$ – hoffmale Oct 9 '17 at 22:58
  • \$\begingroup\$ @hoffmale I see and I think using the state pattern is pretty nice. As you said, the only down side is probably the boiler plate codes in RecordingGLState. \$\endgroup\$ – Snowfish Oct 11 '17 at 0:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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