refactor globals into a context object (#265)

* refactor globals into a context object

* add missing vector include

* add missing array include

* minor cleanup

include/polyscope/context.h

@@ -0,0 +1,111 @@
+// Copyright 2017-2023, Nicholas Sharp and the Polyscope contributors. https://polyscope.run
+
+#pragma once
+
+#include <polyscope/types.h>
+#include <polyscope/weak_handle.h>
+
+#include <glm/glm.hpp>
+#define GLM_ENABLE_EXPERIMENTAL
+#include "glm/gtx/dual_quaternion.hpp"
+#include "glm/gtx/norm.hpp" // necessary for dual_quaternion below
+#undef GLM_ENABLE_EXPERIMENTAL
+
+#include <array>
+#include <map>
+#include <memory>
+#include <set>
+#include <string>
+#include <unordered_set>
+#include <vector>
+
+
+namespace polyscope {
+
+
+// forward declarations
+class Structure;
+class Group;
+class SlicePlane;
+class Widget;
+class FloatingQuantityStructure;
+namespace view {
+extern const double defaultNearClipRatio;
+extern const double defaultFarClipRatio;
+extern const double defaultFov;
+} // namespace view
+
+// A context object wrapping all global state used by Polyscope.
+//
+// In theory the user could explicitly manage multiple contexts explicitly. However for now that is not supported, there
+// is always exactly one global context.
+//
+// Historically, these globals were simply `static` members scattered through a few different files. However, this
+// was a persistent source of bugs at shutdown time, because the order in which destructors are called during shutdown
+// is platform-dependent. Bugs often arose because one global member often depends on another; if destructed in an
+// unexpected order, they would reference one-another and cause platform-dependent errors. The global context solves
+// this because destruction always happens in a predictable order.
+
+struct Context {
+
+  // ======================================================
+  // === General globals from polyscope.h
+  // ======================================================
+
+  bool initialized = false;
+  std::string backend = "";
+  std::map<std::string, std::map<std::string, std::unique_ptr<Structure>>> structures;
+  std::map<std::string, std::unique_ptr<Group>> groups;
+  float lengthScale = 1.;
+  std::tuple<glm::vec3, glm::vec3> boundingBox =
+      std::tuple<glm::vec3, glm::vec3>{glm::vec3{-1., -1., -1.}, glm::vec3{1., 1., 1.}};
+  std::vector<std::unique_ptr<SlicePlane>> slicePlanes;
+  std::vector<WeakHandle<Widget>> widgets;
+  bool doDefaultMouseInteraction = true;
+  std::function<void()> userCallback = nullptr;
+
+
+  // ======================================================
+  // === Render engine globals from engine.h
+  // ======================================================
+
+
+  // ======================================================
+  // === View globals from view.h
+  // ======================================================
+
+  int bufferWidth = -1;
+  int bufferHeight = -1;
+  int windowWidth = 1280;
+  int windowHeight = 720;
+  int initWindowPosX = 20;
+  int initWindowPosY = 20;
+  bool windowResizable = true;
+  NavigateStyle navigateStyle = NavigateStyle::Turntable;
+  UpDir upDir = UpDir::YUp;
+  FrontDir frontDir = FrontDir::ZFront;
+  double moveScale = 1.0;
+  double nearClipRatio = view::defaultNearClipRatio;
+  double farClipRatio = view::defaultFarClipRatio;
+  std::array<float, 4> bgColor{{1.0, 1.0, 1.0, 0.0}};
+  glm::mat4x4 viewMat;
+  double fov = view::defaultFov;
+  ProjectionMode projectionMode = ProjectionMode::Perspective;
+  bool midflight = false;
+  float flightStartTime = -1;
+  float flightEndTime = -1;
+  glm::dualquat flightTargetViewR, flightInitialViewR;
+  glm::vec3 flightTargetViewT, flightInitialViewT;
+  float flightTargetFov, flightInitialFov;
+
+
+  // ======================================================
+  // === Internal globals from internal.h
+  // ======================================================
+
+  bool pointCloudEfficiencyWarningReported = false;
+  FloatingQuantityStructure* globalFloatingQuantityStructure = nullptr;
+};
+
+
+}; // namespace polyscope

include/polyscope/internal.h

@@ -21,10 +21,10 @@ namespace internal {
 uint64_t getNextUniqueID();
 
 // track various fire-once warnings
-extern bool pointCloudEfficiencyWarningReported;
+extern bool& pointCloudEfficiencyWarningReported;
 
 // global members
-extern FloatingQuantityStructure* globalFloatingQuantityStructure;
+extern FloatingQuantityStructure*& globalFloatingQuantityStructure;
 
 } // namespace internal
 } // namespace polyscope

