From 42c2032d815309ef94be724abcad2dcddfc46682 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 28 Dec 2013 18:51:31 +0100 Subject: [PATCH] Simplified and tested Texture move constructor/assignment. Move constructor and move assignment is now noexcept and it just swaps the data of the two instances instead of calling GL API, thus it can be inline. Also removed unneded limitations in BufferTexture. --- src/AbstractTexture.cpp | 32 ++++---------------- src/AbstractTexture.h | 16 +++++++--- src/BufferTexture.h | 5 ---- src/Test/AbstractTextureGLTest.cpp | 48 +++++++++++++++++++++++++++++- 4 files changed, 65 insertions(+), 36 deletions(-) diff --git a/src/AbstractTexture.cpp b/src/AbstractTexture.cpp index c97bbc0f0..3d7c33135 100644 --- a/src/AbstractTexture.cpp +++ b/src/AbstractTexture.cpp @@ -124,8 +124,12 @@ Int AbstractTexture::maxIntegerSamples() { } #endif -void AbstractTexture::destroy() { - /* Moved out */ +AbstractTexture::AbstractTexture(GLenum target): _target(target) { + glGenTextures(1, &_id); +} + +AbstractTexture::~AbstractTexture() { + /* Moved out, nothing to do */ if(!_id) return; /* Remove all bindings */ @@ -135,30 +139,6 @@ void AbstractTexture::destroy() { glDeleteTextures(1, &_id); } -void AbstractTexture::move() { - _id = 0; -} - -AbstractTexture::AbstractTexture(GLenum target): _target(target) { - glGenTextures(1, &_id); -} - -AbstractTexture::~AbstractTexture() { destroy(); } - -AbstractTexture::AbstractTexture(AbstractTexture&& other): _target(other._target), _id(other._id) { - other.move(); -} - -AbstractTexture& AbstractTexture::operator=(AbstractTexture&& other) { - destroy(); - - _target = other._target; - _id = other._id; - - other.move(); - return *this; -} - std::string AbstractTexture::label() const { return Context::current()->state().debug->getLabelImplementation(GL_TEXTURE, _id); } diff --git a/src/AbstractTexture.h b/src/AbstractTexture.h index 495aeef00..95b4cd4b0 100644 --- a/src/AbstractTexture.h +++ b/src/AbstractTexture.h @@ -158,13 +158,13 @@ class MAGNUM_EXPORT AbstractTexture: public AbstractObject { AbstractTexture(const AbstractTexture&) = delete; /** @brief Move constructor */ - AbstractTexture(AbstractTexture&& other); + AbstractTexture(AbstractTexture&& other) noexcept; /** @brief Copying is not allowed */ AbstractTexture& operator=(const AbstractTexture&) = delete; /** @brief Move assignment */ - AbstractTexture& operator=(AbstractTexture&& other); + AbstractTexture& operator=(AbstractTexture&& other) noexcept; /** * @brief %Texture label @@ -482,8 +482,6 @@ class MAGNUM_EXPORT AbstractTexture: public AbstractObject { #endif static InvalidateSubImageImplementation invalidateSubImageImplementation; - void MAGNUM_LOCAL destroy(); - void MAGNUM_LOCAL move(); ColorFormat MAGNUM_LOCAL imageFormatForInternalFormat(TextureFormat internalFormat); ColorType MAGNUM_LOCAL imageTypeForInternalFormat(TextureFormat internalFormat); @@ -597,6 +595,16 @@ template<> struct MAGNUM_EXPORT AbstractTexture::DataHelper<3> { }; #endif +inline AbstractTexture::AbstractTexture(AbstractTexture&& other) noexcept: _target(other._target), _id(other._id) { + other._id = 0; +} + +inline AbstractTexture& AbstractTexture::operator=(AbstractTexture&& other) noexcept { + std::swap(_target, other._target); + std::swap(_id, other._id); + return *this; +} + } #endif diff --git a/src/BufferTexture.h b/src/BufferTexture.h index ac9d37286..f53c01e29 100644 --- a/src/BufferTexture.h +++ b/src/BufferTexture.h @@ -201,11 +201,6 @@ and respective function documentation for more information. class MAGNUM_EXPORT BufferTexture: private AbstractTexture { friend class Context; - BufferTexture(const BufferTexture&) = delete; - BufferTexture(BufferTexture&&) = delete; - BufferTexture& operator=(const BufferTexture&) = delete; - BufferTexture& operator=(BufferTexture&&) = delete; - public: /** @copydoc AbstractTexture::maxLabelLength() */ static Int maxLabelLength() { return AbstractTexture::maxLabelLength(); } diff --git a/src/Test/AbstractTextureGLTest.cpp b/src/Test/AbstractTextureGLTest.cpp index fdf8d9539..42865ae14 100644 --- a/src/Test/AbstractTextureGLTest.cpp +++ b/src/Test/AbstractTextureGLTest.cpp @@ -33,11 +33,57 @@ class AbstractTextureGLTest: public AbstractOpenGLTester { public: explicit AbstractTextureGLTest(); + void construct(); + void constructCopy(); + void constructMove(); + void label(); }; AbstractTextureGLTest::AbstractTextureGLTest() { - addTests({&AbstractTextureGLTest::label}); + addTests({&AbstractTextureGLTest::construct, + &AbstractTextureGLTest::constructCopy, + &AbstractTextureGLTest::constructMove, + + &AbstractTextureGLTest::label}); +} + +void AbstractTextureGLTest::construct() { + { + const Texture2D texture; + + MAGNUM_VERIFY_NO_ERROR(); + CORRADE_VERIFY(texture.id() > 0); + } + + MAGNUM_VERIFY_NO_ERROR(); +} + +void AbstractTextureGLTest::constructCopy() { + CORRADE_VERIFY(!(std::is_constructible{})); + CORRADE_VERIFY(!(std::is_assignable{})); +} + +void AbstractTextureGLTest::constructMove() { + Texture2D a; + const Int id = a.id(); + + MAGNUM_VERIFY_NO_ERROR(); + CORRADE_VERIFY(id > 0); + + Texture2D b(std::move(a)); + + CORRADE_COMPARE(a.id(), 0); + CORRADE_COMPARE(b.id(), id); + + Texture2D 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 AbstractTextureGLTest::label() {