Merge pull request #1 from jpfeuffer/gui/proteintable

Some cleanup

src/openms_gui/include/OpenMS/VISUAL/APPLICATIONS/TOPPViewBase.h

@@ -476,6 +476,7 @@ protected slots:
 
     /// Main workspace
     EnhancedWorkspace ws_;  // not a pointer, but an actual object, so it gets destroyed before the DefaultParamhandler (on which it depends)
+    int activeSubwindowID_ = -1; // due to Qt bugs we need to save the current Window id in the children of the workspace;
     /// Tab bar. The address of the corresponding window to a tab is stored as an int in tabData()
     EnhancedTabBar tab_bar_;
     /// manages recent list of filenames and the menu that goes with it

src/openms_gui/include/OpenMS/VISUAL/Plot1DCanvas.h

@@ -132,7 +132,7 @@ namespace OpenMS
     void flipLayer(Size index);
 
     /// Returns whether this widget is currently in mirror mode
-    bool mirrorModeActive();
+    bool mirrorModeActive() const;
 
     /// Sets whether this widget is currently in mirror mode
     void setMirrorModeActive(bool b);
@@ -175,7 +175,7 @@ namespace OpenMS
     Size getAlignmentSize();
 
     /// Returns the score of the alignment
-    double getAlignmentScore();
+    double getAlignmentScore() const;
 
     /// Returns aligned_peaks_indices_
     std::vector<std::pair<Size, Size> > getAlignedPeaksIndices();
@@ -324,7 +324,7 @@ protected slots:
     /// Shows dialog and calls addLabelAnnotation_
     void addUserLabelAnnotation_(const QPoint& screen_position);
     /// Adds an annotation item at the given screen position
-    void addLabelAnnotation_(const QPoint& screen_position, QString label_text);
+    void addLabelAnnotation_(const QPoint& screen_position, const QString& label_text);
     /// Shows dialog and calls addPeakAnnotation_
     void addUserPeakAnnotation_(PeakIndex near_peak);
 

src/openms_gui/include/OpenMS/VISUAL/PlotCanvas.h

@@ -543,7 +543,7 @@ public slots:
         Sets the visible area to a new value. Note that it does not emit visibleAreaChanged()
         @param area the new visible area
     */
-    void setVisibleArea(AreaType area);
+    void setVisibleArea(const AreaType& area);
 
     /**
         @brief Notifies the canvas that the horizontal scrollbar has been moved.

src/openms_gui/source/VISUAL/APPLICATIONS/TOPPViewBase.cpp

@@ -176,8 +176,21 @@ namespace OpenMS
     connect(&tab_bar_, &EnhancedTabBar::dropOnTab, this, &TOPPViewBase::copyLayer);
     box_layout->addWidget(&tab_bar_);
 
+    //TODO This triggers always also if there are just focus changes. Very very bad..
     connect(&ws_, &EnhancedWorkspace::subWindowActivated, [this](QMdiSubWindow* window) {
-      if (window != nullptr) /* 0 upon terminate */ updateBarsAndMenus(); 
+      Size s = 0;
+      int oldactiveSubwindowID = activeSubwindowID_;
+      for (const auto& child : ws_.subWindowList())
+      {
+        if (child == window)
+        {
+          activeSubwindowID_ = s;
+          break;
+        }
+        ++s;
+      }
+      if (window != nullptr && (oldactiveSubwindowID != activeSubwindowID_)) /* 0 upon terminate */ updateBarsAndMenus();
+
     });
     connect(&ws_, &EnhancedWorkspace::dropReceived, this, &TOPPViewBase::copyLayer);
     box_layout->addWidget(&ws_);
@@ -1365,7 +1378,7 @@ namespace OpenMS
 
   void TOPPViewBase::updateViewBar()
   {
-    selection_view_->update();
+    selection_view_->callUpdateEntries();
   }
 
   void TOPPViewBase::updateMenu()
@@ -1534,6 +1547,10 @@ namespace OpenMS
   {
     if (!ws_.currentSubWindow())
     {
+      if (activeSubwindowID_ < (Size) ws_.subWindowList().size())
+      {
+        return qobject_cast<PlotWidget*>(ws_.subWindowList()[int(activeSubwindowID_)]->widget());
+      }
       return nullptr;
     }
     return qobject_cast<PlotWidget*>(ws_.currentSubWindow()->widget());
@@ -1674,11 +1691,12 @@ namespace OpenMS
   QStringList TOPPViewBase::chooseFilesDialog_(const String& path_overwrite)
   {
     // store active sub window
+    //TODO Why is this done? And why only here?
     QMdiSubWindow* old_active = ws_.currentSubWindow();
     RAIICleanup clean([&]() { ws_.setActiveSubWindow(old_active); });
 
     QString open_path = current_path_.toQString();
-    if (path_overwrite != "")
+    if (!path_overwrite.empty())
     {
       open_path = path_overwrite.toQString();
     }
@@ -1691,7 +1709,7 @@ namespace OpenMS
     {
        return dialog.selectedFiles();
     }
-    return QStringList();
+    return {};
   }
 
   void TOPPViewBase::openFilesByDialog(const String& dir)
@@ -2004,6 +2022,7 @@ namespace OpenMS
     {
       return;
     }
+    selection_view_->currentTabChanged(1); //switch to ID view
   }
 
   void TOPPViewBase::annotateWithOSW()

