From 77c6950d22eaba4afe251fd8b876f037d67fa5c0 Mon Sep 17 00:00:00 2001 From: Camille Martel Date: Tue, 13 Oct 2026 12:16:53 -0700 Subject: [PATCH] Address review feedback - Match radio button sizes to other sub-elements (smaller inline size) - Also fix alignment of select/unselect all buttons relative to filter text baseline - Extract plot name into GetPlotName helper for uniform use in user vs. built-in plots - Properly handle row-wise plot duplicate IDs through PushID/PopID - Remove truncation hazards from use of snprintf - Allow clicking full hierarchical row to toggle expansion (hover highlights the whole row, similar to the other collapsible rows in the options view) --- .../src/profiler/TracyTimelineItemPlot.cpp | 18 +++++--------- profiler/src/profiler/TracyUtility.cpp | 7 ++++++ profiler/src/profiler/TracyUtility.hpp | 1 + profiler/src/profiler/TracyView_Options.cpp | 24 ++++++++++++++----- 4 files changed, 32 insertions(+), 18 deletions(-) diff --git a/profiler/src/profiler/TracyTimelineItemPlot.cpp b/profiler/src/profiler/TracyTimelineItemPlot.cpp index 07689ccc..f6fa9851 100644 --- a/profiler/src/profiler/TracyTimelineItemPlot.cpp +++ b/profiler/src/profiler/TracyTimelineItemPlot.cpp @@ -31,21 +31,15 @@ const char* TimelineItemPlot::HeaderLabel() const switch( m_plot->type ) { case PlotType::User: - return m_worker.GetString( m_plot->name ); + return GetPlotName( *m_plot, m_worker ); case PlotType::Memory: - if( m_plot->name == 0 ) - { - return ICON_FA_MEMORY " Memory usage"; - } - else - { - sprintf( tmp, ICON_FA_MEMORY " %s", m_worker.GetString( m_plot->name ) ); - return tmp; - } + sprintf( tmp, ICON_FA_MEMORY " %s", GetPlotName( *m_plot, m_worker ) ); + return tmp; case PlotType::SysTime: - return ICON_FA_GAUGE_HIGH " CPU usage"; + sprintf( tmp, ICON_FA_GAUGE_HIGH " %s", GetPlotName( *m_plot, m_worker ) ); + return tmp; case PlotType::Power: - sprintf( tmp, ICON_FA_BOLT " %s", m_worker.GetString( m_plot->name ) ); + sprintf( tmp, ICON_FA_BOLT " %s", GetPlotName( *m_plot, m_worker ) ); return tmp; default: assert( false ); diff --git a/profiler/src/profiler/TracyUtility.cpp b/profiler/src/profiler/TracyUtility.cpp index a717311f..94709a76 100644 --- a/profiler/src/profiler/TracyUtility.cpp +++ b/profiler/src/profiler/TracyUtility.cpp @@ -172,6 +172,13 @@ uint32_t GetPlotColor( const PlotData& plot, const Worker& worker ) } } +const char* GetPlotName( const PlotData& plot, const Worker& worker ) +{ + if( plot.type == PlotType::SysTime ) return "CPU usage"; + if( plot.type == PlotType::Memory && plot.name == 0 ) return "Memory usage"; + return worker.GetString( plot.name ); +} + const char* FormatPlotValue( double val, PlotValueFormatting format ) { static char buf[64]; diff --git a/profiler/src/profiler/TracyUtility.hpp b/profiler/src/profiler/TracyUtility.hpp index 2a24fad6..9c3e87ab 100644 --- a/profiler/src/profiler/TracyUtility.hpp +++ b/profiler/src/profiler/TracyUtility.hpp @@ -56,6 +56,7 @@ const char* ShortenImageName( const char* image ); uint32_t GetThreadColor( uint64_t thread, int depth, bool dynamic ); uint32_t GetPlotColor( const PlotData& plot, const Worker& worker ); +const char* GetPlotName( const PlotData& plot, const Worker& worker ); const char* FormatPlotValue( double val, PlotValueFormatting format ); std::vector SplitLines( const char* data, size_t sz ); diff --git a/profiler/src/profiler/TracyView_Options.cpp b/profiler/src/profiler/TracyView_Options.cpp index 5195593f..6b4e9f79 100644 --- a/profiler/src/profiler/TracyView_Options.cpp +++ b/profiler/src/profiler/TracyView_Options.cpp @@ -2,7 +2,6 @@ #include #include #include -#include #include "TracyFilesystem.hpp" #include "TracyImGui.hpp" @@ -41,6 +40,11 @@ static void BuildPlotTree( PlotTreeNode& root, const std::vectortype != PlotType::User ) + { + root.plots.emplace_back( p ); + continue; + } const char* name = worker.GetString( p->name ); PlotTreeNode* node = &root; const char* segStart = name; @@ -85,7 +89,8 @@ static void DrawPlotTreeNode( const std::string& label, const std::string& fullP bool allVisible = total > 0 && visibleCount == total; const bool mixed = visibleCount > 0 && visibleCount < total; - const bool nodeExpand = ImGui::TreeNodeEx( fullPath.c_str(), ImGuiTreeNodeFlags_None, "" ); + // Row-wide hit box so the label (drawn after the checkbox) is clickable and highlighted; the checkbox overlaps it. + const bool nodeExpand = ImGui::TreeNodeEx( fullPath.c_str(), ImGuiTreeNodeFlags_SpanAvailWidth | ImGuiTreeNodeFlags_AllowOverlap, "" ); ImGui::SameLine(); const std::string checkboxId = "##hdr_" + fullPath; ImGui::PushItemFlag( ImGuiItemFlags_MixedValue, mixed ); @@ -110,17 +115,17 @@ static void DrawPlotTreeNode( const std::string& label, const std::string& fullP const char* slash = strrchr( fullName, '/' ); const char* leaf = slash ? slash + 1 : fullName; + ImGui::PushID( p ); SmallColorBox( GetPlotColor( *p, worker ) ); ImGui::SameLine(); bool visible = tc.GetItem( p ).IsVisible(); - char leafId[256]; - snprintf( leafId, sizeof( leafId ), "%s##%p", leaf, (const void*)p ); - if( SmallCheckbox( leafId, &visible ) ) + if( SmallCheckbox( leaf, &visible ) ) { tc.GetItem( p ).SetVisible( visible ); } ImGui::SameLine(); ImGui::TextDisabled( "%s data points", RealToString( p->data.size() ) ); + ImGui::PopID(); } ImGui::TreePop(); } @@ -778,21 +783,24 @@ void View::DrawOptions() { ImGui::SameLine(); bool hierarchy = m_vd.plotsHierarchy; + ImGui::PushStyleVar( ImGuiStyleVar_FramePadding, ImVec2( 0, 0 ) ); if( ImGui::RadioButton( "Flat", !hierarchy ) ) m_vd.plotsHierarchy = false; ImGui::SameLine(); if( ImGui::RadioButton( "Hierarchical", hierarchy ) ) m_vd.plotsHierarchy = true; + ImGui::PopStyleVar(); hierarchy = m_vd.plotsHierarchy; std::vector filteredPlots; filteredPlots.reserve( m_worker.GetPlots().size() ); for( const auto& p : m_worker.GetPlots() ) { - if( m_plotFilter.PassFilter( m_worker.GetString( p->name ) ) ) + if( m_plotFilter.PassFilter( GetPlotName( *p, m_worker ) ) ) { filteredPlots.emplace_back( p ); } } + ImGui::AlignTextToFramePadding(); if( ImGui::SmallButton( "Select all" ) ) { for( const auto* p : filteredPlots ) @@ -827,22 +835,26 @@ void View::DrawOptions() } for( const auto* p : root.plots ) { + ImGui::PushID( p ); SmallColorBox( GetPlotColor( *p, m_worker ) ); ImGui::SameLine(); m_tc.GetItem( p ).VisibilityCheckbox(); ImGui::SameLine(); ImGui::TextDisabled( "%s data points", RealToString( p->data.size() ) ); + ImGui::PopID(); } } else { for( const auto* p : filteredPlots ) { + ImGui::PushID( p ); SmallColorBox( GetPlotColor( *p, m_worker ) ); ImGui::SameLine(); m_tc.GetItem( p ).VisibilityCheckbox(); ImGui::SameLine(); ImGui::TextDisabled( "%s data points", RealToString( p->data.size() ) ); + ImGui::PopID(); } } ImGui::TreePop();