ENH: Minor bugfixes and improvements

Most important changes:
- Select existing color in the simple color table when opening it with an existing color
- Fix loading color table from file: terminology was saved to the wrong color index

Re Slicer#6975, Slicer#7593

Libs/MRML/Core/vtkMRMLColorNode.cxx

@@ -611,6 +611,59 @@ int vtkMRMLColorNode::GetColorIndexByName(const char *name)
   return -1;
 }
 
+//---------------------------------------------------------------------------
+int vtkMRMLColorNode::GetColorIndexByTerminology(const char* terminology, bool ignoreContextName/*=true*/)
+{
+  if (terminology == nullptr)
+  {
+    vtkErrorMacro("GetColorIndexByTerminology: need a valid terminology string as argument");
+    return -1;
+  }
+
+  // Utility function to get part of the terminology string without the terminology context
+  auto GetTerminologyNoContext = [](std::string fullTerminologyStr)
+  {
+    std::vector<std::string> entryComponents;
+    vtksys::SystemTools::Split(fullTerminologyStr, entryComponents, '~');
+    if (entryComponents.size() != 7)
+    {
+      return fullTerminologyStr; // Not a full terminology string, return the input
+    }
+    std::string terminologyNoContext;
+    for (int i=0; i<7; i++)
+    {
+      if (i != 0 && i != 4) // Skip context names
+      {
+        terminologyNoContext.append(entryComponents[i]);
+      }
+      terminologyNoContext.append("~");
+    }
+    return terminologyNoContext;
+  };
+
+  // Get terminology string to compare
+  std::string terminologyStr(terminology);
+  if (ignoreContextName)
+  {
+    terminologyStr = GetTerminologyNoContext(terminologyStr);
+  }
+
+  for (int colorIdx=0; colorIdx<this->GetNumberOfColors(); ++colorIdx)
+  {
+    std::string currentTerminologyStr = this->GetTerminologyAsString(colorIdx);
+    if (ignoreContextName)
+    {
+      currentTerminologyStr = GetTerminologyNoContext(currentTerminologyStr);
+    }
+    if (terminologyStr == currentTerminologyStr)
+    {
+      return colorIdx;
+    }
+  }
+
+  return -1; // Not found
+}
+
 //---------------------------------------------------------------------------
 std::string vtkMRMLColorNode::GetColorNameWithoutSpaces(int ind, const char *subst)
 {

Libs/MRML/Core/vtkMRMLColorNode.h

@@ -120,7 +120,13 @@ class VTK_MRML_EXPORT vtkMRMLColorNode : public vtkMRMLStorableNode
   /// Return the index associated with this color name, which can then be used
   /// to get the color. Returns -1 on failure.
   /// \sa GetColorName()
-  int GetColorIndexByName(const char *name);
+  int GetColorIndexByName(const char* name);
+
+  /// Return the index associated with this terminology entry to get the color. Returns -1 on failure.
+  /// \param terminologyStr The string representation of the searched terminology entry.
+  /// \param ignoreContextName Only consider the category, type, etc. coded entries if true,
+  ///   otherwise look for exact match in the terminology context name as well. True by default.
+  int GetColorIndexByTerminology(const char* terminologyStr, bool ignoreContextName=true);
 
   /// Get the 0'th based \a colorIndex'th name of this color, replacing all
   /// file name sensitive color name characters with safer character(s).

Libs/MRML/Core/vtkMRMLColorTableStorageNode.cxx

@@ -404,7 +404,7 @@ int vtkMRMLColorTableStorageNode::ReadCsvFile(std::string fullFileName, vtkMRMLC
       vtksys::SystemTools::Join(anatomicRegionComponents, "^"),
       vtksys::SystemTools::Join(anatomicRegionModifierComponents, "^")
     };
-    colorNode->SetTerminologyFromString(row, vtksys::SystemTools::Join(terminologyComponents, "~"));
+    colorNode->SetTerminologyFromString(validLabelValues[row], vtksys::SystemTools::Join(terminologyComponents, "~"));
   } // For each row in the color table
 
   return 1;

Libs/MRML/Logic/vtkMRMLColorLogic.cxx

@@ -186,8 +186,7 @@ void vtkMRMLColorLogic::RemoveDefaultColorNodes()
   }
   basicNode->Delete();
 
-   // remove the procedural color nodes (after the fs proc nodes as
-   // getting them by class)
+  // remove the procedural color nodes (after the fs proc nodes as getting them by class)
   std::vector<vtkMRMLNode *> procNodes;
   int numProcNodes = this->GetMRMLScene()->GetNodesByClass("vtkMRMLProceduralColorNode", procNodes);
   for (int i = 0; i < numProcNodes; i++)
@@ -886,7 +885,6 @@ void vtkMRMLColorLogic::AddUserFileNodes()
   {
     this->AddUserFileNode(i);
   }
-
 }
 
 //----------------------------------------------------------------------------------------

Modules/Loadable/Segmentations/Logic/vtkSlicerSegmentationsModuleLogic.cxx

