From de3d0d30ffe52f8c73d796028deaa83c90106ae0 Mon Sep 17 00:00:00 2001 From: Sam Horvath Date: Thu, 26 Apr 2018 12:59:59 -0400 Subject: [PATCH 1/3] BUG: Fiducial PlacingNode pointer can get corrupted in certain workflows Refactored fiducial placing to not use an intermediate placeholder node --- Planner/qSlicerPlannerModuleWidget.cxx | 96 ++++++++++---------------- 1 file changed, 37 insertions(+), 59 deletions(-) diff --git a/Planner/qSlicerPlannerModuleWidget.cxx b/Planner/qSlicerPlannerModuleWidget.cxx index b0861e1..be36e1d 100644 --- a/Planner/qSlicerPlannerModuleWidget.cxx +++ b/Planner/qSlicerPlannerModuleWidget.cxx @@ -82,6 +82,7 @@ //STD includes #include #include +#include #define D(x) std::cout << x << std::endl; @@ -150,9 +151,7 @@ class qSlicerPlannerModuleWidgetPrivate: public Ui_qSlicerPlannerModuleWidget vtkMRMLCommandLineModuleNode* cmdNode; //Bending Variables - vtkMRMLMarkupsFiducialNode* MovingPointA; - vtkMRMLMarkupsFiducialNode* MovingPointB; - vtkMRMLMarkupsFiducialNode* PlacingNode; + std::array BendPoints; vtkMRMLNode* CurrentBendNode; vtkSmartPointer Fiducials; vtkSmartPointer BendingData; @@ -165,7 +164,7 @@ class qSlicerPlannerModuleWidgetPrivate: public Ui_qSlicerPlannerModuleWidget bool BendASide; //Bending methods - void beginPlacement(vtkMRMLScene* scene, int id); + int beginPlacement(vtkMRMLScene* scene, int id); void endPlacement(); void computeAndSetSourcePoints(vtkMRMLScene* scene); void computeTransform(vtkMRMLScene* scene); @@ -191,21 +190,17 @@ class qSlicerPlannerModuleWidgetPrivate: public Ui_qSlicerPlannerModuleWidget //Clear fiducials used for bending void qSlicerPlannerModuleWidgetPrivate::clearControlPoints(vtkMRMLScene* scene) { - if(this->MovingPointA) + if(this->BendPoints[0]) { - scene->RemoveNode(this->MovingPointA); - this->MovingPointA = NULL; + scene->RemoveNode(this->BendPoints[0]); + this->BendPoints[0] = NULL; } - if(this->MovingPointB) + if(this->BendPoints[1]) { - scene->RemoveNode(this->MovingPointB); - this->MovingPointB = NULL; - } - if(this->PlacingNode) - { - scene->RemoveNode(this->PlacingNode); - this->PlacingNode = NULL; + scene->RemoveNode(this->BendPoints[1]); + this->BendPoints[1] = NULL; } + } //----------------------------------------------------------------------------- @@ -249,9 +244,8 @@ qSlicerPlannerModuleWidgetPrivate::qSlicerPlannerModuleWidgetPrivate() this->BendASide = true; this->ScalarsVsBrain = true; - this->MovingPointA = NULL; - this->MovingPointB = NULL; - this->PlacingNode = NULL; + this->BendPoints[0] = NULL; + this->BendPoints[1] = NULL; this->Fiducials = NULL; this->BendMagnitude = 0; @@ -275,7 +269,6 @@ void qSlicerPlannerModuleWidgetPrivate::endPlacement() this->MovingPointAButton->setEnabled(true); this->MovingPointBButton->setEnabled(true); this->placingActive = false; - this->PlacingNode = NULL; } //----------------------------------------------------------------------------- @@ -286,8 +279,8 @@ void qSlicerPlannerModuleWidgetPrivate::computeAndSetSourcePoints(vtkMRMLScene* double posa[3]; double posb[3]; - this->MovingPointA->GetNthFiducialPosition(0, posa); - this->MovingPointB->GetNthFiducialPosition(0, posb); + this->BendPoints[0]->GetNthFiducialPosition(0, posa); + this->BendPoints[1]->GetNthFiducialPosition(0, posb); this->Fiducials->InsertNextPoint(posa[0], posa[1], posa[2]); this->Fiducials->InsertNextPoint(posb[0], posb[1], posb[2]); @@ -365,54 +358,35 @@ void qSlicerPlannerModuleWidgetPrivate::computeTransform(vtkMRMLScene* scene) } //----------------------------------------------------------------------------- //Initialize placement of a fiducial -void qSlicerPlannerModuleWidgetPrivate::beginPlacement(vtkMRMLScene* scene, int id) +int qSlicerPlannerModuleWidgetPrivate::beginPlacement(vtkMRMLScene* scene, int id) { vtkNew fiducial; - - if(id == 2) //Place fiducial MA - { - if(this->MovingPointA) - { - scene->RemoveNode(this->MovingPointA); - MovingPointA = NULL; - } - this->MovingPointA = fiducial.GetPointer(); - this->MovingPointA->SetName("A"); - this->PlacingNode = this->MovingPointA; - - } - if(id == 3) //Place fiducial MB + if (this->BendPoints[id]) { - if(this->MovingPointB) - { - scene->RemoveNode(this->MovingPointB); - MovingPointB = NULL; - } - this->MovingPointB = fiducial.GetPointer(); - this->MovingPointB->SetName("B"); - this->PlacingNode = this->MovingPointB; - + scene->RemoveNode(this->BendPoints[id]); + BendPoints[id] = NULL; } - - //add new fiducial to scene - scene->AddNode(this->PlacingNode); - this->PlacingNode->CreateDefaultDisplayNodes(); - vtkMRMLMarkupsDisplayNode* disp = vtkMRMLMarkupsDisplayNode::SafeDownCast(this->PlacingNode->GetDisplayNode()); + this->BendPoints[id] = fiducial.GetPointer(); + std::string name = "BendPoint" + std::to_string(id); + this->BendPoints[id]->SetName(name.c_str()); + scene->AddNode(this->BendPoints[id]); + this->BendPoints[id]->CreateDefaultDisplayNodes(); + vtkMRMLMarkupsDisplayNode* disp = vtkMRMLMarkupsDisplayNode::SafeDownCast(this->BendPoints[id]->GetDisplayNode()); disp->SetTextScale(0.1); - this->placingActive = true; - - //activate placing vtkMRMLInteractionNode* interaction = qSlicerCoreApplication::application()->applicationLogic()->GetInteractionNode(); vtkMRMLSelectionNode* selection = qSlicerCoreApplication::application()->applicationLogic()->GetSelectionNode(); selection->SetReferenceActivePlaceNodeClassName("vtkMRMLMarkupsFiducialNode"); - selection->SetActivePlaceNodeID(this->PlacingNode->GetID()); + selection->SetActivePlaceNodeID(this->BendPoints[id]->GetID()); interaction->SetCurrentInteractionMode(vtkMRMLInteractionNode::Place); - + + + //deactivate buttons this->MovingPointAButton->setEnabled(false); this->MovingPointBButton->setEnabled(false); + return EXIT_SUCCESS; } //----------------------------------------------------------------------------- @@ -1532,7 +1506,7 @@ void qSlicerPlannerModuleWidget::updateWidgetFromMRML() d->TemplateReferenceOpacitySliderWidget); //Bending init button - if(d->MovingPointA && d->MovingPointB && d->CurrentBendNode) + if(d->BendPoints[0] && d->BendPoints[1] && d->CurrentBendNode) { d->InitButton->setEnabled(true); d->BendingInfoLabel->setText("You can move the points after they are placed by clicking and dragging in the" @@ -1735,14 +1709,18 @@ void qSlicerPlannerModuleWidget::placeFiducialButtonClicked() { Q_D(qSlicerPlannerModuleWidget); QObject* obj = sender(); - + int success; if(obj == d->MovingPointAButton) { - d->beginPlacement(this->mrmlScene(), 2); + success = d->beginPlacement(this->mrmlScene(), 0); } if(obj == d->MovingPointBButton) { - d->beginPlacement(this->mrmlScene(), 3); + success = d->beginPlacement(this->mrmlScene(), 1); + } + if (success != EXIT_SUCCESS) + { + return; } qvtkConnect(qSlicerCoreApplication::application()->applicationLogic()->GetInteractionNode(), vtkMRMLInteractionNode::EndPlacementEvent, this, SLOT(cancelFiducialButtonClicked())); From d3945e1db0e150b6da6ab12a6a9387a86dac5a2c Mon Sep 17 00:00:00 2001 From: Sam Horvath Date: Thu, 26 Apr 2018 13:08:04 -0400 Subject: [PATCH 2/3] ENH: Restrict placement of fiducials to current active bone --- Planner/Logic/vtkSlicerPlannerLogic.cxx | 17 ++++++++++ Planner/Logic/vtkSlicerPlannerLogic.h | 6 ++-- Planner/qSlicerPlannerModuleWidget.cxx | 45 +++++++++++++++++++++++-- 3 files changed, 63 insertions(+), 5 deletions(-) diff --git a/Planner/Logic/vtkSlicerPlannerLogic.cxx b/Planner/Logic/vtkSlicerPlannerLogic.cxx index dea691c..367e098 100644 --- a/Planner/Logic/vtkSlicerPlannerLogic.cxx +++ b/Planner/Logic/vtkSlicerPlannerLogic.cxx @@ -812,4 +812,21 @@ vtkVector3d vtkSlicerPlannerLogic::getNormalAtPoint(vtkVector3d point, vtkCellLo normal.SetZ(n[2]); return normal; +} + +double vtkSlicerPlannerLogic::getDistanceToModel(vtkVector3d point, vtkPolyData* model) +{ + vtkNew locator; + vtkNew triangulate; + triangulate->SetInputData(model); + triangulate->Update(); + locator->SetDataSet(triangulate->GetOutput()); + locator->BuildLocator(); + + double closestPoint[3];//the coordinates of the closest point will be returned here + double closestPointDist2; //the squared distance to the closest point will be returned here + vtkIdType cellId; //the cell id of the cell containing the closest point will be returned here + int subId; //this is rarely used (in triangle strips only, I believe) + locator->FindClosestPoint(point.GetData(), closestPoint, cellId, subId, closestPointDist2); + return std::sqrt(closestPointDist2); } \ No newline at end of file diff --git a/Planner/Logic/vtkSlicerPlannerLogic.h b/Planner/Logic/vtkSlicerPlannerLogic.h index 4408d37..d168e09 100644 --- a/Planner/Logic/vtkSlicerPlannerLogic.h +++ b/Planner/Logic/vtkSlicerPlannerLogic.h @@ -99,6 +99,8 @@ class VTK_SLICER_PLANNER_MODULE_LOGIC_EXPORT vtkSlicerPlannerLogic : vtkSmartPointer getTargetPoints() { return this->TargetPoints; } void setBendType(BendModeType type) {this->bendMode = type;} void setBendSide(BendSide side) { this->bendSide = side; } + double getDistanceToModel(vtkVector3d point, vtkPolyData* model); + protected: vtkSlicerPlannerLogic(); @@ -116,9 +118,9 @@ class VTK_SLICER_PLANNER_MODULE_LOGIC_EXPORT vtkSlicerPlannerLogic : vtkMRMLModelNode* mergeModel(vtkMRMLModelHierarchyNode* HierarchyNode, std::string name); void generateSourcePoints(); vtkVector3d projectToModel(vtkVector3d point); - vtkVector3d projectToModel(vtkVector3d point, vtkPlane* plane); - vtkVector3d projectToModel(vtkVector3d point, vtkPolyData* model); + vtkVector3d projectToModel(vtkVector3d point, vtkPlane* plane); vtkVector3d projectToModel(vtkVector3d point, vtkCellLocator* locator); + vtkVector3d projectToModel(vtkVector3d point, vtkPolyData* model); vtkVector3d getNormalAtPoint(vtkVector3d point, vtkCellLocator* locator, vtkPolyData* model); vtkSmartPointer createPlane(vtkVector3d A, vtkVector3d B, vtkVector3d C, vtkVector3d D); void createBendingLocator(); diff --git a/Planner/qSlicerPlannerModuleWidget.cxx b/Planner/qSlicerPlannerModuleWidget.cxx index be36e1d..87914d9 100644 --- a/Planner/qSlicerPlannerModuleWidget.cxx +++ b/Planner/qSlicerPlannerModuleWidget.cxx @@ -162,10 +162,12 @@ class qSlicerPlannerModuleWidgetPrivate: public Ui_qSlicerPlannerModuleWidget bool BendDoubleSide; bool ScalarsVsBrain; bool BendASide; + vtkMRMLScene* scene; //Bending methods int beginPlacement(vtkMRMLScene* scene, int id); void endPlacement(); + int ActivePoint; void computeAndSetSourcePoints(vtkMRMLScene* scene); void computeTransform(vtkMRMLScene* scene); void clearControlPoints(vtkMRMLScene* scene); @@ -186,6 +188,7 @@ class qSlicerPlannerModuleWidgetPrivate: public Ui_qSlicerPlannerModuleWidget //----------------------------------------------------------------------------- // qSlicerPlannerModuleWidgetPrivate methods + //----------------------------------------------------------------------------- //Clear fiducials used for bending void qSlicerPlannerModuleWidgetPrivate::clearControlPoints(vtkMRMLScene* scene) @@ -200,6 +203,8 @@ void qSlicerPlannerModuleWidgetPrivate::clearControlPoints(vtkMRMLScene* scene) scene->RemoveNode(this->BendPoints[1]); this->BendPoints[1] = NULL; } + + this->ActivePoint = -1; } @@ -239,6 +244,7 @@ qSlicerPlannerModuleWidgetPrivate::qSlicerPlannerModuleWidgetPrivate() this->cmdNode = NULL; this->PreOpSet = false; this->cliFreeze = false; + this->scene = NULL; this->BendDoubleSide = true; this->BendASide = true; @@ -248,6 +254,7 @@ qSlicerPlannerModuleWidgetPrivate::qSlicerPlannerModuleWidgetPrivate() this->BendPoints[1] = NULL; this->Fiducials = NULL; this->BendMagnitude = 0; + this->ActivePoint = -1; qSlicerAbstractCoreModule* splitModule = qSlicerCoreApplication::application()->moduleManager()->module("SplitModel"); @@ -266,9 +273,35 @@ qSlicerPlannerModuleWidgetPrivate::qSlicerPlannerModuleWidgetPrivate() //Complete placement of current fiducial void qSlicerPlannerModuleWidgetPrivate::endPlacement() { - this->MovingPointAButton->setEnabled(true); - this->MovingPointBButton->setEnabled(true); - this->placingActive = false; + + //check that point in close to (i.e. on surface of) model to bend + //if not, retrigger placing and give it another go + + vtkVector3d point; + double posa[3]; + this->BendPoints[this->ActivePoint]->GetNthFiducialPosition(0, posa); + point.SetX(posa[0]); + point.SetY(posa[1]); + point.SetZ(posa[2]); + + double dist = this->logic->getDistanceToModel(point, vtkMRMLModelNode::SafeDownCast(this->CurrentBendNode)->GetPolyData()); + std::cout << "Distance: " << dist << std::endl; + if (dist > 1.0) + { + + this->scene->RemoveNode(this->BendPoints[this->ActivePoint]); + BendPoints[this->ActivePoint] = NULL; + this->beginPlacement(this->scene, this->ActivePoint); + + } + else + { + this->MovingPointAButton->setEnabled(true); + this->MovingPointBButton->setEnabled(true); + this->placingActive = false; + this->ActivePoint = -1; + } + } //----------------------------------------------------------------------------- @@ -375,6 +408,8 @@ int qSlicerPlannerModuleWidgetPrivate::beginPlacement(vtkMRMLScene* scene, int i this->BendPoints[id]->CreateDefaultDisplayNodes(); vtkMRMLMarkupsDisplayNode* disp = vtkMRMLMarkupsDisplayNode::SafeDownCast(this->BendPoints[id]->GetDisplayNode()); disp->SetTextScale(0.1); + disp->SetGlyphScale(5); + this->placingActive = true; vtkMRMLInteractionNode* interaction = qSlicerCoreApplication::application()->applicationLogic()->GetInteractionNode(); vtkMRMLSelectionNode* selection = qSlicerCoreApplication::application()->applicationLogic()->GetSelectionNode(); selection->SetReferenceActivePlaceNodeClassName("vtkMRMLMarkupsFiducialNode"); @@ -386,6 +421,7 @@ int qSlicerPlannerModuleWidgetPrivate::beginPlacement(vtkMRMLScene* scene, int i //deactivate buttons this->MovingPointAButton->setEnabled(false); this->MovingPointBButton->setEnabled(false); + this->ActivePoint = id; return EXIT_SUCCESS; } @@ -1148,6 +1184,8 @@ void qSlicerPlannerModuleWidget::setup() qMRMLPlannerModelHierarchyModel* sceneModel = new qMRMLPlannerModelHierarchyModel(this); + d->scene = this->mrmlScene(); + d->logic = this->plannerLogic(); qSlicerAbstractCoreModule* wrapperModule = qSlicerCoreApplication::application()->moduleManager()->module("ShrinkWrap"); @@ -1723,6 +1761,7 @@ void qSlicerPlannerModuleWidget::placeFiducialButtonClicked() return; } + d->scene = this->mrmlScene(); qvtkConnect(qSlicerCoreApplication::application()->applicationLogic()->GetInteractionNode(), vtkMRMLInteractionNode::EndPlacementEvent, this, SLOT(cancelFiducialButtonClicked())); return; } From 9ad29ce60bee929afede5d4488f12337e9668bcb Mon Sep 17 00:00:00 2001 From: Sam Horvath Date: Thu, 26 Apr 2018 13:37:42 -0400 Subject: [PATCH 3/3] BUG: Peristent slot connection caused planes to pass into bending fiducial logic Disconnect slot after use --- Planner/qSlicerPlannerModuleWidget.cxx | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/Planner/qSlicerPlannerModuleWidget.cxx b/Planner/qSlicerPlannerModuleWidget.cxx index 87914d9..a7368b9 100644 --- a/Planner/qSlicerPlannerModuleWidget.cxx +++ b/Planner/qSlicerPlannerModuleWidget.cxx @@ -276,7 +276,7 @@ void qSlicerPlannerModuleWidgetPrivate::endPlacement() //check that point in close to (i.e. on surface of) model to bend //if not, retrigger placing and give it another go - + vtkVector3d point; double posa[3]; this->BendPoints[this->ActivePoint]->GetNthFiducialPosition(0, posa); @@ -285,7 +285,6 @@ void qSlicerPlannerModuleWidgetPrivate::endPlacement() point.SetZ(posa[2]); double dist = this->logic->getDistanceToModel(point, vtkMRMLModelNode::SafeDownCast(this->CurrentBendNode)->GetPolyData()); - std::cout << "Distance: " << dist << std::endl; if (dist > 1.0) { @@ -642,21 +641,14 @@ vtkMRMLMarkupsPlanesNode* qSlicerPlannerModuleWidgetPrivate::createPlaneNode( QString planesName = refNode->GetName(); planesName += "Planner_Planes"; planes->SetName(planesName.toLatin1()); - vtkNew newDisplay; vtkMRMLNode* display = scene->AddNode(newDisplay.GetPointer()); planes->SetAndObserveDisplayNodeID(display->GetID()); refNode->SetNodeReferenceID( this->sceneModel()->planesReferenceRole(), planes->GetID()); - /** - vtkMRMLTransformDisplayNode* display2 = - vtkMRMLTransformDisplayNode::SafeDownCast(refNode ? - refNode->GetNodeReference(this->sceneModel()->transformDisplayReferenceRole()) : NULL); - planes->SetNodeReferenceID( - this->sceneModel()->transformDisplayReferenceRole(), display2->GetID()); - **/ - //planes->SetAndObserveTransformNodeID(display2->GetID()); + + return planes; } @@ -1781,6 +1773,7 @@ void qSlicerPlannerModuleWidget::cancelBendButtonClicked() d->clearBendingData(this->mrmlScene()); d->bendingActive = false; d->bendingOpen = false; + qvtkDisconnect(qSlicerCoreApplication::application()->applicationLogic()->GetInteractionNode(), vtkMRMLInteractionNode::EndPlacementEvent, this, SLOT(cancelFiducialButtonClicked())); this->updateWidgetFromMRML(); } @@ -1851,6 +1844,7 @@ void qSlicerPlannerModuleWidget::finshBendClicked() d->clearBendingData(this->mrmlScene()); d->bendingActive = false; d->bendingOpen = false; + qvtkDisconnect(qSlicerCoreApplication::application()->applicationLogic()->GetInteractionNode(), vtkMRMLInteractionNode::EndPlacementEvent, this, SLOT(cancelFiducialButtonClicked())); this->updateWidgetFromMRML(); } @@ -1858,7 +1852,7 @@ void qSlicerPlannerModuleWidget::finshBendClicked() //Cancel placement of current fiducial void qSlicerPlannerModuleWidget::cancelFiducialButtonClicked() { - Q_D(qSlicerPlannerModuleWidget); + Q_D(qSlicerPlannerModuleWidget); d->endPlacement(); this->updateWidgetFromMRML();