Add a left margin to TimeGraph

src/OrbitGl/CaptureWindow.cpp

@@ -297,7 +297,9 @@ void CaptureWindow::RightUp() {
 void CaptureWindow::ZoomHorizontally(int delta, int mouse_x) {
   if (delta == 0) return;
   if (time_graph_ != nullptr) {
-    double mouse_ratio = static_cast<double>(mouse_x) / time_graph_->GetTimelineWidth();
+    float timeline_pos_x = time_graph_->GetTimelinePos()[0];
+    double mouse_ratio =
+        static_cast<double>(mouse_x - timeline_pos_x) / time_graph_->GetTimelineWidth();
     time_graph_->ZoomTime(delta, mouse_ratio);
   }
 }

src/OrbitGl/CaptureWindowTest.cpp

@@ -63,7 +63,8 @@ class NavigationTestCaptureWindow : public testing::Test {
     // viewport height should be changed and the test adjusted accordingly
     ORBIT_CHECK(capture_window_.GetTimeGraph()->GetTimelineUi()->GetHeight() <
                 kViewportHeight - kBottomSafetyMargin);
-    ORBIT_CHECK(capture_window_.GetTimeGraph()->GetTimelineUi()->GetPos()[0] == 0);
+    ORBIT_CHECK(capture_window_.GetTimeGraph()->GetTimelineUi()->GetPos()[0] ==
+                time_graph_layout_.GetLeftMargin());
   }
 
  protected:

src/OrbitGl/SliderTest.cpp

@@ -19,6 +19,35 @@
 
 namespace orbit_gl {
 
+namespace {
+
+template <typename T>
+[[nodiscard]] std::shared_ptr<T> CreateTestSlider(Viewport*, TimeGraphLayout*) {}
+
+template <>
+[[nodiscard]] std::shared_ptr<GlHorizontalSlider> CreateTestSlider(Viewport* viewport,
+                                                                   TimeGraphLayout* layout) {
+  auto result = std::make_shared<GlHorizontalSlider>(/*parent=*/nullptr, viewport, layout,
+                                                     /*timeline_info=*/nullptr);
+  // The tests implicitly assume that the horizontal slider has a specific width which was
+  // previously the default, and only possible width, returned by the HorizontalSlider::GetWidth()
+  // function override: "viewport->GetScreenWidth() - layout->GetSliderWidth()". Since we can now
+  // set the slider width, and the default width is 0, we replicate the previous behavior by
+  // explicitly setting this width.
+  float test_width = viewport->GetScreenWidth() - layout->GetSliderWidth();
+  result->SetWidth(test_width);
+  return result;
+}
+
+template <>
+[[nodiscard]] std::shared_ptr<GlVerticalSlider> CreateTestSlider(Viewport* viewport,
+                                                                 TimeGraphLayout* layout) {
+  return std::make_shared<GlVerticalSlider>(/*parent=*/nullptr, viewport, layout,
+                                            /*timeline_info=*/nullptr);
+}
+
+}  // namespace
+
 template <int dim>
 void Pick(GlSlider& slider, int start, int other_dim = 0) {
   static_assert(dim >= 0 && dim <= 1);
@@ -57,7 +86,7 @@ void PickDragRelease(GlSlider& slider, int start, int end = -1, int other_dim =
 }
 
 template <typename SliderClass>
-std::tuple<std::unique_ptr<SliderClass>, std::unique_ptr<Viewport>,
+std::tuple<std::shared_ptr<SliderClass>, std::unique_ptr<Viewport>,
            std::unique_ptr<CaptureViewElementTester>>
 Setup() {
   std::unique_ptr<CaptureViewElementTester> tester = std::make_unique<CaptureViewElementTester>();
@@ -66,8 +95,9 @@ Setup() {
   std::unique_ptr<Viewport> viewport =
       std::make_unique<Viewport>(100 + orthogonal_slider_width, 1000 + orthogonal_slider_width);
 
-  std::unique_ptr<SliderClass> slider =
-      std::make_unique<SliderClass>(nullptr, viewport.get(), tester->GetLayout(), nullptr);
+  // Sliders use "shared_from_this", so we can't use std::unique_ptr here.
+  std::shared_ptr<SliderClass> slider =
+      CreateTestSlider<SliderClass>(viewport.get(), tester->GetLayout());
 
   // Set the slider to be 50% of the maximum size, position in the middle
   slider->SetNormalizedPosition(0.5f);
@@ -346,10 +376,14 @@ TEST(Slider, MouseMoveRequestRedraw) {
   CaptureViewElementTester tester;
   Viewport* viewport = tester.GetViewport();
 
-  std::shared_ptr<GlHorizontalSlider> slider{
-      std::make_shared<GlHorizontalSlider>(nullptr, viewport, tester.GetLayout(), nullptr)};
+  std::shared_ptr<GlHorizontalSlider> slider =
+      CreateTestSlider<GlHorizontalSlider>(viewport, tester.GetLayout());
   tester.SimulatePreRender(slider.get());
 
+  // In CreateTestSlider, we have called SetWidth() which sets "update_primitives_requested_" to
+  // true. Simulate a draw loop to reinitialize the draw flags before testing their values below.
+  tester.SimulateDrawLoop(slider.get(), true, true);
+
   Vec2 kPosInSlider = slider->GetPos();
 
   // Any mouse move or mouse leave should trigger a redraw in the slider.

src/OrbitGl/TimeGraph.cpp

@@ -397,7 +397,7 @@ orbit_gl::CaptureViewElement::EventResult TimeGraph::OnMouseWheel(
   const float kScrollingRatioPerDelta = 0.05f;
 
   if (modifiers.ctrl) {
-    double mouse_ratio = (mouse_pos[0] - GetPos()[0]) / GetTimelineWidth();
+    double mouse_ratio = (mouse_pos[0] - GetTimelinePos()[0]) / GetTimelineWidth();
     ZoomTime(delta, mouse_ratio);
   } else {
     track_container_->IncrementVerticalScroll(delta * kScrollingRatioPerDelta);
@@ -424,7 +424,8 @@ float TimeGraph::GetWorldFromTick(uint64_t time) const {
   if (time_window_us > 0) {
     double start = TicksToMicroseconds(capture_min_timestamp_, time) - min_time_us_;
     double normalized_start = start / time_window_us;
-    float pos = static_cast<float>(normalized_start * GetTimelineWidth());
+    float pos =
+        layout_->GetLeftMargin() + static_cast<float>(normalized_start * GetTimelineWidth());
     return pos;
   }
 
@@ -442,7 +443,8 @@ double TimeGraph::GetUsFromTick(uint64_t time) const {
 uint64_t TimeGraph::GetNsSinceStart(uint64_t time) const { return time - capture_min_timestamp_; }
 
 uint64_t TimeGraph::GetTickFromWorld(float world_x) const {
-  double ratio = GetTimelineWidth() > 0 ? static_cast<double>(world_x / GetTimelineWidth()) : 0;
+  float relative_x = world_x - GetTimelinePos()[0];
+  double ratio = GetTimelineWidth() > 0 ? static_cast<double>(relative_x / GetTimelineWidth()) : 0;
   auto time_span_ns = static_cast<uint64_t>(1000 * GetTime(ratio));
   return capture_min_timestamp_ + time_span_ns;
 }
@@ -683,21 +685,21 @@ void TimeGraph::DoUpdateLayout() {
 
 void TimeGraph::UpdateChildrenPosAndContainerSize() {
   // TimeGraph's children:
-  // ___________________________________________
-  // |            TIMELINE            |   |  +  |
-  // |                                |   |  -  |
-  // |--------------------------------|   |-----|
-  // |     SPACE TRACKS - TIMELINE              |
-  // |--------------------------------| M |-----|
-  // |                                | A |  S  |
-  // |                                | R |  L  |
-  // |         TRACK CONTAINER        | G |  I  |
-  // |                                | I |  D  |
-  // |                                | N |  E  |
-  // |                                |   |  R  |
-  // |________________________________|___|_____|
-  // |       HORIZONTAL SLIDER            |
-  // |------------------------------------|
+  // _______________________________________________
+  // | L |            TIMELINE            | R |  +  |
+  // | E |                                | I |  -  |
+  // | F |--------------------------------| G |-----|
+  // | T |     SPACE TRACKS - TIMELINE      H       |
+  // |   |--------------------------------| T |-----|
+  // | M |                                |   |  S  |
+  // | A |                                | M |  L  |
+  // | R |         TRACK CONTAINER        | A |  I  |
+  // | G |                                | R |  D  |
+  // | I |                                | G |  E  |
+  // | N |                                | I |  R  |
+  // |   |________________________________|_N_|_____|
+  // |   |       HORIZONTAL SLIDER            |
+  // |----------------------------------------|
 
   // First we calculate TrackContainer's height. TimeGraph will set TrackContainer height based on
   // its free space.
@@ -711,8 +713,8 @@ void TimeGraph::UpdateChildrenPosAndContainerSize() {
   float timegraph_current_y = GetPos()[1];
   const float total_right_margin = layout_->GetRightMargin() + vertical_slider_->GetWidth();
 
-  timeline_ui_->SetWidth(GetWidth() - total_right_margin);
-  timeline_ui_->SetPos(timegraph_current_x, timegraph_current_y);
+  timeline_ui_->SetWidth(GetWidth() - total_right_margin - layout_->GetLeftMargin());
+  timeline_ui_->SetPos(timegraph_current_x + layout_->GetLeftMargin(), timegraph_current_y);
 
   plus_button_->SetWidth(layout_->GetButtonWidth());
   plus_button_->SetHeight(layout_->GetButtonHeight());
@@ -723,18 +725,20 @@ void TimeGraph::UpdateChildrenPosAndContainerSize() {
   minus_button_->SetPos(GetWidth() - minus_button_->GetWidth(),
                         timegraph_current_y + plus_button_->GetHeight());
 
+  float x_offset = layout_->GetLeftMargin();
   timegraph_current_y += timeline_ui_->GetHeight() + layout_->GetSpaceBetweenTracksAndTimeline();
-  track_container_->SetWidth(GetWidth() - total_right_margin);
-  track_container_->SetPos(timegraph_current_x, timegraph_current_y);
+  track_container_->SetWidth(GetWidth() - total_right_margin - x_offset);
+  track_container_->SetPos(timegraph_current_x + x_offset, timegraph_current_y);
 
   vertical_slider_->SetWidth(layout_->GetSliderWidth());
   vertical_slider_->SetPos(GetWidth() - vertical_slider_->GetWidth(), timegraph_current_y);
 
   timegraph_current_y += track_container_->GetHeight();
-  horizontal_slider_->SetWidth(GetWidth() - vertical_slider_->GetWidth());
+  float slider_width = GetWidth() - vertical_slider_->GetWidth() - layout_->GetLeftMargin();
+  horizontal_slider_->SetWidth(slider_width);
   // The horizontal slider should be at the bottom of the TimeGraph. Because how OpenGl renders, the
   // way to assure that there is no pixels below the scrollbar is by making a ceiling.
-  horizontal_slider_->SetPos(timegraph_current_x,
+  horizontal_slider_->SetPos(timegraph_current_x + layout_->GetLeftMargin(),
                              std::ceil(GetHeight() - horizontal_slider_->GetHeight()));
 
   // TODO(b/230442062): Refactor this to be part of Slider::UpdateLayout().
@@ -883,10 +887,19 @@ void TimeGraph::DrawMarginsBetweenChildren(
   primitive_assembler.AddBox(MakeBox(timeline_margin_pos, timeline_margin_size),
                              GlCanvas::kZValueTimeBar, GlCanvas::kBackgroundColor);
 
-  // Margin between the Tracks and the vertical scrollbar.
-  Vec2 right_margin_pos{GetWidth() - GetRightMargin(), GetPos()[1]};
-  Quad box = MakeBox(right_margin_pos, GetSize() - (right_margin_pos - GetPos()));
-  primitive_assembler.AddBox(box, GlCanvas::kZValueMargin, GlCanvas::kBackgroundColor);
+  // Right margin mask for Timeline.
+  float right_margin_width = GetRightMargin();
+  float right_margin_height = GetHeight() - timeline_ui_->GetHeight();
+  Vec2 right_margin_pos{GetWidth() - right_margin_width, GetPos()[1]};
+  Quad right_margin_box = MakeBox(right_margin_pos, Vec2(right_margin_width, right_margin_height));
+  primitive_assembler.AddBox(right_margin_box, GlCanvas::kZValueMargin, GlCanvas::kBackgroundColor);
+
+  // Left margin mask for Timeline.
+  float left_margin_width = layout_->GetLeftMargin();
+  float left_margin_height = GetHeight();
+  Vec2 left_margin_pos = GetPos();
+  Quad left_margin_box = MakeBox(left_margin_pos, Vec2(left_margin_width, left_margin_height));
+  primitive_assembler.AddBox(left_margin_box, GlCanvas::kZValueMargin, GlCanvas::kBackgroundColor);
 }
 
 void TimeGraph::DrawText(QPainter* painter, float layer) {

src/OrbitGl/TimeGraphTest.cpp

@@ -25,7 +25,8 @@ class UnitTestTimeGraph : public testing::Test {
  public:
   explicit UnitTestTimeGraph() {
     capture_data_ = TrackTestData::GenerateTestCaptureData();
-    viewport_ = std::make_unique<Viewport>(100, 200);
+    // Make the viewport big enough to account for a left margin.
+    viewport_ = std::make_unique<Viewport>(1000, 200);
     time_graph_ = std::make_unique<TimeGraph>(nullptr, nullptr, viewport_.get(),
                                               capture_data_.get(), nullptr, &time_graph_layout_);
     time_graph_->ZoomAll();
@@ -117,9 +118,17 @@ TEST_F(UnitTestTimeGraph, MouseWheel) {
   const TimeGraph* time_graph = GetTimeGraph();
 
   double original_time_window_us = time_graph->GetTimeWindowUs();
-  Vec2 kLeftmostTimelineMousePosition = time_graph->GetTimelineUi()->GetPos();
-  Vec2 kCenteredMousePosition{time_graph->GetPos()[0] + time_graph->GetSize()[0] / 2.f,
-                              time_graph->GetPos()[1] + time_graph->GetSize()[1] / 2.f};
+  const orbit_gl::TimelineUi* timeline_ui = time_graph->GetTimelineUi();
+  const Vec2 kLeftmostTimelineMousePosition = timeline_ui->GetPos();
+
+  // The interactive area starts at the position of the timeline.
+  const Vec2 kInteractiveAreaPos = {timeline_ui->GetPos()[0], timeline_ui->GetPos()[1]};
+  // That area is the size of the timeline in X and extends to the size of the timegraph in Y.
+  const Vec2 kInteractiveAreaSize = {timeline_ui->GetSize()[0], time_graph->GetSize()[1]};
+  // Mouse position in the center of the interactive area.
+  const Vec2 kCenteredMousePosition{kInteractiveAreaPos[0] + kInteractiveAreaSize[0] / 2.f,
+                                    kInteractiveAreaPos[1] + kInteractiveAreaSize[1] / 2.f};
+
   MouseWheelUp(kCenteredMousePosition, /*ctrl=*/false);
 
   // MouseWheel should scroll when ctrl is not pressed and therefore not modify the time_window.

src/OrbitGl/include/OrbitGl/GlSlider.h

@@ -143,9 +143,6 @@ class GlHorizontalSlider : public GlSlider {
   void DoDraw(PrimitiveAssembler& primitive_assembler, TextRenderer& text_renderer,
               const DrawContext& draw_context) override;
 
-  [[nodiscard]] float GetWidth() const override {
-    return viewport_->GetScreenWidth() - layout_->GetSliderWidth();
-  }
   [[nodiscard]] float GetHeight() const override { return GetSliderWidth(); }
 
   std::unique_ptr<orbit_accessibility::AccessibleInterface> CreateAccessibleInterface() override;

src/OrbitGl/include/OrbitGl/StaticTimeGraphLayout.h

@@ -64,6 +64,7 @@ class StaticTimeGraphLayout : public TimeGraphLayout {
   [[nodiscard]] float GetRoundingRadius() const override { return kRoundingRadius * scale_; }
   [[nodiscard]] float GetRoundingNumSides() const override { return kRoundingNumSides; }
   [[nodiscard]] float GetTextOffset() const override { return kTextOffset * scale_; }
+  [[nodiscard]] float GetLeftMargin() const override { return kLeftMargin * scale_; }
   [[nodiscard]] float GetRightMargin() const override { return kRightMargin * scale_; }
   [[nodiscard]] float GetMinButtonSize() const override { return kMinButtonSize; }
   [[nodiscard]] float GetButtonWidth() const override { return kButtonWidth * scale_; }
@@ -131,6 +132,7 @@ class StaticTimeGraphLayout : public TimeGraphLayout {
   constexpr static float kRoundingRadius = 8.f;
   constexpr static float kRoundingNumSides = 16;
   constexpr static float kTextOffset = 5.f;
+  constexpr static float kLeftMargin = 100.f;
   constexpr static float kRightMargin = 10.f;
   constexpr static float kMinButtonSize = 5.f;
   constexpr static float kButtonWidth = 15.f;

src/OrbitGl/include/OrbitGl/TimeGraph.h

@@ -85,6 +85,7 @@ class TimeGraph : public orbit_gl::CaptureViewElement, public orbit_gl::Timeline
   [[nodiscard]] orbit_gl::TrackManager* GetTrackManager() const {
     return track_container_->GetTrackManager();
   }
+  [[nodiscard]] Vec2 GetTimelinePos() const { return GetTimelineUi()->GetPos(); }
   [[nodiscard]] float GetTimelineWidth() const { return GetTimelineUi()->GetWidth(); }
 
   [[nodiscard]] float GetWorldFromTick(uint64_t time) const override;

src/OrbitGl/include/OrbitGl/TimeGraphLayout.h

@@ -29,6 +29,7 @@ class TimeGraphLayout {
   [[nodiscard]] virtual float GetRoundingRadius() const = 0;
   [[nodiscard]] virtual float GetRoundingNumSides() const = 0;
   [[nodiscard]] virtual float GetTextOffset() const = 0;
+  [[nodiscard]] virtual float GetLeftMargin() const = 0;
   [[nodiscard]] virtual float GetRightMargin() const = 0;
   [[nodiscard]] virtual float GetMinButtonSize() const = 0;
   [[nodiscard]] virtual float GetButtonWidth() const = 0;

src/OrbitQt/TimeGraphLayoutWidget.cpp

@@ -32,6 +32,7 @@ TimeGraphLayoutWidget::TimeGraphLayoutWidget(QWidget* parent)
   AddWidgetForProperty(&rounding_radius_);
   AddWidgetForProperty(&rounding_num_sides_);
   AddWidgetForProperty(&text_offset_);
+  AddWidgetForProperty(&left_margin_);
   AddWidgetForProperty(&right_margin_);
   AddWidgetForProperty(&min_button_size_);
   AddWidgetForProperty(&button_width_);

src/OrbitQt/include/OrbitQt/TimeGraphLayoutWidget.h

@@ -70,6 +70,9 @@ class TimeGraphLayoutWidget : public orbit_config_widgets::PropertyConfigWidget,
   [[nodiscard]] float GetTextOffset() const override {
     return text_offset_.value() * scale_.value();
   }
+  [[nodiscard]] float GetLeftMargin() const override {
+    return left_margin_.value() * scale_.value();
+  }
   [[nodiscard]] float GetRightMargin() const override {
     return right_margin_.value() * scale_.value();
   }
@@ -230,6 +233,12 @@ class TimeGraphLayoutWidget : public orbit_config_widgets::PropertyConfigWidget,
       .initial_value = 5.f,
       .label = "Text Offset:",
   }};
+  FloatProperty left_margin_{{
+      .initial_value = 0.f,
+      .min = 0.f,
+      .max = 1000.f,
+      .label = "Left Margin:",
+  }};
   FloatProperty right_margin_{{
       .initial_value = 10.f,
       .label = "Right Margin:",