Skip to content

Commit

Permalink
reorder duplicate quantity adds to be sequential, rmeove PersistentVa…
Browse files Browse the repository at this point in the history
…lue destruct write
  • Loading branch information
nmwsharp committed Oct 9, 2023
1 parent 7914fda commit d7c98ce
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 33 deletions.
7 changes: 3 additions & 4 deletions include/polyscope/persistent_value.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class PersistentValue {
}

// Ensure in cache on deletion (see not above reference conversion)
~PersistentValue() { set(value); }
~PersistentValue() {}

// Don't want copy or move constructors, only operators
PersistentValue(const PersistentValue&) = delete;
Expand Down Expand Up @@ -82,8 +82,8 @@ class PersistentValue {
return *this;
}

// NOTE if you write via this reference, the value will not _actually_ be cached until destruction or
// manuallyChanged() is called, rather than immediately (ugly, but seems necessary to use with imgui)...
// NOTE if you write via this reference, the value will not _actually_ be cached until
// manuallyChanged() is called, rather than immediately (ugly, but seems necessary to use with imgui)
T& get() { return value; }
void manuallyChanged() { set(value); }

Expand All @@ -107,7 +107,6 @@ class PersistentValue {
template <typename>
friend class PersistentValue;

private:
const std::string name;
T value;
bool holdsDefaultValue = true; // True if the value was set on construction and never changed. False if it was pulled
Expand Down
3 changes: 1 addition & 2 deletions include/polyscope/structure.h
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ class QuantityStructure : public Structure {
QuantityType*
getQuantity(std::string name); // NOTE: will _not_ return floating quantities, must use other version below
FloatingQuantity* getFloatingQuantity(std::string name);
void checkForQuantityWithNameAndDeleteOrError(std::string name, bool allowReplacement = true);
void removeQuantity(std::string name, bool errorIfAbsent = false);
void removeAllQuantities();

Expand Down Expand Up @@ -258,8 +259,6 @@ class QuantityStructure : public Structure {
ImageOrigin imageOrigin, DataType type);

protected:
// helper
bool checkForQuantityWithNameAndDeleteOrError(std::string name, bool allowReplacement);
};


Expand Down
34 changes: 8 additions & 26 deletions include/polyscope/structure.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ template <typename S>
QuantityStructure<S>::~QuantityStructure(){};

template <typename S>
bool QuantityStructure<S>::checkForQuantityWithNameAndDeleteOrError(std::string name, bool allowReplacement) {
void QuantityStructure<S>::checkForQuantityWithNameAndDeleteOrError(std::string name, bool allowReplacement) {

// Look for an existing quantity with this name
bool quantityExists = quantities.find(name) != quantities.end();
Expand All @@ -30,55 +30,32 @@ bool QuantityStructure<S>::checkForQuantityWithNameAndDeleteOrError(std::string
exception("Tried to add quantity with name: [" + name +
"], but a quantity with that name already exists on the structure [" + name +
"]. Use the allowReplacement option like addQuantity(..., true) to replace.");
return false;
}

// Track if the previous quantity was enabled
// TODO why is isn't this handled by the persistence cache like everything else?
bool existingQuantityWasEnabled = false;
if (quantityExists) {
existingQuantityWasEnabled = quantities.find(name)->second->isEnabled();
}
if (floatingQuantityExists) {
existingQuantityWasEnabled = floatingQuantities.find(name)->second->isEnabled();
}

// Remove the old quantity
if (quantityExists || floatingQuantityExists) {
removeQuantity(name);
}

return existingQuantityWasEnabled;
}

template <typename S>
void QuantityStructure<S>::addQuantity(QuantityType* q, bool allowReplacement) {

// Check if a quantity with this name exists, remove it or throw and error if so
bool existingQuantityWasEnabled = checkForQuantityWithNameAndDeleteOrError(q->name, allowReplacement);
checkForQuantityWithNameAndDeleteOrError(q->name, allowReplacement);

// Add the new quantity
quantities[q->name] = std::unique_ptr<QuantityType>(q);

// Re-enable the quantity if we're replacing an enabled quantity
if (existingQuantityWasEnabled) {
q->setEnabled(true);
}
}

template <typename S>
void QuantityStructure<S>::addQuantity(FloatingQuantity* q, bool allowReplacement) {

// Check if a quantity with this name exists, remove it or throw and error if so
bool existingQuantityWasEnabled = checkForQuantityWithNameAndDeleteOrError(q->name, allowReplacement);
checkForQuantityWithNameAndDeleteOrError(q->name, allowReplacement);

// Add the new quantity
floatingQuantities[q->name] = std::unique_ptr<FloatingQuantity>(q);

// Re-enable the quantity if we're replacing an enabled quantity
if (existingQuantityWasEnabled) {
q->setEnabled(true);
}
}

template <typename S>
Expand Down Expand Up @@ -330,6 +307,7 @@ template <typename S>
ScalarImageQuantity* QuantityStructure<S>::addScalarImageQuantityImpl(std::string name, size_t dimX, size_t dimY,
const std::vector<double>& values,
ImageOrigin imageOrigin, DataType type) {
checkForQuantityWithNameAndDeleteOrError(name);
ScalarImageQuantity* q = createScalarImageQuantity(*this, name, dimX, dimY, values, imageOrigin, type);
addQuantity(q);
return q;
Expand All @@ -339,6 +317,7 @@ template <typename S>
ColorImageQuantity* QuantityStructure<S>::addColorImageQuantityImpl(std::string name, size_t dimX, size_t dimY,
const std::vector<glm::vec4>& values,
ImageOrigin imageOrigin) {
checkForQuantityWithNameAndDeleteOrError(name);
ColorImageQuantity* q = createColorImageQuantity(*this, name, dimX, dimY, values, imageOrigin);
addQuantity(q);
return q;
Expand All @@ -348,6 +327,7 @@ template <typename S>
DepthRenderImageQuantity* QuantityStructure<S>::addDepthRenderImageQuantityImpl(
std::string name, size_t dimX, size_t dimY, const std::vector<float>& depthData,
const std::vector<glm::vec3>& normalData, ImageOrigin imageOrigin) {
checkForQuantityWithNameAndDeleteOrError(name);
DepthRenderImageQuantity* q = createDepthRenderImage(*this, name, dimX, dimY, depthData, normalData, imageOrigin);
addQuantity(q);
return q;
Expand All @@ -357,6 +337,7 @@ template <typename S>
ColorRenderImageQuantity* QuantityStructure<S>::addColorRenderImageQuantityImpl(
std::string name, size_t dimX, size_t dimY, const std::vector<float>& depthData,
const std::vector<glm::vec3>& normalData, const std::vector<glm::vec3>& colorData, ImageOrigin imageOrigin) {
checkForQuantityWithNameAndDeleteOrError(name);
ColorRenderImageQuantity* q =
createColorRenderImage(*this, name, dimX, dimY, depthData, normalData, colorData, imageOrigin);
addQuantity(q);
Expand All @@ -368,6 +349,7 @@ ScalarRenderImageQuantity* QuantityStructure<S>::addScalarRenderImageQuantityImp
std::string name, size_t dimX, size_t dimY, const std::vector<float>& depthData,
const std::vector<glm::vec3>& normalData, const std::vector<double>& scalarData, ImageOrigin imageOrigin,
DataType type) {
checkForQuantityWithNameAndDeleteOrError(name);
ScalarRenderImageQuantity* q =
createScalarRenderImage(*this, name, dimX, dimY, depthData, normalData, scalarData, imageOrigin, type);
addQuantity(q);
Expand Down
6 changes: 6 additions & 0 deletions src/curve_network.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -497,13 +497,15 @@ void CurveNetworkQuantity::buildEdgeInfoGUI(size_t edgeInd) {}
CurveNetworkNodeColorQuantity* CurveNetwork::addNodeColorQuantityImpl(std::string name,
const std::vector<glm::vec3>& colors) {
checkForQuantityWithNameAndDeleteOrError(name);
CurveNetworkNodeColorQuantity* q = new CurveNetworkNodeColorQuantity(name, colors, *this);
addQuantity(q);
return q;
}
CurveNetworkEdgeColorQuantity* CurveNetwork::addEdgeColorQuantityImpl(std::string name,
const std::vector<glm::vec3>& colors) {
checkForQuantityWithNameAndDeleteOrError(name);
CurveNetworkEdgeColorQuantity* q = new CurveNetworkEdgeColorQuantity(name, colors, *this);
addQuantity(q);
return q;
Expand All @@ -512,13 +514,15 @@ CurveNetworkEdgeColorQuantity* CurveNetwork::addEdgeColorQuantityImpl(std::strin
CurveNetworkNodeScalarQuantity*
CurveNetwork::addNodeScalarQuantityImpl(std::string name, const std::vector<double>& data, DataType type) {
checkForQuantityWithNameAndDeleteOrError(name);
CurveNetworkNodeScalarQuantity* q = new CurveNetworkNodeScalarQuantity(name, data, *this, type);
addQuantity(q);
return q;
}
CurveNetworkEdgeScalarQuantity*
CurveNetwork::addEdgeScalarQuantityImpl(std::string name, const std::vector<double>& data, DataType type) {
checkForQuantityWithNameAndDeleteOrError(name);
CurveNetworkEdgeScalarQuantity* q = new CurveNetworkEdgeScalarQuantity(name, data, *this, type);
addQuantity(q);
return q;
Expand All @@ -527,6 +531,7 @@ CurveNetwork::addEdgeScalarQuantityImpl(std::string name, const std::vector<doub
CurveNetworkNodeVectorQuantity* CurveNetwork::addNodeVectorQuantityImpl(std::string name,
const std::vector<glm::vec3>& vectors,
VectorType vectorType) {
checkForQuantityWithNameAndDeleteOrError(name);
CurveNetworkNodeVectorQuantity* q = new CurveNetworkNodeVectorQuantity(name, vectors, *this, vectorType);
addQuantity(q);
return q;
Expand All @@ -535,6 +540,7 @@ CurveNetworkNodeVectorQuantity* CurveNetwork::addNodeVectorQuantityImpl(std::str
CurveNetworkEdgeVectorQuantity* CurveNetwork::addEdgeVectorQuantityImpl(std::string name,
const std::vector<glm::vec3>& vectors,
VectorType vectorType) {
checkForQuantityWithNameAndDeleteOrError(name);
CurveNetworkEdgeVectorQuantity* q = new CurveNetworkEdgeVectorQuantity(name, vectors, *this, vectorType);
addQuantity(q);
return q;
Expand Down
5 changes: 5 additions & 0 deletions src/point_cloud.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -371,13 +371,15 @@ void PointCloudQuantity::buildInfoGUI(size_t pointInd) {}


PointCloudColorQuantity* PointCloud::addColorQuantityImpl(std::string name, const std::vector<glm::vec3>& colors) {
checkForQuantityWithNameAndDeleteOrError(name);
PointCloudColorQuantity* q = new PointCloudColorQuantity(name, colors, *this);
addQuantity(q);
return q;
}

PointCloudScalarQuantity* PointCloud::addScalarQuantityImpl(std::string name, const std::vector<double>& data,
DataType type) {
checkForQuantityWithNameAndDeleteOrError(name);
PointCloudScalarQuantity* q = new PointCloudScalarQuantity(name, data, *this, type);
addQuantity(q);
return q;
Expand All @@ -386,6 +388,7 @@ PointCloudScalarQuantity* PointCloud::addScalarQuantityImpl(std::string name, co
PointCloudParameterizationQuantity* PointCloud::addParameterizationQuantityImpl(std::string name,
const std::vector<glm::vec2>& param,
ParamCoordsType type) {
checkForQuantityWithNameAndDeleteOrError(name);
PointCloudParameterizationQuantity* q =
new PointCloudParameterizationQuantity(name, *this, param, type, ParamVizStyle::CHECKER);
addQuantity(q);
Expand All @@ -395,6 +398,7 @@ PointCloudParameterizationQuantity* PointCloud::addParameterizationQuantityImpl(
PointCloudParameterizationQuantity*
PointCloud::addLocalParameterizationQuantityImpl(std::string name, const std::vector<glm::vec2>& param,
ParamCoordsType type) {
checkForQuantityWithNameAndDeleteOrError(name);
PointCloudParameterizationQuantity* q =
new PointCloudParameterizationQuantity(name, *this, param, type, ParamVizStyle::LOCAL_CHECK);
addQuantity(q);
Expand All @@ -403,6 +407,7 @@ PointCloud::addLocalParameterizationQuantityImpl(std::string name, const std::ve

PointCloudVectorQuantity* PointCloud::addVectorQuantityImpl(std::string name, const std::vector<glm::vec3>& vectors,
VectorType vectorType) {
checkForQuantityWithNameAndDeleteOrError(name);
PointCloudVectorQuantity* q = new PointCloudVectorQuantity(name, vectors, *this, vectorType);
addQuantity(q);
return q;
Expand Down
2 changes: 1 addition & 1 deletion src/quantity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace polyscope {
// (subclasses could be a structure-specific quantity or a floating quantity)

Quantity::Quantity(std::string name_, Structure& parentStructure_)
: parent(parentStructure_), name(name_), enabled(parent.typeName() + "#" + parent.name + "#" + name, false) {
: parent(parentStructure_), name(name_), enabled(uniquePrefix() + "enabled", false) {
validateName(name);
}

Expand Down
Loading

0 comments on commit d7c98ce

Please sign in to comment.