From 14c235f8ee29e46d5043daf594b88201c8cd7052 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Fri, 13 Apr 2018 16:18:54 +0200 Subject: [PATCH] Deprecated {Compressed,}Image::setData() functions. They just mirror what the constructor already does. The classes are also movable, so why not just move a new instance over. --- doc/changelog.dox | 3 +++ src/Magnum/AbstractFramebuffer.cpp | 2 +- src/Magnum/AbstractTexture.cpp | 8 +++--- src/Magnum/CubeMapTexture.cpp | 8 +++--- src/Magnum/DebugTools/TextureImage.cpp | 4 +-- src/Magnum/Image.cpp | 23 ---------------- src/Magnum/Image.h | 32 +++++++++++++++------- src/Magnum/Test/ImageTest.cpp | 37 -------------------------- 8 files changed, 37 insertions(+), 80 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 60f503eed..df7790140 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -118,6 +118,9 @@ See also: @subsection changelog-latest-deprecated Deprecated APIs +- `setData()` functions in the @ref Image and @ref CompressedImage classes + are deprecated because they don't offer anything extra over simple + move-assignment of a new instance. - Class @cpp Primitives::Capsule2D @ce and @cpp Primitives::Capsule3D @ce is deprecated, use @ref Primitives::capsule2DWireframe(), @ref Primitives::capsule3DSolid() and @ref Primitives::capsule3DWireframe() diff --git a/src/Magnum/AbstractFramebuffer.cpp b/src/Magnum/AbstractFramebuffer.cpp index e848bed47..fa08cec79 100644 --- a/src/Magnum/AbstractFramebuffer.cpp +++ b/src/Magnum/AbstractFramebuffer.cpp @@ -313,7 +313,7 @@ void AbstractFramebuffer::read(const Range2Di& rectangle, Image2D& image) { + Implementation::pixelStorageSkipOffsetFor(image, rectangle.size()) #endif ); - image.setData(image.storage(), image.format(), image.type(), rectangle.size(), std::move(data)); + image = Image2D{image.storage(), image.format(), image.type(), rectangle.size(), std::move(data)}; } Image2D AbstractFramebuffer::read(const Range2Di& rectangle, Image2D&& image) { diff --git a/src/Magnum/AbstractTexture.cpp b/src/Magnum/AbstractTexture.cpp index f53996d05..d774406c1 100644 --- a/src/Magnum/AbstractTexture.cpp +++ b/src/Magnum/AbstractTexture.cpp @@ -1702,7 +1702,7 @@ template void AbstractTexture::image(GLint level, Image< Buffer::unbindInternal(Buffer::TargetHint::PixelPack); image.storage().applyPack(); (this->*Context::current().state().texture->getImageImplementation)(level, image.format(), image.type(), data.size(), data); - image.setData(image.storage(), image.format(), image.type(), size, std::move(data)); + image = Image{image.storage(), image.format(), image.type(), size, std::move(data)}; } template void MAGNUM_EXPORT AbstractTexture::image<1>(GLint, Image<1>&); @@ -1752,7 +1752,7 @@ template void AbstractTexture::compressedImage(const GLi Buffer::unbindInternal(Buffer::TargetHint::PixelPack); image.storage().applyPack(); (this->*Context::current().state().texture->getCompressedImageImplementation)(level, data.size(), data); - image.setData(image.storage(), CompressedPixelFormat(format), size, std::move(data)); + image = CompressedImage{image.storage(), CompressedPixelFormat(format), size, std::move(data)}; } template void MAGNUM_EXPORT AbstractTexture::compressedImage<1>(GLint, CompressedImage<1>&); @@ -1806,7 +1806,7 @@ template void AbstractTexture::subImage(const GLint leve Buffer::unbindInternal(Buffer::TargetHint::PixelPack); image.storage().applyPack(); glGetTextureSubImage(_id, level, paddedOffset.x(), paddedOffset.y(), paddedOffset.z(), paddedSize.x(), paddedSize.y(), paddedSize.z(), GLenum(image.format()), GLenum(image.type()), data.size(), data); - image.setData(image.storage(), image.format(), image.type(), size, std::move(data)); + image = Image{image.storage(), image.format(), image.type(), size, std::move(data)}; } template void MAGNUM_EXPORT AbstractTexture::subImage<1>(GLint, const Range1Di&, Image<1>&); @@ -1871,7 +1871,7 @@ template void AbstractTexture::compressedSubImage(const Buffer::unbindInternal(Buffer::TargetHint::PixelPack); image.storage().applyPack(); glGetCompressedTextureSubImage(_id, level, paddedOffset.x(), paddedOffset.y(), paddedOffset.z(), paddedSize.x(), paddedSize.y(), paddedSize.z(), data.size(), data); - image.setData(CompressedPixelFormat(format), size, std::move(data)); + image = CompressedImage{CompressedPixelFormat(format), size, std::move(data)}; } template void MAGNUM_EXPORT AbstractTexture::compressedSubImage<1>(GLint, const Range1Di&, CompressedImage<1>&); diff --git a/src/Magnum/CubeMapTexture.cpp b/src/Magnum/CubeMapTexture.cpp index c6587af25..340626869 100644 --- a/src/Magnum/CubeMapTexture.cpp +++ b/src/Magnum/CubeMapTexture.cpp @@ -75,7 +75,7 @@ void CubeMapTexture::image(const Int level, Image3D& image) { Buffer::unbindInternal(Buffer::TargetHint::PixelPack); image.storage().applyPack(); glGetTextureImage(_id, level, GLenum(image.format()), GLenum(image.type()), data.size(), data); - image.setData(image.storage(), image.format(), image.type(), size, std::move(data)); + image = Image3D{image.storage(), image.format(), image.type(), size, std::move(data)}; } Image3D CubeMapTexture::image(const Int level, Image3D&& image) { @@ -130,7 +130,7 @@ void CubeMapTexture::compressedImage(const Int level, CompressedImage3D& image) Buffer::unbindInternal(Buffer::TargetHint::PixelPack); image.storage().applyPack(); (this->*Context::current().state().texture->getFullCompressedCubeImageImplementation)(level, size.xy(), dataOffset, dataSize, data); - image.setData(image.storage(), CompressedPixelFormat(format), size, std::move(data)); + image = CompressedImage3D{image.storage(), CompressedPixelFormat(format), size, std::move(data)}; } CompressedImage3D CubeMapTexture::compressedImage(const Int level, CompressedImage3D&& image) { @@ -183,7 +183,7 @@ void CubeMapTexture::image(const CubeMapCoordinate coordinate, const Int level, Buffer::unbindInternal(Buffer::TargetHint::PixelPack); image.storage().applyPack(); (this->*Context::current().state().texture->getCubeImageImplementation)(coordinate, level, size, image.format(), image.type(), data.size(), data); - image.setData(image.storage(), image.format(), image.type(), size, std::move(data)); + image = Image2D{image.storage(), image.format(), image.type(), size, std::move(data)}; } Image2D CubeMapTexture::image(const CubeMapCoordinate coordinate, const Int level, Image2D&& image) { @@ -234,7 +234,7 @@ void CubeMapTexture::compressedImage(const CubeMapCoordinate coordinate, const I Buffer::unbindInternal(Buffer::TargetHint::PixelPack); image.storage().applyPack(); (this->*Context::current().state().texture->getCompressedCubeImageImplementation)(coordinate, level, size, data.size(), data); - image.setData(image.storage(), CompressedPixelFormat(format), size, std::move(data)); + image = CompressedImage2D{image.storage(), CompressedPixelFormat(format), size, std::move(data)}; } CompressedImage2D CubeMapTexture::compressedImage(const CubeMapCoordinate coordinate, const Int level, CompressedImage2D&& image) { diff --git a/src/Magnum/DebugTools/TextureImage.cpp b/src/Magnum/DebugTools/TextureImage.cpp index f54d070d8..176b662fe 100644 --- a/src/Magnum/DebugTools/TextureImage.cpp +++ b/src/Magnum/DebugTools/TextureImage.cpp @@ -149,7 +149,7 @@ void textureSubImage(Texture2D& texture, const Int level, const Range2Di& range, /* release() needs to be called after querying the size to avoid zeroing it out */ { Vector2i imageSize = image.size(); - image.setData(image.storage(), reinterpretFormat, PixelType::UnsignedInt, imageSize, image.release()); + image = Image2D{image.storage(), reinterpretFormat, PixelType::UnsignedInt, imageSize, image.release()}; } fb.read(range, image); @@ -157,7 +157,7 @@ void textureSubImage(Texture2D& texture, const Int level, const Range2Di& range, /* release() needs to be called after querying the size to avoid zeroing it out */ { Vector2i imageSize = image.size(); - image.setData(image.storage(), imageFormat, PixelType::Float, imageSize, image.release()); + image = Image2D{image.storage(), imageFormat, PixelType::Float, imageSize, image.release()}; } return; } diff --git a/src/Magnum/Image.cpp b/src/Magnum/Image.cpp index e226c0fc7..92fb1a589 100644 --- a/src/Magnum/Image.cpp +++ b/src/Magnum/Image.cpp @@ -31,29 +31,6 @@ template Image::Image(PixelStorage storage, CORRADE_ASSERT(Implementation::imageDataSize(*this) <= _data.size(), "Image::Image(): bad image data size, got" << _data.size() << "but expected at least" << Implementation::imageDataSize(*this), ); } -template void Image::setData(PixelStorage storage, PixelFormat format, PixelType type, const VectorTypeFor& size, Containers::Array&& data) { - _storage = storage; - _format = format; - _type = type; - _size = size; - CORRADE_ASSERT(Implementation::imageDataSize(*this) <= data.size(), "Image::setData(): bad image data size, got" << data.size() << "but expected at least" << Implementation::imageDataSize(*this), ); - _data = std::move(data); -} - -template void CompressedImage::setData( - #ifndef MAGNUM_TARGET_GLES - CompressedPixelStorage storage, - #endif - CompressedPixelFormat format, const VectorTypeFor& size, Containers::Array&& data) -{ - #ifndef MAGNUM_TARGET_GLES - _storage = storage; - #endif - _format = format; - _size = size; - _data = std::move(data); -} - #ifndef DOXYGEN_GENERATING_OUTPUT template class MAGNUM_EXPORT Image<1>; template class MAGNUM_EXPORT Image<2>; diff --git a/src/Magnum/Image.h b/src/Magnum/Image.h index 6e88f7ee6..a5cc99713 100644 --- a/src/Magnum/Image.h +++ b/src/Magnum/Image.h @@ -165,6 +165,7 @@ template class Image { return reinterpret_cast(_data.data()); } + #ifdef MAGNUM_BUILD_DEPRECATED /** * @brief Set image data * @param storage Storage of pixel data @@ -173,18 +174,25 @@ template class Image { * @param size Image size * @param data Image data * + * @deprecated Move-assign a new instance instead. + * * Deletes previous data and replaces them with new. The data are * expected to be of proper size for given @p storage parameters. * @see @ref release() */ - void setData(PixelStorage storage, PixelFormat format, PixelType type, const VectorTypeFor& size, Containers::Array&& data); + CORRADE_DEPRECATED("move-assign a new instance instead") void setData(PixelStorage storage, PixelFormat format, PixelType type, const VectorTypeFor& size, Containers::Array&& data) { + *this = Image{storage, format, type, size, std::move(data)}; + } /** @overload * Similar to the above, but uses default @ref PixelStorage parameters. + * + * @deprecated Move-assign a new instance instead. */ - void setData(PixelFormat format, PixelType type, const VectorTypeFor& size, Containers::Array&& data) { - setData({}, format, type, size, std::move(data)); + CORRADE_DEPRECATED("move-assign a new instance instead") void setData(PixelFormat format, PixelType type, const VectorTypeFor& size, Containers::Array&& data) { + *this = Image{format, type, size, std::move(data)}; } + #endif /** * @brief Release data storage @@ -346,6 +354,7 @@ template class CompressedImage { return reinterpret_cast(_data.data()); } + #ifdef MAGNUM_BUILD_DEPRECATED #ifndef MAGNUM_TARGET_GLES /** * @brief Set image data @@ -354,6 +363,8 @@ template class CompressedImage { * @param size Image size * @param data Image data * + * @deprecated Move-assign a new instance instead. + * * Deletes previous data and replaces them with new. Note that the * data are not copied, but they are deleted on destruction. * @see @ref release() @@ -361,7 +372,9 @@ template class CompressedImage { * @requires_gl Compressed pixel storage is hardcoded in OpenGL ES and * WebGL. */ - void setData(CompressedPixelStorage storage, CompressedPixelFormat format, const VectorTypeFor& size, Containers::Array&& data); + CORRADE_DEPRECATED("move-assign a new instance instead") void setData(CompressedPixelStorage storage, CompressedPixelFormat format, const VectorTypeFor& size, Containers::Array&& data) { + *this = CompressedImage{storage, format, size, std::move(data)}; + } #endif /** @@ -370,10 +383,15 @@ template class CompressedImage { * @param size Image size * @param data Image data * + * @deprecated Move-assign a new instance instead. + * * Similar the above, but uses default @ref CompressedPixelStorage * parameters (or the hardcoded ones in OpenGL ES and WebGL). */ - void setData(CompressedPixelFormat format, const VectorTypeFor& size, Containers::Array&& data); + CORRADE_DEPRECATED("move-assign a new instance instead") void setData(CompressedPixelFormat format, const VectorTypeFor& size, Containers::Array&& data) { + *this = CompressedImage{format, size, std::move(data)}; + } + #endif /** * @brief Release data storage @@ -486,10 +504,6 @@ template inline CompressedImage::CompressedI template inline CompressedImage::CompressedImage(const CompressedPixelFormat format, const VectorTypeFor& size, Containers::Array&& data): CompressedImage{{}, format, size, std::move(data)} {} template inline CompressedImage::CompressedImage(): CompressedImage{CompressedPixelStorage{}} {} - -template inline void CompressedImage::setData(const CompressedPixelFormat format, const VectorTypeFor& size, Containers::Array&& data) { - setData({}, format, size, std::move(data)); -} #endif } diff --git a/src/Magnum/Test/ImageTest.cpp b/src/Magnum/Test/ImageTest.cpp index 3376d14e7..e90fc1f57 100644 --- a/src/Magnum/Test/ImageTest.cpp +++ b/src/Magnum/Test/ImageTest.cpp @@ -40,8 +40,6 @@ struct ImageTest: TestSuite::Tester { void constructMove(); void constructMoveCompressed(); - void setData(); - void setDataCompressed(); void toView(); void toViewCompressed(); void release(); @@ -56,8 +54,6 @@ ImageTest::ImageTest() { &ImageTest::constructMove, &ImageTest::constructMoveCompressed, - &ImageTest::setData, - &ImageTest::setDataCompressed, &ImageTest::toView, &ImageTest::toViewCompressed, &ImageTest::release, @@ -169,39 +165,6 @@ void ImageTest::constructMoveCompressed() { CORRADE_COMPARE(c.data().size(), 8); } -void ImageTest::setData() { - auto data = new char[3*3]; - Image2D a{PixelStorage{}.setAlignment(1), - PixelFormat::RGB, PixelType::UnsignedByte, {1, 3}, Containers::Array{data, 3*3}}; - auto data2 = new char[2*2*4]; - a.setData(PixelFormat::RGBA, PixelType::UnsignedShort, {2, 1}, Containers::Array{data2, 2*2*4}); - - CORRADE_COMPARE(a.storage().alignment(), 4); - CORRADE_COMPARE(a.format(), PixelFormat::RGBA); - CORRADE_COMPARE(a.type(), PixelType::UnsignedShort); - CORRADE_COMPARE(a.size(), Vector2i(2, 1)); - CORRADE_COMPARE(a.data(), data2); -} - -void ImageTest::setDataCompressed() { - auto data = new char[8]; - CompressedImage2D a{CompressedPixelFormat::RGBAS3tcDxt1, {4, 4}, Containers::Array{data, 8}}; - auto data2 = new char[16]; - a.setData( - #ifndef MAGNUM_TARGET_GLES - CompressedPixelStorage{}.setCompressedBlockSize(Vector3i{4}), - #endif - CompressedPixelFormat::RGBAS3tcDxt3, {8, 4}, Containers::Array{data2, 16}); - - #ifndef MAGNUM_TARGET_GLES - CORRADE_COMPARE(a.storage().compressedBlockSize(), Vector3i{4}); - #endif - CORRADE_COMPARE(a.format(), CompressedPixelFormat::RGBAS3tcDxt3); - CORRADE_COMPARE(a.size(), Vector2i(8, 4)); - CORRADE_COMPARE(a.data(), data2); - CORRADE_COMPARE(a.data().size(), 16); -} - void ImageTest::toView() { auto data = new char[3*3]; const Image2D a{PixelStorage{}.setAlignment(1),