src/openms_gui/source/VISUAL/DataSelectionTabs.cpp

@@ -151,6 +151,7 @@ namespace OpenMS
     Size current_index = currentIndex();
     // update the currently visible tab (might be disabled if no data is shown)
     tab_ptrs_[current_index]->updateEntries(layer_ptr);
+    this->update(); // not sure if necessary. Should update itself if changes occurred.
   }
 
   void DataSelectionTabs::currentTabChanged(int tab_index)

src/openms_gui/source/VISUAL/Plot1DCanvas.cpp

@@ -467,7 +467,7 @@ namespace OpenMS
           newHi = overall_data_range_.maxX();
           newLo = newHi - visible_area_.width();
         }
-        //chage data area
+        //change data area
         changeVisibleArea_(newLo, newHi);
         last_mouse_pos_ = p;
       }
@@ -1615,7 +1615,7 @@ namespace OpenMS
     }
   }
 
-  void Plot1DCanvas::addLabelAnnotation_(const QPoint& screen_position, QString text)
+  void Plot1DCanvas::addLabelAnnotation_(const QPoint& screen_position, const QString& text)
   {
     updatePercentageFactor_(getCurrentLayerIndex());
 
@@ -1853,7 +1853,7 @@ namespace OpenMS
   }
 
   /// Returns whether this widget is currently in mirror mode
-  bool Plot1DCanvas::mirrorModeActive()
+  bool Plot1DCanvas::mirrorModeActive() const
   {
     return mirror_mode_;
   }
@@ -2045,7 +2045,7 @@ namespace OpenMS
     return aligned_peaks_mz_delta_.size();
   }
 
-  double Plot1DCanvas::getAlignmentScore()
+  double Plot1DCanvas::getAlignmentScore() const
   {
     return alignment_score_;
   }

src/openms_gui/source/VISUAL/PlotCanvas.cpp

@@ -296,7 +296,7 @@ namespace OpenMS
     changeVisibleArea_(tmp, repaint, true);
   }
 
