From 3a5625706979145224c0fef904c7b6b8671754a1 Mon Sep 17 00:00:00 2001 From: Richard Underhill Date: Thu, 15 Oct 2015 10:15:27 +0100 Subject: [PATCH] BufferImage Fix for multiple area updates. Change-Id: I65785011e84f83c4fb11247f8c5bec49ee212247 Signed-off-by: Richard Underhill --- automated-tests/src/dali/utc-Dali-BufferImage.cpp | 19 +-- dali/internal/event/images/buffer-image-impl.cpp | 189 +++++++++++----------- dali/internal/event/images/buffer-image-impl.h | 16 +- 3 files changed, 108 insertions(+), 116 deletions(-) diff --git a/automated-tests/src/dali/utc-Dali-BufferImage.cpp b/automated-tests/src/dali/utc-Dali-BufferImage.cpp index bd3a05c..a48cefa 100644 --- a/automated-tests/src/dali/utc-Dali-BufferImage.cpp +++ b/automated-tests/src/dali/utc-Dali-BufferImage.cpp @@ -406,12 +406,6 @@ int UtcDaliBufferImageUpdate02(void) SignalReceived = false; image.UploadedSignal().Connect( ImageUploaded ); - std::vector ids; - ids.push_back(200); - ids.push_back(201); - ids.push_back(202); - application.GetGlAbstraction().SetNextTextureIds(ids); - application.SendNotification(); application.Render(0); application.Render(16); @@ -422,7 +416,10 @@ int UtcDaliBufferImageUpdate02(void) DALI_TEST_CHECK( image.IsDataExternal() ); application.GetGlAbstraction().EnableTextureCallTrace(true); - image.Update(RectArea(9,9,5,5)); // notify Core that the image has been updated + // Check that multiple updates in a frame will be properly uploaded + image.Update(RectArea(9,9,5,5)); + image.Update(RectArea(2,2,4,4)); + image.Update(RectArea(3,3,1,6)); application.SendNotification(); application.Render(16); @@ -432,11 +429,9 @@ int UtcDaliBufferImageUpdate02(void) application.SendNotification(); const TraceCallStack& callStack = application.GetGlAbstraction().GetTextureTrace(); - DALI_TEST_EQUALS( callStack.TestMethodAndParams(0, "TexSubImage2D", "9, 9, 5, 1"), true, TEST_LOCATION); - DALI_TEST_EQUALS( callStack.TestMethodAndParams(1, "TexSubImage2D", "9, 10, 5, 1"), true, TEST_LOCATION); - DALI_TEST_EQUALS( callStack.TestMethodAndParams(2, "TexSubImage2D", "9, 11, 5, 1"), true, TEST_LOCATION); - DALI_TEST_EQUALS( callStack.TestMethodAndParams(3, "TexSubImage2D", "9, 12, 5, 1"), true, TEST_LOCATION); - DALI_TEST_EQUALS( callStack.TestMethodAndParams(4, "TexSubImage2D", "9, 13, 5, 1"), true, TEST_LOCATION); + DALI_TEST_EQUALS( callStack.TestMethodAndParams(0, "TexSubImage2D", "9, 9, 5, 5"), true, TEST_LOCATION); + DALI_TEST_EQUALS( callStack.TestMethodAndParams(1, "TexSubImage2D", "2, 2, 4, 4"), true, TEST_LOCATION); + DALI_TEST_EQUALS( callStack.TestMethodAndParams(2, "TexSubImage2D", "3, 3, 1, 6"), true, TEST_LOCATION); DALI_TEST_CHECK( SignalReceived == true ); SignalReceived = false; diff --git a/dali/internal/event/images/buffer-image-impl.cpp b/dali/internal/event/images/buffer-image-impl.cpp index e945930..e09a50b 100644 --- a/dali/internal/event/images/buffer-image-impl.cpp +++ b/dali/internal/event/images/buffer-image-impl.cpp @@ -40,14 +40,22 @@ namespace TypeRegistration mType( typeid( Dali::BufferImage ), typeid( Dali::Image ), NULL ); } // unnamed namespace -BufferImagePtr BufferImage::New( unsigned int width, unsigned int height, Pixel::Format pixelformat, ReleasePolicy releasePol ) +BufferImagePtr BufferImage::New( unsigned int width, + unsigned int height, + Pixel::Format pixelformat, + ReleasePolicy releasePol ) { BufferImagePtr internal = new BufferImage( width, height, pixelformat, releasePol ); internal->Initialize(); return internal; } -BufferImagePtr BufferImage::New( PixelBuffer* pixBuf, unsigned int width, unsigned int height, Pixel::Format pixelformat, unsigned int stride, ReleasePolicy releasePol ) +BufferImagePtr BufferImage::New( PixelBuffer* pixBuf, + unsigned int width, + unsigned int height, + Pixel::Format pixelformat, + unsigned int stride, + ReleasePolicy releasePol ) { BufferImagePtr internal = new BufferImage( pixBuf, width, height, pixelformat, stride, releasePol ); internal->Initialize(); @@ -57,30 +65,37 @@ BufferImagePtr BufferImage::New( PixelBuffer* pixBuf, unsigned int width, unsign BufferImage::BufferImage(unsigned int width, unsigned int height, Pixel::Format pixelformat, ReleasePolicy releasePol) : Image(releasePol), mInternalBuffer(NULL), - mExternalBuffer(NULL), - mBitmap(NULL) + mExternalBuffer(NULL) { - ThreadLocalStorage& tls = ThreadLocalStorage::Get(); - mResourceClient = &tls.GetResourceClient(); - mWidth = width; - mHeight = height; - mPixelFormat = pixelformat; - mBytesPerPixel = Pixel::GetBytesPerPixel( pixelformat ); - mByteStride = width * mBytesPerPixel; - mBufferSize = height * mByteStride; + SetupBuffer( width, height, pixelformat, width, releasePol ); // Allocate a persistent internal buffer mInternalBuffer = new PixelBuffer[ mBufferSize ]; - - // Respect the desired release policy - mResourcePolicy = releasePol == Dali::Image::UNUSED ? ResourcePolicy::OWNED_DISCARD : ResourcePolicy::OWNED_RETAIN; } -BufferImage::BufferImage(PixelBuffer* pixBuf, unsigned int width, unsigned int height, Pixel::Format pixelformat, unsigned int stride, ReleasePolicy releasePol ) +BufferImage::BufferImage(PixelBuffer* pixBuf, + unsigned int width, + unsigned int height, + Pixel::Format pixelformat, + unsigned int stride, + ReleasePolicy releasePol ) : Image(releasePol), mInternalBuffer(NULL), - mExternalBuffer(pixBuf), - mBitmap(NULL) + mExternalBuffer(pixBuf) +{ + SetupBuffer( width, height, pixelformat, stride ? stride: width, releasePol ); +} + +BufferImage::~BufferImage() +{ + delete[] mInternalBuffer; +} + +void BufferImage::SetupBuffer( unsigned int width, + unsigned int height, + Pixel::Format pixelformat, + unsigned int byteStride, + ReleasePolicy releasePol ) { ThreadLocalStorage& tls = ThreadLocalStorage::Get(); mResourceClient = &tls.GetResourceClient(); @@ -88,23 +103,12 @@ BufferImage::BufferImage(PixelBuffer* pixBuf, unsigned int width, unsigned int h mHeight = height; mPixelFormat = pixelformat; mBytesPerPixel = Pixel::GetBytesPerPixel( pixelformat ); - mByteStride = ( stride ? stride : mWidth ) * mBytesPerPixel; + + mByteStride = byteStride * mBytesPerPixel; mBufferSize = height * mByteStride; // Respect the desired release policy mResourcePolicy = releasePol == Dali::Image::UNUSED ? ResourcePolicy::OWNED_DISCARD : ResourcePolicy::OWNED_RETAIN; - - // Create a bitmap to hold copy of external buffer - ReserveBitmap(); - - // Take a copy of the external buffer immediately, so it can be released if desired - RectArea area; - MirrorExternal( area ); -} - -BufferImage::~BufferImage() -{ - delete[] mInternalBuffer; } bool BufferImage::IsDataExternal() const @@ -114,101 +118,92 @@ bool BufferImage::IsDataExternal() const void BufferImage::Update( RectArea& updateArea ) { - ValidateBitmap(); - UpdateBitmap( updateArea ); - - if (mTicket) + if ( !mTicket ) { - DALI_ASSERT_DEBUG( updateArea.x + updateArea.width <= mWidth && updateArea.y + updateArea.height <= mHeight ); - mResourceClient->UpdateBitmapArea( mTicket, updateArea ); - - // Bitmap ownership has been passed on, so any subsequent update will need another bitmap - mBitmap = NULL; + CreateHostBitmap(); } + DALI_ASSERT_DEBUG( updateArea.x + updateArea.width <= mWidth && updateArea.y + updateArea.height <= mHeight ); + UploadArea( mTicket->GetId(), updateArea ); } -void BufferImage::UpdateBitmap( RectArea& updateArea ) +void BufferImage::CreateHostBitmap() { - if ( mExternalBuffer ) - { - MirrorExternal( updateArea ); - } - else - { - // Copy the internal buffer to the bitmap area - memcpy( mBitmap->GetBuffer(), mInternalBuffer, mBufferSize ); - } -} + Integration::Bitmap* bitmap = Bitmap::New( Bitmap::BITMAP_2D_PACKED_PIXELS, mResourcePolicy ); + Bitmap::PackedPixelsProfile* const packedBitmap = bitmap->GetPackedPixelsProfile(); + DALI_ASSERT_DEBUG(packedBitmap); -void BufferImage::ValidateBitmap() -{ - ReserveBitmap(); - if ( !mTicket ) - { - mTicket = mResourceClient->AddBitmapImage( mBitmap ); - mTicket->AddObserver(*this); - } + packedBitmap->ReserveBuffer( mPixelFormat, mWidth, mHeight ); + DALI_ASSERT_DEBUG(bitmap->GetBuffer() != 0); + DALI_ASSERT_DEBUG(bitmap->GetBufferSize() >= mHeight * mWidth * mBytesPerPixel ); + + mTicket = mResourceClient->AddBitmapImage( bitmap ); + mTicket->AddObserver(*this); } -void BufferImage::ReserveBitmap() +void BufferImage::UploadArea( ResourceId destId, const RectArea& area ) { - // Does a bitmap currently exist ? - if ( !mBitmap ) + Integration::Bitmap* bitmap = Bitmap::New( Bitmap::BITMAP_2D_PACKED_PIXELS, mResourcePolicy ); + Bitmap::PackedPixelsProfile* const packedBitmap = bitmap->GetPackedPixelsProfile(); + DALI_ASSERT_DEBUG(packedBitmap); + DALI_ASSERT_DEBUG( area.width <= mWidth && area.height <= mHeight ); + + mBufferWidth = area.width ? area.width : mWidth; + packedBitmap->ReserveBuffer( mPixelFormat, mBufferWidth, area.height ? area.height : mHeight ); + DALI_ASSERT_DEBUG(bitmap->GetBuffer() != 0); + DALI_ASSERT_DEBUG(bitmap->GetBufferSize() >= mBufferWidth * ( area.height ? area.height : mHeight ) * mBytesPerPixel ); + + // Are we uploading from an external or internal buffer ? + if ( mExternalBuffer ) { - mBitmap = Bitmap::New( Bitmap::BITMAP_2D_PACKED_PIXELS, mResourcePolicy ); + // Check if we're doing the entire area without stride mismatch between source and dest ? + if( ( mByteStride == mWidth * mBytesPerPixel ) && area.IsEmpty() ) + { + memcpy( bitmap->GetBuffer(), mExternalBuffer, mBufferSize ); + } + else + { + UpdateBufferArea( mExternalBuffer, bitmap->GetBuffer(), area ); + } } - - if ( !mBitmap->GetBuffer() ) + else { - Bitmap::PackedPixelsProfile* const packedBitmap = mBitmap->GetPackedPixelsProfile(); - DALI_ASSERT_DEBUG(packedBitmap); - - packedBitmap->ReserveBuffer( mPixelFormat, mWidth, mHeight, mByteStride / mBytesPerPixel, mHeight ); - DALI_ASSERT_DEBUG(mBitmap->GetBuffer() != 0); - DALI_ASSERT_DEBUG(mBitmap->GetBufferSize() >= mWidth * mHeight * Pixel::GetBytesPerPixel( mBitmap->GetPixelFormat() ) ); + // Check if we're doing the entire internal buffer ? + if( area.IsEmpty() ) + { + memcpy( bitmap->GetBuffer(), mInternalBuffer, bitmap->GetBufferSize() ); + } + else + { + UpdateBufferArea( mInternalBuffer, bitmap->GetBuffer(), area ); + } } + mResourceClient->UploadBitmap( destId, bitmap, area.x, area.y ); + } void BufferImage::UploadBitmap( ResourceId destId, std::size_t xOffset, std::size_t yOffset ) { - // Make sure we have a bitmap for transport - ReserveBitmap(); - - // Copy source pixel data into bitmap - RectArea area; - UpdateBitmap( area ); + RectArea area( xOffset, yOffset, 0, 0 ); + if ( !mTicket ) + { + CreateHostBitmap(); + } - mResourceClient->UploadBitmap( destId, mBitmap, xOffset, yOffset); - mBitmap = NULL; + UploadArea( destId, area ); } -void BufferImage::UpdateBufferArea( PixelBuffer* src, const RectArea& area ) +void BufferImage::UpdateBufferArea( PixelBuffer* src, PixelBuffer* dest, const RectArea& area ) { DALI_ASSERT_DEBUG( area.x + area.width <= mWidth && area.y + area.height <= mHeight ); - PixelBuffer* dest = mBitmap->GetBuffer(); - uint32_t width = area.width * mBytesPerPixel; - uint32_t stride = mWidth * mBytesPerPixel; + uint32_t width = mBufferWidth * mBytesPerPixel; src += ( area.y * mByteStride ) + ( area.x * mBytesPerPixel ); - dest +=( ( area.y * mWidth ) + area.x ) * mBytesPerPixel; for ( uint32_t i = 0; i < area.height; ++i ) { memcpy( dest, src, width ); src += mByteStride; - dest += stride; - } -} - -void BufferImage::MirrorExternal( const RectArea& area ) -{ - if( ( mByteStride == mWidth * mBytesPerPixel ) && area.IsEmpty() ) - { - memcpy( mBitmap->GetBuffer(), mExternalBuffer, mBufferSize ); - } - else - { - UpdateBufferArea( mExternalBuffer, area ); + dest += width; } } diff --git a/dali/internal/event/images/buffer-image-impl.h b/dali/internal/event/images/buffer-image-impl.h index fa67aec..72db2e2 100644 --- a/dali/internal/event/images/buffer-image-impl.h +++ b/dali/internal/event/images/buffer-image-impl.h @@ -204,25 +204,27 @@ protected: // From Image private: - void ValidateBitmap(); + void SetupBuffer( unsigned int width, + unsigned int height, + Pixel::Format pixelformat, + unsigned int byteStride, + ReleasePolicy releasePol ); - void ReserveBitmap(); + void CreateHostBitmap(); - void UpdateBitmap( RectArea& updateArea ); + void UploadArea( ResourceId destId, const RectArea& area ); - void MirrorExternal( const RectArea& area ); - - void UpdateBufferArea( PixelBuffer* src, const RectArea& area ); + void UpdateBufferArea( PixelBuffer* src, PixelBuffer* dest, const RectArea& area ); private: PixelBuffer* mInternalBuffer; ///< NULL if the data is supplied by an external buffer. PixelBuffer* mExternalBuffer; ///< NULL if there is no external pixel data (this is never owned by BufferImage). ResourceClient* mResourceClient; ///< pointer to the resource client. - Integration::Bitmap* mBitmap; ///< pointer to the bitmap object containing the pixel buffer used to update GL. uint32_t mBufferSize; ///< size of the pixel buffer. uint32_t mByteStride; ///< width of the pixel buffer in bytes. uint32_t mBytesPerPixel; ///< width of a pixel in bytes. + uint32_t mBufferWidth; ///< cached pixel width of bitmap used for transport. Pixel::Format mPixelFormat; ///< pixel format of bitmap. ResourcePolicy::Discardable mResourcePolicy; ///< whether to discard the pixel buffer when removed from the stage or to retain the data. }; -- 2.7.4