Improved BufferImage API, added test.

Move constructor and move assignment now behaves similarly to Image
(not only the buffer but also size is swapped). Added constructor taking
size + data, reordered setData() parameters to match order in Image. The
old setData() function is now alias to the new one, is marked as
deprecated and will be removed in future release.

src/AbstractFramebuffer.cpp

@@ -208,7 +208,7 @@ void AbstractFramebuffer::read(const Vector2i& offset, const Vector2i& size, Buf
     /* If the buffer doesn't have sufficient size, resize it */
     /** @todo Explicitly reset also when buffer usage changes */
     if(image.size() != size)
-        image.setData(size, image.format(), image.type(), nullptr, usage);
+        image.setData(image.format(), image.type(), size, nullptr, usage);
 
     image.buffer().bind(Buffer::Target::PixelPack);
     /** @todo De-duplicate buffer size computation */

src/AbstractTexture.cpp

@@ -999,7 +999,7 @@ template<UnsignedInt dimensions> void AbstractTexture::image(GLenum target, GLin
     const Math::Vector<dimensions, Int> size = DataHelper<dimensions>::imageSize(this, target, level);
     const std::size_t dataSize = size.product()*image.pixelSize();
     if(image.size() != size)
-        image.setData(size, image.format(), image.type(), nullptr, usage);
+        image.setData(image.format(), image.type(), size, nullptr, usage);
 
     image.buffer().bind(Buffer::Target::PixelPack);
     (this->*getImageImplementation)(target, level, image.format(), image.type(), dataSize, nullptr);

src/BufferImage.cpp

@@ -27,7 +27,13 @@
 namespace Magnum {
 
 #ifndef MAGNUM_TARGET_GLES2
-template<UnsignedInt dimensions> void BufferImage<dimensions>::setData(const typename DimensionTraits<Dimensions, Int>::VectorType& size, ColorFormat format, ColorType type, const void* data, BufferUsage usage) {
+template<UnsignedInt dimensions> BufferImage<dimensions>::BufferImage(ColorFormat format, ColorType type, const typename DimensionTraits< Dimensions, Int >::VectorType& size, const void* data, BufferUsage usage): AbstractImage(format, type), _size(size), _buffer(Buffer::Target::PixelPack) {
+    _buffer.setData({data, pixelSize()*size.product()}, usage);
+}
+
+template<UnsignedInt dimensions> BufferImage<dimensions>::BufferImage(ColorFormat format, ColorType type): AbstractImage(format, type), _buffer(Buffer::Target::PixelPack) {}
+
+template<UnsignedInt dimensions> void BufferImage<dimensions>::setData(ColorFormat format, ColorType type, const typename DimensionTraits<Dimensions, Int>::VectorType& size, const void* data, BufferUsage usage) {
     _format = format;
     _type = type;
     _size = size;

src/BufferImage.h

@@ -54,13 +54,36 @@ template<UnsignedInt dimensions> class MAGNUM_EXPORT BufferImage: public Abstrac
          * @brief Constructor
          * @param format            Format of pixel data
          * @param type              Data type of pixel data
+         * @param size              %Image size
+         * @param data              %Image data
+         * @param usage             %Image buffer usage
          *
-         * Dimensions and buffer are empty, call @ref setData() to fill the
+         * Note that the image data are not copied on construction, but they
+         * are deleted on class destruction.
+         */
+        explicit BufferImage(ColorFormat format, ColorType type, const typename DimensionTraits<Dimensions, Int>::VectorType& size, const void* data, BufferUsage usage);
+
+        /**
+         * @brief Constructor
+         * @param format            Format of pixel data
+         * @param type              Data type of pixel data
+         *
+         * Size is zero and buffer are empty, call @ref setData() to fill the
          * image with data.
          */
-        explicit BufferImage(ColorFormat format, ColorType type): AbstractImage(format, type) {
+        explicit BufferImage(ColorFormat format, ColorType type);
-            _buffer.setTargetHint(Buffer::Target::PixelPack);
+
-        }
+        /** @brief Copying is not allowed */
+        BufferImage(const BufferImage<dimensions>&) = delete;
+
+        /** @brief Move constructor */
+        BufferImage(BufferImage<dimensions>&& other) noexcept;
+
+        /** @brief Copying is not allowed */
+        BufferImage<dimensions>& operator=(const BufferImage<dimensions>&) = delete;
+
+        /** @brief Move assignment */
+        BufferImage<dimensions>& operator=(BufferImage<dimensions>&& other) noexcept;
 
         /** @brief %Image size */
         typename DimensionTraits<Dimensions, Int>::VectorType size() const { return _size; }
@@ -70,9 +93,9 @@ template<UnsignedInt dimensions> class MAGNUM_EXPORT BufferImage: public Abstrac
 
         /**
          * @brief Set image data
-         * @param size              %Image size
          * @param format            Format of pixel data
          * @param type              Data type of pixel data
+         * @param size              %Image size
          * @param data              %Image data
          * @param usage             %Image buffer usage
          *
@@ -80,7 +103,17 @@ template<UnsignedInt dimensions> class MAGNUM_EXPORT BufferImage: public Abstrac
          * after filling the buffer.
          * @see @ref Buffer::setData()
          */
-        void setData(const typename DimensionTraits<Dimensions, Int>::VectorType& size, ColorFormat format, ColorType type, const void* data, BufferUsage usage);
+        void setData(ColorFormat format, ColorType type, const typename DimensionTraits<Dimensions, Int>::VectorType& size, const void* data, BufferUsage usage);
+
+        #ifdef MAGNUM_BUILD_DEPRECATED
+        /**
+         * @copybrief setData(ColorFormat, ColorType, const typename DimensionTraits<Dimensions, Int>::VectorType&, const void*, BufferUsage)
+         * @deprecated Use @ref Magnum::BufferImage::setData(ColorFormat, ColorType, const typename DimensionTraits<Dimensions, Int>::VectorType&, const void*, BufferUsage) "setData(ColorFormat, ColorType, const typename DimensionTraits<Dimensions, Int>::VectorType&, const void*, BufferUsage)" instead.
+         */
+        CORRADE_DEPRECATED("use setData(ColorFormat, ColorType, VectorNi, const void*, BufferUsage) instead") void setData(const typename DimensionTraits<Dimensions, Int>::VectorType& size, ColorFormat format, ColorType type, const void* data, BufferUsage usage) {
+            setData(format, type, size, data, usage);
+        }
+        #endif
 
     private:
         Math::Vector<Dimensions, Int> _size;
@@ -95,6 +128,17 @@ typedef BufferImage<2> BufferImage2D;
 
 /** @brief Three-dimensional buffer image */
 typedef BufferImage<3> BufferImage3D;
+
+template<UnsignedInt dimensions> inline BufferImage<dimensions>::BufferImage(BufferImage<dimensions>&& other) noexcept: AbstractImage(std::move(other)), _size(other._size), _buffer(std::move(other._buffer)) {
+    other._size = {};
+}
+
+template<UnsignedInt dimensions> inline BufferImage<dimensions>& BufferImage<dimensions>::operator=(BufferImage<dimensions>&& other) noexcept {
+    AbstractImage::operator=(std::move(other));
+    std::swap(_size, other._size);
+    std::swap(_buffer, other._buffer);
+    return *this;
+}
 #endif
 
 }

src/Test/BufferImageGLTest.cpp

@@ -0,0 +1,127 @@
+/*
+    This file is part of Magnum.
+
+    Copyright © 2010, 2011, 2012, 2013 Vladimír Vondruš <mosra@centrum.cz>
+
+    Permission is hereby granted, free of charge, to any person obtaining a
+    copy of this software and associated documentation files (the "Software"),
+    to deal in the Software without restriction, including without limitation
+    the rights to use, copy, modify, merge, publish, distribute, sublicense,
+    and/or sell copies of the Software, and to permit persons to whom the
+    Software is furnished to do so, subject to the following conditions:
+
+    The above copyright notice and this permission notice shall be included
+    in all copies or substantial portions of the Software.
+
+    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+    THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+    FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+    DEALINGS IN THE SOFTWARE.
+*/
+
+#include <TestSuite/Compare/Container.h>
+
+#include "BufferImage.h"
+#include "ColorFormat.h"
+#include "Test/AbstractOpenGLTester.h"
+
+namespace Magnum { namespace Test {
+
+class BufferImageTest: public AbstractOpenGLTester {
+    public:
+        explicit BufferImageTest();
+
+        void construct();
+        void constructCopy();
+        void constructMove();
+
+        void setData();
+};
+
+BufferImageTest::BufferImageTest() {
+    addTests({&BufferImageTest::construct,
+              &BufferImageTest::constructCopy,
+              &BufferImageTest::constructMove,
+
+              &BufferImageTest::setData});
+}
+
+void BufferImageTest::construct() {
+    const unsigned char data[] = { 'a', 'b', 'c' };
+    BufferImage2D a(ColorFormat::Red, ColorType::UnsignedByte, {1, 3}, data, BufferUsage::StaticDraw);
+    const auto imageData = a.buffer().data<UnsignedByte>();
+
+    MAGNUM_VERIFY_NO_ERROR();
+
+    CORRADE_COMPARE(a.format(), ColorFormat::Red);
+    CORRADE_COMPARE(a.type(), ColorType::UnsignedByte);
+    CORRADE_COMPARE(a.size(), Vector2i(1, 3));
+    CORRADE_COMPARE_AS(std::vector<UnsignedByte>(imageData.begin(), imageData.end()),
+                       std::vector<UnsignedByte>(data, data + 3),
+                       TestSuite::Compare::Container);
+}
+
+void BufferImageTest::constructCopy() {
+     CORRADE_VERIFY(!(std::is_constructible<BufferImage2D, const BufferImage2D&>{}));
+     CORRADE_VERIFY(!(std::is_assignable<BufferImage2D, const BufferImage2D&>{}));
+}
+
+void BufferImageTest::constructMove() {
+    const unsigned char data[3] = { 'a', 'b', 'c' };
+    BufferImage2D a(ColorFormat::Red, ColorType::UnsignedByte, {1, 3}, data, BufferUsage::StaticDraw);
+    const Int id = a.buffer().id();
+
+    MAGNUM_VERIFY_NO_ERROR();
+    CORRADE_VERIFY(id > 0);
+
+    BufferImage2D b(std::move(a));
+
+    CORRADE_COMPARE(a.buffer().id(), 0);
+    CORRADE_COMPARE(a.size(), Vector2i());
+
+    CORRADE_COMPARE(b.format(), ColorFormat::Red);
+    CORRADE_COMPARE(b.type(), ColorType::UnsignedByte);
+    CORRADE_COMPARE(b.size(), Vector2i(1, 3));
+    CORRADE_COMPARE(b.buffer().id(), id);
+
+    const unsigned short data2[2*4] = { 1, 2, 3, 4, 5, 6, 7, 8 };
+    BufferImage2D c(ColorFormat::RGBA, ColorType::UnsignedShort, {2, 1}, data2, BufferUsage::StaticDraw);
+    const Int cId = c.buffer().id();
+    c = std::move(b);
+
+    MAGNUM_VERIFY_NO_ERROR();
+
+    CORRADE_VERIFY(cId > 0);
+    CORRADE_COMPARE(b.buffer().id(), cId);
+    CORRADE_COMPARE(b.size(), Vector2i(2, 1));
+
+    CORRADE_COMPARE(c.format(), ColorFormat::Red);
+    CORRADE_COMPARE(c.type(), ColorType::UnsignedByte);
+    CORRADE_COMPARE(c.size(), Vector2i(1, 3));
+    CORRADE_COMPARE(c.buffer().id(), id);
+}
+
+void BufferImageTest::setData() {
+    const unsigned char data[3] = { 'a', 'b', 'c' };
+    BufferImage2D a(ColorFormat::Red, ColorType::UnsignedByte, {1, 3}, data, BufferUsage::StaticDraw);
+
+    const unsigned short data2[2*4] = { 1, 2, 3, 4, 5, 6, 7, 8 };
+    a.setData(ColorFormat::RGBA, ColorType::UnsignedShort, {2, 1}, data2, BufferUsage::StaticDraw);
+    const auto imageData = a.buffer().data<UnsignedShort>();
+
+    MAGNUM_VERIFY_NO_ERROR();
+
+    CORRADE_COMPARE(a.format(), ColorFormat::RGBA);
+    CORRADE_COMPARE(a.type(), ColorType::UnsignedShort);
+    CORRADE_COMPARE(a.size(), Vector2i(2, 1));
+    CORRADE_COMPARE_AS(std::vector<UnsignedShort>(imageData.begin(), imageData.end()),
+                       std::vector<UnsignedShort>(data2, data2 + 8),
+                       TestSuite::Compare::Container);
+}
+
+}}
+
+CORRADE_TEST_MAIN(Magnum::Test::BufferImageTest)

src/Test/CMakeLists.txt

@@ -48,6 +48,10 @@ if(BUILD_GL_TESTS)
     corrade_add_test(MeshGLTest MeshGLTest.cpp LIBRARIES ${GL_TEST_LIBRARIES})
     corrade_add_test(RenderbufferGLTest RenderbufferGLTest.cpp LIBRARIES ${GL_TEST_LIBRARIES})
     corrade_add_test(ShaderGLTest ShaderGLTest.cpp LIBRARIES ${GL_TEST_LIBRARIES})
+
+    if(NOT MAGNUM_TARGET_GLES2)
+        corrade_add_test(BufferImageGLTest BufferImageGLTest.cpp LIBRARIES ${GL_TEST_LIBRARIES})
+    endif()
 endif()
 
 set_target_properties(ResourceManagerTest PROPERTIES COMPILE_FLAGS -DCORRADE_GRACEFUL_ASSERT)