Fix a macOS segfault in simulate.cc.

The segfault was caused by updating the UI outside the main thread.

PiperOrigin-RevId: 526019783
Change-Id: I3a211ce8a65f10716778bb3310bb568e00309a32
This commit is contained in:
Saran Tunyasuvunakool
2023-04-21 06:45:13 -07:00
committed by Copybara-Service
parent 0295d972cb
commit da17fdfcfd
2 changed files with 69 additions and 63 deletions
+28 -23
View File
@@ -19,6 +19,7 @@
#include <cstdio> #include <cstdio>
#include <cstring> #include <cstring>
#include <memory> #include <memory>
#include <optional>
#include <ratio> #include <ratio>
#include <string> #include <string>
#include <type_traits> #include <type_traits>
@@ -1032,13 +1033,22 @@ void CopyCamera(mj::Simulate* sim) {
void UpdateSettings(mj::Simulate* sim, const mjModel* m) { void UpdateSettings(mj::Simulate* sim, const mjModel* m) {
// physics flags // physics flags
for (int i=0; i<mjNDISABLE; i++) { for (int i=0; i<mjNDISABLE; i++) {
sim->disable[i] = ((m->opt.disableflags & (1<<i)) !=0); int new_value = ((m->opt.disableflags & (1<<i)) !=0);
if (sim->disable[i] != new_value) {
sim->disable[i] = new_value;
sim->pending_.ui_update_physics = true;
}
} }
for (int i=0; i<mjNENABLE; i++) { for (int i=0; i<mjNENABLE; i++) {
sim->enable[i] = ((m->opt.enableflags & (1<<i)) !=0); int new_value = ((m->opt.enableflags & (1<<i)) !=0);
if (sim->enable[i] != new_value) {
sim->enable[i] = new_value;
sim->pending_.ui_update_physics = true;
}
} }
// camera // camera
int old_camera = sim->camera;
if (sim->cam.type==mjCAMERA_FIXED) { if (sim->cam.type==mjCAMERA_FIXED) {
sim->camera = 2 + sim->cam.fixedcamid; sim->camera = 2 + sim->cam.fixedcamid;
} else if (sim->cam.type==mjCAMERA_TRACKING) { } else if (sim->cam.type==mjCAMERA_TRACKING) {
@@ -1046,9 +1056,9 @@ void UpdateSettings(mj::Simulate* sim, const mjModel* m) {
} else { } else {
sim->camera = 0; sim->camera = 0;
} }
if (old_camera != sim->camera) {
// update UI sim->pending_.ui_update_rendering = true;
sim->pending_.full_ui_update = true; }
} }
// Compute suitable font scale. // Compute suitable font scale.
@@ -1557,7 +1567,6 @@ void Simulate::Sync() {
bool update_profiler = this->profiler && (this->run || !this->m_); bool update_profiler = this->profiler && (this->run || !this->m_);
bool update_sensor = this->sensor && (this->run || !this->m_); bool update_sensor = this->sensor && (this->run || !this->m_);
bool update_settings = false;
for (int i = 0; i < m_->njnt; ++i) { for (int i = 0; i < m_->njnt; ++i) {
std::optional<std::pair<mjtNum, mjtNum>> range; std::optional<std::pair<mjtNum, mjtNum>> range;
@@ -1681,13 +1690,11 @@ void Simulate::Sync() {
mj_forward(m_, d_); mj_forward(m_, d_);
update_profiler = true; update_profiler = true;
update_sensor = true; update_sensor = true;
update_settings = true;
pending_.reset = false; pending_.reset = false;
} }
if (pending_.align) { if (pending_.align) {
AlignAndScaleView(this, m_); AlignAndScaleView(this, m_);
update_settings = true;
pending_.align = false; pending_.align = false;
} }
@@ -1709,7 +1716,6 @@ void Simulate::Sync() {
mj_forward(m_, d_); mj_forward(m_, d_);
update_profiler = true; update_profiler = true;
update_sensor = true; update_sensor = true;
update_settings = true;
pending_.load_key = false; pending_.load_key = false;
} }
@@ -1774,8 +1780,7 @@ void Simulate::Sync() {
// UI camera // UI camera
this->camera = 1; this->camera = 1;
mjui_update(SECT_RENDERING, -1, &this->ui0, &pending_.select_state, pending_.ui_update_rendering = true;
&this->platform_ui->mjr_context());
} }
} }
@@ -1807,6 +1812,9 @@ void Simulate::Sync() {
warn_vgeomfull_prev_ = scnstate_.data.warning[mjWARN_VGEOMFULL].number; warn_vgeomfull_prev_ = scnstate_.data.warning[mjWARN_VGEOMFULL].number;
} }
// update settings
UpdateSettings(this, m_);
// update watch // update watch
if (this->ui0_enable && this->ui0.sect[SECT_WATCH].state) { if (this->ui0_enable && this->ui0.sect[SECT_WATCH].state) {
UpdateWatch(this, m_, d_); UpdateWatch(this, m_, d_);
@@ -1818,7 +1826,6 @@ void Simulate::Sync() {
} }
if (update_profiler) { UpdateProfiler(this, m_, d_); } if (update_profiler) { UpdateProfiler(this, m_, d_); }
if (update_sensor) { UpdateSensor(this, m_, d_); } if (update_sensor) { UpdateSensor(this, m_, d_); }
if (update_settings) { UpdateSettings(this, m_); }
// clear timers once profiler info has been copied // clear timers once profiler info has been copied
ClearTimers(d_); ClearTimers(d_);
@@ -2057,14 +2064,6 @@ void Simulate::Render() {
} }
// update UI sections from last sync // update UI sections from last sync
if (pending_.full_ui_update) {
mjui_update(-1, -1, &this->ui0, &this->uistate, &this->platform_ui->mjr_context());
mjui_update(-1, -1, &this->ui1, &this->uistate, &this->platform_ui->mjr_context());
pending_.full_ui_update = false;
pending_.ui_update_physics = false;
pending_.ui_update_joint = false;
pending_.ui_update_ctrl = false;
} else {
if (this->ui0_enable && this->ui0.sect[SECT_WATCH].state) { if (this->ui0_enable && this->ui0.sect[SECT_WATCH].state) {
mjui_update(SECT_WATCH, -1, &this->ui0, &this->uistate, &this->platform_ui->mjr_context()); mjui_update(SECT_WATCH, -1, &this->ui0, &this->uistate, &this->platform_ui->mjr_context());
} }
@@ -2083,8 +2082,7 @@ void Simulate::Render() {
cam_prev_.trackbodyid != cam.trackbodyid || cam_prev_.trackbodyid != cam.trackbodyid ||
opt_prev_.label != opt.label || opt_prev_.frame != opt.frame || opt_prev_.label != opt.label || opt_prev_.frame != opt.frame ||
Differ(opt_prev_.flags, opt.flags))) { Differ(opt_prev_.flags, opt.flags))) {
mjui_update(SECT_RENDERING, -1, &this->ui0, &this->uistate, pending_.ui_update_rendering = true;
&this->platform_ui->mjr_context());
} }
if (this->ui0_enable && this->ui0.sect[SECT_RENDERING].state && if (this->ui0_enable && this->ui0.sect[SECT_RENDERING].state &&
@@ -2102,6 +2100,14 @@ void Simulate::Render() {
cam_prev_ = cam; cam_prev_ = cam;
} }
if (pending_.ui_update_rendering) {
if (this->ui0_enable && this->ui0.sect[SECT_RENDERING].state) {
mjui_update(SECT_RENDERING, -1, &this->ui0, &this->uistate,
&this->platform_ui->mjr_context());
}
pending_.ui_update_rendering = false;
}
if (pending_.ui_update_joint) { if (pending_.ui_update_joint) {
if (this->ui1_enable && this->ui1.sect[SECT_JOINT].state) { if (this->ui1_enable && this->ui1.sect[SECT_JOINT].state) {
mjui_update(SECT_JOINT, -1, &this->ui1, &this->uistate, &this->platform_ui->mjr_context()); mjui_update(SECT_JOINT, -1, &this->ui1, &this->uistate, &this->platform_ui->mjr_context());
@@ -2115,7 +2121,6 @@ void Simulate::Render() {
} }
pending_.ui_update_ctrl = false; pending_.ui_update_ctrl = false;
} }
}
// render scene // render scene
mjr_render(rect, &this->scn, &this->platform_ui->mjr_context()); mjr_render(rect, &this->scn, &this->platform_ui->mjr_context());
+2 -1
View File
@@ -22,6 +22,7 @@
#include <mutex> #include <mutex>
#include <optional> #include <optional>
#include <ratio> #include <ratio>
#include <string>
#include <utility> #include <utility>
#include <vector> #include <vector>
@@ -127,8 +128,8 @@ class Simulate {
int newperturb; int newperturb;
bool select; bool select;
mjuiState select_state; mjuiState select_state;
bool full_ui_update;
bool ui_update_physics; bool ui_update_physics;
bool ui_update_rendering;
bool ui_update_joint; bool ui_update_joint;
bool ui_update_ctrl; bool ui_update_ctrl;
} pending_ = {}; } pending_ = {};