From 3fd165e9bbb700f8b047ba06d5c6c15eac2d084c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 28 Dec 2013 19:04:01 +0100 Subject: [PATCH] Added move constructor/assignment to Framebuffer classes. Also updated the tests. --- src/AbstractFramebuffer.h | 5 ---- src/DefaultFramebuffer.h | 12 ++++++++ src/Framebuffer.cpp | 3 ++ src/Framebuffer.h | 25 ++++++++++++++++ src/Renderbuffer.cpp | 5 ++++ src/Renderbuffer.h | 28 ++++++++++++++---- src/Test/FramebufferGLTest.cpp | 51 ++++++++++++++++++++++++++++++++- src/Test/RenderbufferGLTest.cpp | 48 ++++++++++++++++++++++++++++++- 8 files changed, 164 insertions(+), 13 deletions(-) diff --git a/src/AbstractFramebuffer.h b/src/AbstractFramebuffer.h index a82058fe4..886da9d2b 100644 --- a/src/AbstractFramebuffer.h +++ b/src/AbstractFramebuffer.h @@ -145,11 +145,6 @@ from buffer overflow. class MAGNUM_EXPORT AbstractFramebuffer { friend class Context; - AbstractFramebuffer(const AbstractFramebuffer&) = delete; - AbstractFramebuffer(AbstractFramebuffer&&) = delete; - AbstractFramebuffer& operator=(const AbstractFramebuffer&) = delete; - AbstractFramebuffer& operator=(AbstractFramebuffer&&) = delete; - public: /** @todo `GL_IMPLEMENTATION_COLOR_READ_FORMAT`, `GL_IMPLEMENTATION_COLOR_READ_TYPE`, seems to be depending on currently bound FB (aargh) (@extension{ARB,ES2_compatibility}). */ diff --git a/src/DefaultFramebuffer.h b/src/DefaultFramebuffer.h index 245fc4998..1c902b408 100644 --- a/src/DefaultFramebuffer.h +++ b/src/DefaultFramebuffer.h @@ -291,6 +291,18 @@ class MAGNUM_EXPORT DefaultFramebuffer: public AbstractFramebuffer { explicit MAGNUM_LOCAL DefaultFramebuffer(); + /** @brief Copying is not allowed */ + DefaultFramebuffer(const DefaultFramebuffer&) = delete; + + /** @brief Moving is not allowed */ + DefaultFramebuffer(DefaultFramebuffer&&) = delete; + + /** @brief Copying is not allowed */ + DefaultFramebuffer& operator=(const DefaultFramebuffer&) = delete; + + /** @brief Moving is not allowed */ + DefaultFramebuffer& operator=(DefaultFramebuffer&& other) = delete; + /** * @brief Check framebuffer status * @param target Target for which to check the status diff --git a/src/Framebuffer.cpp b/src/Framebuffer.cpp index 79942aad7..2c733127b 100644 --- a/src/Framebuffer.cpp +++ b/src/Framebuffer.cpp @@ -79,6 +79,9 @@ Framebuffer::Framebuffer(const Range2Di& viewport) { } Framebuffer::~Framebuffer() { + /* Moved out, nothing to do */ + if(!_id) return; + /* If bound, remove itself from state */ Implementation::FramebufferState* state = Context::current()->state().framebuffer; if(state->readBinding == _id) state->readBinding = 0; diff --git a/src/Framebuffer.h b/src/Framebuffer.h index 97ce8058f..49a8c34ec 100644 --- a/src/Framebuffer.h +++ b/src/Framebuffer.h @@ -296,6 +296,12 @@ class MAGNUM_EXPORT Framebuffer: public AbstractFramebuffer, public AbstractObje */ explicit Framebuffer(const Range2Di& viewport); + /** @brief Copying is not allowed */ + Framebuffer(const Framebuffer&) = delete; + + /** @brief Move constructor */ + Framebuffer(Framebuffer&& other) noexcept; + /** * @brief Destructor * @@ -304,6 +310,12 @@ class MAGNUM_EXPORT Framebuffer: public AbstractFramebuffer, public AbstractObje */ ~Framebuffer(); + /** @brief Copying is not allowed */ + Framebuffer& operator=(const Framebuffer&) = delete; + + /** @brief Move assignment */ + Framebuffer& operator=(Framebuffer&& other) noexcept; + /** @brief OpenGL framebuffer ID */ GLuint id() const { return _id; } @@ -577,6 +589,19 @@ class MAGNUM_EXPORT Framebuffer: public AbstractFramebuffer, public AbstractObje /** @debugoperator{DefaultFramebuffer} */ Debug MAGNUM_EXPORT operator<<(Debug debug, Framebuffer::Status value); +inline Framebuffer::Framebuffer(Framebuffer&& other) noexcept { + _id = other._id; + _viewport = other._viewport; + other._id = 0; + other._viewport = {}; +} + +inline Framebuffer& Framebuffer::operator=(Framebuffer&& other) noexcept { + std::swap(_id, other._id); + std::swap(_viewport, other._viewport); + return *this; +} + } #endif diff --git a/src/Renderbuffer.cpp b/src/Renderbuffer.cpp index 10900c100..03fd45c74 100644 --- a/src/Renderbuffer.cpp +++ b/src/Renderbuffer.cpp @@ -71,7 +71,12 @@ Int Renderbuffer::maxSamples() { return value; } +Renderbuffer::Renderbuffer() { glGenRenderbuffers(1, &_id); } + Renderbuffer::~Renderbuffer() { + /* Moved out, nothing to do */ + if(!_id) return; + /* If bound, remove itself from state */ GLuint& binding = Context::current()->state().framebuffer->renderbufferBinding; if(binding == _id) binding = 0; diff --git a/src/Renderbuffer.h b/src/Renderbuffer.h index a97ed7c91..c616c2dc6 100644 --- a/src/Renderbuffer.h +++ b/src/Renderbuffer.h @@ -56,11 +56,6 @@ See its documentation for more information. class MAGNUM_EXPORT Renderbuffer: public AbstractObject { friend class Context; - Renderbuffer(const Renderbuffer&) = delete; - Renderbuffer(Renderbuffer&&) = delete; - Renderbuffer& operator=(const Renderbuffer&) = delete; - Renderbuffer& operator=(Renderbuffer&&) = delete; - public: /** * @brief Max supported renderbuffer size @@ -89,7 +84,13 @@ class MAGNUM_EXPORT Renderbuffer: public AbstractObject { * Generates new OpenGL renderbuffer. * @see @fn_gl{GenRenderbuffers} */ - explicit Renderbuffer() { glGenRenderbuffers(1, &_id); } + explicit Renderbuffer(); + + /** @brief Copying is not allowed */ + Renderbuffer(const Renderbuffer&) = delete; + + /** @brief Move constructor */ + Renderbuffer(Renderbuffer&& other) noexcept; /** * @brief Destructor @@ -99,6 +100,12 @@ class MAGNUM_EXPORT Renderbuffer: public AbstractObject { */ ~Renderbuffer(); + /** @brief Copying is not allowed */ + Renderbuffer& operator=(const Renderbuffer&) = delete; + + /** @brief Move assignment */ + Renderbuffer& operator=(Renderbuffer&& other) noexcept; + /** @brief OpenGL internal renderbuffer ID */ GLuint id() const { return _id; } @@ -189,6 +196,15 @@ class MAGNUM_EXPORT Renderbuffer: public AbstractObject { GLuint _id; }; +inline Renderbuffer::Renderbuffer(Renderbuffer&& other) noexcept: _id(other._id) { + other._id = 0; +} + +inline Renderbuffer& Renderbuffer::operator=(Renderbuffer&& other) noexcept { + std::swap(_id, other._id); + return *this; +} + } #endif diff --git a/src/Test/FramebufferGLTest.cpp b/src/Test/FramebufferGLTest.cpp index 3b3dd7b9e..298d1d684 100644 --- a/src/Test/FramebufferGLTest.cpp +++ b/src/Test/FramebufferGLTest.cpp @@ -33,11 +33,60 @@ class FramebufferGLTest: public AbstractOpenGLTester { public: explicit FramebufferGLTest(); + void construct(); + void constructCopy(); + void constructMove(); + void label(); }; FramebufferGLTest::FramebufferGLTest() { - addTests({&FramebufferGLTest::label}); + addTests({&FramebufferGLTest::construct, + &FramebufferGLTest::constructCopy, + &FramebufferGLTest::constructMove, + + &FramebufferGLTest::label}); +} + +void FramebufferGLTest::construct() { + { + const Framebuffer framebuffer({{32, 16}, {128, 256}}); + + MAGNUM_VERIFY_NO_ERROR(); + CORRADE_VERIFY(framebuffer.id() > 0); + CORRADE_COMPARE(framebuffer.viewport(), Range2Di({32, 16}, {128, 256})); + } + + MAGNUM_VERIFY_NO_ERROR(); +} + +void FramebufferGLTest::constructCopy() { + CORRADE_VERIFY(!(std::is_constructible{})); + CORRADE_VERIFY(!(std::is_assignable{})); +} + +void FramebufferGLTest::constructMove() { + Framebuffer a({{32, 16}, {128, 256}}); + const Int id = a.id(); + + MAGNUM_VERIFY_NO_ERROR(); + CORRADE_VERIFY(id > 0); + + Framebuffer b(std::move(a)); + + CORRADE_COMPARE(a.id(), 0); + CORRADE_COMPARE(b.id(), id); + CORRADE_COMPARE(b.viewport(), Range2Di({32, 16}, {128, 256})); + + Framebuffer c({{128, 256}, {32, 16}}); + const Int cId = c.id(); + c = std::move(b); + + MAGNUM_VERIFY_NO_ERROR(); + CORRADE_VERIFY(cId > 0); + CORRADE_COMPARE(b.id(), cId); + CORRADE_COMPARE(c.id(), id); + CORRADE_COMPARE(c.viewport(), Range2Di({32, 16}, {128, 256})); } void FramebufferGLTest::label() { diff --git a/src/Test/RenderbufferGLTest.cpp b/src/Test/RenderbufferGLTest.cpp index f6cd2862c..4f44ac41f 100644 --- a/src/Test/RenderbufferGLTest.cpp +++ b/src/Test/RenderbufferGLTest.cpp @@ -33,11 +33,57 @@ class RenderbufferGLTest: public AbstractOpenGLTester { public: explicit RenderbufferGLTest(); + void construct(); + void constructCopy(); + void constructMove(); + void label(); }; RenderbufferGLTest::RenderbufferGLTest() { - addTests({&RenderbufferGLTest::label}); + addTests({&RenderbufferGLTest::construct, + &RenderbufferGLTest::constructCopy, + &RenderbufferGLTest::constructMove, + + &RenderbufferGLTest::label}); +} + +void RenderbufferGLTest::construct() { + { + const Renderbuffer renderbuffer; + + MAGNUM_VERIFY_NO_ERROR(); + CORRADE_VERIFY(renderbuffer.id() > 0); + } + + MAGNUM_VERIFY_NO_ERROR(); +} + +void RenderbufferGLTest::constructCopy() { + CORRADE_VERIFY(!(std::is_constructible{})); + CORRADE_VERIFY(!(std::is_assignable{})); +} + +void RenderbufferGLTest::constructMove() { + Renderbuffer a; + const Int id = a.id(); + + MAGNUM_VERIFY_NO_ERROR(); + CORRADE_VERIFY(id > 0); + + Renderbuffer b(std::move(a)); + + CORRADE_COMPARE(a.id(), 0); + CORRADE_COMPARE(b.id(), id); + + Renderbuffer c; + const Int cId = c.id(); + c = std::move(b); + + MAGNUM_VERIFY_NO_ERROR(); + CORRADE_VERIFY(cId > 0); + CORRADE_COMPARE(b.id(), cId); + CORRADE_COMPARE(c.id(), id); } void RenderbufferGLTest::label() {