Refactor Region into RegionConverter and add tests

CMakeLists.txt

@@ -192,7 +192,7 @@ set(LINK_LIBS
 
 set(SOURCE_FILES
         ${SOURCE_FILES}
-	third-party/pugixml/src/pugixml.cpp
+        third-party/pugixml/src/pugixml.cpp
         src/Cache/LoaderCache.cc
         src/Cache/TileCache.cc
         src/Cache/TilePool.cc
@@ -225,14 +225,15 @@ set(SOURCE_FILES
         src/ImageStats/Histogram.cc
         src/ImageStats/StatsCalculator.cc
         src/Logger/Logger.cc
-		src/Logger/CartaLogSink.cc
+        src/Logger/CartaLogSink.cc
         src/Main/Main.cc
         src/Main/ProgramSettings.cc
         src/Main/WebBrowser.cc
         src/Region/CrtfImportExport.cc
         src/Region/Ds9ImportExport.cc
         src/Region/LineBoxRegions.cc
         src/Region/Region.cc
+		src/Region/RegionConverter.cc
         src/Region/RegionHandler.cc
         src/Region/RegionImportExport.cc
         src/Session/CursorSettings.cc

src/Region/Region.h

@@ -4,7 +4,7 @@
    SPDX-License-Identifier: GPL-3.0-or-later
 */
 
-//# Region.h: class for managing 2D region parameters
+//# Region.h: class for managing 2D region parameters in reference image
 
 #ifndef CARTA_SRC_REGION_REGION_H_
 #define CARTA_SRC_REGION_REGION_H_
@@ -15,112 +15,53 @@
 #include <vector>
 
 #include <casacore/coordinates/Coordinates/CoordinateSystem.h>
-#include <casacore/images/Regions/WCRegion.h>
-#include <casacore/lattices/LRegions/LCPolygon.h>
 #include <casacore/lattices/LRegions/LCRegion.h>
 #include <casacore/lattices/Lattices/ArrayLattice.h>
 #include <casacore/tables/Tables/TableRecord.h>
 
-#include "Util/Message.h"
+#include "RegionConverter.h"
+#include "RegionState.h"
 #include "Util/Stokes.h"
 
-#define DEFAULT_VERTEX_COUNT 1000
-
 namespace carta {
 
-inline std::string RegionName(CARTA::RegionType type) {
-    std::unordered_map<CARTA::RegionType, std::string> region_names = {{CARTA::POINT, "point"}, {CARTA::LINE, "line"},
-        {CARTA::POLYLINE, "polyline"}, {CARTA::RECTANGLE, "rectangle"}, {CARTA::ELLIPSE, "ellipse"}, {CARTA::ANNULUS, "annulus"},
-        {CARTA::POLYGON, "polygon"}, {CARTA::ANNPOINT, "ann point"}, {CARTA::ANNLINE, "ann line"}, {CARTA::ANNPOLYLINE, "ann polyline"},
-        {CARTA::ANNRECTANGLE, "ann rectangle"}, {CARTA::ANNELLIPSE, "ann ellipse"}, {CARTA::ANNPOLYGON, "ann polygon"},
-        {CARTA::ANNVECTOR, "vector"}, {CARTA::ANNRULER, "ruler"}, {CARTA::ANNTEXT, "text"}, {CARTA::ANNCOMPASS, "compass"}};
-    return region_names[type];
-}
-
-struct RegionState {
-    // struct used for region parameters
-    int reference_file_id;
-    CARTA::RegionType type;
-    std::vector<CARTA::Point> control_points;
-    float rotation;
-
-    RegionState() : reference_file_id(-1), type(CARTA::POINT), rotation(0) {}
-    RegionState(int ref_file_id_, CARTA::RegionType type_, const std::vector<CARTA::Point>& control_points_, float rotation_)
-        : reference_file_id(ref_file_id_), type(type_), control_points(control_points_), rotation(rotation_) {}
-
-    bool operator==(const RegionState& rhs) {
-        return (reference_file_id == rhs.reference_file_id) && (type == rhs.type) && !RegionChanged(rhs);
-    }
-
-    bool operator!=(const RegionState& rhs) {
-        return (reference_file_id != rhs.reference_file_id) || (type != rhs.type) || RegionChanged(rhs);
-    }
-
-    bool RegionDefined() {
-        return !control_points.empty();
-    }
-
-    bool RegionChanged(const RegionState& rhs) {
-        // Ignores annotation params (for interrupting region calculations)
-        return (rotation != rhs.rotation) || PointsChanged(rhs);
-    }
-    bool PointsChanged(const RegionState& rhs) {
-        // Points must be same size, order, and value to be unchanged
-        if (control_points.size() != rhs.control_points.size()) {
-            return true;
-        }
-        for (int i = 0; i < control_points.size(); ++i) {
-            float x(control_points[i].x()), y(control_points[i].y());
-            float rhs_x(rhs.control_points[i].x()), rhs_y(rhs.control_points[i].y());
-            if ((x != rhs_x) || (y != rhs_y)) {
-                return true;
-            }
-        }
-        return false;
-    }
-};
-
 class Region {
 public:
     Region(const RegionState& state, std::shared_ptr<casacore::CoordinateSystem> csys);
 
-    inline bool IsValid() { // control points validated
+    inline bool IsValid() {
         return _valid;
     };
 
-    // set new region parameters
+    inline bool RegionChanged() {
+        return _region_changed;
+    }
+
+    // Set new region parameters
     bool UpdateRegion(const RegionState& state);
 
-    // state accessors
+    // RegionState accessors, including region type information
     inline RegionState GetRegionState() {
         std::lock_guard<std::mutex> guard(_region_state_mutex);
         RegionState region_state = _region_state;
         return region_state;
     }
 
-    inline bool RegionChanged() { // reference image, type, points, or rotation changed
-        return _region_changed;
-    }
-
     inline bool IsPoint() {
-        return GetRegionState().type == CARTA::POINT;
+        return GetRegionState().IsPoint();
     }
 
     inline bool IsLineType() {
         // Not enclosed region defined by 2 or more points
-        std::vector<CARTA::RegionType> line_types{
-            CARTA::LINE, CARTA::POLYLINE, CARTA::ANNLINE, CARTA::ANNPOLYLINE, CARTA::ANNVECTOR, CARTA::ANNRULER};
-        auto type = GetRegionState().type;
-        return std::find(line_types.begin(), line_types.end(), type) != line_types.end();
+        return GetRegionState().IsLineType();
     }
 
-    inline bool IsRotbox() {
-        RegionState rs = GetRegionState();
-        return ((rs.type == CARTA::RECTANGLE || rs.type == CARTA::ANNRECTANGLE) && (rs.rotation != 0.0));
+    inline bool IsInReferenceImage(int file_id) {
+        return file_id == GetRegionState().reference_file_id;
     }
 
     inline bool IsAnnotation() {
-        return GetRegionState().type > CARTA::POLYGON;
+        return GetRegionState().IsAnnotation();
     }
 
     inline std::shared_ptr<casacore::CoordinateSystem> CoordinateSystem() {
@@ -130,106 +71,53 @@ class Region {
     // Communication
     bool IsConnected();
     void WaitForTaskCancellation();
+    std::shared_mutex& GetActiveTaskMutex();
 
-    // Converted region as approximate LCPolygon and its mask
-    std::shared_ptr<casacore::LCRegion> GetImageRegion(int file_id, std::shared_ptr<casacore::CoordinateSystem> image_csys,
-        const casacore::IPosition& image_shape, const StokesSource& stokes_source = StokesSource(), bool report_error = true);
+    // LCRegion and mask for region applied to image.  Must be a closed region (not line) and not annotation.
+    std::shared_ptr<casacore::LCRegion> GetImageRegion(int file_id, std::shared_ptr<casacore::CoordinateSystem> csys,
+        const casacore::IPosition& shape, const StokesSource& stokes_source = StokesSource(), bool report_error = true);
     casacore::ArrayLattice<casacore::Bool> GetImageRegionMask(int file_id);
 
-    // Converted region in Record for export
+    // Record for region applied to image, for export.  Not for converting to LCRegion for analytics.
     casacore::TableRecord GetImageRegionRecord(
-        int file_id, std::shared_ptr<casacore::CoordinateSystem> output_csys, const casacore::IPosition& output_shape);
-
-    std::shared_mutex& GetActiveTaskMutex();
+        int file_id, std::shared_ptr<casacore::CoordinateSystem> csys, const casacore::IPosition& shape);
 
 private:
-    // check points: number required for region type, and values are finite
+    // Check points: number required for region type, and values are finite
     bool CheckPoints(const std::vector<CARTA::Point>& points, CARTA::RegionType type);
     bool PointsFinite(const std::vector<CARTA::Point>& points);
 
-    // Reset cache when region changes
+    // Cached LCRegion
     void ResetRegionCache();
+    std::shared_ptr<casacore::LCRegion> GetCachedLCRegion(int file_id, const StokesSource& stokes_source);
 
-    // Check if reference region is set successfully
-    bool ReferenceRegionValid();
-
-    // Apply region to reference image, set WCRegion and wcs control points.
-    void SetReferenceRegion();
-    bool RectanglePointsToWorld(std::vector<CARTA::Point>& pixel_points, std::vector<casacore::Quantity>& wcs_points);
-    void RectanglePointsToCorners(std::vector<CARTA::Point>& pixel_points, float rotation, casacore::Vector<casacore::Double>& x,
-        casacore::Vector<casacore::Double>& y);
-    bool EllipsePointsToWorld(std::vector<CARTA::Point>& pixel_points, std::vector<casacore::Quantity>& wcs_points, float& rotation);
-
-    // Reference region as approximate polygon converted to image coordinates; used for data streams
-    bool UseApproximatePolygon(std::shared_ptr<casacore::CoordinateSystem> output_csys);
-    std::vector<CARTA::Point> GetRectangleMidpoints();
-    std::shared_ptr<casacore::LCRegion> GetCachedPolygonRegion(int file_id);
-    std::shared_ptr<casacore::LCRegion> GetAppliedPolygonRegion(
-        int file_id, std::shared_ptr<casacore::CoordinateSystem> output_csys, const casacore::IPosition& output_shape);
-    std::vector<std::vector<CARTA::Point>> GetReferencePolygonPoints(int num_vertices);
-    std::vector<std::vector<CARTA::Point>> GetApproximatePolygonPoints(int num_vertices);
-    std::vector<CARTA::Point> GetApproximateEllipsePoints(int num_vertices);
-    double GetTotalSegmentLength(std::vector<CARTA::Point>& points);
-    bool ConvertPointsToImagePixels(const std::vector<CARTA::Point>& points, std::shared_ptr<casacore::CoordinateSystem> output_csys,
-        casacore::Vector<casacore::Double>& x, casacore::Vector<casacore::Double>& y);
-    void RemoveHorizontalPolygonPoints(casacore::Vector<casacore::Double>& x, casacore::Vector<casacore::Double>& y);
-    bool ValuesNear(float val1, float val2);
-
-    // Region applied to any image; used for export
-    std::shared_ptr<casacore::LCRegion> GetCachedLCRegion(int file_id);
-    std::shared_ptr<casacore::LCRegion> GetConvertedLCRegion(int file_id, std::shared_ptr<casacore::CoordinateSystem> output_csys,
-        const casacore::IPosition& output_shape, const StokesSource& stokes_source = StokesSource(), bool report_error = true);
-
-    // Control points converted to pixel coords in output image, returned in LCRegion Record format for export
-    casacore::TableRecord GetRegionPointsRecord(
-        int file_id, std::shared_ptr<casacore::CoordinateSystem> output_csys, const casacore::IPosition& output_shape);
+    // Record in pixel coordinates from control points, for reference image
     casacore::TableRecord GetControlPointsRecord(const casacore::IPosition& shape);
-    void CompleteLCRegionRecord(casacore::TableRecord& record, const casacore::IPosition& shape);
-    casacore::TableRecord GetPointRecord(std::shared_ptr<casacore::CoordinateSystem> output_csys, const casacore::IPosition& output_shape);
-    casacore::TableRecord GetLineRecord(
-        int file_id, std::shared_ptr<casacore::CoordinateSystem> image_csys, const casacore::IPosition& image_shape);
-    casacore::TableRecord GetPolygonRecord(std::shared_ptr<casacore::CoordinateSystem> output_csys);
-    casacore::TableRecord GetRotboxRecord(std::shared_ptr<casacore::CoordinateSystem> output_csys);
-    casacore::TableRecord GetEllipseRecord(std::shared_ptr<casacore::CoordinateSystem> output_csys);
+    casacore::TableRecord GetRotboxRecordForLCRegion(const casacore::IPosition& shape);
     void CompleteRegionRecord(casacore::TableRecord& record, const casacore::IPosition& image_shape);
 
-    // Utilities to convert control points
-    // Input: CARTA::Point. Returns: point (x, y) in reference world coords
-    bool ConvertCartaPointToWorld(const CARTA::Point& point, std::vector<casacore::Quantity>& world_point);
-    // Input: point (x,y) in reference world coords. Returns: point (x,y) in output pixel coords
-    bool ConvertWorldToPixel(std::vector<casacore::Quantity>& world_point, std::shared_ptr<casacore::CoordinateSystem> output_csys,
-        casacore::Vector<casacore::Double>& pixel_point);
-
-    // region parameters struct
-    RegionState _region_state;
-
-    // coord sys and shape of reference image
+    // Coordinate system of reference image
     std::shared_ptr<casacore::CoordinateSystem> _coord_sys;
 
-    // Reference region cache
-    std::mutex _region_mutex; // creation of casacore regions is not threadsafe
-    std::mutex _region_approx_mutex;
+    // Region parameters
+    RegionState _region_state;
     std::mutex _region_state_mutex;
 
-    // Use a shared lock for long time calculations, use an exclusive lock for the object destruction
-    mutable std::shared_mutex _active_task_mutex;
-
-    // Region cached as original type
-    std::shared_ptr<casacore::WCRegion> _reference_region; // 2D region applied to reference image
-    std::vector<casacore::Quantity> _wcs_control_points;   // for manual region conversion
+    // Region flags
+    bool _valid;          // RegionState set properly
+    bool _region_changed; // control points or rotation changed
 
-    // Converted regions
-    // Reference region converted to image; key is file_id
-    std::unordered_map<int, std::shared_ptr<casacore::LCRegion>> _applied_regions;
-    // Polygon approximation region converted to image; key is file_id
-    std::unordered_map<int, std::shared_ptr<casacore::LCRegion>> _polygon_regions;
+    // Region applied to reference image
+    std::shared_ptr<casacore::LCRegion> _lcregion;
+    std::mutex _lcregion_mutex;
+    bool _lcregion_set; // may be nullptr if outside image
 
-    // region flags
-    bool _valid;                // RegionState set properly
-    bool _region_changed;       // control points or rotation changed
-    bool _reference_region_set; // indicates attempt was made; may be null wcregion outside image
+    // Converter to handle region applied to matched image
+    std::unique_ptr<RegionConverter> _region_converter;
 
-    // Communication
+    // Communication:
+    // Use a shared lock for long time calculations, use an exclusive lock for the object destruction
+    mutable std::shared_mutex _active_task_mutex;
     volatile bool _connected = true;
 };