include/polyscope/polyscope.h

@@ -10,6 +10,7 @@
 
 #include "imgui.h"
 
+#include "polyscope/context.h"
 #include "polyscope/group.h"
 #include "polyscope/internal.h"
 #include "polyscope/messages.h"
@@ -31,6 +32,8 @@ namespace polyscope {
 // forward declarations
 class Structure;
 class Group;
+class SlicePlane;
+class Widget;
 
 // Initialize polyscope, including windowing system and openGL. Should be called exactly once at the beginning of a
 // program. If initialization fails in any way, an exception will be thrown.
@@ -63,40 +66,47 @@ void shutdown();
 namespace state {
 
 // has polyscope::init() been called?
-extern bool initialized;
+extern bool& initialized;
 
 // what backend was set on initialization
-extern std::string backend;
+extern std::string& backend;
 
 // lists of all structures in Polyscope, by category
 // TODO unique pointer
-extern std::map<std::string, std::map<std::string, std::unique_ptr<Structure>>> structures;
+extern std::map<std::string, std::map<std::string, std::unique_ptr<Structure>>>& structures;
 
 // lists of all groups in Polyscope
-extern std::map<std::string, std::unique_ptr<Group>> groups;
+extern std::map<std::string, std::unique_ptr<Group>>& groups;
 
 // representative length scale for all registered structures
-extern float lengthScale;
+extern float& lengthScale;
 
 // axis-aligned bounding box for all registered structures
-extern std::tuple<glm::vec3, glm::vec3> boundingBox;
+extern std::tuple<glm::vec3, glm::vec3>& boundingBox;
 
 // list of all slice planes in the scene
-extern std::vector<std::unique_ptr<SlicePlane>> slicePlanes;
+extern std::vector<std::unique_ptr<SlicePlane>>& slicePlanes;
 
 // list of all widgets in the scene (the memory is NOT owned here, they're just refs)
-extern std::vector<WeakHandle<Widget>> widgets;
+extern std::vector<WeakHandle<Widget>>& widgets;
 
 // should we allow default trackball mouse camera interaction?
 // Needs more interactions on when to turn this on/off
-extern bool doDefaultMouseInteraction;
+extern bool& doDefaultMouseInteraction;
 
 // a callback function used to render a "user" gui
-extern std::function<void()> userCallback;
+extern std::function<void()>& userCallback;
 
 // representative center for all registered structures
 glm::vec3 center();
 
+// The global context, all of the variables above are secretly references to members of this context.
+// This is useful because it means the lists get destructed in a predictable order on shutdown, rather than the
+// platform-defined order we get if they are just static globals.
+// One day we may refactor Polyscope to explicitly track contexts and allow the use of multiple contexts. For now there
+// is always exactly one global context object.
+extern Context globalContext;
+
 } // namespace state
 
 // === Manage structures tracked by polyscope

include/polyscope/scaled_value.h

@@ -12,7 +12,7 @@ namespace polyscope {
 
 // forward declare
 namespace state {
-extern float lengthScale;
+extern float& lengthScale;
 }
 
 template <typename T>

include/polyscope/view.h

@@ -37,35 +37,38 @@ using polyscope::UpDir;
 
 // NOTE: users should use setters to set these if they exist, otherwise updates
 // may not be applied immediately.
-extern int bufferWidth;
-extern int bufferHeight;
-extern int windowWidth;
-extern int windowHeight;
-extern int initWindowPosX;
-extern int initWindowPosY;
-extern bool windowResizable;
-extern NavigateStyle style;
-extern UpDir upDir;
-extern FrontDir frontDir;
-extern double moveScale;
-extern double nearClipRatio;
-extern double farClipRatio;
-extern std::array<float, 4> bgColor;
+extern int& bufferWidth;
+extern int& bufferHeight;
+extern int& windowWidth;
+extern int& windowHeight;
+extern int& initWindowPosX;
+extern int& initWindowPosY;
+extern bool& windowResizable;
+extern NavigateStyle& style;
+extern UpDir& upDir;
+extern FrontDir& frontDir;
+extern double& moveScale;
+extern double& nearClipRatio;
+extern double& farClipRatio;
+extern std::array<float, 4>& bgColor;
 
 // Current view camera parameters
 // TODO deprecate these one day, and just use a CameraParameters member instead. But this would break existing code, so
 // for now we leave these as-is and wrap inputs/outputs to a CameraParameters
-extern glm::mat4x4 viewMat;
-extern double fov; // in the y direction
-extern ProjectionMode projectionMode;
+extern glm::mat4x4& viewMat;
+extern double& fov; // in the y direction
+extern ProjectionMode& projectionMode;
 
 // "Flying" view
-extern bool midflight;
-extern float flightStartTime;
-extern float flightEndTime;
-extern glm::dualquat flightTargetViewR, flightInitialViewR;
-extern glm::vec3 flightTargetViewT, flightInitialViewT;
-extern float flightTargetFov, flightInitialFov;
+extern bool& midflight;
+extern float& flightStartTime;
+extern float& flightEndTime;
+extern glm::dualquat& flightTargetViewR;
+extern glm::dualquat& flightInitialViewR;
+extern glm::vec3& flightTargetViewT;
+extern glm::vec3& flightInitialViewT;
+extern float& flightTargetFov;
+extern float& flightInitialFov;
 
 // Default values
 extern const double defaultNearClipRatio;

src/CMakeLists.txt

@@ -227,6 +227,7 @@ SET(HEADERS
   ${INCLUDE_ROOT}/color_quantity.h
   ${INCLUDE_ROOT}/color_quantity.ipp
   ${INCLUDE_ROOT}/combining_hash_functions.h
+  ${INCLUDE_ROOT}/context.h
   ${INCLUDE_ROOT}/curve_network.h
   ${INCLUDE_ROOT}/curve_network.ipp
   ${INCLUDE_ROOT}/curve_network_color_quantity.h

src/internal.cpp

@@ -2,15 +2,17 @@
 
 #include "polyscope/internal.h"
 
+#include "polyscope/polyscope.h"
+
 namespace polyscope {
 namespace internal {
 
 uint64_t uniqueID = 42;
 
 uint64_t getNextUniqueID() { return uniqueID++; }
 
-bool pointCloudEfficiencyWarningReported = false;
-FloatingQuantityStructure* globalFloatingQuantityStructure = nullptr;
+bool& pointCloudEfficiencyWarningReported = state::globalContext.pointCloudEfficiencyWarningReported;
+FloatingQuantityStructure*& globalFloatingQuantityStructure = state::globalContext.globalFloatingQuantityStructure;
 
 } // namespace internal
 } // namespace polyscope

src/state.cpp

@@ -6,19 +6,20 @@ namespace polyscope {
 
 namespace state {
 
-bool initialized = false;
-std::string backend = "";
-float lengthScale = 1.0;
-std::tuple<glm::vec3, glm::vec3> boundingBox =
-    std::tuple<glm::vec3, glm::vec3>{glm::vec3{-1., -1., -1.}, glm::vec3{1., 1., 1.}};
-std::map<std::string, std::map<std::string, std::unique_ptr<Structure>>> structures;
-std::map<std::string, std::unique_ptr<Group>> groups;
-std::function<void()> userCallback = nullptr;
-bool doDefaultMouseInteraction = true;
 
-// Lists of things
-std::vector<WeakHandle<Widget>> widgets;
-std::vector<std::unique_ptr<SlicePlane>> slicePlanes;
+Context globalContext;
+
+// Map all of the named global variables as references to the context struct
+bool& initialized = globalContext.initialized;
+std::string& backend = globalContext.backend;
+std::map<std::string, std::map<std::string, std::unique_ptr<Structure>>>& structures = globalContext.structures;
+std::map<std::string, std::unique_ptr<Group>>& groups = globalContext.groups;
+float& lengthScale = globalContext.lengthScale;
+std::tuple<glm::vec3, glm::vec3>& boundingBox = globalContext.boundingBox;
+std::vector<std::unique_ptr<SlicePlane>>& slicePlanes = globalContext.slicePlanes;
+std::vector<WeakHandle<Widget>>& widgets = globalContext.widgets;
+bool& doDefaultMouseInteraction = globalContext.doDefaultMouseInteraction;
+std::function<void()>& userCallback = globalContext.userCallback;
 
 } // namespace state
 } // namespace polyscope