From 75d285b13c1ac2e0cf90691f797cf6e40837d16a Mon Sep 17 00:00:00 2001
From: Matthias Puchner <github@mpuchner.de>
Date: Tue, 23 Nov 2021 09:27:16 +0100
Subject: [PATCH] reduce SessionModel exposure in MaskContainerItem

Add and use dedicated methods. This will ease SessionModel migration
---
 GUI/Model/Data/IntensityDataItem.cpp   |  2 +-
 GUI/Model/Data/MaskItems.cpp           | 26 ++++++++++++++++++++++
 GUI/Model/Data/MaskItems.h             | 29 +++++++++++++++++--------
 GUI/Model/Data/MaskUnitsConverter.cpp  |  2 +-
 GUI/Model/Data/RealDataItem.cpp        |  3 +--
 GUI/Model/From/FromDomain.cpp          | 14 ++++++------
 GUI/Model/Instrument/DetectorItems.cpp | 30 +++++++++++++-------------
 GUI/View/Mask/MaskContainerView.h      |  2 +-
 GUI/View/Mask/MaskEditorCanvas.cpp     |  4 ++--
 9 files changed, 74 insertions(+), 38 deletions(-)

diff --git a/GUI/Model/Data/IntensityDataItem.cpp b/GUI/Model/Data/IntensityDataItem.cpp
index 6e4083c9122..2756a66e17d 100644
--- a/GUI/Model/Data/IntensityDataItem.cpp
+++ b/GUI/Model/Data/IntensityDataItem.cpp
@@ -474,7 +474,7 @@ ProjectionContainerItem* IntensityDataItem::getOrCreateProjectionContainerItem()
 
 bool IntensityDataItem::hasMasks() const
 {
-    return (maskContainerItem() && maskContainerItem()->hasChildren());
+    return (maskContainerItem() && !maskContainerItem()->maskItems().isEmpty());
 }
 
 bool IntensityDataItem::hasProjections() const
diff --git a/GUI/Model/Data/MaskItems.cpp b/GUI/Model/Data/MaskItems.cpp
index 2c48959ba88..decbb48187c 100644
--- a/GUI/Model/Data/MaskItems.cpp
+++ b/GUI/Model/Data/MaskItems.cpp
@@ -18,6 +18,7 @@
 #include "Device/Mask/Line.h"
 #include "Device/Mask/Polygon.h"
 #include "Device/Mask/Rectangle.h"
+#include "GUI/Model/Session/SessionModel.h"
 #include "GUI/Model/Types/DoubleDescriptor.h"
 #include "GUI/Util/Error.h"
 
@@ -32,6 +33,31 @@ MaskContainerItem::MaskContainerItem() : SessionItem(M_TYPE)
     setDefaultTag(T_MASKS);
 }
 
+QVector<MaskItem*> MaskContainerItem::maskItems() const
+{
+    QVector<MaskItem*> result;
+    for (auto* child : children())
+        if (auto* mask = dynamic_cast<MaskItem*>(child))
+            result.append(mask);
+
+    return result;
+}
+
+void MaskContainerItem::insertMask(int row, MaskItem* maskItem)
+{
+    insertItem(row, maskItem);
+}
+
+RegionOfInterestItem* MaskContainerItem::regionOfInterestItem() const
+{
+    return firstChildOfType<RegionOfInterestItem>();
+}
+
+void MaskContainerItem::clear()
+{
+    model()->removeRows(0, numberOfChildren(), index());
+}
+
 /* ------------------------------------------------------------------------- */
 
 bool MaskItem::maskValue() const
diff --git a/GUI/Model/Data/MaskItems.h b/GUI/Model/Data/MaskItems.h
index 0123973bf46..54a2adf7513 100644
--- a/GUI/Model/Data/MaskItems.h
+++ b/GUI/Model/Data/MaskItems.h
@@ -20,15 +20,6 @@
 
 class IShape2D;
 
-//! Container holding various masks as children
-
-class MaskContainerItem : public SessionItem {
-public:
-    static constexpr auto M_TYPE{"MaskContainer"};
-
-    MaskContainerItem();
-};
-
 //! A base class for all mask items
 
 class MaskItem : public SessionItem {
@@ -182,4 +173,24 @@ public:
     std::unique_ptr<IShape2D> createShape(double scale) const override;
 };
 
+//! Container holding various masks as children
+
+class MaskContainerItem : public SessionItem {
+public:
+    static constexpr auto M_TYPE{"MaskContainer"};
+
+    MaskContainerItem();
+
+    QVector<MaskItem*> maskItems() const;
+
+    //! Insert mask at given row.
+    void insertMask(int row, MaskItem* maskItem);
+
+    //! Can be nullptr.
+    RegionOfInterestItem* regionOfInterestItem() const;
+
+    //! Remove all masks
+    void clear();
+};
+
 #endif // BORNAGAIN_GUI_MODEL_DATA_MASKITEMS_H
