The animation callbacks need machine pointers to be stable

If an machine has an animation active it cannot be moved as
the animation has a pointer to the machine object.

1 files changed, 61 insertions, 52 deletions

diff --git a/src/main.cc b/src/main.cc
index 62620c8..9285ccb 100644
--- a/src/main.cc
+++ b/src/main.cc
@@ -393,6 +393,9 @@ private:
       : id(id), jobs(0), requests(0), x(0.0) {
     }
 
+    Machine(Machine const&) = delete;
+    Machine& operator=(Machine const&) = delete;
+
     ~Machine() override {
       assert(!animation);
     }
@@ -572,19 +575,19 @@ private:
   void stop_all_animations() {
     if (!animator_) return;
     for (auto& machine : machines_) {
-      if (machine.animation) animator_->stop(machine.animation.get());
+      if (machine->animation) animator_->stop(machine->animation.get());
     }
   }
 
-  void update(Machine& machine) {
-    auto old = machine.data.max_jobs;
-    machine.data = monitor_->machine(machine.id);
-    if (old < machine.data.max_jobs) {
-      max_jobs_ += machine.data.max_jobs - old;
-      if (machine.jobs) animate(machine);
-    } else if (old > machine.data.max_jobs) {
-      max_jobs_ -= old - machine.data.max_jobs;
-      if (machine.jobs) animate(machine);
+  void update(Machine* machine) {
+    auto old = machine->data.max_jobs;
+    machine->data = monitor_->machine(machine->id);
+    if (old < machine->data.max_jobs) {
+      max_jobs_ += machine->data.max_jobs - old;
+      if (machine->jobs) animate(machine);
+    } else if (old > machine->data.max_jobs) {
+      max_jobs_ -= old - machine->data.max_jobs;
+      if (machine->jobs) animate(machine);
     }
   }
 
@@ -643,19 +646,19 @@ private:
                            (box_width - margin_x * 2) * PANGO_SCALE);
     unsigned col = 0;
     for (auto const& machine : machines_) {
-      pango_layout_set_text(layout_.get(), machine.data.name.data(),
-                            machine.data.name.size());
+      pango_layout_set_text(layout_.get(), machine->data.name.data(),
+                            machine->data.name.size());
       auto x = pad_x + col * (box_width + pad_x);
       rounded_path(cairo_.get(), x, y, box_width, box_height);
       cairo_set_source_rgba(cairo_.get(), 0.1, 0.1, 0.1, 0.7);
 
-      if (machine.x > 0.0) {
+      if (machine->x > 0.0) {
         cairo_fill_preserve(cairo_.get());
         auto old = std::unique_ptr<cairo_path_t, CairoPathDelete>(
             cairo_copy_path(cairo_.get()));
         cairo_new_path(cairo_.get());
         cairo_rectangle(cairo_.get(), x, y,
-                        machine.x * box_width, box_height);
+                        machine->x * box_width, box_height);
         cairo_clip(cairo_.get());
         cairo_append_path(cairo_.get(), old.get());
         cairo_matrix_t matrix;
@@ -667,13 +670,13 @@ private:
       } else {
         cairo_fill(cairo_.get());
       }
-      if (machine.requests > 0) {
+      if (machine->requests > 0) {
         auto radius = box_height / 10.0;
         cairo_set_line_width(cairo_.get(), 1.5);
         cairo_new_path(cairo_.get());
         cairo_move_to(cairo_.get(), x + box_width - radius, y);
         cairo_rel_line_to(cairo_.get(),
-                          -(machine.requests * (box_width - radius * 2))
+                          -(machine->requests * (box_width - radius * 2))
                           / requests_, 0);
         cairo_set_source_rgb(cairo_.get(), 0, 0.6, 0);
         cairo_stroke(cairo_.get());
@@ -700,22 +703,28 @@ private:
   }
 
   void added_machine(Monitor*, uint32_t id) override {
-    Machine machine(id);
-    update(machine);
-    machines_.insert(
-        std::lower_bound(machines_.begin(), machines_.end(), machine),
-        machine);
+    auto machine = std::make_unique<Machine>(id);
+    update(machine.get());
+    machines_.emplace(
+        std::lower_bound(machines_.begin(), machines_.end(), machine,
+                         compare_machine),
+        std::move(machine));
     draw();
   }
 
+  static bool compare_machine(std::unique_ptr<Machine> const& m1,
+                              std::unique_ptr<Machine> const& m2) {
+    return *m1 < *m2;
+  }
+
   void updated_machine(Monitor*, uint32_t id) override {
     for (auto& machine : machines_) {
-      if (machine.id == id) {
-        auto old = machine.data.name;
-        update(machine);
-        if (machine.data.name != old) {
+      if (machine->id == id) {
+        auto old = machine->data.name;
+        update(machine.get());
+        if (machine->data.name != old) {
           // TODO: Perhaps remove and insert instead?
-          std::sort(machines_.begin(), machines_.end());
+          std::sort(machines_.begin(), machines_.end(), compare_machine);
         }
         draw();
         return;
@@ -726,11 +735,11 @@ private:
 
   void removed_machine(Monitor*, uint32_t id) override {
     for (auto it = machines_.begin(); it != machines_.end(); ++it) {
-      if (it->id == id) {
-        max_jobs_ -= it->data.max_jobs;
-        requests_ -= it->requests;
-        jobs_ -= it->jobs;
-        if (animator_) animator_->stop(it->animation.get());
+      if ((*it)->id == id) {
+        max_jobs_ -= (*it)->data.max_jobs;
+        requests_ -= (*it)->requests;
+        jobs_ -= (*it)->jobs;
+        if (animator_) animator_->stop((*it)->animation.get());
         machines_.erase(it);
         draw();
         return;
@@ -742,19 +751,19 @@ private:
   void added_job(Monitor*, uint32_t source, uint32_t target) override {
     if (source == target) {
       for (auto& machine : machines_) {
-        if (machine.id == source) {
-          ++machine.jobs;
-          animate(machine);
+        if (machine->id == source) {
+          ++machine->jobs;
+          animate(machine.get());
         }
       }
     } else {
       for (auto& machine : machines_) {
-        if (machine.id == source) {
-          ++machine.requests;
+        if (machine->id == source) {
+          ++machine->requests;
         }
-        if (machine.id == target) {
-          ++machine.jobs;
-          animate(machine);
+        if (machine->id == target) {
+          ++machine->jobs;
+          animate(machine.get());
         }
       }
       ++requests_;
@@ -766,19 +775,19 @@ private:
   void removed_job(Monitor*, uint32_t source, uint32_t target) override {
     if (source == target) {
       for (auto& machine : machines_) {
-        if (machine.id == source) {
-          --machine.jobs;
-          animate(machine);
+        if (machine->id == source) {
+          --machine->jobs;
+          animate(machine.get());
         }
       }
     } else {
       for (auto& machine : machines_) {
-        if (machine.id == source) {
-          --machine.requests;
+        if (machine->id == source) {
+          --machine->requests;
         }
-        if (machine.id == target) {
-          --machine.jobs;
-          animate(machine);
+        if (machine->id == target) {
+          --machine->jobs;
+          animate(machine.get());
         }
       }
       --requests_;
@@ -796,11 +805,11 @@ private:
     force_draw();
   }
 
-  void animate(Machine& machine) {
+  void animate(Machine* machine) {
     if (!animator_) return;
-    if (machine.animation) return;
-    machine.animation = std::make_unique<MachineAnimation>(&machine);
-    animator_->start(machine.animation, &machine);
+    if (machine->animation) return;
+    machine->animation = std::make_unique<MachineAnimation>(machine);
+    animator_->start(machine->animation, machine);
   }
 
   std::shared_ptr<PollLooper> looper_;
@@ -816,7 +825,7 @@ private:
   uint32_t black_pixel_;
   xcb_screen_t const* screen_;
   double box_height_;
-  std::vector<Machine> machines_;
+  std::vector<std::unique_ptr<Machine>> machines_;
   unsigned max_jobs_;
   unsigned jobs_;
   unsigned requests_;