-  void PlotCanvas::setVisibleArea(AreaType area)
+  void PlotCanvas::setVisibleArea(const AreaType& area)
   {
     //cout << OPENMS_PRETTY_FUNCTION << endl;
     changeVisibleArea_(area);
@@ -1029,7 +1029,7 @@ namespace OpenMS
 
   LayerData& LayerStack::getCurrentLayer()
   {
-    if (current_layer_ >= layers_.size())
+    if (current_layer_ >= layers_.size() || current_layer_ < 0)
     {
       throw Exception::IndexOverflow(__FILE__, __LINE__, OPENMS_PRETTY_FUNCTION, current_layer_, layers_.size());
     }
@@ -1068,7 +1068,7 @@ namespace OpenMS
     }
     layers_.erase(layers_.begin() + layer_index);
 
-    // update current layer if it became invalid
+    // update current layer if it became invalid TODO dont you have to adjust the index to stay on the same layer??
     if (current_layer_ >= getLayerCount())
     {
       current_layer_ = getLayerCount() - 1; // overflow is intentional

src/openms_gui/source/VISUAL/SequenceVisualizer.cpp

@@ -8,7 +8,6 @@
 #include <QMessageBox>
 #include <QJsonArray>
 #include <QString>
-#include <String>
 
 #include <QtWebEngineWidgets/QWebEngineView>
 

src/openms_gui/source/VISUAL/SpectraIDViewTab.cpp

@@ -403,7 +403,20 @@ namespace OpenMS
     {
       // deselect whatever is currently shown
       int last_spectrum_index = int(layer_->getCurrentSpectrumIndex());
-      emit spectrumDeselected(last_spectrum_index);
+      // Deselecting spectrum does not do what you think it does. It still paints stuff. Without annotations..
+      // so just leave it for now.
+      //
+      // PARTLY SOLVED: The problem was, that if you defocus the TOPPView window, somehow
+      // selectionChange is called, with EMPTY selection. Maybe this is a feature and we have to store the
+      // selected spectrum indices as well. I want to support multi-selection in the future to see shared peptides
+      // Actually this might be solved by the removal of the unnecessary updates in activateSubWindow.
+      // I think updateEntries resets selections as well.. not sure how we could avoid that. We really have to avoid
+      // calling this crazy function when only small updates are needed.
+      //emit spectrumDeselected(last_spectrum_index);
+      // TODO also currently, the current active spectrum can be restored after deselection by clicking on
+      //  the Scans tab and then switching back to ID tab. (Scans will get the current scan in the 1D View, which
+      //  is still there. I guess I have to deselect in the 1D view, too, after all.
+
       updateProteinEntries_(-1);
     }
     //TODO if you deselected the current spectrum, you currently cannot click on/navigate to the same spectrum
@@ -412,6 +425,7 @@ namespace OpenMS
 
   void SpectraIDViewTab::currentCellChanged_(int row, int column, int /*old_row*/, int /*old_column*/)
   {
+    // TODO you actually only have to do repainting if the row changes..
     // sometimes Qt calls this function when table empty during refreshing
     if (row < 0 || column < 0)
       return;
@@ -428,6 +442,7 @@ namespace OpenMS
     int current_spectrum_index = table_widget_->item(row, Clmn::SPEC_INDEX)->data(Qt::DisplayRole).toInt();
     const auto& exp = *layer_->getPeakData();
     const auto& spec2 = exp[current_spectrum_index];
+
     //
     // Signal for a new spectrum to be shown
     //
@@ -534,6 +549,7 @@ namespace OpenMS
 
     // Update the protein table with data of the id row that was clicked
     updateProteinEntries_(row);
+
   }
 
   bool SpectraIDViewTab::hasData(const LayerData* layer)
@@ -547,6 +563,7 @@ namespace OpenMS
 
   void SpectraIDViewTab::updateEntries(LayerData* cl)
   {
+
     // do not try to be smart and check if layer_ == cl; to return early
     // since the layer content might have changed, e.g. pepIDs were added
     layer_ = cl;
@@ -557,7 +574,7 @@ namespace OpenMS
     is_first_time_loading = true;
     createProteinToPeptideIDMap_();
     updateEntries_(); // we need this extra function since it's an internal slot
-    
+
   }
 
   LayerData* SpectraIDViewTab::getLayer()
@@ -584,7 +601,7 @@ namespace OpenMS
     // no valid peak layer attached
     if (!hasData(layer_) || layer_->getPeakData()->getProteinIdentifications().empty())
     {
-      clear();
+      //clear(); this was done in updateEntries_() already.
       return;
     }
 
@@ -675,6 +692,7 @@ namespace OpenMS
 
   void SpectraIDViewTab::updateEntries_()
   {
+
     // no valid peak layer attached
     if (!hasData(layer_))
     {
@@ -740,6 +758,7 @@ namespace OpenMS
       headers << ck.toQString();
     }
 
+    table_widget_->blockSignals(true); // to be safe, that clear does not trigger anything.
     table_widget_->clear();
     table_widget_->setRowCount(0);
     table_widget_->setColumnCount(headers.size());
@@ -1037,4 +1056,4 @@ namespace OpenMS
       table_widget_->setAtBottomRow(first_precursor.getIntensity(), Clmn::PREC_INT, background_color);
     }
   }
-}
+}

src/openms_gui/source/VISUAL/TOPPViewMenu.cpp

@@ -90,7 +90,12 @@ namespace OpenMS
     m_file->addMenu(recent_files->getMenu()); // updates automatically via RecentFilesMenu class, since this is just a pointer
 
     m_file->addSeparator();
-    m_file->addAction("&Preferences", parent, &TOPPViewBase::preferencesDialog);
+    //m_file->addAction("&Preferences", parent, &TOPPViewBase::preferencesDialog);
+    auto pref = new QAction("&Preferences", parent);
+    pref->setMenuRole(QAction::PreferencesRole);
+    pref->setEnabled(true);
+    m_file->addAction(pref);
+    connect(pref, &QAction::triggered, parent, &TOPPViewBase::preferencesDialog);
     m_file->addAction("&Quit", qApp, SLOT(quit()));
 
     // Tools menu
@@ -149,7 +154,7 @@ namespace OpenMS
       TV_STATUS::IS_1D_VIEW);
     action->setToolTip("Only available in 1D View");
     m_layer->addSeparator();
-    addAction_(m_layer->addAction("Preferences", parent, &TOPPViewBase::showPreferences),
+    addAction_(m_layer->addAction("Layer preferences", parent, &TOPPViewBase::showPreferences),
       TV_STATUS::HAS_LAYER);
 
     // Windows menu
@@ -175,7 +180,7 @@ namespace OpenMS
     m_help->addAction("Tutorials and documentation", [&]() { GUIHelpers::openURL("html/index.html"); }, Qt::Key_F1);
 
     m_help->addSeparator();
-    m_help->addAction("&About", [&]() {QApplicationTOPP::showAboutDialog(parent, "TOPPView"); });
+    m_help->addAction("&About", [parent]() {QApplicationTOPP::showAboutDialog(parent, "TOPPView"); });
   }
 
   void TOPPViewMenu::update(const FS_TV status, const LayerData::DataType layer_type)