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 GLState
s, 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!
initializeGL()
probably ought to first callQOpenGLWidget::initializeGL()
before executing the stored functions. \$\endgroup\$ – Toby Speight Oct 6 '17 at 7:55