diff --git a/frontend/app/components/trace_viewer_v2/timeline/constants.h b/frontend/app/components/trace_viewer_v2/timeline/constants.h index 5526acf0..0274cd00 100644 --- a/frontend/app/components/trace_viewer_v2/timeline/constants.h +++ b/frontend/app/components/trace_viewer_v2/timeline/constants.h @@ -75,8 +75,15 @@ inline constexpr Pixel kSelectedBorderThickness = 2.0f; // go/keep-sorted start // A solid blue for the curtain border. #A1C9FFFF at 100% opacity. inline constexpr ImU32 kSelectedTimeRangeBorderColor = 0xFFFFC9A1; -// A semi-transparent blue for the curtain highlight. #A1C9FF4D at 30% opacity. +// The color for the bottom of the selected time range gradient. #a1c9ff99 at +// 60% opacity. +inline constexpr ImU32 kSelectedTimeRangeBottomColor = 0x99FFC9A1; +// A semi-transparent blue for the curtain highlight. #a1c9ff4d at 30% opacity. inline constexpr ImU32 kSelectedTimeRangeColor = 0x4DFFC9A1; +// The color for the top half of the selected time range gradient. #a1c9ff1a +// at 10% opacity. +inline constexpr ImU32 kSelectedTimeRangeTopColor = 0x1AFFC9A1; +inline constexpr Pixel kSelectedTimeRangeTextBottomPadding = 10.0f; // go/keep-sorted end // Zooming and Panning Constants diff --git a/frontend/app/components/trace_viewer_v2/timeline/timeline.cc b/frontend/app/components/trace_viewer_v2/timeline/timeline.cc index 1a4a4d4d..ac430030 100644 --- a/frontend/app/components/trace_viewer_v2/timeline/timeline.cc +++ b/frontend/app/components/trace_viewer_v2/timeline/timeline.cc @@ -59,6 +59,7 @@ void Timeline::Draw() { ImGui::SetNextWindowViewport(viewport->ID); ImGui::PushStyleVar(ImGuiStyleVar_WindowRounding, 0.f); + ImGui::PushStyleVar(ImGuiStyleVar_WindowPadding, ImVec2(0.0f, 0.0f)); ImGui::PushStyleVar(ImGuiStyleVar_CellPadding, ImVec2(0.0f, 0.0f)); ImGui::PushStyleVar(ImGuiStyleVar_ItemSpacing, ImVec2(0.0f, 0.0f)); @@ -115,16 +116,25 @@ void Timeline::Draw() { HandleWheel(); HandleMouse(); - // Keep this at the end. - // `DrawSelectedTimeRanges` should be called after all other timeline content - // (events, ruler, etc.) has been drawn. This ensures that the selected time - // range is rendered on top of everything else within the current ImGui - // window, without affecting global foreground elements like tooltips. - DrawSelectedTimeRanges(timeline_width, px_per_time_unit_val); + ImGui::EndChild(); + // Draw the selected time range in a separate overlay child window. + // This ensures it is drawn on top of the "Tracks" child window (because it's + // declared after) but below tooltips (because it's a child window, not + // in the foreground draw list). + ImGui::SetCursorPos(ImVec2(0, 0)); + ImGui::BeginChild("SelectionOverlay", ImVec2(0, 0), ImGuiChildFlags_None, + ImGuiWindowFlags_NoTitleBar | ImGuiWindowFlags_NoResize | + ImGuiWindowFlags_NoMove | ImGuiWindowFlags_NoScrollbar | + ImGuiWindowFlags_NoSavedSettings | + ImGuiWindowFlags_NoInputs | + ImGuiWindowFlags_NoBackground); + DrawSelectedTimeRanges(timeline_width, px_per_time_unit_val); ImGui::EndChild(); + ImGui::PopStyleVar(); // ItemSpacing ImGui::PopStyleVar(); // CellPadding + ImGui::PopStyleVar(); // WindowPadding ImGui::PopStyleVar(); // WindowRounding ImGui::End(); // Timeline viewer } @@ -743,9 +753,8 @@ void Timeline::DrawGroup(int group_index, double px_per_time_unit_val) { void Timeline::DrawSelectedTimeRange(const TimeRange& range, Pixel timeline_width, double px_per_time_unit_val) { - const ImVec2 table_rect_min = ImGui::GetItemRectMin(); - const ImVec2 table_rect_max = ImGui::GetItemRectMax(); - const Pixel timeline_x_start = table_rect_min.x + label_width_; + const ImGuiViewport* viewport = ImGui::GetMainViewport(); + const Pixel timeline_x_start = viewport->Pos.x + label_width_; const Pixel time_range_x_start = TimeToScreenX(range.start(), timeline_x_start, px_per_time_unit_val); @@ -763,18 +772,34 @@ void Timeline::DrawSelectedTimeRange(const TimeRange& range, if (clipped_x_end > clipped_x_start) { // Use the window draw list to render over all other timeline content. ImDrawList* const draw_list = ImGui::GetWindowDrawList(); - draw_list->AddRectFilled(ImVec2(clipped_x_start, table_rect_min.y), - ImVec2(clipped_x_end, table_rect_max.y), - kSelectedTimeRangeColor); + + const float rect_y_min = viewport->Pos.y; + const float rect_y_max = viewport->Pos.y + viewport->Size.y; + const float rect_y_mid = (rect_y_min + rect_y_max) * 0.5f; + + // Draw the top half with a lighter color to keep the timeline content + // visible. + draw_list->AddRectFilled(ImVec2(clipped_x_start, rect_y_min), + ImVec2(clipped_x_end, rect_y_mid), + kSelectedTimeRangeTopColor); + + // Apply the gradient only to the bottom half of the timeline. + // Increase the opacity of the bottom part to make the text area less + // transparent and the text more visible. + draw_list->AddRectFilledMultiColor( + ImVec2(clipped_x_start, rect_y_mid), ImVec2(clipped_x_end, rect_y_max), + kSelectedTimeRangeTopColor, kSelectedTimeRangeTopColor, + kSelectedTimeRangeBottomColor, kSelectedTimeRangeBottomColor); + // Only draw the border if the edge of the time range is visible. if (time_range_x_start >= timeline_x_start) { - draw_list->AddLine(ImVec2(time_range_x_start, table_rect_min.y), - ImVec2(time_range_x_start, table_rect_max.y), + draw_list->AddLine(ImVec2(time_range_x_start, rect_y_min), + ImVec2(time_range_x_start, rect_y_max), kSelectedTimeRangeBorderColor); } if (time_range_x_end <= timeline_x_start + timeline_width) { - draw_list->AddLine(ImVec2(time_range_x_end, table_rect_min.y), - ImVec2(time_range_x_end, table_rect_max.y), + draw_list->AddLine(ImVec2(time_range_x_end, rect_y_min), + ImVec2(time_range_x_end, rect_y_max), kSelectedTimeRangeBorderColor); } @@ -784,10 +809,10 @@ void Timeline::DrawSelectedTimeRange(const TimeRange& range, if (clipped_x_end - clipped_x_start > text_size.x) { const float text_x = clipped_x_start + (clipped_x_end - clipped_x_start - text_size.x) / 2; - const ImVec2 window_pos = ImGui::GetWindowPos(); - const ImVec2 window_size = ImGui::GetWindowSize(); + // Move the text up a little bit to avoid being too close to the bottom + // edge. const float text_y = - window_pos.y + window_size.y - text_size.y - kRulerTextPadding; + rect_y_max - text_size.y - kSelectedTimeRangeTextBottomPadding; draw_list->AddText(ImVec2(text_x, text_y), kRulerTextColor, text.c_str()); } } diff --git a/frontend/app/components/trace_viewer_v2/timeline/timeline_test.cc b/frontend/app/components/trace_viewer_v2/timeline/timeline_test.cc index a56ca9a7..dd030bff 100644 --- a/frontend/app/components/trace_viewer_v2/timeline/timeline_test.cc +++ b/frontend/app/components/trace_viewer_v2/timeline/timeline_test.cc @@ -6,6 +6,7 @@ #include "testing/base/public/gmock.h" #include "" +#include "absl/strings/match.h" #include "absl/strings/string_view.h" #include "third_party/dear_imgui/imgui.h" #include "third_party/dear_imgui/imgui_internal.h" @@ -834,24 +835,24 @@ TEST_F(MockTimelineImGuiFixture, ShiftClickAndReleaseShiftMidDragContinuesSelection) { // Setup similar to TimelineDragSelectionTest to ensure predictable // coordinates. - timeline_.SetVisibleRange({0.0, 165.3}); - timeline_.set_data_time_range({0.0, 165.3}); + timeline_.SetVisibleRange({0.0, 166.9}); + timeline_.set_data_time_range({0.0, 166.9}); ImGuiIO& io = ImGui::GetIO(); // Start with Shift held down. io.AddKeyEvent(ImGuiMod_Shift, true); // Start drag in timeline area. - // X=308 is safely inside the timeline (250 + padding < 308). - io.MousePos = ImVec2(308.0f, 50.0f); + // X=300 is safely inside the timeline (250 + padding < 300). + io.MousePos = ImVec2(300.0f, 50.0f); io.AddMouseButtonEvent(0, true); SimulateFrame(); // Release Shift key. io.AddKeyEvent(ImGuiMod_Shift, false); - // Drag mouse to X=508. - io.MousePos = ImVec2(508.0f, 50.0f); + // Drag mouse to X=500. + io.MousePos = ImVec2(500.0f, 50.0f); SimulateFrame(); // End drag. @@ -863,22 +864,22 @@ TEST_F(MockTimelineImGuiFixture, const TimeRange& range = timeline_.selected_time_ranges()[0]; // Calculate expected range based on pixel movement. // 10px/us assumption from TimelineDragSelectionTest. - // 308 -> 5.0. 508 -> 25.0. + // 300 -> 5.0. 500 -> 25.0. EXPECT_NEAR(range.start(), 5.0, 1e-5); EXPECT_NEAR(range.end(), 25.0, 1e-5); } TEST_F(MockTimelineImGuiFixture, ClickAndPressShiftMidDragContinuesPanning) { // Setup similar to TimelineDragSelectionTest. - timeline_.SetVisibleRange({0.0, 165.3}); - timeline_.set_data_time_range({0.0, 165.3}); + timeline_.SetVisibleRange({0.0, 166.9}); + timeline_.set_data_time_range({0.0, 166.9}); ImGuiIO& io = ImGui::GetIO(); // Start without Shift. io.AddKeyEvent(ImGuiMod_Shift, false); // Start drag in timeline area. - io.MousePos = ImVec2(308.0f, 50.0f); + io.MousePos = ImVec2(300.0f, 50.0f); io.AddMouseButtonEvent(0, true); EXPECT_CALL(timeline_, Pan(0.0f)); @@ -889,8 +890,8 @@ TEST_F(MockTimelineImGuiFixture, ClickAndPressShiftMidDragContinuesPanning) { io.AddKeyEvent(ImGuiMod_Shift, true); // Drag mouse to left (simulate pan right). - // Move from 308 to 208 (-100px). - io.MousePos = ImVec2(208.0f, 50.0f); + // Move from 300 to 200 (-100px). + io.MousePos = ImVec2(200.0f, 50.0f); EXPECT_CALL(timeline_, Pan(FloatEq(100.0f))); EXPECT_CALL(timeline_, Scroll(FloatEq(0.0f))); @@ -953,6 +954,21 @@ TEST_F(MockTimelineImGuiFixture, using RealTimelineImGuiFixture = TimelineImGuiTestFixture; +// Add a sanity check that the window padding is set to zero. +// This is the presumption for all the drawing logic. And all tests below assume +// this. +TEST_F(RealTimelineImGuiFixture, DrawSetsWindowPaddingToZero) { + ImGui::NewFrame(); + timeline_.Draw(); + + ImGuiWindow* window = ImGui::FindWindowByName("Timeline viewer"); + ASSERT_NE(window, nullptr); + EXPECT_EQ(window->WindowPadding.x, 0.0f); + EXPECT_EQ(window->WindowPadding.y, 0.0f); + + ImGui::EndFrame(); +} + TEST_F(RealTimelineImGuiFixture, ClickEventSelectsEvent) { FlameChartTimelineData data; @@ -978,7 +994,9 @@ TEST_F(RealTimelineImGuiFixture, ClickEventSelectsEvent) { // Set a mouse position that is guaranteed to be over the event, since the // event spans the entire timeline. - ImGui::GetIO().MousePos = ImVec2(300.f, 40.f); + // y=28 is safely within the event rect (starts at 20, height 16 -> ends at + // 36). + ImGui::GetIO().MousePos = ImVec2(300.f, 28.f); ImGui::GetIO().MouseDown[0] = true; SimulateFrame(); @@ -1040,7 +1058,7 @@ TEST_F(RealTimelineImGuiFixture, callback_count++; }); - ImGui::GetIO().MousePos = ImVec2(300.f, 40.f); + ImGui::GetIO().MousePos = ImVec2(300.f, 28.f); // First click. ImGui::GetIO().MouseDown[0] = true; @@ -1074,7 +1092,7 @@ TEST_F(RealTimelineImGuiFixture, ClickEmptyAreaDeselectsEvent) { timeline_.SetVisibleRange({0.0, 100.0}); // First, select an event. - ImGui::GetIO().MousePos = ImVec2(300.f, 40.f); // A position over the event. + ImGui::GetIO().MousePos = ImVec2(300.f, 28.f); // A position over the event. ImGui::GetIO().MouseDown[0] = true; SimulateFrame(); ImGui::GetIO().MouseDown[0] = false; // Release the mouse. @@ -1117,7 +1135,7 @@ TEST_F(RealTimelineImGuiFixture, ClickEmptyAreaDeselectsOnlyOnce) { timeline_.SetVisibleRange({0.0, 100.0}); // First, select an event. - ImGui::GetIO().MousePos = ImVec2(300.f, 40.f); // A position over the event. + ImGui::GetIO().MousePos = ImVec2(300.f, 28.f); // A position over the event. ImGui::GetIO().MouseDown[0] = true; SimulateFrame(); ImGui::GetIO().MouseDown[0] = false; // Release the mouse. @@ -1209,7 +1227,7 @@ TEST_F(RealTimelineImGuiFixture, ShiftClickEventTogglesCurtain) { timeline_.SetVisibleRange({0.0, 100.0}); // Mouse is over the event - ImGui::GetIO().MousePos = ImVec2(500.f, 40.f); + ImGui::GetIO().MousePos = ImVec2(500.f, 28.f); ImGui::GetIO().AddKeyEvent(ImGuiMod_Shift, true); ImGui::GetIO().MouseDown[0] = true; @@ -1255,7 +1273,7 @@ TEST_F(RealTimelineImGuiFixture, ImGui::GetIO().AddKeyEvent(ImGuiMod_Shift, true); // First shift-click on event 1. - ImGui::GetIO().MousePos = ImVec2(500.f, 40.f); // Position over event 1. + ImGui::GetIO().MousePos = ImVec2(500.f, 28.f); // Position over event 1. ImGui::GetIO().MouseDown[0] = true; SimulateFrame(); @@ -1268,7 +1286,7 @@ TEST_F(RealTimelineImGuiFixture, SimulateFrame(); // Second shift-click on event 2. - ImGui::GetIO().MousePos = ImVec2(1100.f, 40.f); // Position over event 2. + ImGui::GetIO().MousePos = ImVec2(1100.f, 28.f); // Position over event 2. ImGui::GetIO().MouseDown[0] = true; SimulateFrame(); @@ -1283,7 +1301,7 @@ TEST_F(RealTimelineImGuiFixture, SimulateFrame(); // Third shift-click on event 1 again to deselect. - ImGui::GetIO().MousePos = ImVec2(500.f, 40.f); // Position over event 1. + ImGui::GetIO().MousePos = ImVec2(500.f, 28.f); // Position over event 1. ImGui::GetIO().MouseDown[0] = true; SimulateFrame(); @@ -1344,11 +1362,11 @@ class TimelineDragSelectionTest : public RealTimelineImGuiFixture { void SetUp() override { RealTimelineImGuiFixture::SetUp(); // Set a visible range that results in a round number for px_per_time_unit - // to make test calculations predictable. With a timeline width of 1653px - // (based on 1920px window width and paddings), a duration of 165.3 gives - // 10px per microsecond. - timeline_.SetVisibleRange({0.0, 165.3}); - timeline_.set_data_time_range({0.0, 165.3}); + // to make test calculations predictable. With a timeline width of 1669px + // (based on 1920px window width, 250px label width, and 1px padding), + // a duration of 166.9 gives 10px per microsecond. + timeline_.SetVisibleRange({0.0, 166.9}); + timeline_.set_data_time_range({0.0, 166.9}); ImGui::GetIO().AddKeyEvent(ImGuiMod_Shift, true); } @@ -1364,12 +1382,12 @@ TEST_F(TimelineDragSelectionTest, ShiftDragCreatesTimeSelection) { // Start drag in timeline area. // The label column is 250px wide, so timeline starts after that. - io.MousePos = ImVec2(308.0f, 50.0f); + io.MousePos = ImVec2(300.0f, 50.0f); io.AddMouseButtonEvent(0, true); SimulateFrame(); // Dragging - io.MousePos = ImVec2(508.0f, 50.0f); + io.MousePos = ImVec2(500.0f, 50.0f); SimulateFrame(); // End drag @@ -1386,10 +1404,10 @@ TEST_F(TimelineDragSelectionTest, ShiftDragCreatesMultipleTimeSelections) { ImGuiIO& io = ImGui::GetIO(); // First drag - io.MousePos = ImVec2(308.0f, 50.0f); + io.MousePos = ImVec2(300.0f, 50.0f); io.AddMouseButtonEvent(0, true); SimulateFrame(); - io.MousePos = ImVec2(408.0f, 50.0f); + io.MousePos = ImVec2(400.0f, 50.0f); SimulateFrame(); io.AddMouseButtonEvent(0, false); SimulateFrame(); @@ -1399,10 +1417,10 @@ TEST_F(TimelineDragSelectionTest, ShiftDragCreatesMultipleTimeSelections) { EXPECT_DOUBLE_EQ(timeline_.selected_time_ranges()[0].end(), 15.0); // Second drag - io.MousePos = ImVec2(508.0f, 50.0f); + io.MousePos = ImVec2(500.0f, 50.0f); io.AddMouseButtonEvent(0, true); SimulateFrame(); - io.MousePos = ImVec2(608.0f, 50.0f); + io.MousePos = ImVec2(600.0f, 50.0f); SimulateFrame(); io.AddMouseButtonEvent(0, false); SimulateFrame(); @@ -1418,12 +1436,12 @@ TEST_F(TimelineDragSelectionTest, DraggingUpdatesCurrentSelectedTimeRange) { ImGuiIO& io = ImGui::GetIO(); // Start drag in timeline area. - io.MousePos = ImVec2(308.0f, 50.0f); + io.MousePos = ImVec2(300.0f, 50.0f); io.AddMouseButtonEvent(0, true); SimulateFrame(); // Dragging - io.MousePos = ImVec2(508.0f, 50.0f); + io.MousePos = ImVec2(500.0f, 50.0f); SimulateFrame(); // During drag, current_selected_time_range_ should be set, but @@ -1579,7 +1597,7 @@ TEST_F(RealTimelineImGuiFixture, ClickEventSetsSelectionIndices) { timeline_.SetVisibleRange({0.0, 100.0}); // Set a mouse position that is guaranteed to be over the event. - ImGui::GetIO().MousePos = ImVec2(300.f, 40.f); + ImGui::GetIO().MousePos = ImVec2(300.f, 28.f); ImGui::GetIO().MouseDown[0] = true; SimulateFrame(); @@ -1671,7 +1689,7 @@ TEST_F(RealTimelineImGuiFixture, SelectionMutualExclusion) { timeline_.SetVisibleRange({0.0, 100.0}); // Step 1: Select Flame Event - ImGui::GetIO().MousePos = ImVec2(300.f, 40.f); // Over flame event + ImGui::GetIO().MousePos = ImVec2(300.f, 28.f); // Over flame event ImGui::GetIO().MouseDown[0] = true; SimulateFrame(); ImGui::GetIO().MouseDown[0] = false; @@ -1710,7 +1728,7 @@ TEST_F(RealTimelineImGuiFixture, SelectionMutualExclusion) { EXPECT_EQ(timeline_.selected_counter_index(), 0); // Step 3: Select Flame Event Again - ImGui::GetIO().MousePos = ImVec2(300.f, 40.f); + ImGui::GetIO().MousePos = ImVec2(300.f, 28.f); ImGui::GetIO().MouseDown[0] = true; SimulateFrame(); @@ -1732,7 +1750,7 @@ TEST_F(RealTimelineImGuiFixture, ClickEmptyAreaClearsSelectionIndices) { timeline_.SetVisibleRange({0.0, 100.0}); // Select event - ImGui::GetIO().MousePos = ImVec2(300.f, 40.f); + ImGui::GetIO().MousePos = ImVec2(300.f, 28.f); ImGui::GetIO().MouseDown[0] = true; SimulateFrame(); ImGui::GetIO().MouseDown[0] = false; @@ -1750,6 +1768,41 @@ TEST_F(RealTimelineImGuiFixture, ClickEmptyAreaClearsSelectionIndices) { EXPECT_EQ(timeline_.selected_counter_index(), -1); } +TEST_F(RealTimelineImGuiFixture, SelectionOverlayIsDrawnOnTopOfTracks) { + // Ensure we have some data so tracks are drawn. + FlameChartTimelineData data; + data.groups.push_back({.name = "Group 1", .start_level = 0}); + timeline_.set_timeline_data(std::move(data)); + + ImGui::NewFrame(); + timeline_.Draw(); + + ImGuiWindow* timeline_window = ImGui::FindWindowByName("Timeline viewer"); + ASSERT_NE(timeline_window, nullptr); + + bool found_tracks = false; + bool found_overlay = false; + bool overlay_is_after_tracks = false; + + for (ImGuiWindow* child : timeline_window->DC.ChildWindows) { + if (absl::StrContains(child->Name, "Tracks")) { + found_tracks = true; + } else if (absl::StrContains(child->Name, "SelectionOverlay")) { + found_overlay = true; + if (found_tracks) { + overlay_is_after_tracks = true; + } + } + } + + EXPECT_TRUE(found_tracks) << "Tracks child window not found"; + EXPECT_TRUE(found_overlay) << "SelectionOverlay child window not found"; + EXPECT_TRUE(overlay_is_after_tracks) + << "SelectionOverlay should be drawn after Tracks to appear on top"; + + ImGui::EndFrame(); +} + } // namespace } // namespace testing } // namespace traceviewer