diff --git a/GUI/Model/Data/MaskUnitsConverter.cpp b/GUI/Model/Data/MaskUnitsConverter.cpp
index 93b431fdd90..6abd1d82b6c 100644
--- a/GUI/Model/Data/MaskUnitsConverter.cpp
+++ b/GUI/Model/Data/MaskUnitsConverter.cpp
@@ -50,7 +50,7 @@ void MaskUnitsConverter::convertIntensityDataItem(IntensityDataItem* intensityDa
     m_data = intensityData->getOutputData();
 
     if (intensityData->maskContainerItem())
-        for (SessionItem* maskItem : intensityData->maskContainerItem()->getItems())
+        for (auto* maskItem : intensityData->maskContainerItem()->maskItems())
             convertMask(maskItem);
 
     if (intensityData->projectionContainerItem())
diff --git a/GUI/Model/Data/RealDataItem.cpp b/GUI/Model/Data/RealDataItem.cpp
index d4200cb18ac..cff275185af 100644
--- a/GUI/Model/Data/RealDataItem.cpp
+++ b/GUI/Model/Data/RealDataItem.cpp
@@ -406,8 +406,7 @@ void RealDataItem::rotateData()
     clearInstrumentId();
 
     if (auto* maskContainer = intensityDataItem()->maskContainerItem())
-        maskContainer->model()->removeRows(0, maskContainer->numberOfChildren(),
-                                           maskContainer->index());
+        maskContainer->clear();
 
     if (auto* projectionsContainer = intensityDataItem()->projectionContainerItem())
         projectionsContainer->model()->removeRows(0, projectionsContainer->numberOfChildren(),
diff --git a/GUI/Model/From/FromDomain.cpp b/GUI/Model/From/FromDomain.cpp
index 9a85a6bf4ac..18d532778f6 100644
--- a/GUI/Model/From/FromDomain.cpp
+++ b/GUI/Model/From/FromDomain.cpp
@@ -640,7 +640,7 @@ void GUI::Transform::FromDomain::setMaskContainer(MaskContainerItem* container_i
             ellipseItem->setYRadius(scale * ellipse->getRadiusY());
             ellipseItem->setAngle(scale * ellipse->getTheta());
             ellipseItem->setMaskValue(mask_value);
-            container_item->insertItem(0, ellipseItem);
+            container_item->insertMask(0, ellipseItem);
         }
 
         else if (const auto* rectangle = dynamic_cast<const Rectangle*>(shape)) {
@@ -650,7 +650,7 @@ void GUI::Transform::FromDomain::setMaskContainer(MaskContainerItem* container_i
             rectangleItem->setXUp(scale * rectangle->getXup());
             rectangleItem->setYUp(scale * rectangle->getYup());
             rectangleItem->setMaskValue(mask_value);
-            container_item->insertItem(0, rectangleItem);
+            container_item->insertMask(0, rectangleItem);
 
         }
 
@@ -668,28 +668,28 @@ void GUI::Transform::FromDomain::setMaskContainer(MaskContainerItem* container_i
             polygonItem->setMaskValue(mask_value);
             polygonItem->setIsClosed(true);
 
-            container_item->insertItem(0, polygonItem);
+            container_item->insertMask(0, polygonItem);
         }
 
         else if (const auto* vline = dynamic_cast<const VerticalLine*>(shape)) {
             auto* lineItem = new VerticalLineItem();
             lineItem->setPosX(scale * vline->getXpos());
             lineItem->setMaskValue(mask_value);
-            container_item->insertItem(0, lineItem);
+            container_item->insertMask(0, lineItem);
         }
 
         else if (const auto* hline = dynamic_cast<const HorizontalLine*>(shape)) {
             auto* lineItem = new HorizontalLineItem();
             lineItem->setPosY(scale * hline->getYpos());
             lineItem->setMaskValue(mask_value);
-            container_item->insertItem(0, lineItem);
+            container_item->insertMask(0, lineItem);
         }
 
         else if (const auto* plane = dynamic_cast<const InfinitePlane*>(shape)) {
             Q_UNUSED(plane);
             auto* planeItem = new MaskAllItem();
             planeItem->setMaskValue(mask_value);
-            container_item->insertItem(-1, planeItem);
+            container_item->insertMask(-1, planeItem);
         }
 
         else {
@@ -707,7 +707,7 @@ void GUI::Transform::FromDomain::setMaskContainer(MaskContainerItem* container_i
         roiItem->setYLow(scale * yBounds.first);
         roiItem->setXUp(scale * xBounds.second);
         roiItem->setYUp(scale * yBounds.second);
-        container_item->insertItem(-1, roiItem);
+        container_item->insertMask(-1, roiItem);
     }
 }
 
diff --git a/GUI/Model/Instrument/DetectorItems.cpp b/GUI/Model/Instrument/DetectorItems.cpp
index aa01acb7811..240af9477bd 100644
--- a/GUI/Model/Instrument/DetectorItems.cpp
+++ b/GUI/Model/Instrument/DetectorItems.cpp
@@ -131,21 +131,21 @@ void DetectorItem::addMasksToDomain(IDetector2D* detector) const
 
     const double scale = axesToDomainUnitsFactor();
 
-    for (int i_row = maskContainer->children().size(); i_row > 0; --i_row) {
-        if (auto* maskItem = dynamic_cast<MaskItem*>(maskContainer->children().at(i_row - 1))) {
-
-            if (maskItem->modelType() == RegionOfInterestItem::M_TYPE) {
-                auto* rectItem = dynamic_cast<RectangleItem*>(maskItem);
-                double xlow = scale * rectItem->xLow();
-                double ylow = scale * rectItem->yLow();
-                double xup = scale * rectItem->xUp();
-                double yup = scale * rectItem->yUp();
-                detector->setRegionOfInterest(xlow, ylow, xup, yup);
-            } else {
-                std::unique_ptr<IShape2D> shape(maskItem->createShape(scale));
-                bool mask_value = maskItem->maskValue();
-                detector->addMask(*shape, mask_value);
-            }
+    const auto maskItems = maskContainer->maskItems();
+    for (auto m = maskItems.rbegin(); m != maskItems.rend(); m++) {
+        MaskItem* maskItem = *m;
+
+        if (maskItem->modelType() == RegionOfInterestItem::M_TYPE) {
+            auto* rectItem = dynamic_cast<RectangleItem*>(maskItem);
+            double xlow = scale * rectItem->xLow();
+            double ylow = scale * rectItem->yLow();
+            double xup = scale * rectItem->xUp();
+            double yup = scale * rectItem->yUp();
+            detector->setRegionOfInterest(xlow, ylow, xup, yup);
+        } else {
+            std::unique_ptr<IShape2D> shape(maskItem->createShape(scale));
+            bool mask_value = maskItem->maskValue();
+            detector->addMask(*shape, mask_value);
         }
     }
 }
diff --git a/GUI/View/Mask/MaskContainerView.h b/GUI/View/Mask/MaskContainerView.h
index 0ceb91fa047..5db60847e2e 100644
--- a/GUI/View/Mask/MaskContainerView.h
+++ b/GUI/View/Mask/MaskContainerView.h
@@ -22,7 +22,7 @@
 class MaskContainerItem;
 class ProjectionContainerItem;
 
-//! The MaskContainerView is nothing move than just transparent rectangle to cover axes area
+//! The MaskContainerView is nothing more than just transparent rectangle to cover axes area
 //! of ColorMapPlot inside MaskGraphicsScene. The goal of this rectangle is to hide all MaskViews
 //! if they are going outside of ColorMapPlot.
 //!
diff --git a/GUI/View/Mask/MaskEditorCanvas.cpp b/GUI/View/Mask/MaskEditorCanvas.cpp
index c7e28113a3c..70daade9990 100644
--- a/GUI/View/Mask/MaskEditorCanvas.cpp
+++ b/GUI/View/Mask/MaskEditorCanvas.cpp
@@ -84,7 +84,7 @@ void MaskEditorCanvas::onPresentationTypeRequest(MaskEditorFlags::PresentationTy
 
     if (auto* container = m_intensityDataItem->maskContainerItem()) {
         bool isVisible = presentationType.testFlag(MaskEditorFlags::MASK_EDITOR);
-        for (MaskItem* mask : container->items<MaskItem>())
+        for (MaskItem* mask : container->maskItems())
             mask->setIsVisibleValue(isVisible);
     }
 }
@@ -129,7 +129,7 @@ bool MaskEditorCanvas::isAxisRangeMatchData() const
 void MaskEditorCanvas::setZoomToROI()
 {
     if (MaskContainerItem* maskContainer = m_intensityDataItem->maskContainerItem()) {
-        if (auto* roiItem = maskContainer->firstChildOfType<RegionOfInterestItem>()) {
+        if (auto* roiItem = maskContainer->regionOfInterestItem()) {
             m_intensityDataItem->setLowerX(roiItem->xLow());
             m_intensityDataItem->setUpperX(roiItem->xUp());
             m_intensityDataItem->setLowerY(roiItem->yLow());
-- 
GitLab