From db8c5fedaa4d7f69e623a64dc69a37ba473b6368 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 28 Jan 2019 08:54:32 +0100 Subject: [PATCH] GL: clean up Framebuffer constructors. The AbstractFramebuffer constructor is now constexpr, but that doesn't mean we can have GL::defaultFramebuffer constexpr -- all member functions are (by design) non-const and we also need to modify its viewport value quite often. So this has to stay as-is. --- src/Magnum/GL/AbstractFramebuffer.h | 19 +++++++++++++++++-- src/Magnum/GL/DefaultFramebuffer.cpp | 5 ----- src/Magnum/GL/DefaultFramebuffer.h | 11 ++++++++++- src/Magnum/GL/Framebuffer.cpp | 2 +- src/Magnum/GL/Framebuffer.h | 24 +++--------------------- 5 files changed, 31 insertions(+), 30 deletions(-) diff --git a/src/Magnum/GL/AbstractFramebuffer.h b/src/Magnum/GL/AbstractFramebuffer.h index 3704fd0c1..bb2355549 100644 --- a/src/Magnum/GL/AbstractFramebuffer.h +++ b/src/Magnum/GL/AbstractFramebuffer.h @@ -707,11 +707,26 @@ class MAGNUM_GL_EXPORT AbstractFramebuffer { #else protected: #endif - explicit AbstractFramebuffer(): _flags{ObjectFlag::DeleteOnDestruction} {} - explicit AbstractFramebuffer(GLuint id, const Range2Di& viewport, ObjectFlags flags) noexcept: _id{id}, _viewport{viewport}, _flags{flags} {} + /* Used by the (constexpr) DefaultFramebuffer constructor and both + the NoCreate and normal constructor of Framebuffer */ + constexpr explicit AbstractFramebuffer(GLuint id, const Range2Di& viewport, ObjectFlags flags) noexcept: _id{id}, _viewport{viewport}, _flags{flags} {} ~AbstractFramebuffer() = default; + AbstractFramebuffer(const AbstractFramebuffer&) = delete; + AbstractFramebuffer(AbstractFramebuffer&& other) noexcept: _id{other._id}, _viewport{other._viewport}, _flags{other._flags} { + other._id = 0; + other._viewport = {}; + } + AbstractFramebuffer& operator=(const AbstractFramebuffer&) = delete; + AbstractFramebuffer& operator=(AbstractFramebuffer&& other) noexcept { + using std::swap; + swap(_id, other._id); + swap(_viewport, other._viewport); + swap(_flags, other._flags); + return *this; + } + void MAGNUM_GL_LOCAL createIfNotAlready(); void MAGNUM_GL_LOCAL bindInternal(FramebufferTarget target); diff --git a/src/Magnum/GL/DefaultFramebuffer.cpp b/src/Magnum/GL/DefaultFramebuffer.cpp index 9c654c8f7..ef0dcfc37 100644 --- a/src/Magnum/GL/DefaultFramebuffer.cpp +++ b/src/Magnum/GL/DefaultFramebuffer.cpp @@ -46,11 +46,6 @@ namespace GL { DefaultFramebuffer defaultFramebuffer; -DefaultFramebuffer::DefaultFramebuffer() { - _id = 0; - _flags |= ObjectFlag::Created; -} - DefaultFramebuffer::Status DefaultFramebuffer::checkStatus(const FramebufferTarget target) { return Status((this->*Context::current().state().framebuffer->checkStatusImplementation)(target)); } diff --git a/src/Magnum/GL/DefaultFramebuffer.h b/src/Magnum/GL/DefaultFramebuffer.h index ba5ed937d..248d3f54c 100644 --- a/src/Magnum/GL/DefaultFramebuffer.h +++ b/src/Magnum/GL/DefaultFramebuffer.h @@ -306,7 +306,13 @@ class MAGNUM_GL_EXPORT DefaultFramebuffer: public AbstractFramebuffer { }; #endif - explicit MAGNUM_GL_LOCAL DefaultFramebuffer(); + /** + * @brief Constructor + * + * Not meant to be constructed on the application side, use the + * @ref GL::defaultFramebuffer instance directly. + */ + constexpr explicit DefaultFramebuffer(): AbstractFramebuffer{0, {}, ObjectFlag::Created|ObjectFlag::DeleteOnDestruction} {} /** @brief Copying is not allowed */ DefaultFramebuffer(const DefaultFramebuffer&) = delete; @@ -504,6 +510,9 @@ class MAGNUM_GL_EXPORT DefaultFramebuffer: public AbstractFramebuffer { }; /** @brief Default framebuffer instance */ +/* Even though the constructor is constexpr, this variable can't -- all + framebuffer APIs are non-const since they modify global GL state and besides + that we also need to modify its private _viewport member quite a lot */ extern DefaultFramebuffer MAGNUM_GL_EXPORT defaultFramebuffer; /** @debugoperatorclassenum{DefaultFramebuffer,DefaultFramebuffer::Status} */ diff --git a/src/Magnum/GL/Framebuffer.cpp b/src/Magnum/GL/Framebuffer.cpp index e34e4f80c..972d5e5b1 100644 --- a/src/Magnum/GL/Framebuffer.cpp +++ b/src/Magnum/GL/Framebuffer.cpp @@ -97,7 +97,7 @@ Int Framebuffer::maxColorAttachments() { return value; } -Framebuffer::Framebuffer(const Range2Di& viewport) { +Framebuffer::Framebuffer(const Range2Di& viewport): AbstractFramebuffer{0, viewport, ObjectFlag::DeleteOnDestruction} { CORRADE_INTERNAL_ASSERT(viewport != Implementation::FramebufferState::DisengagedViewport); _viewport = viewport; (this->*Context::current().state().framebuffer->createImplementation)(); diff --git a/src/Magnum/GL/Framebuffer.h b/src/Magnum/GL/Framebuffer.h index a86ab6267..ffb67e7fb 100644 --- a/src/Magnum/GL/Framebuffer.h +++ b/src/Magnum/GL/Framebuffer.h @@ -361,14 +361,13 @@ class MAGNUM_GL_EXPORT Framebuffer: public AbstractFramebuffer, public AbstractO * destructing) objects even without any OpenGL context being active. * @see @ref Framebuffer(const Range2Di&), @ref wrap() */ - explicit Framebuffer(NoCreateT) noexcept { _id = 0; } + explicit Framebuffer(NoCreateT) noexcept: AbstractFramebuffer{{}, {}, {}} {} /** @brief Copying is not allowed */ Framebuffer(const Framebuffer&) = delete; /** @brief Move constructor */ - /* MinGW complains loudly if the declaration doesn't also have inline */ - inline Framebuffer(Framebuffer&& other) noexcept; + Framebuffer(Framebuffer&&) noexcept = default; /** * @brief Destructor @@ -382,8 +381,7 @@ class MAGNUM_GL_EXPORT Framebuffer: public AbstractFramebuffer, public AbstractO Framebuffer& operator=(const Framebuffer&) = delete; /** @brief Move assignment */ - /* MinGW complains loudly if the declaration doesn't also have inline */ - inline Framebuffer& operator=(Framebuffer&& other) noexcept; + Framebuffer& operator=(Framebuffer&&) noexcept = default; /** @brief OpenGL framebuffer ID */ GLuint id() const { return _id; } @@ -939,22 +937,6 @@ class MAGNUM_GL_EXPORT Framebuffer: public AbstractFramebuffer, public AbstractO /** @debugoperatorclassenum{Framebuffer,Framebuffer::Status} */ MAGNUM_GL_EXPORT Debug& operator<<(Debug& debug, Framebuffer::Status value); -inline Framebuffer::Framebuffer(Framebuffer&& other) noexcept { - _id = other._id; - _viewport = other._viewport; - _flags = other._flags; - other._id = 0; - other._viewport = {}; -} - -inline Framebuffer& Framebuffer::operator=(Framebuffer&& other) noexcept { - using std::swap; - swap(_id, other._id); - swap(_viewport, other._viewport); - swap(_flags, other._flags); - return *this; -} - inline GLuint Framebuffer::release() { const GLuint id = _id; _id = 0;