From da3cb5a8ad13c739aadd68da8d43006bc9f528fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Thu, 23 Jul 2015 13:46:48 +0200 Subject: [PATCH] Compressed image support, part 1: generalized AbstractImage class. When pixel pack/unpack parameter support is done, this class will contain only stuff that's common to both compressed and uncompressed images. Currently that's nothing, so the class is empty. Note the hilarious bug in both GCC and Clang: if you remove the `_dummy` member, both of them start complaining about weird completely unrelated stuff. --- src/Magnum/AbstractImage.cpp | 16 ++++---- src/Magnum/AbstractImage.h | 54 +++++++-------------------- src/Magnum/BufferImage.cpp | 4 +- src/Magnum/BufferImage.h | 17 ++++++++- src/Magnum/Image.h | 23 +++++++++--- src/Magnum/ImageReference.h | 17 +++++++-- src/Magnum/Test/AbstractImageTest.cpp | 8 ++-- src/Magnum/Trade/ImageData.h | 21 +++++++++-- 8 files changed, 92 insertions(+), 68 deletions(-) diff --git a/src/Magnum/AbstractImage.cpp b/src/Magnum/AbstractImage.cpp index 42e89cc01..135609ea0 100644 --- a/src/Magnum/AbstractImage.cpp +++ b/src/Magnum/AbstractImage.cpp @@ -30,9 +30,9 @@ #include "Magnum/ColorFormat.h" #include "Magnum/Math/Vector.h" -namespace Magnum { +namespace Magnum { namespace Implementation { -std::size_t AbstractImage::pixelSize(ColorFormat format, ColorType type) { +std::size_t imagePixelSize(ColorFormat format, ColorType type) { std::size_t size = 0; switch(type) { case ColorType::UnsignedByte: @@ -151,18 +151,18 @@ std::size_t AbstractImage::pixelSize(ColorFormat format, ColorType type) { CORRADE_ASSERT_UNREACHABLE(); } -template std::size_t AbstractImage::dataSize(Math::Vector size) const { +template std::size_t imageDataSize(const AbstractImage&, 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]*pixelSize() + 3)/4)*4; + const std::size_t rowSize = ((size[0]*imagePixelSize(format, type) + 3)/4)*4; /** @todo Can't this be done somewhat nicer? */ size[0] = 1; return rowSize*size.product(); } -template MAGNUM_EXPORT std::size_t AbstractImage::dataSize<1>(Math::Vector<1, Int>) const; -template MAGNUM_EXPORT std::size_t AbstractImage::dataSize<2>(Math::Vector<2, Int>) const; -template MAGNUM_EXPORT std::size_t AbstractImage::dataSize<3>(Math::Vector<3, Int>) const; +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>); -} +}} diff --git a/src/Magnum/AbstractImage.h b/src/Magnum/AbstractImage.h index 818c2504f..70cda0581 100644 --- a/src/Magnum/AbstractImage.h +++ b/src/Magnum/AbstractImage.h @@ -36,60 +36,34 @@ 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 images -See @ref Image, @ref ImageReference, @ref BufferImage, @ref Trade::ImageData +See @ref Image, @ref ImageReference, @ref BufferImage and @ref Trade::ImageData documentation for more information. @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 MAGNUM_EXPORT AbstractImage { - public: - /** - * @brief Pixel size (in bytes) - * @param format Format of the pixel - * @param type Data type of the pixel - * - * @see @ref pixelSize() - */ - static std::size_t pixelSize(ColorFormat format, ColorType type); - - /** @brief Format of pixel data */ - constexpr ColorFormat format() const { return _format; } - - /** @brief Data type of pixel data */ - constexpr ColorType type() const { return _type; } - - /** - * @brief Pixel size (in bytes) - * - * Convenience member alternative for - * @ref pixelSize(ColorFormat, ColorType). - */ - std::size_t pixelSize() const { return pixelSize(_format, _type); } - +class AbstractImage { protected: - /** - * @brief Constructor - * @param format Format of pixel data - * @param type Data type of pixel data - */ - constexpr explicit AbstractImage(ColorFormat format, ColorType type): _format(format), _type(type) {} - ~AbstractImage() = default; - template std::size_t dataSize(Math::Vector size) const; - - #ifdef DOXYGEN_GENERATING_OUTPUT - private: - #else + #ifndef DOXYGEN_GENERATING_OUTPUT protected: + #else + private: #endif - ColorFormat _format; - ColorType _type; + /** @todo remove this when the class has actual contents */ + /* Otherwise both (heh) GCC and Clang complain when move-constructing using {} */ + Int _dummy; }; } diff --git a/src/Magnum/BufferImage.cpp b/src/Magnum/BufferImage.cpp index 8ad193fce..2c59369c9 100644 --- a/src/Magnum/BufferImage.cpp +++ b/src/Magnum/BufferImage.cpp @@ -28,11 +28,11 @@ namespace Magnum { #ifndef MAGNUM_TARGET_GLES2 -template BufferImage::BufferImage(ColorFormat format, ColorType type, const VectorTypeFor& size, const void* data, BufferUsage usage): AbstractImage(format, type), _size(size), _buffer(Buffer::TargetHint::PixelPack) { +template BufferImage::BufferImage(ColorFormat format, ColorType type, const VectorTypeFor& size, const void* data, BufferUsage usage): _format{format}, _type{type}, _size{size}, _buffer{Buffer::TargetHint::PixelPack} { _buffer.setData({data, dataSize(size)}, usage); } -template BufferImage::BufferImage(ColorFormat format, ColorType type): AbstractImage(format, type), _buffer(Buffer::TargetHint::PixelPack) {} +template BufferImage::BufferImage(ColorFormat format, ColorType type): _format{format}, _type{type}, _buffer{Buffer::TargetHint::PixelPack} {} template void BufferImage::setData(ColorFormat format, ColorType type, const VectorTypeFor& size, const void* data, BufferUsage usage) { _format = format; diff --git a/src/Magnum/BufferImage.h b/src/Magnum/BufferImage.h index 344367fec..cd8a3853d 100644 --- a/src/Magnum/BufferImage.h +++ b/src/Magnum/BufferImage.h @@ -90,12 +90,21 @@ template class BufferImage: public AbstractImage { /** @brief Move assignment */ BufferImage& operator=(BufferImage&& other) noexcept; + /** @brief Format of pixel data */ + ColorFormat format() const { return _format; } + + /** @brief Data type of pixel data */ + ColorType type() const { return _type; } + + /** @brief Pixel size (in bytes) */ + std::size_t pixelSize() const { return Implementation::imagePixelSize(_format, _type); } + /** @brief Image size */ VectorTypeFor size() const { return _size; } /** @copydoc Image::dataSize() */ std::size_t dataSize(const VectorTypeFor& size) const { - return AbstractImage::dataSize(size); + return Implementation::imageDataSize(*this, _format, _type, size); } /** @brief Image buffer */ @@ -118,6 +127,8 @@ template class BufferImage: public AbstractImage { void setData(ColorFormat format, ColorType type, const VectorTypeFor& size, const void* data, BufferUsage usage); private: + ColorFormat _format; + ColorType _type; Math::Vector _size; Buffer _buffer; }; @@ -131,13 +142,15 @@ typedef BufferImage<2> BufferImage2D; /** @brief Three-dimensional buffer image */ typedef BufferImage<3> BufferImage3D; -template inline BufferImage::BufferImage(BufferImage&& other) noexcept: AbstractImage(std::move(other)), _size(other._size), _buffer(std::move(other._buffer)) { +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)} { other._size = {}; } template inline BufferImage& BufferImage::operator=(BufferImage&& other) noexcept { AbstractImage::operator=(std::move(other)); using std::swap; + swap(_format, other._format); + swap(_type, other._type); swap(_size, other._size); swap(_buffer, other._buffer); return *this; diff --git a/src/Magnum/Image.h b/src/Magnum/Image.h index 0a41dc0d0..c65f8ac71 100644 --- a/src/Magnum/Image.h +++ b/src/Magnum/Image.h @@ -56,7 +56,7 @@ template class Image: public AbstractImage { * Note that the image data are not copied on construction, but they * are deleted on class destruction. */ - explicit Image(ColorFormat format, ColorType type, const VectorTypeFor& size, void* data): AbstractImage{format, type}, _size{size}, _data{reinterpret_cast(data)} {} + explicit Image(ColorFormat format, ColorType type, const VectorTypeFor& size, void* data): _format{format}, _type{type}, _size{size}, _data{reinterpret_cast(data)} {} /** * @brief Constructor @@ -66,7 +66,7 @@ template class Image: public AbstractImage { * Dimensions are set to zero and data pointer to `nullptr`, call * @ref setData() to fill the image with data. */ - /*implicit*/ Image(ColorFormat format, ColorType type): AbstractImage(format, type), _data{} {} + /*implicit*/ Image(ColorFormat format, ColorType type): _format{format}, _type{type}, _data{} {} /** @brief Copying is not allowed */ Image(const Image&) = delete; @@ -96,6 +96,15 @@ template class Image: public AbstractImage { /*implicit*/ operator ImageReference() const && = delete; #endif + /** @brief Format of pixel data */ + ColorFormat format() const { return _format; } + + /** @brief Data type of pixel data */ + ColorType type() const { return _type; } + + /** @brief Pixel size (in bytes) */ + std::size_t pixelSize() const { return Implementation::imagePixelSize(_format, _type); } + /** @brief Image size */ VectorTypeFor size() const { return _size; } @@ -107,7 +116,7 @@ template class Image: public AbstractImage { * @see @ref pixelSize() */ std::size_t dataSize(const VectorTypeFor& size) const { - return AbstractImage::dataSize(size); + return Implementation::imageDataSize(*this, _format, _type, size); } /** @@ -147,6 +156,8 @@ template class Image: public AbstractImage { char* release(); private: + ColorFormat _format; + ColorType _type; Math::Vector _size; char* _data; }; @@ -160,7 +171,7 @@ typedef Image<2> Image2D; /** @brief Three-dimensional image */ typedef Image<3> Image3D; -template inline Image::Image(Image&& other) noexcept: AbstractImage(std::move(other)), _size(std::move(other._size)), _data(std::move(other._data)) { +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)} { other._size = {}; other._data = nullptr; } @@ -168,6 +179,8 @@ template inline Image::Image(Image inline Image& Image::operator=(Image&& other) noexcept { AbstractImage::operator=(std::move(other)); using std::swap; + swap(_format, other._format); + swap(_type, other._type); swap(_size, other._size); swap(_data, other._data); return *this; @@ -180,7 +193,7 @@ const & const #endif { - return ImageReference(AbstractImage::format(), AbstractImage::type(), _size, _data); + return ImageReference{_format, _type, _size, _data}; } template inline char* Image::release() { diff --git a/src/Magnum/ImageReference.h b/src/Magnum/ImageReference.h index b48960e4c..c0d8286d4 100644 --- a/src/Magnum/ImageReference.h +++ b/src/Magnum/ImageReference.h @@ -63,7 +63,7 @@ template class ImageReference: public AbstractImage { * @param size Image size * @param data Image data */ - constexpr explicit ImageReference(ColorFormat format, ColorType type, const VectorTypeFor& size, const void* data): AbstractImage(format, type), _size(size), _data(reinterpret_cast(data)) {} + constexpr explicit ImageReference(ColorFormat format, ColorType type, const VectorTypeFor& size, const void* data): _format{format}, _type{type}, _size{size}, _data{reinterpret_cast(data)} {} /** * @brief Constructor @@ -74,14 +74,23 @@ template class ImageReference: public AbstractImage { * Data pointer is set to `nullptr`, call @ref setData() to fill the * image with data. */ - constexpr explicit ImageReference(ColorFormat format, ColorType type, const VectorTypeFor& size): AbstractImage(format, type), _size(size), _data(nullptr) {} + constexpr explicit ImageReference(ColorFormat format, ColorType type, const VectorTypeFor& size): _format{format}, _type{type}, _size{size}, _data{nullptr} {} + + /** @brief Format of pixel data */ + ColorFormat format() const { return _format; } + + /** @brief Data type of pixel data */ + ColorType type() const { return _type; } + + /** @brief Pixel size (in bytes) */ + std::size_t pixelSize() const { return Implementation::imagePixelSize(_format, _type); } /** @brief Image size */ constexpr VectorTypeFor size() const { return _size; } /** @copydoc Image::dataSize() */ std::size_t dataSize(const VectorTypeFor& size) const { - return AbstractImage::dataSize(size); + return Implementation::imageDataSize(*this, _format, _type, size); } /** @brief Pointer to raw data */ @@ -105,6 +114,8 @@ template class ImageReference: public AbstractImage { } private: + ColorFormat _format; + ColorType _type; Math::Vector _size; const char* _data; }; diff --git a/src/Magnum/Test/AbstractImageTest.cpp b/src/Magnum/Test/AbstractImageTest.cpp index b2ff35bff..7e3d9fe23 100644 --- a/src/Magnum/Test/AbstractImageTest.cpp +++ b/src/Magnum/Test/AbstractImageTest.cpp @@ -44,10 +44,10 @@ AbstractImageTest::AbstractImageTest() { } void AbstractImageTest::pixelSize() { - CORRADE_COMPARE(AbstractImage::pixelSize(ColorFormat::RGBA, ColorType::UnsignedInt), 4*4); - CORRADE_COMPARE(AbstractImage::pixelSize(ColorFormat::DepthComponent, ColorType::UnsignedShort), 2); - CORRADE_COMPARE(AbstractImage::pixelSize(ColorFormat::StencilIndex, ColorType::UnsignedByte), 1); - CORRADE_COMPARE(AbstractImage::pixelSize(ColorFormat::DepthStencil, ColorType::UnsignedInt248), 4); + 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() { diff --git a/src/Magnum/Trade/ImageData.h b/src/Magnum/Trade/ImageData.h index 3b0e6f449..f09848d47 100644 --- a/src/Magnum/Trade/ImageData.h +++ b/src/Magnum/Trade/ImageData.h @@ -56,7 +56,7 @@ template class ImageData: public AbstractImage { * Note that the image data are not copied on construction, but they * are deleted on class destruction. */ - explicit ImageData(ColorFormat format, ColorType type, const VectorTypeFor& size, void* data): AbstractImage{format, type}, _size{size}, _data{reinterpret_cast(data)} {} + explicit ImageData(ColorFormat format, ColorType type, const VectorTypeFor& size, void* data): _format{format}, _type{type}, _size{size}, _data{reinterpret_cast(data)} {} /** @brief Copying is not allowed */ ImageData(const ImageData&) = delete; @@ -86,12 +86,21 @@ template class ImageData: public AbstractImage { /*implicit*/ operator ImageReference() const && = delete; #endif + /** @brief Format of pixel data */ + ColorFormat format() const { return _format; } + + /** @brief Data type of pixel data */ + ColorType type() const { return _type; } + + /** @brief Pixel size (in bytes) */ + std::size_t pixelSize() const { return Implementation::imagePixelSize(_format, _type); } + /** @brief Image size */ VectorTypeFor size() const { return _size; } /** @copydoc Image::dataSize() */ std::size_t dataSize(const VectorTypeFor& size) const { - return AbstractImage::dataSize(size); + return Implementation::imageDataSize(*this, _format, _type, size); } /** @@ -118,6 +127,8 @@ template class ImageData: public AbstractImage { char* release(); private: + ColorFormat _format; + ColorType _type; Math::Vector _size; char* _data; }; @@ -131,7 +142,7 @@ typedef ImageData<2> ImageData2D; /** @brief Three-dimensional image */ typedef ImageData<3> ImageData3D; -template inline ImageData::ImageData(ImageData&& other) noexcept: AbstractImage(std::move(other)), _size(std::move(other._size)), _data(std::move(other._data)) { +template inline ImageData::ImageData(ImageData&& 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)} { other._size = {}; other._data = nullptr; } @@ -139,6 +150,8 @@ template inline ImageData::ImageData(ImageDa template inline ImageData& ImageData::operator=(ImageData&& other) noexcept { AbstractImage::operator=(std::move(other)); using std::swap; + swap(_format, other._format); + swap(_type, other._type); swap(_size, other._size); swap(_data, other._data); return *this; @@ -151,7 +164,7 @@ const & const #endif { - return ImageReference(AbstractImage::format(), AbstractImage::type(), _size, _data); + return ImageReference(_format, _type, _size, _data); } template inline char* ImageData::release() {