Many O3DVisualizer functions cause crashes/hangs if called during OnAnimationTick/ OnAnimationFrame

[EwingKang]

As title stated, it took me some time going through the source code to understand the issue. One of the most easily encountered example is SetAnimating(). This is used when an user is trying to stop the animation automatically after reaching some animation time.
I believe, and please correct me if I'm worng, the root cause arises from the SetAnimating() function. The function unsets the callback functor during the callback. This causes some kind of callstack corruption.

Open3D/cpp/open3d/visualization/visualizer/O3DVisualizer.cpp

Lines 1737 to 1758 in d7a2cf6

	void SetAnimating(bool is_animating) {
	if (is_animating == ui_state_.is_animating) {
	return;
	}
	ui_state_.is_animating = is_animating;
	if (is_animating) {
	ui_state_.current_time = max_time_;
	auto now = Application::GetInstance().Now();
	start_animation_clock_time_ = now;
	last_animation_tick_clock_time_ = now;
	if (on_animation_tick_) {
	window_->SetOnTickEvent(on_animation_tick_);
	} else {
	window_->SetOnTickEvent(
	[this]() -> bool { return this->OnAnimationTick(); });
	}
	} else {
	window_->SetOnTickEvent(nullptr);
	SetCurrentTime(0.0);
	next_animation_tick_clock_time_ = 0.0;
	}

This can be captured with GDB, which shows memory is not accessible:

Thread 1 "my_test" received signal SIGSEGV, Segmentation fault.
open3d::visualization::visualizer::O3DVisualizer::Impl::SetOnAnimationTick(open3d::visualization::visualizer::O3DVisualizer&, std::function<open3d::visualization::visualizer::O3DVisualizer::TickResult (open3d::visualization::visualizer::O3DVisualizer&, double, double)>)::{lambda()#1}::operator()() const (__closure=0x55555ff73d10) at /Open3D/cpp/open3d/visualization/visualizer/O3DVisualizer.cpp:1776
1776                        this->scene_->ForceRedraw();
(gdb) bt
#0  open3d::visualization::visualizer::O3DVisualizer::Impl::SetOnAnimationTick(open3d::visualization::visualizer::O3DVisualizer&, std::function<open3d::visualization::visualizer::O3DVisualizer::TickResult (open3d::visualization::visualizer::O3DVisualizer&, double, double)>)::{lambda()#1}::operator()() const (__closure=0x55555ff73d10)
    at /Open3D/cpp/open3d/visualization/visualizer/O3DVisualizer.cpp:1776
#1  0x0000555556c8a7ae in std::__invoke_impl<bool, open3d::visualization::visualizer::O3DVisualizer::Impl::SetOnAnimationTick(open3d::visualization::visualizer::O3DVisualizer&, std::function<open3d::visualization::visualizer::O3DVisualizer::TickResult (open3d::visualization::visualizer::O3DVisualizer&, double, double)>)::{lambda()#1}&>(std::__invoke_other, open3d::visualization::visualizer::O3DVisualizer::Impl::SetOnAnimationTick(open3d::visualization::visualizer::O3DVisualizer&, std::function<open3d::visualization::visualizer::O3DVisualizer::TickResult (open3d::visualization::visualizer::O3DVisualizer&, double, double)>)::{lambda()#1}&) (__f=...) at /usr/include/c++/11/bits/invoke.h:61
#2  0x0000555556c85110 in std::__invoke_r<bool, open3d::visualization::visualizer::O3DVisualizer::Impl::SetOnAnimationTick(open3d::visualization::visualizer::O3DVisualizer&, std::function<open3d::visualization::visualizer::O3DVisualizer::TickResult (open3d::visualization::visualizer::O3DVisualizer&, double, double)>)::{lambda()#1}&>(open3d::visualization::visualizer::O3DVisualizer::Impl::SetOnAnimationTick(open3d::visualization::visualizer::O3DVisualizer&, std::function<open3d::visualization::visualizer::O3DVisualizer::TickResult (open3d::visualization::visualizer::O3DVisualizer&, double, double)>)::{lambda()#1}&) (__fn=...)
    at /usr/include/c++/11/bits/invoke.h:142
#3  0x0000555556c7f2e2 in std::_Function_handler<bool (), open3d::visualization::visualizer::O3DVisualizer::Impl::SetOnAnimationTick(open3d::visualization::visualizer::O3DVisualizer&, std::function<open3d::visualization::visualizer::O3DVisualizer::TickResult (open3d::visualization::visualizer::O3DVisualizer&, double, double)>)::{lambda()#1}>::_M_invoke(std::_Any_data const&) (__functor=...)
    at /usr/include/c++/11/bits/std_function.h:290
#4  0x0000555556d12ea6 in std::function<bool ()>::operator()() const (this=0x55555f0eda88) at /usr/include/c++/11/bits/std_function.h:590
#5  0x0000555556d1013a in open3d::visualization::gui::Window::OnTickEvent (this=0x55555ed8e990, e=...) at /Open3D/cpp/open3d/visualization/gui/Window.cpp:1209
#6  0x0000555556ca4b2e in open3d::visualization::gui::Application::ProcessQueuedEvents (this=0x55555e97d340 <open3d::visualization::gui::Application::GetInstance()::g_app>, unlocker=...)
    at /Open3D/cpp/open3d/visualization/gui/Application.cpp:596
#7  0x0000555556ca4790 in open3d::visualization::gui::Application::RunOneTick (this=0x55555e97d340 <open3d::visualization::gui::Application::GetInstance()::g_app>, unlocker=..., cleanup_if_no_windows=true)
    at /Open3D/cpp/open3d/visualization/gui/Application.cpp:562
#8  0x0000555556ca4473 in open3d::visualization::gui::Application::Run (this=0x55555e97d340 <open3d::visualization::gui::Application::GetInstance()::g_app>)
    at /Open3D/cpp/open3d/visualization/gui/Application.cpp:525
#9  ...main
(gdb) p this
$1 = (open3d::visualization::visualizer::O3DVisualizer::Impl * const) 0x55500aa29683
(gdb) p *this
Cannot access memory at address 0x55500aa29683
(gdb)

Another similar problem is with AddGeometry and RemoveGeometry within the animation callback. Since it actually calls SetAnnimating(false) during the callback. However, I believe that this is the intended way of updating the geometries since UpdateGeometry only works with TGeometry PointClouds.

Open3D/cpp/open3d/visualization/visualizer/O3DVisualizer.cpp

Lines 1078 to 1081 in d7a2cf6

	if (min_time_ == max_time_) {
	SetAnimating(false);
	}
	UpdateTimeUIRange();

My current workaround is to call the geometry update function directly from the rendering scene object.

open3d::visualization::visualizer::O3DVisualizer* o3d_vis;
//...
window->SetOnAnimationTick([](o3d_vis::O3DVisualizer &this_o3d, double dt, double total_time) -> o3d_vis::O3DVisualizer::TickResult {
  this_o3d.GetScene()->RemoveGeometry("my_mesh");
  open3d::visualization::rendering::MaterialRecord mat;
  //mat.base_color = CalcDefaultUnlitColor();
  mat.shader = "defaultUnlit";
  this_o3d.GetScene()->AddGeometry("my_mesh", my_mesh.get(), mat);
  //...
}

So what is the best way to work around this?
I found a similar case in Widget class, which uses a flag to avoid the issue.

Open3D/cpp/open3d/visualization/gui/Widget.cpp

Lines 111 to 124 in d7a2cf6

	void Widget::DrawImGuiPushEnabledState() {
	if (!IsEnabled()) {
	ImGui::PushItemFlag(ImGuiItemFlags_Disabled, true);
	ImGui::PushStyleVar(ImGuiStyleVar_Alpha,
	ImGui::GetStyle().Alpha * 0.5f);
	}
	// As an immediate mode GUI, responses to UI events can happen
	// during a draw. Store what the disabled flag was at the
	// beginning of the draw, so we know how many things to pop
	// to clean up. (An example of when this is needed is a reset
	// button: after clicking it, it will probably disable itself
	// since there is nothing to reset now)
	impl_->pop_disabled_flags_at_end_of_draw_ = !IsEnabled();
	}

I think that even we're not going to implement anything for change of behavior, it should at least implement some kind of preprocessor function that warns/prevents user from calling invalid functions within the animation callback.

[cwreynolds]

I do not know the answer to @EwingKang's issue, but wanted to note this possibly related issue I filed back in April 2023: #6094 “how to use animation callbacks with o3d.visualization.draw()?”

[EwingKang]

@cwreynolds you mentioned in #6094 that

The bad news is the callback is either hung or no longer getting invoked by the Visualizer

I think this is exactly the same thing I discovered here! Since in your code you did called geometry related functions, it probably "unbinds" the callback and caused some weird/ crashing behavior

One of the obscure code that I discovered but didn't mentioned here is about the time parameter in draw()./
It appears that if you set any animation related functions, such as setting animation_duration or making any one of the drawing geometry has a non-zero time parameter, any drawing geometry will only be shown in a single frame as controlled by following code.

Open3D/cpp/open3d/visualization/visualizer/O3DVisualizer.cpp

Lines 1995 to 2003 in d7a2cf6

	bool IsGeometryVisible(const DrawObject &o) {
	bool is_current =
	(o.time >= ui_state_.current_time &&
	o.time < ui_state_.current_time + ui_state_.time_step);
	bool is_group_enabled = (ui_state_.enabled_groups.find(o.group) !=
	ui_state_.enabled_groups.end());
	bool is_visible = o.is_visible;
	return (is_visible & is_current & is_group_enabled);
	}

By that logic, If I want to show a 30 second animation with 0.025 (40Hz) time step, I would have to add 1200 geometry just for one single object. I think that is good for no computational efforts, but very impractical consider the memory usage. Doing transformation should be quick enough that can be performed in real time.

I'm not sure if this is a separate issue or not, but I do think we probably need to re think the design of this