@@ -73,6 +73,7 @@
 // MRML includes
 #include <vtkEventBroker.h>
 #include <vtkMRMLColorTableNode.h>
+#include "vtkMRMLI18N.h"
 #include <vtkMRMLLabelMapVolumeNode.h>
 #include <vtkMRMLLabelMapVolumeDisplayNode.h>
 #include <vtkMRMLMessageCollection.h>
@@ -1067,9 +1068,11 @@ vtkMRMLColorTableNode* vtkSlicerSegmentationsModuleLogic::CreateColorTableNodeFo
   newColorTable->SetNumberOfColors(1);
   newColorTable->GetLookupTable()->SetRange(0, 0);
   newColorTable->GetLookupTable()->SetNumberOfTableValues(1);
-  // Use NoName as color name to not list the "background" color in the color legend.
-  newColorTable->SetColor(0, newColorTable->GetNoName(), 0.0, 0.0, 0.0, 0.0);
+  newColorTable->SetColor(0, vtkMRMLTr("vtkSlicerSegmentationsModuleLogic", "background").c_str(), 0.0, 0.0, 0.0, 0.0);
   segmentationNode->GetScene()->AddNode(newColorTable);
+  // Associate exported color table to the segmentation node
+  segmentationNode->SetLabelmapConversionColorTableNodeID(newColorTable->GetID());
+
   return newColorTable;
 }
 
@@ -1138,7 +1141,7 @@ bool vtkSlicerSegmentationsModuleLogic::ExportSegmentsToColorTableNode(vtkMRMLSe
     }
     else
     {
-       vtkWarningWithObjectMacro(segmentationNode, "ExportSegmentsToColorTableNode: failed to get terminology tag from segment " << segmentName);
+      vtkWarningWithObjectMacro(segmentationNode, "ExportSegmentsToColorTableNode: failed to get terminology tag from segment " << segmentName);
     }
   }
 

Modules/Loadable/Terminologies/Logic/vtkSlicerTerminologiesModuleLogic.cxx

@@ -2541,6 +2541,10 @@ std::vector<std::string> vtkSlicerTerminologiesModuleLogic::FindTerminologyNames
   // Find terminology entries in each preferred terminology
   for (std::string terminologyName : preferredTerminologyNames)
   {
+    if (!this->IsTerminologyContextLoaded(terminologyName))
+    {
+      continue; // It is possible that some preferred terminologies are not loaded in this session
+    }
     vtkNew<vtkSlicerTerminologyType> typeObject;
     if (!this->GetTypeInTerminologyCategory(terminologyName, categoryId, typeId, typeObject))
     {
@@ -3027,6 +3031,17 @@ bool vtkSlicerTerminologiesModuleLogic::LoadColorTable(vtkMRMLColorNode* colorNo
   return true;
 }
 
+//---------------------------------------------------------------------------
+bool vtkSlicerTerminologiesModuleLogic::IsTerminologyContextLoaded(std::string terminologyName)
+{
+  if (terminologyName.empty())
+  {
+    return false;
+  }
+  rapidjson::Value& root = this->Internal->GetTerminologyRootByName(terminologyName);
+  return !root.IsNull();
+}
+
 //---------------------------------------------------------------------------
 const char* vtkSlicerTerminologiesModuleLogic::IsTerminologyColorTable(std::string terminologyName)
 {

Modules/Loadable/Terminologies/Logic/vtkSlicerTerminologiesModuleLogic.h

@@ -134,6 +134,8 @@ class VTK_SLICER_TERMINOLOGIES_LOGIC_EXPORT vtkSlicerTerminologiesModuleLogic :
     std::vector<std::string> preferredAnatomicContextNames,
     vtkCollection* foundEntries=nullptr);
 
+  /// Determine if terminology with a given name is loaded.
+  bool IsTerminologyContextLoaded(std::string terminologyName);
   /// Determine if terminology is from a color table.
   /// \return ID of the color table node if terminology comes from color table, else nullptr.
   const char* IsTerminologyColorTable(std::string terminologyName);

Modules/Loadable/Terminologies/Widgets/qMRMLSimpleColorTableView.cxx

@@ -19,6 +19,7 @@
 ==============================================================================*/
 
 // Qt includes
+#include <QDebug>
 #include <QHeaderView>
 #include <QSortFilterProxyModel>
 
@@ -117,3 +118,28 @@ vtkMRMLColorNode* qMRMLSimpleColorTableView::mrmlColorNode()const
   Q_ASSERT(mrmlModel);
   return mrmlModel->mrmlColorNode();
 }
+
+//------------------------------------------------------------------------------
+void qMRMLSimpleColorTableView::selectColorByIndex(int colorIndex)const
+{
+  QSortFilterProxyModel* sortFilterModel = this->sortFilterProxyModel();
+  qMRMLColorModel* colorModel = this->colorModel();
+  vtkMRMLColorNode* colorNode = this->mrmlColorNode();
+  if (colorNode == nullptr)
+  {
+    qCritical() << Q_FUNC_INFO << ": Invalid color node in table view";
+    return;
+  }
+
+  QModelIndexList foundIndices = colorModel->match(colorModel->index(0,0), qMRMLItemDelegate::ColorEntryRole,
+    colorIndex, 1, Qt::MatchExactly | Qt::MatchRecursive);
+  if (foundIndices.size() == 0)
+  {
+    qCritical() << Q_FUNC_INFO << ": Failed to find color model index by color index " << colorIndex
+      << " in color node " << colorNode->GetName();
+    return;
+  }
+
+  QModelIndex foundIndex = sortFilterModel->mapFromSource(foundIndices[0]);
+  this->selectionModel()->select(foundIndex, QItemSelectionModel::Select | QItemSelectionModel::Rows);
+}

Modules/Loadable/Terminologies/Widgets/qMRMLSimpleColorTableView.h

@@ -51,6 +51,8 @@ public slots:
   void setMRMLColorNode(vtkMRMLColorNode* colorNode);
   /// Utility function to simply connect signals/slots with Qt Designer
   void setMRMLColorNode(vtkMRMLNode* colorNode);
+  /// Select row in table by color index
+  void selectColorByIndex(int colorIndex)const;
 
 protected:
   QScopedPointer<qMRMLSimpleColorTableViewPrivate> d_ptr;

Modules/Loadable/Terminologies/Widgets/qSlicerTerminologyNavigatorWidget.cxx

@@ -632,7 +632,7 @@ QTableWidgetItem* qSlicerTerminologyNavigatorWidgetPrivate::findTableWidgetItemF
     }
   }
 
