From 43388898b61dfbc19a987001a1f5f37ea804ba58 Mon Sep 17 00:00:00 2001 From: Nicholas Sharp Date: Sat, 13 Dec 2025 12:00:18 -0800 Subject: [PATCH 1/2] improve slice plane API, allow custom names --- include/polyscope/slice_plane.h | 28 +++++++++++++ src/slice_plane.cpp | 71 ++++++++++++++++++++++++++++----- src/structure.cpp | 2 +- test/src/combo_test.cpp | 4 +- test/src/floating_test.cpp | 18 ++++----- test/src/misc_test.cpp | 62 ++++++++++++++++++++++++++++ test/src/volume_mesh_test.cpp | 4 +- 7 files changed, 164 insertions(+), 25 deletions(-) diff --git a/include/polyscope/slice_plane.h b/include/polyscope/slice_plane.h index ecb617f87..907471e9e 100644 --- a/include/polyscope/slice_plane.h +++ b/include/polyscope/slice_plane.h @@ -42,6 +42,10 @@ class SlicePlane { const std::string postfix; std::string uniquePrefix(); + // Remove the slice plane from the scene + // (destroys this object, pointer is now invalid) + void remove(); + // Set the position and orientation of the plane // planePosition is any 3D position which the plane touches (the center of the plane) // planeNormal is a vector giving the normal direction of the plane, objects @@ -109,10 +113,34 @@ class SlicePlane { void updateWidgetEnabled(); }; +// Manually specify a name for the slice plane +SlicePlane* addSlicePlane(std::string name); + +// Automatically generates a name for the plane like +// "Scene Slice Plane 0", "Scene Slice Plane 1", etc. +SlicePlane* addSlicePlane(); + +// DEPRECATED: there's on reason to offer this variant, it could be set manually SlicePlane* addSceneSlicePlane(bool initiallyVisible = false); + +// Get a slice plane by name +SlicePlane* getSlicePlane(std::string name); + +// Remove a slice plane by name or pointer +void removeSlicePlane(std::string name); +void removeSlicePlane(SlicePlane* plane); + +// Remove the last-added slice plane void removeLastSceneSlicePlane(); + +// Remove all slice planes void removeAllSlicePlanes(); + + +// === Internal helpers + void buildSlicePlaneGUI(); +void notifySlicePlanesChanged(); // call after adding/remove any slice plane // flag to open the slice plane menu after adding a slice plane extern bool openSlicePlaneMenu; diff --git a/src/slice_plane.cpp b/src/slice_plane.cpp index 27d185f02..cbc69e74c 100644 --- a/src/slice_plane.cpp +++ b/src/slice_plane.cpp @@ -13,28 +13,64 @@ namespace polyscope { // Storage for global options bool openSlicePlaneMenu = false; -SlicePlane* addSceneSlicePlane(bool initiallyVisible) { +SlicePlane* addSlicePlane(std::string name) { + // check if the name is unique, throw an error if not + for (const std::unique_ptr& s : state::slicePlanes) { + if (s->name == name) { + error("Slice plane with name " + name + " already exists"); + return nullptr; + } + } + + state::slicePlanes.emplace_back(std::unique_ptr(new SlicePlane(name))); + SlicePlane* newPlane = state::slicePlanes.back().get(); + notifySlicePlanesChanged(); + return newPlane; +} + +SlicePlane* addSlicePlane() { size_t nPlanes = state::slicePlanes.size(); std::string newName = "Scene Slice Plane " + std::to_string(nPlanes); - state::slicePlanes.emplace_back(std::unique_ptr(new SlicePlane(newName))); - nPlanes++; - SlicePlane* newPlane = state::slicePlanes.back().get(); + return addSlicePlane(newName); +} + +// DEPRECATED: there's on reason to offer this variant, it could be set manually +SlicePlane* addSceneSlicePlane(bool initiallyVisible) { + SlicePlane* newPlane = addSlicePlane(); if (!initiallyVisible) { newPlane->setDrawPlane(false); newPlane->setDrawWidget(false); } - for (std::unique_ptr& s : state::slicePlanes) { - s->resetVolumeSliceProgram(); - } return newPlane; } +SlicePlane* getSlicePlane(std::string name) { + for (const std::unique_ptr& s : state::slicePlanes) { + if (s->name == name) { + return s.get(); + } + } + error("No slice plane with name " + name + " exists"); + return nullptr; +} + +void removeSlicePlane(std::string name) { + for (auto it = state::slicePlanes.begin(); it != state::slicePlanes.end(); it++) { + if ((*it)->name == name) { + state::slicePlanes.erase(it); + notifySlicePlanesChanged(); + return; + } + } + warning("No slice plane with name " + name + " exists, cannot remove"); +} + +void removeSlicePlane(SlicePlane* plane) { removeSlicePlane(plane->name); } + void removeLastSceneSlicePlane() { if (state::slicePlanes.empty()) return; state::slicePlanes.pop_back(); - for (std::unique_ptr& s : state::slicePlanes) { - s->resetVolumeSliceProgram(); - } + notifySlicePlanesChanged(); } void removeAllSlicePlanes() { @@ -43,6 +79,13 @@ void removeAllSlicePlanes() { } } +void notifySlicePlanesChanged() { + // NSHARP: I've forgotten why this is needed. Perhaps it is not? + for (std::unique_ptr& s : state::slicePlanes) { + s->resetVolumeSliceProgram(); + } +} + void buildSlicePlaneGUI() { @@ -53,7 +96,7 @@ void buildSlicePlaneGUI() { } if (ImGui::TreeNode("Slice Planes")) { if (ImGui::Button("Add plane")) { - addSceneSlicePlane(true); + addSlicePlane(); } ImGui::SameLine(); if (ImGui::Button("Remove plane")) { @@ -94,6 +137,11 @@ SlicePlane::~SlicePlane() { std::string SlicePlane::uniquePrefix() { return "SlicePlane#" + name + "#"; } +void SlicePlane::remove() { + removeSlicePlane(name); + // now invalid! can't do anything else +} + void SlicePlane::prepare() { planeProgram = render::engine->requestShader("SLICE_PLANE", {}, render::ShaderReplacementDefaults::Process); @@ -380,6 +428,7 @@ void SlicePlane::updateWidgetEnabled() { transformGizmo.enabled = enabled; } + void SlicePlane::setPose(glm::vec3 planePosition, glm::vec3 planeNormal) { // Choose the other axes to be most similar to the current ones, which will make animations look smoother diff --git a/src/structure.cpp b/src/structure.cpp index 9b23f645f..9cab497e8 100644 --- a/src/structure.cpp +++ b/src/structure.cpp @@ -93,7 +93,7 @@ void Structure::buildUI() { // if there are none, show a helpful message if (ImGui::Button("Add slice plane")) { openSlicePlaneMenu = true; - addSceneSlicePlane(true); + addSlicePlane(); } } else { // otherwise, show toggles for each diff --git a/test/src/combo_test.cpp b/test/src/combo_test.cpp index ea546d58a..e49fa1170 100644 --- a/test/src/combo_test.cpp +++ b/test/src/combo_test.cpp @@ -111,7 +111,7 @@ TEST_F(PolyscopeTest, SlicePlaneTest) { polyscope::show(3); // render with one slice plane - polyscope::addSceneSlicePlane(); + polyscope::addSlicePlane(); polyscope::show(3); // try a few variations of point cloud settings @@ -126,7 +126,7 @@ TEST_F(PolyscopeTest, SlicePlaneTest) { polyscope::show(3); // add another and rotate it - polyscope::SlicePlane* p = polyscope::addSceneSlicePlane(); + polyscope::SlicePlane* p = polyscope::addSlicePlane(); p->setTransform(glm::translate(p->getTransform(), glm::vec3{-1., 0., 0.})); polyscope::show(3); diff --git a/test/src/floating_test.cpp b/test/src/floating_test.cpp index bee1f6921..8f29d9559 100644 --- a/test/src/floating_test.cpp +++ b/test/src/floating_test.cpp @@ -106,7 +106,7 @@ TEST_F(PolyscopeTest, FloatingRenderImageTest) { polyscope::pickAtScreenCoords(glm::vec2(0.3, 0.8)); // add a slice plane - polyscope::addSceneSlicePlane(); + polyscope::addSlicePlane(); polyscope::show(3); polyscope::pickAtScreenCoords(glm::vec2(0.3, 0.8)); polyscope::removeLastSceneSlicePlane(); @@ -120,7 +120,7 @@ TEST_F(PolyscopeTest, FloatingRenderImageTest) { polyscope::pickAtScreenCoords(glm::vec2(0.3, 0.8)); // add a slice plane - polyscope::addSceneSlicePlane(); + polyscope::addSlicePlane(); polyscope::show(3); polyscope::pickAtScreenCoords(glm::vec2(0.3, 0.8)); polyscope::removeLastSceneSlicePlane(); @@ -135,7 +135,7 @@ TEST_F(PolyscopeTest, FloatingRenderImageTest) { polyscope::pickAtScreenCoords(glm::vec2(0.3, 0.8)); // add a slice plane - polyscope::addSceneSlicePlane(); + polyscope::addSlicePlane(); polyscope::show(3); polyscope::pickAtScreenCoords(glm::vec2(0.3, 0.8)); polyscope::removeLastSceneSlicePlane(); @@ -149,7 +149,7 @@ TEST_F(PolyscopeTest, FloatingRenderImageTest) { polyscope::pickAtScreenCoords(glm::vec2(0.3, 0.8)); // add a slice plane - polyscope::addSceneSlicePlane(); + polyscope::addSlicePlane(); polyscope::show(3); polyscope::pickAtScreenCoords(glm::vec2(0.3, 0.8)); polyscope::removeLastSceneSlicePlane(); @@ -164,7 +164,7 @@ TEST_F(PolyscopeTest, FloatingRenderImageTest) { polyscope::pickAtScreenCoords(glm::vec2(0.3, 0.8)); // add a slice plane - polyscope::addSceneSlicePlane(); + polyscope::addSlicePlane(); polyscope::show(3); polyscope::pickAtScreenCoords(glm::vec2(0.3, 0.8)); polyscope::removeLastSceneSlicePlane(); @@ -178,7 +178,7 @@ TEST_F(PolyscopeTest, FloatingRenderImageTest) { polyscope::pickAtScreenCoords(glm::vec2(0.3, 0.8)); // add a slice plane - polyscope::addSceneSlicePlane(); + polyscope::addSlicePlane(); polyscope::show(3); polyscope::pickAtScreenCoords(glm::vec2(0.3, 0.8)); polyscope::removeLastSceneSlicePlane(); @@ -194,7 +194,7 @@ TEST_F(PolyscopeTest, FloatingRenderImageTest) { polyscope::pickAtScreenCoords(glm::vec2(0.3, 0.8)); // add a slice plane - polyscope::addSceneSlicePlane(); + polyscope::addSlicePlane(); polyscope::show(3); polyscope::pickAtScreenCoords(glm::vec2(0.3, 0.8)); polyscope::removeLastSceneSlicePlane(); @@ -209,7 +209,7 @@ TEST_F(PolyscopeTest, FloatingRenderImageTest) { polyscope::pickAtScreenCoords(glm::vec2(0.3, 0.8)); // add a slice plane - polyscope::addSceneSlicePlane(); + polyscope::addSlicePlane(); polyscope::show(3); polyscope::pickAtScreenCoords(glm::vec2(0.3, 0.8)); polyscope::removeLastSceneSlicePlane(); @@ -226,7 +226,7 @@ TEST_F(PolyscopeTest, FloatingRenderImageTest) { polyscope::pickAtScreenCoords(glm::vec2(0.3, 0.8)); // add a slice plane - polyscope::addSceneSlicePlane(); + polyscope::addSlicePlane(); polyscope::show(3); polyscope::pickAtScreenCoords(glm::vec2(0.3, 0.8)); diff --git a/test/src/misc_test.cpp b/test/src/misc_test.cpp index e796ddcc9..14adc59c1 100644 --- a/test/src/misc_test.cpp +++ b/test/src/misc_test.cpp @@ -66,3 +66,65 @@ TEST_F(PolyscopeTest, FlatMaterialTest) { polyscope::removeAllStructures(); } + + +// ============================================================ +// =============== Slice Plane Tests +// ============================================================ + +// We test these on a point cloud because it is convenient, but really we are testing the scalar quantity + +TEST_F(PolyscopeTest, TestSlicePlane) { + + auto psPoints = registerPointCloud(); // add some structure to the scene + + // Basic add + polyscope::SlicePlane* sp1 = polyscope::addSlicePlane(); + polyscope::show(3); + + // Set properties + sp1->setColor(glm::vec3(1.0, 0.0, 0.0)); + EXPECT_EQ(sp1->getColor(), glm::vec3(1.0, 0.0, 0.0)); + + sp1->setTransparency(0.5); + EXPECT_EQ(sp1->getTransparency(), 0.5); + + sp1->setGridLineColor(glm::vec3(0.5, 0.5, 0.5)); + EXPECT_EQ(sp1->getGridLineColor(), glm::vec3(0.5, 0.5, 0.5)); + + polyscope::show(3); + + // Transform stuff + glm::mat4 transform = glm::mat4(1.0); + transform[3][0] = 1.0; + sp1->setTransform(transform); + EXPECT_EQ(sp1->getTransform(), transform); + + glm::vec3 center = sp1->getCenter(); + glm::vec3 normal = sp1->getNormal(); + + // Enable/disable drawing styles + sp1->setDrawPlane(false); + sp1->setDrawWidget(false); + polyscope::show(3); + + // Add/remove with custom names + polyscope::SlicePlane* sp2 = polyscope::addSlicePlane("custom_name"); + EXPECT_EQ(sp2->name, "custom_name"); + polyscope::SlicePlane* sp3 = polyscope::addSlicePlane(); + polyscope::show(3); + polyscope::removeSlicePlane("custom_name"); + polyscope::show(3); + polyscope::removeLastSceneSlicePlane(); + polyscope::show(3); + polyscope::removeSlicePlane(sp1); + polyscope::show(3); + polyscope::SlicePlane* sp4 = polyscope::addSlicePlane(); + sp4->remove(); + // for now, still test that the the old deprecated function works + polyscope::SlicePlane* sp5 = polyscope::addSceneSlicePlane(); + polyscope::show(3); + + polyscope::removeAllSlicePlanes(); + polyscope::removeAllStructures(); +} \ No newline at end of file diff --git a/test/src/volume_mesh_test.cpp b/test/src/volume_mesh_test.cpp index 1a81a786c..e883f4745 100644 --- a/test/src/volume_mesh_test.cpp +++ b/test/src/volume_mesh_test.cpp @@ -245,7 +245,7 @@ TEST_F(PolyscopeTest, VolumeMeshInspect) { polyscope::VolumeMesh* psVol = polyscope::registerVolumeMesh("vol", verts, cells); // plain old inspecting - polyscope::SlicePlane* p = polyscope::addSceneSlicePlane(); + polyscope::SlicePlane* p = polyscope::addSlicePlane(); p->setVolumeMeshToInspect("vol"); polyscope::show(3); @@ -280,7 +280,7 @@ TEST_F(PolyscopeTest, VolumeMeshInspectWithExtra) { polyscope::VolumeMesh* psVolExtra = polyscope::registerVolumeMesh("vol extra", verts, cells); // plain old inspecting - polyscope::SlicePlane* p = polyscope::addSceneSlicePlane(); + polyscope::SlicePlane* p = polyscope::addSlicePlane(); p->setVolumeMeshToInspect("vol"); polyscope::show(3); From b76149e3ec87d098a85eafd6b8b629566deaa3a0 Mon Sep 17 00:00:00 2001 From: Nicholas Sharp Date: Sat, 13 Dec 2025 12:56:44 -0800 Subject: [PATCH 2/2] change slice plane 'active' to 'enabled' --- include/polyscope/slice_plane.h | 8 ++++++-- src/slice_plane.cpp | 25 ++++++++++++++----------- test/src/misc_test.cpp | 5 +++++ 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/include/polyscope/slice_plane.h b/include/polyscope/slice_plane.h index 907471e9e..5aa719b99 100644 --- a/include/polyscope/slice_plane.h +++ b/include/polyscope/slice_plane.h @@ -56,6 +56,10 @@ class SlicePlane { glm::vec3 getCenter(); glm::vec3 getNormal(); + bool getEnabled(); + void setEnabled(bool newVal); + + // deprecated, should have been called 'enabled' bool getActive(); void setActive(bool newVal); @@ -82,7 +86,7 @@ class SlicePlane { protected: // = State - PersistentValue active; // is it actually slicing? + PersistentValue enabled; // is it actually slicing? PersistentValue drawPlane; // do we draw the plane onscreen? PersistentValue drawWidget; // do we draw the widget onscreen? PersistentValue objectTransform; @@ -121,7 +125,7 @@ SlicePlane* addSlicePlane(std::string name); SlicePlane* addSlicePlane(); // DEPRECATED: there's on reason to offer this variant, it could be set manually -SlicePlane* addSceneSlicePlane(bool initiallyVisible = false); +SlicePlane* addSceneSlicePlane(bool initiallyVisible=false); // Get a slice plane by name SlicePlane* getSlicePlane(std::string name); diff --git a/src/slice_plane.cpp b/src/slice_plane.cpp index cbc69e74c..604968ff2 100644 --- a/src/slice_plane.cpp +++ b/src/slice_plane.cpp @@ -111,7 +111,7 @@ void buildSlicePlaneGUI() { SlicePlane::SlicePlane(std::string name_) - : name(name_), postfix(std::to_string(state::slicePlanes.size())), active(uniquePrefix() + "#active", true), + : name(name_), postfix(std::to_string(state::slicePlanes.size())), enabled(uniquePrefix() + "#enabled", true), drawPlane(uniquePrefix() + "#drawPlane", true), drawWidget(uniquePrefix() + "#drawWidget", true), objectTransform(uniquePrefix() + "#object_transform", glm::mat4(1.0)), color(uniquePrefix() + "#color", getNextUniqueColor()), @@ -253,7 +253,7 @@ void SlicePlane::setSliceAttributes(render::ShaderProgram& p) { } void SlicePlane::drawGeometry() { - if (!active.get()) return; + if (!enabled.get()) return; ensureVolumeInspectValid(); @@ -292,7 +292,7 @@ void SlicePlane::drawGeometry() { void SlicePlane::draw() { - if (!active.get()) return; + if (!enabled.get()) return; if (drawPlane.get()) { // Set uniforms @@ -320,8 +320,8 @@ void SlicePlane::draw() { void SlicePlane::buildGUI() { ImGui::PushID(name.c_str()); - if (ImGui::Checkbox(name.c_str(), &active.get())) { - setActive(getActive()); + if (ImGui::Checkbox(name.c_str(), &enabled.get())) { + setEnabled(getEnabled()); } ImGui::SameLine(); @@ -403,7 +403,7 @@ void SlicePlane::setSceneObjectUniforms(render::ShaderProgram& p, bool alwaysPas } glm::vec3 SlicePlane::getCenter() { - if (active.get()) { + if (enabled.get()) { glm::vec3 center{objectTransform.get()[3][0], objectTransform.get()[3][1], objectTransform.get()[3][2]}; return center; } else { @@ -413,7 +413,7 @@ glm::vec3 SlicePlane::getCenter() { } glm::vec3 SlicePlane::getNormal() { - if (active.get()) { + if (enabled.get()) { glm::vec3 normal{objectTransform.get()[0][0], objectTransform.get()[0][1], objectTransform.get()[0][2]}; normal = glm::normalize(normal); return normal; @@ -424,7 +424,7 @@ glm::vec3 SlicePlane::getNormal() { } void SlicePlane::updateWidgetEnabled() { - bool enabled = getActive() && getDrawWidget(); + bool enabled = getEnabled() && getDrawWidget(); transformGizmo.enabled = enabled; } @@ -455,13 +455,16 @@ void SlicePlane::setPose(glm::vec3 planePosition, glm::vec3 planeNormal) { polyscope::requestRedraw(); } -bool SlicePlane::getActive() { return active.get(); } -void SlicePlane::setActive(bool newVal) { - active = newVal; +bool SlicePlane::getEnabled() { return enabled.get(); } +void SlicePlane::setEnabled(bool newVal) { + enabled = newVal; updateWidgetEnabled(); polyscope::requestRedraw(); } +bool SlicePlane::getActive() { return getEnabled(); } +void SlicePlane::setActive(bool newVal) { return setEnabled(newVal); } + bool SlicePlane::getDrawPlane() { return drawPlane.get(); } void SlicePlane::setDrawPlane(bool newVal) { drawPlane = newVal; diff --git a/test/src/misc_test.cpp b/test/src/misc_test.cpp index 14adc59c1..49aa454f1 100644 --- a/test/src/misc_test.cpp +++ b/test/src/misc_test.cpp @@ -80,7 +80,12 @@ TEST_F(PolyscopeTest, TestSlicePlane) { // Basic add polyscope::SlicePlane* sp1 = polyscope::addSlicePlane(); + EXPECT_TRUE(sp1->getEnabled()); polyscope::show(3); + sp1->setEnabled(false); + EXPECT_FALSE(sp1->getEnabled()); + polyscope::show(3); + sp1->setEnabled(true); // Set properties sp1->setColor(glm::vec3(1.0, 0.0, 0.0));