From fe97c608c212c34976b9d260aaa2cd4c70e70247 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 11 Aug 2015 13:26:08 +0200 Subject: [PATCH] Pixel storage support, part 1: removed needles AbstractImage bases. I hoped to use them to store pixel pack/unpack configuration, but I have better solution now. The AbstractImage.h header is now removed along with the base classes, I'm not expecting that anyone used these empty classes, so I'm not doing anything to preserve backwards compatibility. --- src/Magnum/AbstractImage.h | 84 ------------------- src/Magnum/BufferImage.h | 14 ++-- src/Magnum/CMakeLists.txt | 4 +- src/Magnum/Image.h | 12 ++- src/Magnum/ImageView.h | 8 +- src/Magnum/Magnum.h | 1 - .../{AbstractImage.cpp => PixelStorage.cpp} | 10 +-- src/Magnum/PixelStorage.h | 47 +++++++++++ src/Magnum/Test/CMakeLists.txt | 2 +- ...ractImageTest.cpp => PixelStorageTest.cpp} | 18 ++-- src/Magnum/Trade/ImageData.cpp | 2 +- src/Magnum/Trade/ImageData.h | 5 +- 12 files changed, 82 insertions(+), 125 deletions(-) delete mode 100644 src/Magnum/AbstractImage.h rename src/Magnum/{AbstractImage.cpp => PixelStorage.cpp} (91%) create mode 100644 src/Magnum/PixelStorage.h rename src/Magnum/Test/{AbstractImageTest.cpp => PixelStorageTest.cpp} (86%) diff --git a/src/Magnum/AbstractImage.h b/src/Magnum/AbstractImage.h deleted file mode 100644 index 7f76718d5..000000000 --- a/src/Magnum/AbstractImage.h +++ /dev/null @@ -1,84 +0,0 @@ -#ifndef Magnum_AbstractImage_h -#define Magnum_AbstractImage_h -/* - This file is part of Magnum. - - Copyright © 2010, 2011, 2012, 2013, 2014, 2015 - Vladimír Vondruš - - 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. -*/ - -/** @file - * @brief Class @ref Magnum::AbstractImage, @ref Magnum::AbstractCompressedImage - */ - -#include - -#include "Magnum/Magnum.h" -#include "Magnum/visibility.h" - -namespace Magnum { - -namespace Implementation { - std::size_t MAGNUM_EXPORT imagePixelSize(ColorFormat format, ColorType type); - - template std::size_t imageDataSize(const AbstractImage& image, ColorFormat format, ColorType type, Math::Vector size); -} - -/** -@brief Non-templated base for one-, two- or three-dimensional uncompressed images - -See @ref Image, @ref ImageView, @ref BufferImage and @ref Trade::ImageData -documentation for more information. -@see @ref AbstractCompressedImage -@todo Where to put glClampColor() and glPixelStore() encapsulation? It is - needed in AbstractFramebuffer::read(), Texture::setImage() etc (i.e. all - functions operating with images). It also possibly needs to be "stackable" - to easily revert the state back. -*/ -class AbstractImage { - protected: - ~AbstractImage() = default; - - #ifndef DOXYGEN_GENERATING_OUTPUT - protected: - #else - private: - #endif - /** @todo remove this when the class has actual contents */ - /* Otherwise both (heh) GCC and Clang complain when move-constructing using {} */ - Int _dummy; -}; - -/** -@brief Non-templated base for one-, two- or three-dimensional compressed images - -See @ref CompressedImage, @ref CompressedImageView, @ref CompressedBufferImage -and @ref Trade::ImageData documentation for more information. -@see @ref AbstractImage -*/ -class AbstractCompressedImage: public AbstractImage { - protected: - ~AbstractCompressedImage() = default; -}; - -} - -#endif diff --git a/src/Magnum/BufferImage.h b/src/Magnum/BufferImage.h index d6b9dc908..fc0661743 100644 --- a/src/Magnum/BufferImage.h +++ b/src/Magnum/BufferImage.h @@ -31,9 +31,9 @@ */ #endif -#include "Magnum/AbstractImage.h" #include "Magnum/Buffer.h" #include "Magnum/DimensionTraits.h" +#include "Magnum/PixelStorage.h" #include "Magnum/Math/Vector3.h" namespace Magnum { @@ -49,7 +49,7 @@ Stores image data in GPU memory. Interchangeable with @ref Image, @requires_gles30 Pixel buffer objects are not available in OpenGL ES 2.0. @requires_webgl20 Pixel buffer objects are not available in WebGL 1.0. */ -template class BufferImage: public AbstractImage { +template class BufferImage { public: enum: UnsignedInt { Dimensions = dimensions /**< Image dimension count */ @@ -105,7 +105,7 @@ template class BufferImage: public AbstractImage { /** @copydoc Image::dataSize() */ std::size_t dataSize(const VectorTypeFor& size) const { - return Implementation::imageDataSize(*this, _format, _type, size); + return Implementation::imageDataSize(_format, _type, size); } /** @brief Image buffer */ @@ -155,7 +155,7 @@ See @ref BufferImage for more information. Interchangeable with @ref CompressedI @requires_gles30 Pixel buffer objects are not available in OpenGL ES 2.0. @requires_webgl20 Pixel buffer objects are not available in WebGL 1.0. */ -template class CompressedBufferImage: public AbstractCompressedImage { +template class CompressedBufferImage { public: enum: UnsignedInt { Dimensions = dimensions /**< Image dimension count */ @@ -237,17 +237,16 @@ typedef CompressedBufferImage<2> CompressedBufferImage2D; /** @brief Three-dimensional compressed buffer image */ typedef CompressedBufferImage<3> CompressedBufferImage3D; -template inline BufferImage::BufferImage(BufferImage&& other) noexcept: AbstractImage{std::move(other)}, _format{std::move(other._format)}, _type{std::move(other._type)}, _size{std::move(other._size)}, _buffer{std::move(other._buffer)} { +template inline BufferImage::BufferImage(BufferImage&& other) noexcept: _format{std::move(other._format)}, _type{std::move(other._type)}, _size{std::move(other._size)}, _buffer{std::move(other._buffer)} { other._size = {}; } -template inline CompressedBufferImage::CompressedBufferImage(CompressedBufferImage&& other) noexcept: AbstractCompressedImage{std::move(other)}, _format{std::move(other._format)}, _size{std::move(other._size)}, _buffer{std::move(other._buffer)}, _dataSize{std::move(other._dataSize)} { +template inline CompressedBufferImage::CompressedBufferImage(CompressedBufferImage&& other) noexcept: _format{std::move(other._format)}, _size{std::move(other._size)}, _buffer{std::move(other._buffer)}, _dataSize{std::move(other._dataSize)} { other._size = {}; other._dataSize = {}; } template inline BufferImage& BufferImage::operator=(BufferImage&& other) noexcept { - AbstractImage::operator=(std::move(other)); using std::swap; swap(_format, other._format); swap(_type, other._type); @@ -257,7 +256,6 @@ template inline BufferImage& BufferImage inline CompressedBufferImage& CompressedBufferImage::operator=(CompressedBufferImage&& other) noexcept { - AbstractCompressedImage::operator=(std::move(other)); using std::swap; swap(_format, other._format); swap(_size, other._size); diff --git a/src/Magnum/CMakeLists.txt b/src/Magnum/CMakeLists.txt index ac9bcca11..0e5220d5b 100644 --- a/src/Magnum/CMakeLists.txt +++ b/src/Magnum/CMakeLists.txt @@ -29,7 +29,6 @@ configure_file(${CMAKE_CURRENT_SOURCE_DIR}/configure.h.cmake # Files shared between main library and unit test library set(Magnum_SRCS AbstractFramebuffer.cpp - AbstractImage.cpp AbstractObject.cpp AbstractTexture.cpp AbstractShaderProgram.cpp @@ -44,6 +43,7 @@ set(Magnum_SRCS Mesh.cpp MeshView.cpp OpenGL.cpp + PixelStorage.cpp Renderbuffer.cpp Renderer.cpp Resource.cpp @@ -79,7 +79,6 @@ set(Magnum_SRCS set(Magnum_HEADERS AbstractFramebuffer.h - AbstractImage.h AbstractObject.h AbstractResourceLoader.h AbstractShaderProgram.h @@ -100,6 +99,7 @@ set(Magnum_HEADERS Mesh.h MeshView.h OpenGL.h + PixelStorage.h Renderbuffer.h RenderbufferFormat.h Renderer.h diff --git a/src/Magnum/Image.h b/src/Magnum/Image.h index 4402b958a..da064fa2f 100644 --- a/src/Magnum/Image.h +++ b/src/Magnum/Image.h @@ -42,7 +42,7 @@ Stores image data on client memory. Interchangeable with @ref ImageView, @ref BufferImage or @ref Trade::ImageData. @see @ref Image1D, @ref Image2D, @ref Image3D, @ref CompressedImage */ -template class Image: public AbstractImage { +template class Image { public: enum: UnsignedInt { Dimensions = dimensions /**< Image dimension count */ @@ -118,7 +118,7 @@ template class Image: public AbstractImage { * @see @ref pixelSize() */ std::size_t dataSize(const VectorTypeFor& size) const { - return Implementation::imageDataSize(*this, _format, _type, size); + return Implementation::imageDataSize(_format, _type, size); } /** @@ -182,7 +182,7 @@ See @ref Image for more information. Interchangeable with @ref CompressedImageView, @ref CompressedBufferImage or @ref Trade::ImageData. @see @ref CompressedImage1D, @ref CompressedImage2D, @ref CompressedImage3D */ -template class CompressedImage: public AbstractCompressedImage { +template class CompressedImage { public: enum: UnsignedInt { Dimensions = dimensions /**< Image dimension count */ @@ -291,18 +291,17 @@ typedef CompressedImage<2> CompressedImage2D; /** @brief Three-dimensional compressed image */ typedef CompressedImage<3> CompressedImage3D; -template inline Image::Image(Image&& other) noexcept: AbstractImage{std::move(other)}, _format{std::move(other._format)}, _type{std::move(other._type)}, _size{std::move(other._size)}, _data{std::move(other._data)} { +template inline Image::Image(Image&& other) noexcept: _format{std::move(other._format)}, _type{std::move(other._type)}, _size{std::move(other._size)}, _data{std::move(other._data)} { other._size = {}; other._data = nullptr; } -template inline CompressedImage::CompressedImage(CompressedImage&& other) noexcept: AbstractCompressedImage{std::move(other)}, _format{std::move(other._format)}, _size{std::move(other._size)}, _data{std::move(other._data)} { +template inline CompressedImage::CompressedImage(CompressedImage&& other) noexcept: _format{std::move(other._format)}, _size{std::move(other._size)}, _data{std::move(other._data)} { other._size = {}; other._data = nullptr; } template inline Image& Image::operator=(Image&& other) noexcept { - AbstractImage::operator=(std::move(other)); using std::swap; swap(_format, other._format); swap(_type, other._type); @@ -312,7 +311,6 @@ template inline Image& Image::op } template inline CompressedImage& CompressedImage::operator=(CompressedImage&& other) noexcept { - AbstractCompressedImage::operator=(std::move(other)); using std::swap; swap(_format, other._format); swap(_size, other._size); diff --git a/src/Magnum/ImageView.h b/src/Magnum/ImageView.h index e7fc96d72..1a38ce5b5 100644 --- a/src/Magnum/ImageView.h +++ b/src/Magnum/ImageView.h @@ -31,9 +31,9 @@ #include -#include "Magnum/Math/Vector3.h" -#include "Magnum/AbstractImage.h" #include "Magnum/DimensionTraits.h" +#include "Magnum/PixelStorage.h" +#include "Magnum/Math/Vector3.h" namespace Magnum { @@ -53,7 +53,7 @@ Interchangeable with @ref Image, @ref BufferImage or @ref Trade::ImageData. @see @ref ImageView1D, @ref ImageView2D, @ref ImageView3D, @ref CompressedImageView */ -template class ImageView: public AbstractImage { +template class ImageView { public: enum: UnsignedInt { Dimensions = dimensions /**< Image dimension count */ @@ -142,7 +142,7 @@ See @ref ImageView for more information. Interchangeable with @see @ref CompressedImageView1D, @ref CompressedImageView2D, @ref CompressedImageView3D */ -template class CompressedImageView: public AbstractCompressedImage { +template class CompressedImageView { public: enum: UnsignedInt { Dimensions = dimensions /**< Image dimension count */ diff --git a/src/Magnum/Magnum.h b/src/Magnum/Magnum.h index 51314a2f7..f659a3fb8 100644 --- a/src/Magnum/Magnum.h +++ b/src/Magnum/Magnum.h @@ -435,7 +435,6 @@ using Math::operator "" _radf; FramebufferTarget enums used only directly with framebuffer instance */ class AbstractFramebuffer; -class AbstractImage; /* AbstractQuery is not used directly */ class AbstractShaderProgram; class AbstractTexture; diff --git a/src/Magnum/AbstractImage.cpp b/src/Magnum/PixelStorage.cpp similarity index 91% rename from src/Magnum/AbstractImage.cpp rename to src/Magnum/PixelStorage.cpp index 135609ea0..75d8ad64c 100644 --- a/src/Magnum/AbstractImage.cpp +++ b/src/Magnum/PixelStorage.cpp @@ -23,7 +23,7 @@ DEALINGS IN THE SOFTWARE. */ -#include "AbstractImage.h" +#include "PixelStorage.h" #include @@ -151,7 +151,7 @@ std::size_t imagePixelSize(ColorFormat format, ColorType type) { CORRADE_ASSERT_UNREACHABLE(); } -template std::size_t imageDataSize(const AbstractImage&, const ColorFormat format, const ColorType type, Math::Vector size) { +template std::size_t imageDataSize(const ColorFormat format, const ColorType type, Math::Vector size) { /** @todo Code this properly when all @fn_gl{PixelStore} parameters are implemented */ /* Row size, rounded to multiple of 4 bytes */ const std::size_t rowSize = ((size[0]*imagePixelSize(format, type) + 3)/4)*4; @@ -161,8 +161,8 @@ template std::size_t imageDataSize(const AbstractImage&, return rowSize*size.product(); } -template MAGNUM_EXPORT std::size_t imageDataSize<1>(const AbstractImage&, ColorFormat, ColorType, Math::Vector<1, Int>); -template MAGNUM_EXPORT std::size_t imageDataSize<2>(const AbstractImage&, ColorFormat, ColorType, Math::Vector<2, Int>); -template MAGNUM_EXPORT std::size_t imageDataSize<3>(const AbstractImage&, ColorFormat, ColorType, Math::Vector<3, Int>); +template MAGNUM_EXPORT std::size_t imageDataSize<1>(ColorFormat, ColorType, Math::Vector<1, Int>); +template MAGNUM_EXPORT std::size_t imageDataSize<2>(ColorFormat, ColorType, Math::Vector<2, Int>); +template MAGNUM_EXPORT std::size_t imageDataSize<3>(ColorFormat, ColorType, Math::Vector<3, Int>); }} diff --git a/src/Magnum/PixelStorage.h b/src/Magnum/PixelStorage.h new file mode 100644 index 000000000..79c898564 --- /dev/null +++ b/src/Magnum/PixelStorage.h @@ -0,0 +1,47 @@ +#ifndef Magnum_PixelStorage_h +#define Magnum_PixelStorage_h +/* + This file is part of Magnum. + + Copyright © 2010, 2011, 2012, 2013, 2014, 2015 + Vladimír Vondruš + + 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. +*/ + +/** @file + * @brief + */ + +#include + +#include "Magnum/Magnum.h" +#include "Magnum/visibility.h" + +namespace Magnum { + +namespace Implementation { + std::size_t MAGNUM_EXPORT imagePixelSize(ColorFormat format, ColorType type); + + template std::size_t imageDataSize(ColorFormat format, ColorType type, Math::Vector size); +} + +} + +#endif diff --git a/src/Magnum/Test/CMakeLists.txt b/src/Magnum/Test/CMakeLists.txt index ecf1168bc..108a6a9a0 100644 --- a/src/Magnum/Test/CMakeLists.txt +++ b/src/Magnum/Test/CMakeLists.txt @@ -23,7 +23,6 @@ # DEALINGS IN THE SOFTWARE. # -corrade_add_test(AbstractImageTest AbstractImageTest.cpp LIBRARIES Magnum) corrade_add_test(AbstractShaderProgramTest AbstractShaderProgramTest.cpp LIBRARIES Magnum) corrade_add_test(ArrayTest ArrayTest.cpp) corrade_add_test(FormatTest FormatTest.cpp LIBRARIES Magnum) @@ -34,6 +33,7 @@ corrade_add_test(FramebufferTest FramebufferTest.cpp LIBRARIES Magnum) corrade_add_test(ImageTest ImageTest.cpp LIBRARIES Magnum) corrade_add_test(ImageViewTest ImageViewTest.cpp LIBRARIES Magnum) corrade_add_test(MeshTest MeshTest.cpp LIBRARIES Magnum) +corrade_add_test(PixelStorageTest PixelStorageTest.cpp LIBRARIES Magnum) corrade_add_test(RendererTest RendererTest.cpp LIBRARIES Magnum) corrade_add_test(ResourceManagerTest ResourceManagerTest.cpp LIBRARIES Magnum) corrade_add_test(SamplerTest SamplerTest.cpp LIBRARIES Magnum) diff --git a/src/Magnum/Test/AbstractImageTest.cpp b/src/Magnum/Test/PixelStorageTest.cpp similarity index 86% rename from src/Magnum/Test/AbstractImageTest.cpp rename to src/Magnum/Test/PixelStorageTest.cpp index 7e3d9fe23..a17abd8ca 100644 --- a/src/Magnum/Test/AbstractImageTest.cpp +++ b/src/Magnum/Test/PixelStorageTest.cpp @@ -25,32 +25,32 @@ #include -#include "Magnum/AbstractImage.h" #include "Magnum/Image.h" #include "Magnum/ColorFormat.h" +#include "Magnum/PixelStorage.h" namespace Magnum { namespace Test { -struct AbstractImageTest: TestSuite::Tester { - explicit AbstractImageTest(); +struct PixelStorageTest: TestSuite::Tester { + explicit PixelStorageTest(); void pixelSize(); void dataSize(); }; -AbstractImageTest::AbstractImageTest() { - addTests({&AbstractImageTest::pixelSize, - &AbstractImageTest::dataSize}); +PixelStorageTest::PixelStorageTest() { + addTests({&PixelStorageTest::pixelSize, + &PixelStorageTest::dataSize}); } -void AbstractImageTest::pixelSize() { +void PixelStorageTest::pixelSize() { CORRADE_COMPARE(Implementation::imagePixelSize(ColorFormat::RGBA, ColorType::UnsignedInt), 4*4); CORRADE_COMPARE(Implementation::imagePixelSize(ColorFormat::DepthComponent, ColorType::UnsignedShort), 2); CORRADE_COMPARE(Implementation::imagePixelSize(ColorFormat::StencilIndex, ColorType::UnsignedByte), 1); CORRADE_COMPARE(Implementation::imagePixelSize(ColorFormat::DepthStencil, ColorType::UnsignedInt248), 4); } -void AbstractImageTest::dataSize() { +void PixelStorageTest::dataSize() { /* Verify that row size is properly rounded */ CORRADE_COMPARE(Image2D(ColorFormat::RGBA, ColorType::UnsignedByte).dataSize({}), 0); CORRADE_COMPARE(Image2D(ColorFormat::Red, ColorType::UnsignedByte).dataSize({4, 2}), 8); @@ -63,4 +63,4 @@ void AbstractImageTest::dataSize() { }} -CORRADE_TEST_MAIN(Magnum::Test::AbstractImageTest) +CORRADE_TEST_MAIN(Magnum::Test::PixelStorageTest) diff --git a/src/Magnum/Trade/ImageData.cpp b/src/Magnum/Trade/ImageData.cpp index b0bc02027..70099a900 100644 --- a/src/Magnum/Trade/ImageData.cpp +++ b/src/Magnum/Trade/ImageData.cpp @@ -49,7 +49,7 @@ template std::size_t ImageData::pixelSize() template std::size_t ImageData::dataSize(const VectorTypeFor< dimensions, Int >& size) const { CORRADE_ASSERT(!_compressed, "Trade::ImageData::dataSize(): the image is compressed", {}); - return Implementation::imageDataSize(*this, _format, _type, size); + return Implementation::imageDataSize(_format, _type, size); } template ImageData::operator ImageView() diff --git a/src/Magnum/Trade/ImageData.h b/src/Magnum/Trade/ImageData.h index 584943ca3..8345a697e 100644 --- a/src/Magnum/Trade/ImageData.h +++ b/src/Magnum/Trade/ImageData.h @@ -50,7 +50,7 @@ Uncompressed image is interchangeable with @ref Image, @ref ImageView or or @ref CompressedBufferImage. @see @ref ImageData1D, @ref ImageData2D, @ref ImageData3D */ -template class ImageData: public AbstractCompressedImage { +template class ImageData { public: enum: UnsignedInt { Dimensions = dimensions /**< Image dimension count */ @@ -223,7 +223,7 @@ typedef ImageData<2> ImageData2D; /** @brief Three-dimensional image */ typedef ImageData<3> ImageData3D; -template inline ImageData::ImageData(ImageData&& other) noexcept: AbstractCompressedImage{std::move(other)}, _compressed{std::move(other._compressed)}, _type{std::move(other._type)}, _size{std::move(other._size)}, _data{std::move(other._data)} { +template inline ImageData::ImageData(ImageData&& other) noexcept: _compressed{std::move(other._compressed)}, _type{std::move(other._type)}, _size{std::move(other._size)}, _data{std::move(other._data)} { if(_compressed) _compressedFormat = std::move(other._compressedFormat); else _format = std::move(other._format); @@ -232,7 +232,6 @@ template inline ImageData::ImageData(ImageDa } template inline ImageData& ImageData::operator=(ImageData&& other) noexcept { - AbstractCompressedImage::operator=(std::move(other)); using std::swap; swap(_compressed, other._compressed); if(_compressed) swap(_compressedFormat, other._compressedFormat);