From 3bcd2abeea8d7a5a58903ece06060cdb8540a71c Mon Sep 17 00:00:00 2001 From: Daniel Nachbaur Date: Thu, 5 Nov 2015 17:32:02 +0100 Subject: [PATCH] Fix regression from #506: Only post-divide alpha for image writing --- doc/Changelog.md | 9 ++-- eq/detail/channel.ipp | 4 -- eq/image.cpp | 114 +++++++++++++++++++++++---------------- eq/image.h | 9 +--- eq/resultImageListener.h | 2 - 5 files changed, 73 insertions(+), 65 deletions(-) diff --git a/doc/Changelog.md b/doc/Changelog.md index 08ccb95452..0a3a33e7b1 100644 --- a/doc/Changelog.md +++ b/doc/Changelog.md @@ -3,11 +3,10 @@ Changelog {#Changelog} # git master -* [506](https://github.com/Eyescale/Equalizer/pull/506): - eq::ResultImageListener::notifyNewImage() now reports post-divided alpha image -* [506](https://github.com/Eyescale/Equalizer/pull/506): - Add eq::Image::postDivideAlpha() to fix premultiplied alpha images from - glReadPixels() +# Release 1.10 (5-Nov-2015) + +* [508](https://github.com/Eyescale/Equalizer/pull/508): + Post-divide alpha from pixels in eq::Image::writeImage() * [504](https://github.com/Eyescale/Equalizer/pull/504): Let the OS choose the server port * [502](https://github.com/Eyescale/Equalizer/pull/500): diff --git a/eq/detail/channel.ipp b/eq/detail/channel.ipp index e31837a31f..1acc7697bf 100644 --- a/eq/detail/channel.ipp +++ b/eq/detail/channel.ipp @@ -123,10 +123,6 @@ public: channel.getObjectManager( ))) { framebufferImage.finishReadback( channel.glewGetContext( )); - - // glReadPixels with alpha has ARGB premultiplied format. - // Post-divide alpha for image saving etc. - framebufferImage.postDivideAlpha(); } } diff --git a/eq/image.cpp b/eq/image.cpp index 557b7cab00..905803ff43 100644 --- a/eq/image.cpp +++ b/eq/image.cpp @@ -169,7 +169,11 @@ namespace detail class Image { public: - Image() : type( eq::Frame::TYPE_MEMORY ), ignoreAlpha( false ) {} + Image() + : type( eq::Frame::TYPE_MEMORY ) + , ignoreAlpha( false ) + , hasPremultipliedAlpha( false ) + {} /** The rectangle of the current pixel data. */ PixelViewport pvp; @@ -186,6 +190,8 @@ class Image /** Alpha channel significance. */ bool ignoreAlpha; + bool hasPremultipliedAlpha; + Attachment& getAttachment( const eq::Frame::Buffer buffer ) { switch( buffer ) @@ -246,6 +252,7 @@ Image::~Image() void Image::reset() { _impl->ignoreAlpha = false; + _impl->hasPremultipliedAlpha = false; setPixelViewport( PixelViewport( )); } @@ -675,6 +682,9 @@ void Image::_finishReadback( const Frame::Buffer buffer, return; } + if( memory.hasAlpha && buffer == Frame::BUFFER_COLOR ) + _impl->hasPremultipliedAlpha = true; + flags |= ( memory.hasAlpha ? 0 : EQ_COMPRESSOR_IGNORE_ALPHA ); uint64_t outDims[4] = {0}; @@ -1154,13 +1164,51 @@ bool Image::writeImage( const std::string& filename, if( nPixels == 0 || memory.state != Memory::VALID ) return false; - std::ofstream image( filename.c_str(), std::ios::out | std::ios::binary ); - if( !image.is_open( )) + const unsigned char* data = + reinterpret_cast( getPixelPointer( buffer )); + + unsigned char* convertedData = nullptr; + + // glReadPixels with alpha has ARGB premultiplied format: post-divide alpha + if( _impl->hasPremultipliedAlpha && + getExternalFormat( buffer ) == EQ_COMPRESSOR_DATATYPE_BGRA ) { - LBERROR << "Can't open " << filename << " for writing" << std::endl; - return false; + convertedData = new unsigned char[nPixels*4]; + + const uint32_t* bgraData = reinterpret_cast< const uint32_t* >( data ); + uint32_t* bgraConverted = reinterpret_cast< uint32_t* >( convertedData); + for( size_t i = 0; i < nPixels; ++i, ++bgraConverted, ++bgraData ) + { + *bgraConverted = *bgraData; + uint32_t& pixel = *bgraConverted; + const uint32_t alpha = pixel >> 24; + if( alpha != 0 ) + { + const uint32_t red = (pixel >> 16) & 0xff; + const uint32_t green = (pixel >> 8) & 0xff; + const uint32_t blue = pixel & 0xff; + *bgraConverted = (( alpha << 24 ) | + (((255 * red) / alpha ) << 16 ) | + (((255 * green) / alpha ) << 8 ) | + ((255 * blue) / alpha )); + } + } } + const bool retVal = _writeImage( filename, buffer, + convertedData ? convertedData : data ); + delete [] convertedData; + return retVal; +} + +bool Image::_writeImage( const std::string& filename, + const Frame::Buffer buffer, + const unsigned char* data_ ) const +{ + const Memory& memory = _impl->getMemory( buffer ); + const PixelViewport& pvp = memory.pvp; + const size_t nPixels = pvp.w * pvp.h; + RGBHeader header; header.width = pvp.w; header.height = pvp.h; @@ -1219,10 +1267,6 @@ bool Image::writeImage( const std::string& filename, } LBASSERT( header.bytesPerChannel > 0 ); - const uint8_t bpc = header.bytesPerChannel; - const uint16_t nChannels = header.depth; - const size_t depth = nChannels * bpc; - // Swap red & blue where needed bool swapRB = false; switch( getExternalFormat( buffer )) @@ -1238,24 +1282,31 @@ bool Image::writeImage( const std::string& filename, swapRB = true; } + const uint8_t bpc = header.bytesPerChannel; + const uint16_t nChannels = header.depth; + const size_t depth = nChannels * bpc; + const boost::filesystem::path path( filename ); #ifdef EQUALIZER_USE_OPENSCENEGRAPH if( path.extension() != ".rgb" ) { - const unsigned char* data = - reinterpret_cast( getPixelPointer( buffer )); osg::ref_ptr osgImage = new osg::Image(); osgImage->setImage( pvp.w, pvp.h, depth, getExternalFormat( buffer ), swapRB ? GL_RGBA : GL_BGRA, GL_UNSIGNED_BYTE, - const_cast< unsigned char* >( data ), + const_cast< unsigned char* >( data_ ), osg::Image::NO_DELETE ); return osgDB::writeImageFile( *osgImage, filename ); } #endif - const size_t nBytes = nPixels * depth; - const char* data = reinterpret_cast( getPixelPointer( buffer)); + std::ofstream image( filename.c_str(), std::ios::out | std::ios::binary ); + if( !image.is_open( )) + { + LBERROR << "Can't open " << filename << " for writing" << std::endl; + return false; + } + const size_t nBytes = nPixels * depth; if( header.bytesPerChannel > 2 ) LBWARN << static_cast< int >( header.bytesPerChannel ) << " bytes per channel not supported by RGB spec" << std::endl; @@ -1265,6 +1316,8 @@ bool Image::writeImage( const std::string& filename, image.write( reinterpret_cast( &header ), sizeof( header )); header.convert(); + const char* data = reinterpret_cast< const char* >( data_ ); + // Each channel is saved separately if( nChannels == 3 || nChannels == 4 ) { @@ -1648,39 +1701,6 @@ bool Image::getAlphaUsage() const return !_impl->ignoreAlpha; } -void Image::postDivideAlpha() -{ - switch( getExternalFormat( Frame::BUFFER_COLOR )) - { - case EQ_COMPRESSOR_DATATYPE_BGRA: - break; - default: - LBERROR << "Unsupported image format for postDivideAlpha(): " - << getExternalFormat( Frame::BUFFER_COLOR ) << std::endl; - return; - } - - uint32_t* data = - reinterpret_cast< uint32_t* >( getPixelPointer( Frame::BUFFER_COLOR )); - const PixelViewport& pvp = _impl->getMemory( Frame::BUFFER_COLOR ).pvp; - - for( int32_t i = 0; i < pvp.w * pvp.h; ++i, ++data ) - { - uint32_t& pixel = *data; - const uint32_t alpha = pixel >> 24; - if( alpha != 0 ) - { - const uint32_t red = (pixel >> 16) & 0xff; - const uint32_t green = (pixel >> 8) & 0xff; - const uint32_t blue = pixel & 0xff; - pixel = (( alpha << 24 ) | - (((255 * red) / alpha ) << 16 ) | - (((255 * green) / alpha ) << 8 ) | - ((255 * blue) / alpha )); - } - } -} - void Image::setOffset( int32_t x, int32_t y ) { _impl->pvp.x = x; diff --git a/eq/image.h b/eq/image.h index 38704f6950..4b6467deac 100644 --- a/eq/image.h +++ b/eq/image.h @@ -244,13 +244,6 @@ class Image /** @return true if alpha data can not be ignored. @version 1.0 */ EQ_API bool getAlphaUsage() const; - /** - * Undo premultiplied alpha for ARGB image, e.g. the result of glReadPixels. - * Only implemented for external format EQ_COMPRESSOR_DATATYPE_BGRA. - * @version 1.10 - */ - EQ_API void postDivideAlpha(); - /** * Set the minimum quality after a full download-compression path. * @@ -435,6 +428,8 @@ class Image void _finishReadback( const Frame::Buffer buffer, const GLEWContext* ); bool _readbackZoom( const Frame::Buffer buffer, util::ObjectManager& om ); + bool _writeImage( const std::string& filename, const Frame::Buffer buffer, + const unsigned char* data ) const; }; }; #endif // EQ_IMAGE_H diff --git a/eq/resultImageListener.h b/eq/resultImageListener.h index d771ad28fa..d31b73853b 100644 --- a/eq/resultImageListener.h +++ b/eq/resultImageListener.h @@ -43,8 +43,6 @@ class ResultImageListener * Notify on new image, called from rendering thread in * Channel::frameViewFinish(). * - * Since version 1.10, the image pixels are post-divided by alpha. - * * @param channel the destination channel * @param image the new image, valid only in the current frame * @version 1.9