diff --git a/qt-models/diveplannermodel.cpp b/qt-models/diveplannermodel.cpp index 6824011b4..3fc4f0278 100644 --- a/qt-models/diveplannermodel.cpp +++ b/qt-models/diveplannermodel.cpp @@ -1118,25 +1118,38 @@ void DivePlannerPointsModel::updateDiveProfile() if (diveplan.is_empty()) return; - // For calculating variations, we need a copy of the plan. We have to copy _before_ - // calling plan(), because that adds deco stops. - bool computeVariations = isPlanner() && shouldComputeVariations(); - struct diveplan plan_copy; - if (computeVariations) - plan_copy = diveplan; - deco_state_cache cache; struct deco_state plan_deco_state; plan(&plan_deco_state, diveplan, d, dcNr, decotimestep, cache, isPlanner(), false); updateMaxDepth(); - if (computeVariations) { + if (isPlanner() && shouldComputeVariations()) { + auto plan_copy = std::make_unique(); + lock_planner(); + *plan_copy = diveplan; + unlock_planner(); #ifdef VARIATIONS_IN_BACKGROUND - QtConcurrent::run([this, plan = std::move(plan_copy), deco = plan_deco_state] () - { this->computeVariations(std::move(plan), deco); }); + // Since we're calling computeVariations asynchronously and plan_deco_state is allocated + // on the stack, it must be copied and freed by the worker-thread. + auto deco_copy = std::make_unique(plan_deco_state); + + // Ideally, we would pass the unique_ptrs to the lambda for QtConcurrent::run(). + // This, in principle, can be done as such: + // [ptr = std::move(ptr)] () mutable { f(std::move(ptr)) }; + // However, this make the lambda uncopyable and QtConcurrent::run() sadly + // uses copy semantics. + // So let's be pragmatic and do a release/reaquire pair. + // Somewhat disappointing, but what do you want to do? + // Note 1: this is now not exception safe, but Qt doesn't support + // exceptions anyway. + // Note 2: We also can't use the function / argument syntax of QtConcurrent::run(), + // because it likewise uses copy-semantics. How annoying. + QtConcurrent::run([this, plan = plan_copy.release(), deco = deco_copy.release()] () + { this->computeVariationsFreeDeco(std::unique_ptr(plan), + std::unique_ptr(deco)); }); #else - computeVariations(std::move(plan_copy), plan_deco_state); + computeVariations(std::move(plan_copy), &plan_deco_state); #endif final_deco_state = plan_deco_state; } @@ -1181,6 +1194,12 @@ int DivePlannerPointsModel::analyzeVariations(const std::vector &min, return (leftsum + rightsum) / 2; } +void DivePlannerPointsModel::computeVariationsFreeDeco(std::unique_ptr original_plan, std::unique_ptr previous_ds) +{ + computeVariations(std::move(original_plan), previous_ds.get()); + // Note: previous ds automatically free()d by virtue of being a unique_ptr. +} + // Return reference to second to last element. // Caller is responsible for checking that there are at least two elements. template @@ -1189,16 +1208,17 @@ auto &second_to_last(T &v) return *std::prev(std::prev(v.end())); } -void DivePlannerPointsModel::computeVariations(struct diveplan original_plan, struct deco_state ds) +void DivePlannerPointsModel::computeVariations(std::unique_ptr original_plan, const struct deco_state *previous_ds) { // nothing to do unless there's an original plan - if (original_plan.dp.empty()) + if (!original_plan) return; auto dive = std::make_unique(); copy_dive(d, dive.get()); deco_state_cache cache, save; struct diveplan plan_copy; + struct deco_state ds = *previous_ds; int my_instance = ++instanceCounter; save.cache(&ds); @@ -1216,7 +1236,7 @@ void DivePlannerPointsModel::computeVariations(struct diveplan original_plan, st depth_units = tr("ft"); } - plan_copy = original_plan; + plan_copy = *original_plan; if (plan_copy.dp.size() < 2) return; if (my_instance != instanceCounter) @@ -1224,7 +1244,7 @@ void DivePlannerPointsModel::computeVariations(struct diveplan original_plan, st auto original = plan(&ds, plan_copy, dive.get(), dcNr, 1, cache, true, false); save.restore(&ds, false); - plan_copy = original_plan; + plan_copy = *original_plan; second_to_last(plan_copy.dp).depth.mm += delta_depth.mm; plan_copy.dp.back().depth.mm += delta_depth.mm; if (my_instance != instanceCounter) @@ -1232,7 +1252,6 @@ void DivePlannerPointsModel::computeVariations(struct diveplan original_plan, st auto deeper = plan(&ds, plan_copy, dive.get(), dcNr, 1, cache, true, false); save.restore(&ds, false); - plan_copy = original_plan; second_to_last(plan_copy.dp).depth.mm -= delta_depth.mm; plan_copy.dp.back().depth.mm -= delta_depth.mm; if (my_instance != instanceCounter) @@ -1240,14 +1259,13 @@ void DivePlannerPointsModel::computeVariations(struct diveplan original_plan, st auto shallower = plan(&ds, plan_copy, dive.get(), dcNr, 1, cache, true, false); save.restore(&ds, false); - plan_copy = original_plan; + plan_copy = *original_plan; plan_copy.dp.back().time += delta_time.seconds; if (my_instance != instanceCounter) return; auto longer = plan(&ds, plan_copy, dive.get(), dcNr, 1, cache, true, false); save.restore(&ds, false); - plan_copy = original_plan; plan_copy.dp.back().time -= delta_time.seconds; if (my_instance != instanceCounter) return; @@ -1290,16 +1308,15 @@ void DivePlannerPointsModel::createPlan(bool saveAsNew) removeDeco(); createTemporaryPlan(); - // For calculating variations, we need a copy of the plan. We have to copy _before_ - // calling plan(), because that adds deco stops. - struct diveplan plan_copy; - if (shouldComputeVariations()) - plan_copy = diveplan; - plan(&ds_after_previous_dives, diveplan, d, dcNr, decotimestep, cache, isPlanner(), true); - if (shouldComputeVariations()) - computeVariations(std::move(plan_copy), ds_after_previous_dives); + if (shouldComputeVariations()) { + auto plan_copy = std::make_unique(); + lock_planner(); + *plan_copy = diveplan; + unlock_planner(); + computeVariations(std::move(plan_copy), &ds_after_previous_dives); + } // Fixup planner notes. if (current_dive && d->id == current_dive->id) { diff --git a/qt-models/diveplannermodel.h b/qt-models/diveplannermodel.h index 1dadfacc7..af958b938 100644 --- a/qt-models/diveplannermodel.h +++ b/qt-models/diveplannermodel.h @@ -131,7 +131,8 @@ private: void createTemporaryPlan(); struct diveplan diveplan; void computeVariationsDone(QString text); - void computeVariations(struct diveplan plan, struct deco_state ds); // Note: works on copies of plan and ds + void computeVariations(std::unique_ptr plan, const struct deco_state *ds); + void computeVariationsFreeDeco(std::unique_ptr plan, std::unique_ptr ds); int analyzeVariations(const std::vector &min, const std::vector &mid, const std::vector &max, const char *unit); struct dive *d; int dcNr;