Skip to content

Commit

Permalink
tidy up on shutdown to allow multiple init/shutdown cycles per pr…
Browse files Browse the repository at this point in the history
…ocess

- use `std::uique_ptr<Engine>` to manage global engine lifetime. This means that we no longer need `bool state::initialized`, we can just use the engine ptr to infer initialized state.
- Hide window.
- Clear `contextStack`.
- Reset cached `lazy` properties. Had to convert from namespace to struct to allow default initialisation reset.
  • Loading branch information
gkoulin committed Oct 2, 2024
1 parent 9f97b70 commit 92cc9ee
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 36 deletions.
1 change: 0 additions & 1 deletion include/polyscope/context.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ 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;
Expand Down
3 changes: 0 additions & 3 deletions include/polyscope/polyscope.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,6 @@ bool windowRequestsClose();
// === Global variables ===
namespace state {

// has polyscope::init() been called?
extern bool& initialized;

// what backend was set on initialization
extern std::string& backend;

Expand Down
2 changes: 1 addition & 1 deletion include/polyscope/render/engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,7 @@ class Engine {

// The global render engine
// Gets initialized by initializeRenderEngine() in polyscope::init();
extern Engine* engine;
extern std::unique_ptr<Engine> engine;

// The backend type of the engine, as initialized above
extern std::string engineBackendName;
Expand Down
57 changes: 34 additions & 23 deletions src/polyscope.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,17 +162,16 @@ void init(std::string backend) {

view::invalidateView();

state::initialized = true;
state::doDefaultMouseInteraction = true;
}

void checkInitialized() {
if (!state::initialized) {
if (!isInitialized()) {
exception("Polyscope has not been initialized");
}
}

bool isInitialized() { return state::initialized; }
bool isInitialized() { return render::engine != nullptr; }

void pushContext(std::function<void()> callbackFunction, bool drawDefaultUI) {

Expand Down Expand Up @@ -890,7 +889,7 @@ void mainLoopIteration() {

void show(size_t forFrames) {

if (!state::initialized) {
if (!isInitialized()) {
exception("must initialize Polyscope with polyscope::init() before calling polyscope::show().");
}
unshowRequested = false;
Expand Down Expand Up @@ -934,14 +933,20 @@ bool windowRequestsClose() {
return false;
}

void resetLazy();

void shutdown() {

// TODO should we make an effort to destruct everything here?
if (options::usePrefsFile) {
writePrefsFile();
}

render::engine->hideWindow();
render::engine->shutdownImGui();
render::engine.reset();
contextStack.clear();
resetLazy();
}

bool registerStructure(Structure* s, bool replaceIfPresent) {
Expand Down Expand Up @@ -1179,16 +1184,18 @@ void refresh() {
}

// Cached versions of lazy properties used for updates
namespace lazy {
struct Lazy {
TransparencyMode transparencyMode = TransparencyMode::None;
int transparencyRenderPasses = 8;
int ssaaFactor = 1;
bool groundPlaneEnabled = true;
GroundPlaneMode groundPlaneMode = GroundPlaneMode::TileReflection;
ScaledValue<float> groundPlaneHeightFactor = 0;
int shadowBlurIters = 2;
float shadowDarkness = .4;
} // namespace lazy
float shadowDarkness = .4f;
};
static Lazy lazy{};


void processLazyProperties() {

Expand All @@ -1203,49 +1210,53 @@ void processLazyProperties() {


// transparency mode
if (lazy::transparencyMode != options::transparencyMode) {
lazy::transparencyMode = options::transparencyMode;
if (lazy.transparencyMode != options::transparencyMode) {
lazy.transparencyMode = options::transparencyMode;
render::engine->setTransparencyMode(options::transparencyMode);
}

// transparency render passes
if (lazy::transparencyRenderPasses != options::transparencyRenderPasses) {
lazy::transparencyRenderPasses = options::transparencyRenderPasses;
if (lazy.transparencyRenderPasses != options::transparencyRenderPasses) {
lazy.transparencyRenderPasses = options::transparencyRenderPasses;
requestRedraw();
}

// ssaa
if (lazy::ssaaFactor != options::ssaaFactor) {
lazy::ssaaFactor = options::ssaaFactor;
if (lazy.ssaaFactor != options::ssaaFactor) {
lazy.ssaaFactor = options::ssaaFactor;
render::engine->setSSAAFactor(options::ssaaFactor);
}

// ground plane
if (lazy::groundPlaneEnabled != options::groundPlaneEnabled || lazy::groundPlaneMode != options::groundPlaneMode) {
lazy::groundPlaneEnabled = options::groundPlaneEnabled;
if (lazy.groundPlaneEnabled != options::groundPlaneEnabled || lazy.groundPlaneMode != options::groundPlaneMode) {
lazy.groundPlaneEnabled = options::groundPlaneEnabled;
if (!options::groundPlaneEnabled) {
// if the (depecated) groundPlaneEnabled = false, set mode to None, so we only have one variable to check
options::groundPlaneMode = GroundPlaneMode::None;
}
lazy::groundPlaneMode = options::groundPlaneMode;
lazy.groundPlaneMode = options::groundPlaneMode;
render::engine->groundPlane.prepare();
requestRedraw();
}
if (lazy::groundPlaneHeightFactor.asAbsolute() != options::groundPlaneHeightFactor.asAbsolute() ||
lazy::groundPlaneHeightFactor.isRelative() != options::groundPlaneHeightFactor.isRelative()) {
lazy::groundPlaneHeightFactor = options::groundPlaneHeightFactor;
if (lazy.groundPlaneHeightFactor.asAbsolute() != options::groundPlaneHeightFactor.asAbsolute() ||
lazy.groundPlaneHeightFactor.isRelative() != options::groundPlaneHeightFactor.isRelative()) {
lazy.groundPlaneHeightFactor = options::groundPlaneHeightFactor;
requestRedraw();
}
if (lazy::shadowBlurIters != options::shadowBlurIters) {
lazy::shadowBlurIters = options::shadowBlurIters;
if (lazy.shadowBlurIters != options::shadowBlurIters) {
lazy.shadowBlurIters = options::shadowBlurIters;
requestRedraw();
}
if (lazy::shadowDarkness != options::shadowDarkness) {
lazy::shadowDarkness = options::shadowDarkness;
if (lazy.shadowDarkness != options::shadowDarkness) {
lazy.shadowDarkness = options::shadowDarkness;
requestRedraw();
}
};

void resetLazy() {
lazy = {};
}

void updateStructureExtents() {

if (!options::automaticallyComputeSceneExtents) {
Expand Down
2 changes: 1 addition & 1 deletion src/render/initialize_backend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace polyscope {
namespace render {

// Storage for the global engine pointer
Engine* engine = nullptr;
std::unique_ptr<Engine> engine;

// Backend we initialized with; written once below
std::string engineBackendName = "";
Expand Down
6 changes: 3 additions & 3 deletions src/render/managed_buffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ std::shared_ptr<render::AttributeBuffer> ManagedBuffer<T>::getRenderAttributeBuf

if (!renderAttributeBuffer) {
ensureHostBufferPopulated(); // warning: the order of these matters because of how hostBufferPopulated works
renderAttributeBuffer = generateAttributeBuffer<T>(render::engine);
renderAttributeBuffer = generateAttributeBuffer<T>(render::engine.get());
renderAttributeBuffer->setData(data);
}
return renderAttributeBuffer;
Expand All @@ -312,7 +312,7 @@ std::shared_ptr<render::TextureBuffer> ManagedBuffer<T>::getRenderTextureBuffer(
if (!renderTextureBuffer) {
ensureHostBufferPopulated(); // warning: the order of these matters because of how hostBufferPopulated works

renderTextureBuffer = generateTextureBuffer<T>(deviceBufferType, render::engine);
renderTextureBuffer = generateTextureBuffer<T>(deviceBufferType, render::engine.get());

// templatize this?
switch (deviceBufferType) {
Expand Down Expand Up @@ -377,7 +377,7 @@ ManagedBuffer<T>::getIndexedRenderAttributeBuffer(ManagedBuffer<uint32_t>& indic

// We don't have it. Create a new one and return that.
ensureHostBufferPopulated();
std::shared_ptr<render::AttributeBuffer> newBuffer = generateAttributeBuffer<T>(render::engine);
std::shared_ptr<render::AttributeBuffer> newBuffer = generateAttributeBuffer<T>(render::engine.get());
indices.ensureHostBufferPopulated();
std::vector<T> expandData = gather(data, indices.data);
newBuffer->setData(expandData); // initially populate
Expand Down
2 changes: 1 addition & 1 deletion src/render/mock_opengl/mock_gl_engine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ MockGLEngine* glEngine = nullptr; // alias for engine pointer

void initializeRenderEngine() {
glEngine = new MockGLEngine();
engine = glEngine;
engine.reset(glEngine);
glEngine->initialize();
engine->allocateGlobalBuffersAndPrograms();
}
Expand Down
3 changes: 2 additions & 1 deletion src/render/opengl/gl_engine_egl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ void initializeRenderEngine_egl() {

glEngineEGL = new GLEngineEGL(); // create the new global engine object

engine = glEngineEGL; // we keep a few copies of this pointer with various types
// we keep a few copies of this pointer with various types
engine.reset(glEngineEGL); // owning
glEngine = glEngineEGL;

// initialize
Expand Down
6 changes: 5 additions & 1 deletion src/render/opengl/gl_engine_glfw.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,13 @@ extern GLEngine* glEngine; // defined in gl_engine.h

void initializeRenderEngine_glfw() {

if (engine) {
exception("ERROR: engine is already initialised");
}
glEngineGLFW = new GLEngineGLFW(); // create the new global engine object

engine = glEngineGLFW; // we keep a few copies of this pointer with various types
// we keep a few copies of this pointer with various types
engine.reset(glEngineGLFW); // owning
glEngine = glEngineGLFW;

// initialize
Expand Down
1 change: 0 additions & 1 deletion src/state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ namespace state {
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;
Expand Down

0 comments on commit 92cc9ee

Please sign in to comment.