From 97f52415b16e17d22f166b19bf659941dbbed05c Mon Sep 17 00:00:00 2001 From: Kimmo Hoikka Date: Wed, 5 Dec 2018 18:17:16 +0000 Subject: [PATCH] Add an easier to use New in FrameBuffer for the 99% of the cases where we use offscreens - Deprecated the unsafe version which allows enum to be passed in as uint32_t Change-Id: I9c28f06d26c25ce1d84e840e7a46056d6760208a --- automated-tests/src/dali/utc-Dali-FrameBuffer.cpp | 80 ++++++++++++++++++++++ .../event/images/frame-buffer-image-impl.cpp | 11 ++- .../internal/event/rendering/frame-buffer-impl.cpp | 8 +-- dali/internal/event/rendering/frame-buffer-impl.h | 22 +++--- .../render/renderers/render-frame-buffer.cpp | 2 +- .../render/renderers/render-frame-buffer.h | 4 +- dali/public-api/rendering/frame-buffer.cpp | 23 ++++++- dali/public-api/rendering/frame-buffer.h | 46 ++++++++++--- 8 files changed, 164 insertions(+), 32 deletions(-) diff --git a/automated-tests/src/dali/utc-Dali-FrameBuffer.cpp b/automated-tests/src/dali/utc-Dali-FrameBuffer.cpp index 041e1bb..e19a7f8 100644 --- a/automated-tests/src/dali/utc-Dali-FrameBuffer.cpp +++ b/automated-tests/src/dali/utc-Dali-FrameBuffer.cpp @@ -140,6 +140,86 @@ int UtcDaliFrameBufferNew06(void) END_TEST; } +int UtcDaliFrameBufferNewWithColor01(void) +{ + TestApplication application; + uint32_t width = 64; + uint32_t height = 64; + FrameBuffer frameBuffer = FrameBuffer::New( width, height ); + application.SendNotification(); + application.Render(); + DALI_TEST_EQUALS(application.GetGlAbstraction().CheckFramebufferColorAttachment(), (GLenum)GL_TRUE, TEST_LOCATION); + DALI_TEST_EQUALS(application.GetGlAbstraction().CheckFramebufferDepthAttachment(), (GLenum)GL_FALSE, TEST_LOCATION); + DALI_TEST_EQUALS(application.GetGlAbstraction().CheckFramebufferStencilAttachment(), (GLenum)GL_FALSE, TEST_LOCATION); + // check that texture is not empty handle + DALI_TEST_CHECK( frameBuffer.GetColorTexture() ); + END_TEST; +} + +int UtcDaliFrameBufferNewWithColor02(void) +{ + TestApplication application; + uint32_t width = 64; + uint32_t height = 64; + FrameBuffer frameBuffer = FrameBuffer::New( width, height, FrameBuffer::Attachment::COLOR ); + application.SendNotification(); + application.Render(); + DALI_TEST_EQUALS(application.GetGlAbstraction().CheckFramebufferColorAttachment(), (GLenum)GL_TRUE, TEST_LOCATION); + DALI_TEST_EQUALS(application.GetGlAbstraction().CheckFramebufferDepthAttachment(), (GLenum)GL_FALSE, TEST_LOCATION); + DALI_TEST_EQUALS(application.GetGlAbstraction().CheckFramebufferStencilAttachment(), (GLenum)GL_FALSE, TEST_LOCATION); + // check that texture is not empty handle + DALI_TEST_CHECK( frameBuffer.GetColorTexture() ); + END_TEST; +} + +int UtcDaliFrameBufferNewWithColor03(void) +{ + TestApplication application; + uint32_t width = 64; + uint32_t height = 64; + FrameBuffer frameBuffer = FrameBuffer::New( width, height, FrameBuffer::Attachment::COLOR_DEPTH ); + application.SendNotification(); + application.Render(); + DALI_TEST_EQUALS(application.GetGlAbstraction().CheckFramebufferColorAttachment(), (GLenum)GL_TRUE, TEST_LOCATION); + DALI_TEST_EQUALS(application.GetGlAbstraction().CheckFramebufferDepthAttachment(), (GLenum)GL_TRUE, TEST_LOCATION); + DALI_TEST_EQUALS(application.GetGlAbstraction().CheckFramebufferStencilAttachment(), (GLenum)GL_FALSE, TEST_LOCATION); + // check that texture is not empty handle + DALI_TEST_CHECK( frameBuffer.GetColorTexture() ); + END_TEST; +} + +int UtcDaliFrameBufferNewWithColor04(void) +{ + TestApplication application; + uint32_t width = 64; + uint32_t height = 64; + FrameBuffer frameBuffer = FrameBuffer::New( width, height, FrameBuffer::Attachment::COLOR_STENCIL ); + application.SendNotification(); + application.Render(); + DALI_TEST_EQUALS(application.GetGlAbstraction().CheckFramebufferColorAttachment(), (GLenum)GL_TRUE, TEST_LOCATION); + DALI_TEST_EQUALS(application.GetGlAbstraction().CheckFramebufferDepthAttachment(), (GLenum)GL_FALSE, TEST_LOCATION); + DALI_TEST_EQUALS(application.GetGlAbstraction().CheckFramebufferStencilAttachment(), (GLenum)GL_TRUE, TEST_LOCATION); + // check that texture is not empty handle + DALI_TEST_CHECK( frameBuffer.GetColorTexture() ); + END_TEST; +} + +int UtcDaliFrameBufferNewWithColor05(void) +{ + TestApplication application; + uint32_t width = 64; + uint32_t height = 64; + FrameBuffer frameBuffer = FrameBuffer::New( width, height, FrameBuffer::Attachment::COLOR_DEPTH_STENCIL ); + application.SendNotification(); + application.Render(); + DALI_TEST_EQUALS(application.GetGlAbstraction().CheckFramebufferColorAttachment(), (GLenum)GL_TRUE, TEST_LOCATION); + DALI_TEST_EQUALS(application.GetGlAbstraction().CheckFramebufferDepthAttachment(), (GLenum)GL_TRUE, TEST_LOCATION); + DALI_TEST_EQUALS(application.GetGlAbstraction().CheckFramebufferStencilAttachment(), (GLenum)GL_TRUE, TEST_LOCATION); + // check that texture is not empty handle + DALI_TEST_CHECK( frameBuffer.GetColorTexture() ); + END_TEST; +} + int UtcDaliFrameBufferCopyConstructor(void) { TestApplication application; diff --git a/dali/internal/event/images/frame-buffer-image-impl.cpp b/dali/internal/event/images/frame-buffer-image-impl.cpp index def730a..b5a4439 100644 --- a/dali/internal/event/images/frame-buffer-image-impl.cpp +++ b/dali/internal/event/images/frame-buffer-image-impl.cpp @@ -30,12 +30,11 @@ namespace Internal namespace { -const int RenderBufferFormatToFrameBufferAttachments[] = { Dali::FrameBuffer::Attachment::NONE, - Dali::FrameBuffer::Attachment::DEPTH, - Dali::FrameBuffer::Attachment::STENCIL, - Dali::FrameBuffer::Attachment::DEPTH_STENCIL - }; - +const Dali::FrameBuffer::Attachment::Mask RenderBufferFormatToFrameBufferAttachments[] = + { Dali::FrameBuffer::Attachment::NONE, + Dali::FrameBuffer::Attachment::DEPTH, + Dali::FrameBuffer::Attachment::STENCIL, + Dali::FrameBuffer::Attachment::DEPTH_STENCIL }; } // unnamed namespace FrameBufferImagePtr FrameBufferImage::New( unsigned int width, diff --git a/dali/internal/event/rendering/frame-buffer-impl.cpp b/dali/internal/event/rendering/frame-buffer-impl.cpp index 20d09d3..ccfc8d0 100644 --- a/dali/internal/event/rendering/frame-buffer-impl.cpp +++ b/dali/internal/event/rendering/frame-buffer-impl.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017 Samsung Electronics Co., Ltd. + * Copyright (c) 2018 Samsung Electronics Co., Ltd. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -28,7 +28,7 @@ namespace Dali namespace Internal { -FrameBufferPtr FrameBuffer::New( unsigned int width, unsigned int height, unsigned int attachments ) +FrameBufferPtr FrameBuffer::New( uint32_t width, uint32_t height, Mask attachments ) { FrameBufferPtr frameBuffer( new FrameBuffer( width, height, attachments ) ); frameBuffer->Initialize(); @@ -41,7 +41,7 @@ Render::FrameBuffer* FrameBuffer::GetRenderObject() const return mRenderObject; } -FrameBuffer::FrameBuffer( unsigned int width, unsigned int height, unsigned int attachments ) +FrameBuffer::FrameBuffer( uint32_t width, uint32_t height, Mask attachments ) : mEventThreadServices( *Stage::GetCurrent() ), mRenderObject( NULL ), mColor( NULL ), @@ -57,7 +57,7 @@ void FrameBuffer::Initialize() AddFrameBuffer( mEventThreadServices.GetUpdateManager(), *mRenderObject ); } -void FrameBuffer::AttachColorTexture( TexturePtr texture, unsigned int mipmapLevel, unsigned int layer ) +void FrameBuffer::AttachColorTexture( TexturePtr texture, uint32_t mipmapLevel, uint32_t layer ) { if( ( texture->GetWidth() / ( 1u << mipmapLevel ) == mWidth ) && ( texture->GetHeight() / ( 1u << mipmapLevel ) == mHeight ) ) diff --git a/dali/internal/event/rendering/frame-buffer-impl.h b/dali/internal/event/rendering/frame-buffer-impl.h index 8b0f776..9389193 100644 --- a/dali/internal/event/rendering/frame-buffer-impl.h +++ b/dali/internal/event/rendering/frame-buffer-impl.h @@ -2,7 +2,7 @@ #define DALI_INTERNAL_FRAME_BUFFER_H /* - * Copyright (c) 2016 Samsung Electronics Co., Ltd. + * Copyright (c) 2018 Samsung Electronics Co., Ltd. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -42,6 +42,8 @@ class FrameBuffer : public BaseObject { public: + using Mask = Dali::FrameBuffer::Attachment::Mask; + /** * @brief Create a new FrameBuffer * @@ -50,7 +52,7 @@ public: * @param[in] attachments The attachments comprising the format of the FrameBuffer (bit-mask) * @return A smart-pointer to the newly allocated Texture. */ - static FrameBufferPtr New( unsigned int width, unsigned int height, unsigned int attachments ); + static FrameBufferPtr New( uint32_t width, uint32_t height, Mask attachments ); /** * @brief Get the FrameBuffer render object @@ -62,7 +64,7 @@ public: /** * @copydoc Dali::FrameBuffer::AttachColorTexture() */ - void AttachColorTexture( TexturePtr texture, unsigned int mipmapLevel, unsigned int layer ); + void AttachColorTexture( TexturePtr texture, uint32_t mipmapLevel, uint32_t layer ); /** * @copydoc Dali::FrameBuffer::GetColorTexture() @@ -77,7 +79,7 @@ private: // implementation * @param[in] height The height of the FrameBuffer * @param[in] attachments The attachments comprising the format of the FrameBuffer (bit-mask) */ - FrameBuffer( unsigned int width, unsigned int height, unsigned int attachments ); + FrameBuffer( uint32_t width, uint32_t height, Mask attachments ); /** * Second stage initialization of the Texture @@ -92,8 +94,10 @@ protected: virtual ~FrameBuffer(); private: // unimplemented methods - FrameBuffer( const FrameBuffer& ); - FrameBuffer& operator=( const FrameBuffer& ); + + FrameBuffer() = delete; + FrameBuffer( const FrameBuffer& ) = delete; + FrameBuffer& operator=( const FrameBuffer& ) = delete; private: // data @@ -101,9 +105,9 @@ private: // data Internal::Render::FrameBuffer* mRenderObject; ///< The Render::Texture associated to this texture TexturePtr mColor; - unsigned int mWidth; - unsigned int mHeight; - unsigned int mAttachments; ///< Bit-mask of type FrameBuffer::Attachment::Mask + uint32_t mWidth; + uint32_t mHeight; + Mask mAttachments; ///< Bit-mask of type FrameBuffer::Attachment::Mask }; diff --git a/dali/internal/render/renderers/render-frame-buffer.cpp b/dali/internal/render/renderers/render-frame-buffer.cpp index a9d351c..2ea746e 100644 --- a/dali/internal/render/renderers/render-frame-buffer.cpp +++ b/dali/internal/render/renderers/render-frame-buffer.cpp @@ -27,7 +27,7 @@ namespace Internal namespace Render { -FrameBuffer::FrameBuffer( uint32_t width, uint32_t height, uint32_t attachments ) +FrameBuffer::FrameBuffer( uint32_t width, uint32_t height, Mask attachments ) :mId( 0u ), mDepthBuffer( attachments & Dali::FrameBuffer::Attachment::DEPTH ), mStencilBuffer( attachments & Dali::FrameBuffer::Attachment::STENCIL ), diff --git a/dali/internal/render/renderers/render-frame-buffer.h b/dali/internal/render/renderers/render-frame-buffer.h index 3cf7110..196630d 100644 --- a/dali/internal/render/renderers/render-frame-buffer.h +++ b/dali/internal/render/renderers/render-frame-buffer.h @@ -35,13 +35,15 @@ class FrameBuffer { public: + using Mask = Dali::FrameBuffer::Attachment::Mask; + /** * Constructor * @param[in] width The width of the FrameBuffer * @param[in] height The height of the FrameBuffer * @param[in] attachments The attachments comprising the format of the FrameBuffer (bit-mask) */ - FrameBuffer( uint32_t width, uint32_t height, uint32_t attachments ); + FrameBuffer( uint32_t width, uint32_t height, Mask attachments ); /** * Destructor diff --git a/dali/public-api/rendering/frame-buffer.cpp b/dali/public-api/rendering/frame-buffer.cpp index 607a79b..0b86092 100644 --- a/dali/public-api/rendering/frame-buffer.cpp +++ b/dali/public-api/rendering/frame-buffer.cpp @@ -19,15 +19,34 @@ #include // INTERNAL INCLUDES +#include // DALI_LOG_WARNING_NOFN #include // Dali::Internal::FrameBuffer - +#include // Dali::Internal::Texture namespace Dali { -FrameBuffer FrameBuffer::New( uint32_t width, uint32_t height, uint32_t attachments ) +FrameBuffer FrameBuffer::New( uint32_t width, uint32_t height ) +{ + return New( width, height, FrameBuffer::Attachment::COLOR ); +} + +FrameBuffer FrameBuffer::New( uint32_t width, uint32_t height, Attachment::Mask attachments ) { Internal::FrameBufferPtr frameBuffer = Internal::FrameBuffer::New( width, height, attachments ); + if( attachments & FrameBuffer::Attachment::COLOR ) + { + Internal::TexturePtr texture = Internal::Texture::New( Dali::TextureType::TEXTURE_2D, Pixel::RGB888, width, height ); + frameBuffer->AttachColorTexture( texture, 0u, 0u ); + } + return FrameBuffer( frameBuffer.Get() ); +} + +FrameBuffer FrameBuffer::New( uint32_t width, uint32_t height, uint32_t attachments ) +{ + DALI_LOG_WARNING_NOFN("DEPRECATION WARNING: FrameBuffer::New(uint32_t,uint32_t,uint32_t) is deprecated and will be removed from next release. use New(uint32_t, uint32_t,Attachment::Mask) instead.\n" ); + // have to static cast, which according to standard since C++11 is undefined behaviour, hence this variant is deprecated + Internal::FrameBufferPtr frameBuffer = Internal::FrameBuffer::New( width, height, static_cast( attachments ) ); return FrameBuffer( frameBuffer.Get() ); } diff --git a/dali/public-api/rendering/frame-buffer.h b/dali/public-api/rendering/frame-buffer.h index 30c5e6d..b80630c 100644 --- a/dali/public-api/rendering/frame-buffer.h +++ b/dali/public-api/rendering/frame-buffer.h @@ -43,7 +43,7 @@ public: /** * @brief The initial attachments to create the FrameBuffer with. - * @note The color attachment is created on calling AttachColorTexture(). If a color attachment is not required, omit this call. + * @note The color attachment can also be created on calling AttachColorTexture(). * @note With "NONE", no attachments are created initially. However color attachments can still be added as described above. * * @SINCE_1_1.45 @@ -51,25 +51,53 @@ public: struct Attachment { /** - * @brief Enumeration for the bit-mask value. + * @brief Enumeration for the attachments and/or textures to be created by default * @SINCE_1_1.45 */ enum Mask { - NONE = 0, ///< No attachments are created initially @SINCE_1_1.45 - - DEPTH = 1 << 0, ///< Depth buffer bit-mask value @SINCE_1_1.45 - STENCIL = 1 << 1, ///< Stencil buffer bit-mask value @SINCE_1_1.45 - - // Preset bit-mask combinations: - DEPTH_STENCIL = DEPTH | STENCIL ///< The Framebuffer will be created with depth and stencil buffer @SINCE_1_1.45 + NONE = 0, ///< No attachments are created initially @SINCE_1_1.45 + DEPTH = 1 << 0, ///< Depth buffer is created @SINCE_1_1.45 + STENCIL = 1 << 1, ///< Stencil buffer is created @SINCE_1_1.45 + DEPTH_STENCIL = DEPTH | STENCIL, ///< Depth and stencil buffer will be created @SINCE_1_1.45 + COLOR = 1 << 2, ///< Color texture will be created @SINCE_1_3.54 + COLOR_DEPTH = COLOR | DEPTH, ///< Color texture and depth buffer will be created @SINCE_1_3.54 + COLOR_STENCIL = COLOR | STENCIL, ///< Color texture and stencil buffer will be created @SINCE_1_3.54 + COLOR_DEPTH_STENCIL = COLOR_DEPTH | STENCIL ///< Color, depth and stencil buffer will be created @SINCE_1_3.54 }; }; /** + * @brief Creates a new FrameBuffer with only COLOR texture attached on it + * + * @SINCE_1_3.54 + * + * @note Call GetColorTexture() to get the COLOR texture + * + * @param[in] width The width of the FrameBuffer and the color texture + * @param[in] height The height of the FrameBuffer and the color texture + * @return A handle to a newly allocated FrameBuffer + */ + static FrameBuffer New( uint32_t width, uint32_t height ); + + /** + * @brief Creates a new FrameBuffer with the specified attachments + * + * @SINCE_1_3.54 + * + * @param[in] width The width of the FrameBuffer and the attachments + * @param[in] height The height of the FrameBuffer and the attachments + * @param[in] attachments Enumeration of the attachments to create + * @return A handle to a newly allocated FrameBuffer + */ + static FrameBuffer New( uint32_t width, uint32_t height, Attachment::Mask attachments ); + + /** + * @DEPRECATED_1_3.54 use New( uint32_t width, uint32_t height ) or New( uint32_t width, uint32_t height, Attachment::Mask attachments ) instead * @brief Creates a new FrameBuffer object. * * @SINCE_1_1.43 + * * @param[in] width The width of the FrameBuffer * @param[in] height The height of the FrameBuffer * @param[in] attachments The attachments comprising the format of the FrameBuffer (the type is int to allow multiple bitmasks to be ORd) -- 2.7.4