From c6c3a0e33492b5a7753072cf20505f0c44a22e49 Mon Sep 17 00:00:00 2001 From: Richard Huang Date: Mon, 8 Jul 2019 13:57:37 +0100 Subject: [PATCH] Synchronize the window removal between main thread and render thread Change-Id: I2ad9c459029c648d66d1dca373f5fd906ec0d6c8 --- dali/integration-api/scene-holder-impl.cpp | 3 + dali/internal/adaptor/common/adaptor-impl.cpp | 10 +++ dali/internal/adaptor/common/adaptor-impl.h | 6 ++ .../common/combined-update-render-controller.cpp | 86 +++++++++++++++++++--- .../common/combined-update-render-controller.h | 21 ++++++ .../adaptor/common/thread-controller-interface.h | 6 ++ dali/internal/system/common/thread-controller.cpp | 5 ++ dali/internal/system/common/thread-controller.h | 6 ++ .../window-system/common/window-render-surface.cpp | 5 ++ .../native-render-surface-ecore-wl.cpp | 7 +- .../ubuntu-x11/pixmap-render-surface-ecore-x.cpp | 4 +- 11 files changed, 145 insertions(+), 14 deletions(-) diff --git a/dali/integration-api/scene-holder-impl.cpp b/dali/integration-api/scene-holder-impl.cpp index 64d0a42..491c5b4 100644 --- a/dali/integration-api/scene-holder-impl.cpp +++ b/dali/integration-api/scene-holder-impl.cpp @@ -131,6 +131,9 @@ SceneHolder::~SceneHolder() { mAdaptor->RemoveObserver( *mLifeCycleObserver.get() ); mAdaptor->RemoveWindow( this ); + + mAdaptor->DeleteSurface( *mSurface.get() ); + mAdaptor = nullptr; } diff --git a/dali/internal/adaptor/common/adaptor-impl.cpp b/dali/internal/adaptor/common/adaptor-impl.cpp index d1c1a5d..a4ab857 100755 --- a/dali/internal/adaptor/common/adaptor-impl.cpp +++ b/dali/internal/adaptor/common/adaptor-impl.cpp @@ -523,6 +523,16 @@ void Adaptor::ReplaceSurface( Dali::Integration::SceneHolder window, Dali::Rende } } +void Adaptor::DeleteSurface( Dali::RenderSurfaceInterface& surface ) +{ + // Flush the event queue to give the update-render thread chance + // to start processing messages for new camera setup etc as soon as possible + ProcessCoreEvents(); + + // This method blocks until the render thread has finished rendering the current surface. + mThreadController->DeleteSurface( &surface ); +} + Dali::RenderSurfaceInterface& Adaptor::GetSurface() const { return *mWindows.front()->GetSurface(); diff --git a/dali/internal/adaptor/common/adaptor-impl.h b/dali/internal/adaptor/common/adaptor-impl.h index 37a7438..a716bea 100755 --- a/dali/internal/adaptor/common/adaptor-impl.h +++ b/dali/internal/adaptor/common/adaptor-impl.h @@ -295,6 +295,12 @@ public: // AdaptorInternalServices implementation bool RemoveWindow( Dali::Internal::Adaptor::SceneHolder* childWindow ); /** + * @brief Deletes the rendering surface + * @param[in] surface to delete + */ + void DeleteSurface( Dali::RenderSurfaceInterface& surface ); + + /** * @brief Retrieve the window that the given actor is added to. * * @param[in] actor The actor diff --git a/dali/internal/adaptor/common/combined-update-render-controller.cpp b/dali/internal/adaptor/common/combined-update-render-controller.cpp index 4486766..aae3340 100644 --- a/dali/internal/adaptor/common/combined-update-render-controller.cpp +++ b/dali/internal/adaptor/common/combined-update-render-controller.cpp @@ -112,6 +112,7 @@ CombinedUpdateRenderController::CombinedUpdateRenderController( AdaptorInternalS mPendingRequestUpdate( FALSE ), mUseElapsedTimeAfterWait( FALSE ), mNewSurface( NULL ), + mDeletedSurface( nullptr ), mPostRendering( FALSE ), mSurfaceResized( FALSE ), mForceClear( FALSE ) @@ -294,23 +295,49 @@ void CombinedUpdateRenderController::ReplaceSurface( Dali::RenderSurfaceInterfac { LOG_EVENT_TRACE; - // Set the ThreadSyncronizationInterface on the new surface - newSurface->SetThreadSynchronization( *this ); + if( mUpdateRenderThread ) + { + // Set the ThreadSyncronizationInterface on the new surface + newSurface->SetThreadSynchronization( *this ); - LOG_EVENT( "Starting to replace the surface, event-thread blocked" ); + LOG_EVENT( "Starting to replace the surface, event-thread blocked" ); - // Start replacing the surface. - { - ConditionalWait::ScopedLock lock( mUpdateRenderThreadWaitCondition ); - mPostRendering = FALSE; // Clear the post-rendering flag as Update/Render thread will replace the surface now - mNewSurface = newSurface; - mUpdateRenderThreadWaitCondition.Notify( lock ); + // Start replacing the surface. + { + ConditionalWait::ScopedLock lock( mUpdateRenderThreadWaitCondition ); + mPostRendering = FALSE; // Clear the post-rendering flag as Update/Render thread will replace the surface now + mNewSurface = newSurface; + mUpdateRenderThreadWaitCondition.Notify( lock ); + } + + // Wait until the surface has been replaced + sem_wait( &mEventThreadSemaphore ); + + LOG_EVENT( "Surface replaced, event-thread continuing" ); } +} - // Wait until the surface has been replaced - sem_wait( &mEventThreadSemaphore ); +void CombinedUpdateRenderController::DeleteSurface( Dali::RenderSurfaceInterface* surface ) +{ + LOG_EVENT_TRACE; + + if( mUpdateRenderThread ) + { + LOG_EVENT( "Starting to delete the surface, event-thread blocked" ); + + // Start replacing the surface. + { + ConditionalWait::ScopedLock lock( mUpdateRenderThreadWaitCondition ); + mPostRendering = FALSE; // Clear the post-rendering flag as Update/Render thread will delete the surface now + mDeletedSurface = surface; + mUpdateRenderThreadWaitCondition.Notify( lock ); + } + + // Wait until the surface has been deleted + sem_wait( &mEventThreadSemaphore ); - LOG_EVENT( "Surface replaced, event-thread continuing" ); + LOG_EVENT( "Surface deleted, event-thread continuing" ); + } } void CombinedUpdateRenderController::ResizeSurface() @@ -608,6 +635,21 @@ void CombinedUpdateRenderController::UpdateRenderThread() AddPerformanceMarker( PerformanceInterface::RENDER_START ); mCore.Render( renderStatus, mForceClear ); + + ////////////////////////////// + // DELETE SURFACE + ////////////////////////////// + + Integration::RenderSurface* deletedSurface = ShouldSurfaceBeDeleted(); + if( DALI_UNLIKELY( deletedSurface ) ) + { + LOG_UPDATE_RENDER_TRACE_FMT( "Deleting Surface" ); + + mCore.SurfaceDeleted( deletedSurface ); + + SurfaceDeleted(); + } + AddPerformanceMarker( PerformanceInterface::RENDER_END ); mForceClear = false; @@ -692,12 +734,14 @@ bool CombinedUpdateRenderController::UpdateRenderReady( bool& useElapsedTime, bo ( mUpdateRenderThreadCanSleep && ! updateRequired && ! mPendingRequestUpdate ) ) && // Ensure we wait if we're supposed to be sleeping AND do not require another update ! mDestroyUpdateRenderThread && // Ensure we don't wait if the update-render-thread is supposed to be destroyed ! mNewSurface && // Ensure we don't wait if we need to replace the surface + ! mDeletedSurface && // Ensure we don't wait if we need to delete the surface ! mSurfaceResized ) // Ensure we don't wait if we need to resize the surface { LOG_UPDATE_RENDER( "WAIT: mUpdateRenderRunCount: %d", mUpdateRenderRunCount ); LOG_UPDATE_RENDER( " mUpdateRenderThreadCanSleep: %d, updateRequired: %d, mPendingRequestUpdate: %d", mUpdateRenderThreadCanSleep, updateRequired, mPendingRequestUpdate ); LOG_UPDATE_RENDER( " mDestroyUpdateRenderThread: %d", mDestroyUpdateRenderThread ); LOG_UPDATE_RENDER( " mNewSurface: %d", mNewSurface ); + LOG_UPDATE_RENDER( " mDeletedSurface: %d", mDeletedSurface ); LOG_UPDATE_RENDER( " mSurfaceResized: %d", mSurfaceResized ); // Reset the time when the thread is waiting, so the sleep-until time for @@ -717,6 +761,7 @@ bool CombinedUpdateRenderController::UpdateRenderReady( bool& useElapsedTime, bo LOG_COUNTER_UPDATE_RENDER( "mUpdateRenderThreadCanSleep: %d, updateRequired: %d, mPendingRequestUpdate: %d", mUpdateRenderThreadCanSleep, updateRequired, mPendingRequestUpdate ); LOG_COUNTER_UPDATE_RENDER( "mDestroyUpdateRenderThread: %d", mDestroyUpdateRenderThread ); LOG_COUNTER_UPDATE_RENDER( "mNewSurface: %d", mNewSurface ); + LOG_COUNTER_UPDATE_RENDER( "mDeletedSurface: %d", mDeletedSurface ); LOG_COUNTER_UPDATE_RENDER( "mSurfaceResized: %d", mSurfaceResized ); mUseElapsedTimeAfterWait = FALSE; @@ -750,6 +795,22 @@ void CombinedUpdateRenderController::SurfaceReplaced() sem_post( &mEventThreadSemaphore ); } +Integration::RenderSurface* CombinedUpdateRenderController::ShouldSurfaceBeDeleted() +{ + ConditionalWait::ScopedLock lock( mUpdateRenderThreadWaitCondition ); + + Integration::RenderSurface* deletedSurface = mDeletedSurface; + mDeletedSurface = NULL; + + return deletedSurface; +} + +void CombinedUpdateRenderController::SurfaceDeleted() +{ + // Just increment the semaphore + sem_post( &mEventThreadSemaphore ); +} + bool CombinedUpdateRenderController::ShouldSurfaceBeResized() { ConditionalWait::ScopedLock lock( mUpdateRenderThreadWaitCondition ); @@ -806,6 +867,7 @@ void CombinedUpdateRenderController::PostRenderWaitForCompletion() ConditionalWait::ScopedLock lock( mUpdateRenderThreadWaitCondition ); while( mPostRendering && ! mNewSurface && // We should NOT wait if we're replacing the surface + ! mDeletedSurface && // We should NOT wait if we're deleting the surface ! mSurfaceResized && // We should NOT wait if we're resizing the surface ! mDestroyUpdateRenderThread ) { diff --git a/dali/internal/adaptor/common/combined-update-render-controller.h b/dali/internal/adaptor/common/combined-update-render-controller.h index a8cb5f2..41f519c 100644 --- a/dali/internal/adaptor/common/combined-update-render-controller.h +++ b/dali/internal/adaptor/common/combined-update-render-controller.h @@ -127,6 +127,11 @@ public: virtual void ReplaceSurface( Dali::RenderSurfaceInterface* surface ); /** + * @copydoc ThreadControllerInterface::DeleteSurface() + */ + virtual void DeleteSurface( Dali::RenderSurfaceInterface* surface ); + + /** * @copydoc ThreadControllerInterface::ResizeSurface() */ virtual void ResizeSurface(); @@ -227,6 +232,21 @@ private: void SurfaceReplaced(); /** + * Checks to see if the surface needs to be deleted. + * This will lock the mutex in mUpdateRenderThreadWaitCondition. + * + * @return Pointer to the deleted surface, nullptr otherwise + */ + Integration::RenderSurface* ShouldSurfaceBeDeleted(); + + /** + * Called by the Update/Render thread after a surface has been deleted. + * + * This will lock the mutex in mEventThreadWaitCondition + */ + void SurfaceDeleted(); + + /** * Checks to see if the surface needs to be resized. * This will lock the mutex in mUpdateRenderThreadWaitCondition. * @@ -336,6 +356,7 @@ private: volatile unsigned int mUseElapsedTimeAfterWait; ///< Whether we should use the elapsed time after waiting (set by the event-thread, read by the update-render-thread). Integration::RenderSurface* volatile mNewSurface; ///< Will be set to the new-surface if requested (set by the event-thread, read & cleared by the update-render thread). + Integration::RenderSurface* volatile mDeletedSurface; ///< Will be set to the deleted surface if requested (set by the event-thread, read & cleared by the update-render thread). volatile unsigned int mPostRendering; ///< Whether post-rendering is taking place (set by the event & render threads, read by the render-thread). volatile unsigned int mSurfaceResized; ///< Will be set to resize the surface (set by the event-thread, read & cleared by the update-render thread). diff --git a/dali/internal/adaptor/common/thread-controller-interface.h b/dali/internal/adaptor/common/thread-controller-interface.h index d805ced..751dceb 100644 --- a/dali/internal/adaptor/common/thread-controller-interface.h +++ b/dali/internal/adaptor/common/thread-controller-interface.h @@ -86,6 +86,12 @@ public: virtual void ReplaceSurface( Dali::RenderSurfaceInterface* surface ) = 0; /** + * Deletes the surface. + * @param[in] surface The surface to be deleted + */ + virtual void DeleteSurface( Dali::RenderSurfaceInterface* surface ) = 0; + + /** * Resize the surface. */ virtual void ResizeSurface() = 0; diff --git a/dali/internal/system/common/thread-controller.cpp b/dali/internal/system/common/thread-controller.cpp index 3d70efe..ac7e49e 100644 --- a/dali/internal/system/common/thread-controller.cpp +++ b/dali/internal/system/common/thread-controller.cpp @@ -90,6 +90,11 @@ void ThreadController::ReplaceSurface( Dali::RenderSurfaceInterface* newSurface mThreadControllerInterface->ReplaceSurface( newSurface ); } +void ThreadController::DeleteSurface( Dali::RenderSurfaceInterface* surface ) +{ + mThreadControllerInterface->DeleteSurface( surface ); +} + void ThreadController::ResizeSurface() { mThreadControllerInterface->ResizeSurface(); diff --git a/dali/internal/system/common/thread-controller.h b/dali/internal/system/common/thread-controller.h index 8e13370..4a7b81e 100644 --- a/dali/internal/system/common/thread-controller.h +++ b/dali/internal/system/common/thread-controller.h @@ -108,6 +108,12 @@ public: void ReplaceSurface( Dali::RenderSurfaceInterface* surface ); /** + * Deletes the surface. + * @param surface The surface to be deleted + */ + void DeleteSurface( Dali::RenderSurfaceInterface* surface ); + + /** * Resize the surface. */ void ResizeSurface(); diff --git a/dali/internal/window-system/common/window-render-surface.cpp b/dali/internal/window-system/common/window-render-surface.cpp index 18ae128..f9eb552 100644 --- a/dali/internal/window-system/common/window-render-surface.cpp +++ b/dali/internal/window-system/common/window-render-surface.cpp @@ -83,6 +83,11 @@ WindowRenderSurface::~WindowRenderSurface() { delete mRotationTrigger; } + + if ( mEGLSurface ) + { + DestroySurface(); + } } void WindowRenderSurface::Initialize( Any surface ) diff --git a/dali/internal/window-system/tizen-wayland/native-render-surface-ecore-wl.cpp b/dali/internal/window-system/tizen-wayland/native-render-surface-ecore-wl.cpp index dd35dcb..54004b1 100644 --- a/dali/internal/window-system/tizen-wayland/native-render-surface-ecore-wl.cpp +++ b/dali/internal/window-system/tizen-wayland/native-render-surface-ecore-wl.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018 Samsung Electronics Co., Ltd. + * Copyright (c) 2019 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. @@ -76,6 +76,11 @@ NativeRenderSurfaceEcoreWl::NativeRenderSurfaceEcoreWl( Dali::PositionSize posit NativeRenderSurfaceEcoreWl::~NativeRenderSurfaceEcoreWl() { + if ( mEGLSurface ) + { + DestroySurface(); + } + // release the surface if we own one if( mOwnSurface ) { diff --git a/dali/internal/window-system/ubuntu-x11/pixmap-render-surface-ecore-x.cpp b/dali/internal/window-system/ubuntu-x11/pixmap-render-surface-ecore-x.cpp index c8e049c..514b049 100644 --- a/dali/internal/window-system/ubuntu-x11/pixmap-render-surface-ecore-x.cpp +++ b/dali/internal/window-system/ubuntu-x11/pixmap-render-surface-ecore-x.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018 Samsung Electronics Co., Ltd. + * Copyright (c) 2019 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. @@ -81,6 +81,8 @@ PixmapRenderSurfaceEcoreX::PixmapRenderSurfaceEcoreX( Dali::PositionSize positio PixmapRenderSurfaceEcoreX::~PixmapRenderSurfaceEcoreX() { + DestroySurface(); + // release the surface if we own one if( mOwnSurface ) { -- 2.7.4