From 0eb9f4acb6555a2ee7c80c2bff92a0bba91d77cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Gr=C3=A9goire?= Date: Thu, 18 Dec 2025 15:44:58 +0100 Subject: [PATCH] Fix frame image race condition + refactor - In the connection state, retrieve the FrameImage while owning the data lock. - Use actual image data pointer as caching key instead of the address of ImageCache which may change during executation (unstable). - Fixes scale Messages image tooltip scale. - Free the connection image --- profiler/src/profiler/TracyView.cpp | 22 +++++++++++++++- profiler/src/profiler/TracyView.hpp | 14 ++++++---- .../profiler/TracyView_ConnectionState.cpp | 26 +++++-------------- .../src/profiler/TracyView_FrameOverview.cpp | 15 +---------- .../src/profiler/TracyView_FrameTimeline.cpp | 16 +----------- profiler/src/profiler/TracyView_Messages.cpp | 15 +---------- 6 files changed, 40 insertions(+), 68 deletions(-) diff --git a/profiler/src/profiler/TracyView.cpp b/profiler/src/profiler/TracyView.cpp index 5ffbdf4d..684116ca 100644 --- a/profiler/src/profiler/TracyView.cpp +++ b/profiler/src/profiler/TracyView.cpp @@ -126,7 +126,9 @@ View::~View() if( m_compare.loadThread.joinable() ) m_compare.loadThread.join(); if( m_saveThread.joinable() ) m_saveThread.join(); - if( m_frameTexture ) FreeTexture( m_frameTexture, m_cbMainThread ); + if( m_FrameTextureCache.textureId ) FreeTexture( m_FrameTextureCache.textureId, m_cbMainThread ); + if( m_FrameTextureCacheConnection.textureId ) FreeTexture( m_FrameTextureCacheConnection.textureId, m_cbMainThread ); + if( m_playback.texture ) FreeTexture( m_playback.texture, m_cbMainThread ); } @@ -1359,6 +1361,24 @@ bool View::DrawImpl() return keepOpen; } +void View::DrawFrameImage( FrameImageCache& cache, const FrameImage& fi, float scale ) +{ + if ( fi.ptr != cache.dataPtr ) + { + if( !cache.textureId ) cache.textureId = MakeTexture(); + UpdateTexture( cache.textureId, m_worker.UnpackFrameImage( fi ), fi.w, fi.h ); + cache.dataPtr = fi.ptr; + } + if( fi.flip ) + { + ImGui::Image( cache.textureId, ImVec2( fi.w * scale, fi.h * scale ), ImVec2( 0, 1 ), ImVec2( 1, 0 ) ); + } + else + { + ImGui::Image( cache.textureId, ImVec2( fi.w * scale, fi.h * scale ) ); + } +} + void View::DrawTextEditor() { const auto scale = GetScale(); diff --git a/profiler/src/profiler/TracyView.hpp b/profiler/src/profiler/TracyView.hpp index 969347e4..ce8c3605 100644 --- a/profiler/src/profiler/TracyView.hpp +++ b/profiler/src/profiler/TracyView.hpp @@ -106,6 +106,12 @@ class View int64_t total; uint16_t threadNum; }; + + struct FrameImageCache + { + ImTextureID textureId = 0; + const void* dataPtr = nullptr; + }; public: struct PlotView @@ -247,6 +253,7 @@ private: void Achieve( const char* id ); bool DrawImpl(); + void DrawFrameImage( FrameImageCache& cache, const FrameImage& fi, float scale = GetScale() ); void DrawNotificationArea(); bool DrawConnection(); void DrawFrames(); @@ -619,11 +626,8 @@ private: std::atomic m_srcFileBytes { 0 }; std::atomic m_dstFileBytes { 0 }; - ImTextureID m_frameTexture = 0; - const void* m_frameTexturePtr = nullptr; - - ImTextureID m_frameTextureConn = 0; - const void* m_frameTextureConnPtr = nullptr; + FrameImageCache m_FrameTextureCache; + FrameImageCache m_FrameTextureCacheConnection; std::vector> m_annotations; UserData m_userData; diff --git a/profiler/src/profiler/TracyView_ConnectionState.cpp b/profiler/src/profiler/TracyView_ConnectionState.cpp index 77c18ac0..1f69ae70 100644 --- a/profiler/src/profiler/TracyView_ConnectionState.cpp +++ b/profiler/src/profiler/TracyView_ConnectionState.cpp @@ -77,6 +77,7 @@ bool View::DrawConnection() } } + FrameImage lastFrameImage{}; { Worker::MainThreadDataLockGuard lock = m_worker.ObtainLockForMainThread(); ImGui::SameLine(); @@ -91,29 +92,16 @@ bool View::DrawConnection() ImGui::Text( "%6.1f", fps ); ImGui::SameLine(); TextFocused( "Frame time:", TimeToString( dt ) ); - } + } + const auto& fis = m_worker.GetFrameImages(); + // Keep a copy here since the worker may modify the frame images vector while we do not own the lock + if( !fis.empty() ) lastFrameImage = *fis.back(); } - const auto& fis = m_worker.GetFrameImages(); - if( !fis.empty() ) + if( lastFrameImage.ptr.get() ) { - const auto fiScale = scale * 0.5f; - const auto& fi = fis.back(); - if( fi != m_frameTextureConnPtr ) - { - if( !m_frameTextureConn ) m_frameTextureConn = MakeTexture(); - UpdateTexture( m_frameTextureConn, m_worker.UnpackFrameImage( *fi ), fi->w, fi->h ); - m_frameTextureConnPtr = fi; - } ImGui::Separator(); - if( fi->flip ) - { - ImGui::Image( m_frameTextureConn, ImVec2( fi->w * fiScale, fi->h * fiScale ), ImVec2( 0, 1 ), ImVec2( 1, 0 ) ); - } - else - { - ImGui::Image( m_frameTextureConn, ImVec2( fi->w * fiScale, fi->h * fiScale ) ); - } + DrawFrameImage( m_FrameTextureCacheConnection, lastFrameImage, scale * 0.5f ); } ImGui::Separator(); diff --git a/profiler/src/profiler/TracyView_FrameOverview.cpp b/profiler/src/profiler/TracyView_FrameOverview.cpp index bdb9b9e3..1eb63fe4 100644 --- a/profiler/src/profiler/TracyView_FrameOverview.cpp +++ b/profiler/src/profiler/TracyView_FrameOverview.cpp @@ -196,21 +196,8 @@ void View::DrawFrames() auto fi = m_worker.GetFrameImage( *m_frames, sel ); if( fi ) { - if( fi != m_frameTexturePtr ) - { - if( !m_frameTexture ) m_frameTexture = MakeTexture(); - UpdateTexture( m_frameTexture, m_worker.UnpackFrameImage( *fi ), fi->w, fi->h ); - m_frameTexturePtr = fi; - } ImGui::Separator(); - if( fi->flip ) - { - ImGui::Image( m_frameTexture, ImVec2( fi->w * scale, fi->h * scale ), ImVec2( 0, 1 ), ImVec2( 1, 0 ) ); - } - else - { - ImGui::Image( m_frameTexture, ImVec2( fi->w * scale, fi->h * scale ) ); - } + DrawFrameImage( m_FrameTextureCache, *fi ); } ImGui::EndTooltip(); diff --git a/profiler/src/profiler/TracyView_FrameTimeline.cpp b/profiler/src/profiler/TracyView_FrameTimeline.cpp index 3c099174..6ee9fd2a 100644 --- a/profiler/src/profiler/TracyView_FrameTimeline.cpp +++ b/profiler/src/profiler/TracyView_FrameTimeline.cpp @@ -153,22 +153,8 @@ void View::DrawTimelineFrames( const FrameData& frames ) auto fi = m_worker.GetFrameImage( frames, i ); if( fi ) { - const auto scale = GetScale(); - if( fi != m_frameTexturePtr ) - { - if( !m_frameTexture ) m_frameTexture = MakeTexture(); - UpdateTexture( m_frameTexture, m_worker.UnpackFrameImage( *fi ), fi->w, fi->h ); - m_frameTexturePtr = fi; - } ImGui::Separator(); - if( fi->flip ) - { - ImGui::Image( m_frameTexture, ImVec2( fi->w * scale, fi->h * scale ), ImVec2( 0, 1 ), ImVec2( 1, 0 ) ); - } - else - { - ImGui::Image( m_frameTexture, ImVec2( fi->w * scale, fi->h * scale ) ); - } + DrawFrameImage( m_FrameTextureCache, *fi ); if( ImGui::GetIO().KeyCtrl && IsMouseClicked( 0 ) ) { diff --git a/profiler/src/profiler/TracyView_Messages.cpp b/profiler/src/profiler/TracyView_Messages.cpp index 4db01435..afb204d2 100644 --- a/profiler/src/profiler/TracyView_Messages.cpp +++ b/profiler/src/profiler/TracyView_Messages.cpp @@ -268,20 +268,7 @@ void View::DrawMessageLine( const MessageData& msg, bool hasCallstack, int& idx if( fi ) { ImGui::BeginTooltip(); - if( fi != m_frameTexturePtr ) - { - if( !m_frameTexture ) m_frameTexture = MakeTexture(); - UpdateTexture( m_frameTexture, m_worker.UnpackFrameImage( *fi ), fi->w, fi->h ); - m_frameTexturePtr = fi; - } - if( fi->flip ) - { - ImGui::Image( m_frameTexture, ImVec2( fi->w, fi->h ), ImVec2( 0, 1 ), ImVec2( 1, 0 ) ); - } - else - { - ImGui::Image( m_frameTexture, ImVec2( fi->w, fi->h ) ); - } + DrawFrameImage( m_FrameTextureCache , *fi ); ImGui::EndTooltip(); } }