-    return nullptr;
+  return nullptr;
 }
 
 //-----------------------------------------------------------------------------
@@ -1024,6 +1024,26 @@ bool qSlicerTerminologyNavigatorWidget::setTerminologyEntry(vtkSlicerTerminology
     }
   }
 
+  // Set selection in color table
+  const char* colorTableNodeID = logic->IsTerminologyColorTable(terminologyContextNameToSelect.toUtf8().constData());
+  if (colorTableNodeID != nullptr)
+  {
+    vtkMRMLColorNode* colorNode = vtkMRMLColorNode::SafeDownCast(
+      logic->GetMRMLScene()->GetNodeByID(colorTableNodeID));
+    if (colorNode == nullptr)
+    {
+      qCritical() << Q_FUNC_INFO << ": Failed to find color node by ID " << colorTableNodeID;
+      return returnValue;
+    }
+    vtkNew<vtkSlicerTerminologyEntry> entry;
+    this->terminologyEntry(entry);
+    int foundColor = colorNode->GetColorIndexByTerminology(logic->SerializeTerminologyEntry(entry).c_str());
+    if (foundColor >= 0)
+    {
+      d->ColorTableView->selectColorByIndex(foundColor);
+    }
+  }
+
   return returnValue;
 }
 
@@ -1563,9 +1583,15 @@ void qSlicerTerminologyNavigatorWidget::onTerminologySelectionChanged(int index)
 {
   Q_D(qSlicerTerminologyNavigatorWidget);
 
-  // Set current terminology
+  // Make selection in the other combobox as well
   ctkComboBox* visibleComboBox = (d->ComboBox_Terminology->isVisible() ?
     d->ComboBox_Terminology : d->ComboBox_Terminology_2);
+  ctkComboBox* invisibleComboBox = (!d->ComboBox_Terminology->isVisible() ?
+    d->ComboBox_Terminology : d->ComboBox_Terminology_2);
+  QSignalBlocker blocker(invisibleComboBox);
+  invisibleComboBox->setCurrentIndex(visibleComboBox->currentIndex());
+
+  // Set current terminology
   QString terminologyName = visibleComboBox->itemText(index);
   this->setCurrentTerminology(terminologyName);
 
@@ -1943,6 +1969,10 @@ void qSlicerTerminologyNavigatorWidget::onColorSelected(const QItemSelection& se
   // Set category and type
   d->CurrentCategoryObject->vtkCodedEntry::Copy(colorNode->GetTerminologyCategory(colorIndex));
   d->CurrentTypeObject->vtkCodedEntry::Copy(colorNode->GetTerminologyType(colorIndex));
+  double color[4] = {0.0};
+  colorNode->GetColor(colorIndex, color);
+  d->CurrentTypeObject->SetRecommendedDisplayRGBValue(
+    (unsigned char)(color[0] * 255.0), (unsigned char)(color[1] * 255.0), (unsigned char)(color[2] * 255.0));
   emit selectionValidityChanged(true);
 
   // Set optional information if any
@@ -1974,6 +2004,9 @@ void qSlicerTerminologyNavigatorWidget::onColorSelected(const QItemSelection& se
     d->resetCurrentRegionModifier();
   }
 
+  // Set name and color
+  d->setNameFromCurrentTerminology();
+  d->setRecommendedColorFromCurrentTerminology();
 }
 //-----------------------------------------------------------------------------
 void qSlicerTerminologyNavigatorWidget::onColorRowDoubleClicked(const QModelIndex &index)