From c948c4576506aa6f5e0f75be876216f443ba7800 Mon Sep 17 00:00:00 2001 From: Sameer Sheorey <41028320+ssheorey@users.noreply.github.com> Date: Fri, 29 Dec 2023 14:50:23 -0800 Subject: [PATCH] Visualizer: remove glfwTerminate() call from destructor (fix #4960) (See PR #4963) [Author: @bchretien] (#6559) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * examples: fix headless_rendering.py * ViewControl.cpp: relax floating-point comparison in ConvertFromPinholeCameraParameters() * Visualizer: improve GLTF context handling (fix #4960) --------- Authored-by: Benjamin Chrétien <2742231+bchretien@users.noreply.github.com> --- .../visualization/visualizer/ViewControl.cpp | 17 ++-- .../visualization/visualizer/Visualizer.cpp | 81 ++++++++++++------- .../visualization/visualizer/Visualizer.h | 6 ++ 3 files changed, 69 insertions(+), 35 deletions(-) diff --git a/cpp/open3d/visualization/visualizer/ViewControl.cpp b/cpp/open3d/visualization/visualizer/ViewControl.cpp index 0f2f21dfcb8..a4dff55a64a 100644 --- a/cpp/open3d/visualization/visualizer/ViewControl.cpp +++ b/cpp/open3d/visualization/visualizer/ViewControl.cpp @@ -174,13 +174,16 @@ bool ViewControl::ConvertFromPinholeCameraParameters( bool allow_arbitrary) { auto intrinsic = parameters.intrinsic_; auto extrinsic = parameters.extrinsic_; - if (!allow_arbitrary && (window_height_ <= 0 || window_width_ <= 0 || - window_height_ != intrinsic.height_ || - window_width_ != intrinsic.width_ || - intrinsic.intrinsic_matrix_(0, 2) != - (double)window_width_ / 2.0 - 0.5 || - intrinsic.intrinsic_matrix_(1, 2) != - (double)window_height_ / 2.0 - 0.5)) { + + constexpr double threshold = 1.e-6; + if (!allow_arbitrary && + (window_height_ <= 0 || window_width_ <= 0 || + window_height_ != intrinsic.height_ || + window_width_ != intrinsic.width_ || + std::abs(intrinsic.intrinsic_matrix_(0, 2) - + ((double)window_width_ / 2.0 - 0.5)) > threshold || + std::abs(intrinsic.intrinsic_matrix_(1, 2) = + ((double)window_height_ / 2.0 - 0.5)) > threshold)) { utility::LogWarning( "[ViewControl] ConvertFromPinholeCameraParameters() failed " "because window height and width do not match."); diff --git a/cpp/open3d/visualization/visualizer/Visualizer.cpp b/cpp/open3d/visualization/visualizer/Visualizer.cpp index 4681a05590d..216c4898a0a 100644 --- a/cpp/open3d/visualization/visualizer/Visualizer.cpp +++ b/cpp/open3d/visualization/visualizer/Visualizer.cpp @@ -7,6 +7,8 @@ #include "open3d/visualization/visualizer/Visualizer.h" +#include + #include "open3d/geometry/TriangleMesh.h" #if defined(__APPLE__) && defined(BUILD_GUI) @@ -18,51 +20,63 @@ void unbind(); namespace open3d { -namespace { +namespace visualization { -class GLFWEnvironmentSingleton { +/// \brief GLFW context, handled as a singleton. +class GLFWContext { private: - GLFWEnvironmentSingleton() { utility::LogDebug("GLFW init."); } - GLFWEnvironmentSingleton(const GLFWEnvironmentSingleton &) = delete; - GLFWEnvironmentSingleton &operator=(const GLFWEnvironmentSingleton &) = - delete; - -public: - ~GLFWEnvironmentSingleton() { - glfwTerminate(); - utility::LogDebug("GLFW destruct."); - } - -public: - static GLFWEnvironmentSingleton &GetInstance() { - static GLFWEnvironmentSingleton singleton; - return singleton; - } + GLFWContext() { + utility::LogDebug("GLFW init."); - static int InitGLFW() { - GLFWEnvironmentSingleton::GetInstance(); #if defined(__APPLE__) // On macOS, GLFW changes the directory to the resource directory, // which will cause an unexpected change of directory if using a // framework build version of Python. glfwInitHint(GLFW_COCOA_CHDIR_RESOURCES, GLFW_FALSE); #endif - return glfwInit(); + init_status_ = glfwInit(); + } + + GLFWContext(const GLFWContext &) = delete; + GLFWContext &operator=(const GLFWContext &) = delete; + +public: + ~GLFWContext() { + if (init_status_ == GLFW_TRUE) { + glfwTerminate(); + utility::LogDebug("GLFW destruct."); + } + } + + /// \brief Get the glfwInit status. + inline int InitStatus() const { return init_status_; } + + /// \brief Get a shared instance of the GLFW context. + static std::shared_ptr GetInstance() { + static std::weak_ptr singleton; + + auto res = singleton.lock(); + if (res == nullptr) { + res = std::shared_ptr(new GLFWContext()); + singleton = res; + } + return res; } static void GLFWErrorCallback(int error, const char *description) { utility::LogWarning("GLFW Error: {}", description); } -}; - -} // unnamed namespace -namespace visualization { +private: + /// \brief Status of the glfwInit call. + int init_status_ = GLFW_FALSE; +}; Visualizer::Visualizer() {} Visualizer::~Visualizer() { - glfwTerminate(); // to be safe + DestroyVisualizerWindow(); + #if defined(__APPLE__) && defined(BUILD_GUI) bluegl::unbind(); #endif @@ -77,6 +91,7 @@ bool Visualizer::CreateVisualizerWindow( const bool visible /* = true*/) { window_name_ = window_name; if (window_) { // window already created + utility::LogDebug("[Visualizer] Reusing window."); UpdateWindowTitle(); glfwSetWindowPos(window_, left, top); glfwSetWindowSize(window_, width, height); @@ -91,8 +106,10 @@ bool Visualizer::CreateVisualizerWindow( return true; } - glfwSetErrorCallback(GLFWEnvironmentSingleton::GLFWErrorCallback); - if (!GLFWEnvironmentSingleton::InitGLFW()) { + utility::LogDebug("[Visualizer] Creating window."); + glfwSetErrorCallback(GLFWContext::GLFWErrorCallback); + glfw_context_ = GLFWContext::GetInstance(); + if (glfw_context_->InitStatus() != GLFW_TRUE) { utility::LogWarning("Failed to initialize GLFW"); return false; } @@ -201,9 +218,17 @@ bool Visualizer::CreateVisualizerWindow( } void Visualizer::DestroyVisualizerWindow() { + if (!is_initialized_) { + return; + } + + utility::LogDebug("[Visualizer] Destroying window."); is_initialized_ = false; glDeleteVertexArrays(1, &vao_id_); + vao_id_ = 0; glfwDestroyWindow(window_); + window_ = nullptr; + glfw_context_.reset(); } void Visualizer::RegisterAnimationCallback( diff --git a/cpp/open3d/visualization/visualizer/Visualizer.h b/cpp/open3d/visualization/visualizer/Visualizer.h index 247ee2c91fe..3d72d54ee41 100644 --- a/cpp/open3d/visualization/visualizer/Visualizer.h +++ b/cpp/open3d/visualization/visualizer/Visualizer.h @@ -37,6 +37,8 @@ class Image; namespace visualization { +class GLFWContext; + /// \class Visualizer /// /// \brief The main Visualizer class. @@ -262,6 +264,10 @@ class Visualizer { // window GLFWwindow *window_ = NULL; std::string window_name_ = "Open3D"; + + /// \brief Shared GLFW context. + std::shared_ptr glfw_context_ = nullptr; + Eigen::Vector2i saved_window_size_ = Eigen::Vector2i::Zero(); Eigen::Vector2i saved_window_pos_ = Eigen::Vector2i::Zero(); std::function animation_callback_func_ = nullptr;