Fixed a bug in isisminer in which bad (e.g. self-intersecting) polygo…

…n geometries were not treated properly. Added pertinent unit tests to GisGeometry and Strategy classes. Fixed incorrect links and minor typos in isisminer documentation. Addresses DOI-USGS#5612.
UA-LPL · Sep 23, 2024 · 38b1cc2 · 38b1cc2
1 parent 3ee9a1a
commit 38b1cc2
Show file tree

Hide file tree

Showing 11 changed files with 210 additions and 33 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -37,6 +37,7 @@ release.
 
 ### Fixed
 
+- Fixed a bug in isisminer in which bad (e.g. self-intersecting) polygon geometries were not treated properly. Added pertinent unit tests to GisGeometry and Strategy classes. Issue: [5612](https://github.com/DOI-USGS/ISIS3/issues/5612)
 - Fixed a bug in kaguyasp2isis that doesn't work for data with a detached label.
 
 ## [8.3.0] - 2024-08-16

diff --git a/isis/src/base/apps/isisminer/isisminer.xml b/isis/src/base/apps/isisminer/isisminer.xml
@@ -49,8 +49,8 @@
       in well-known-text (WKT) or well-known-binary (WKB) as conversions are 
       restricted to text representations of geometries (format requirements are 
       the same as functions like 
-      <a href="http://www.postgis.org/docs/ST_GeomFromText.html">ST_GeomFromWKT</a> and 
-      <a href="http://www.postgis.org/docs/ST_GeomFromWKB.html">ST_GeomFromWKB</a>, 
+      <a href="http://postgis.net/docs/ST_GeomFromText.html">ST_GeomFromWKT</a> and 
+      <a href="http://postgis.net/docs/ST_GeomFromWKB.html">ST_GeomFromWKB</a>, 
       respectively). Resources may also contain Assets. Assets are typically 
       another list of Resources created from Strategies that are attached to a 
       (parent) Resource. If Assets are represented as Resource lists, they can 
@@ -366,7 +366,7 @@
           and is either not repaired (RepairInvalidGeometry = False) or could not be
           sucessfully repaired (RepairInvalidGeometry = True). The options are:
           continue, disable or error. For InvalidGeometryAction = continue, the state
-          of the invalid geoemtry is retained in the Resource (some GIS operations
+          of the invalid geometry is retained in the Resource (some GIS operations
           still function) and it is not disabled and no error occurs - the issue is
           ignored. If InvalidGeometryAction = disable, then the geometry is retained
           in the Resource but the status is set to "discard". It can be re-enabled by
@@ -4707,14 +4707,14 @@ End
        feature.
     </change>
     <change name="Kris Becker" date="2015-06-12">
-      Improved GisOverlap and StereoPair strategies. StereoAngle can now be
-      computed from the GIS centroid of the overlapping region, providing a
-      much higher degree of accuracy. Improved global variable pool management
-      whilst traversing through Strategy/Resource depths in the mining process.
-      Ensure Asset is cleared when Mode = Create is used in AssetSideBar strategy.
+       Improved GisOverlap and StereoPair strategies. StereoAngle can now be
+       computed from the GIS centroid of the overlapping region, providing a
+       much higher degree of accuracy. Improved global variable pool management
+       whilst traversing through Strategy/Resource depths in the mining process.
+       Ensure Asset is cleared when Mode = Create is used in AssetSideBar strategy.
     </change>
     <change name="Kris Becker" date="2015-06-17">
-        Fixed bug in creation of GEOS SRTree when only one geometry occurs 
+       Fixed bug in creation of GEOS SRTree when only one geometry occurs 
        (requires two or more).  Changed name of IsisMiner objects within the 
        GisOverlap strategy (since we now have two of them) to CandidateMiner and 
        OverlapMiner. Added feature to simplify geometry whilst preserving 
@@ -4726,21 +4726,21 @@ End
        Add ability to process the set of GisOverlap results for each Resource
        as they are matched. This is handled in the OverlapMiner Strategy object.
     </change>
-   <change name="Kris Becker" date="2015-07-07">
+    <change name="Kris Becker" date="2015-07-07">
        Added more content to documentation and added an example demonstrating a 
        real application of the isisminer application.
-   </change>
-   <change name="Kris Becker" date="2015-09-27">
+    </change>
+    <change name="Kris Becker" date="2015-09-27">
        Refactored the Resource class to contain all but the active status so 
        that copies can be maintained in separate instances. Reworked the 
        AssetSidebar strategy to take advantage of this work.
-   </change>
-   <change name="Kris Becker" date="2015-10-11">
+    </change>
+    <change name="Kris Becker" date="2015-10-11">
        Fixed bug in argument scanning when the number of arguments reached 10 or 
        more as %1 was replacing %10, etc... Scan and replace i reverse order 
        corrected this problem. Enhanced the Calculator strategy to provide 
        argument replacement in the Initializers group.
-   </change> 
+    </change> 
     <change name="Kris Becker" date="2015-11-01">
         Modified CsvReader strategy to have the provided header length determine 
         the number of keywords created/Resource. This allows the jigsaw residual 
@@ -4750,12 +4750,14 @@ End
         Converted isisminer to callable function. Also converted all Makefile
         tests to gtest format. Moved some data from isis data area to
         "isis/tests/data/isisminer".
-    <change name="Ken Edmundson" date="2024-09-12">
+    </change>
+    <change name="Ken Edmundson" date="2024-09-23">
         Originally implemented in UofA OSIRIS-REx code by Kris Becker, 2018-07-31.
-        Corrected problems with invalid geometries aborting isisminer. Added two
-        new parameters, RepairInvalidGeometry and InvalidGeometryAction, to allow user
-        more control over how invalid geometries are managed. Update some of the
-        documentation.
+        Corrected problems with invalid geometries causeing isisminer to abort.
+        Added two new parameters, RepairInvalidGeometry and InvalidGeometryAction,
+        to allow user more control over how invalid geometries are managed. Updated
+        documentation. Ken Edmundson added unit tests to GisGeometry and Strategy
+        classes to address these changes.
     </change>
   </history>
 

diff --git a/isis/src/base/objs/GisGeometry/GisGeometry.cpp b/isis/src/base/objs/GisGeometry/GisGeometry.cpp
@@ -63,7 +63,7 @@ namespace Isis {
    * This constructor will read the contents of the Polygon blob of an ISIS cube
    * file and create a geometry from its contents.
    *
-   * @param cube  Cube object to create the geomtery from
+   * @param cube  Cube object to create the geometry from
    */
   GisGeometry::GisGeometry(Cube &cube) : m_type(IsisCube), m_geom(0), m_preparedGeom(0) {
 
@@ -825,7 +825,7 @@ namespace Isis {
   /**
    * Reads Polygon from ISIS Cube and returns geometry from contents
    *
-   * @param cube ISIS Cube contaning a Polygon geometry object
+   * @param cube ISIS Cube containing a Polygon geometry object
    * @return GEOSGeometry Pointer to GEOS-C type geometry from Polygon BLOB
    */
   GEOSGeometry *GisGeometry::fromCube(Cube &cube) const {

diff --git a/isis/src/base/objs/GisGeometry/GisGeometry.h b/isis/src/base/objs/GisGeometry/GisGeometry.h
@@ -23,8 +23,8 @@ namespace Isis {
    * @brief Encapsulation class provides support for GEOS-C API 
    *  
    * The Geometry Engine, Open Source (GEOS) software package, developed in C++ 
-   * from a port of the Java Toplogy Suite (JTS) provides a simplied, generic C 
-   * API using an opaque C pointer. This layer is to provide stable API from 
+   * from a port of the Java Topology Suite (JTS) provides a simplified, generic C 
+   * API using an opaque C pointer. This layer is to provide a stable API from 
    * which to develop and maintain applications that are relatively immune from 
    * changes to the underlying C++ implementation. 
    *  
@@ -44,7 +44,7 @@ namespace Isis {
    *   @history 2016-03-02 Ian Humphrey - Updated for coding standards compliance. Added to 
    *                           jwbacker's original unit test to prepare for adding this class to
    *                           ISIS. Fixes #2398.
-   *   @history 2016-03-04 Kris Becker - Completed the documentation and implmented the equals()
+   *   @history 2016-03-04 Kris Becker - Completed the documentation and implemented the equals()
    *                           method.
    *   @history 2018-07-29 Kris Becker - Added buffer() method; isValid() was
    *                           throwing an exception if the geometry was invalid

diff --git a/isis/src/base/objs/GisGeometry/GisGeometry.truth b/isis/src/base/objs/GisGeometry/GisGeometry.truth
@@ -83,6 +83,26 @@ GisGeometry::Types:
     length?           0
     points?           0
 
+"Construct Self-Intersecting Geometry from WKT GIS source"
+    isDefined?        true
+    isValid?          false
+    isValidReason?    "Self-intersection[286.747135842881 51.2716857610475]"
+    isEmpty?          true
+    type?             "WKT"
+    area?             0
+    length?           0
+    points?           0
+
+"Repaired Self-Intersecting Geometry from WKT GIS source"
+    isDefined?        true
+    isValid?          true
+    isValidReason?    "Valid Geometry"
+    isEmpty?          false
+    type?             "GeosGis"
+    area?             26.2303
+    length?           21.0533
+    points?           5
+
 "Construct Copy Geometry from GisGeometry from Cube"
     isDefined?        true
     isValid?          true
@@ -171,6 +191,15 @@ GisGeometry::Types:
     equals?           "No"
     intersect ratio?  "0.0"
 
+"Source: Repaired Self-Intersecting WKT Geometry, Target: GeomGisIsisCube Geometry"
+    distance?         "0.0"
+    intersects?       "Yes"
+    contains?         "No"
+    disjoint?         "No"
+    overlaps?         "Yes"
+    equals?           "No"
+    intersect ratio?  "0.014072849290423"
+
 "Source: GisIsisCube Geometry, Target: WKT Geometry"
     distance?         "0.0"
     intersects?       "Yes"
@@ -282,6 +311,16 @@ Simplified Geometry from Invalid Geometry is NULL.
     length?           8.23376
     points?           187
 
+"Intersection Geometry of GeomCube and Repaired Self-Intersecting WKT Geometries"
+    isDefined?        true
+    isValid?          true
+    isValidReason?    "Valid Geometry"
+    isEmpty?          false
+    type?             "GeosGis"
+    area?             26.2303
+    length?           21.0533
+    points?           5
+
 "Union Geometry of Invalid Geometry with WKT Geometry as target"
     isDefined?        false
     isValid?          false

diff --git a/isis/src/base/objs/GisGeometry/unitTest.cpp b/isis/src/base/objs/GisGeometry/unitTest.cpp
@@ -39,6 +39,11 @@ void printTypes();
  *                           References #2398.
  *   @history 2016-03-04 Ian Humphrey - Updated test and truthdata for equals() method.
  *                           References #2398.
+ *   @history 2024-09-23 Ken Edmundson - Updated test and truthdata for 1) detecting
+ *                           self-intersecting geometries; 2) repairing such geometries
+ *                           with a buffer of size 0; and 3) overlap and intersection
+ *                           of repaired geometry with another.
+ *                           References #5612.
  *
  *
  * NOTE - distance(), intersects(), contains(), disjoin(), overlaps() methods
@@ -93,7 +98,6 @@ int main() {
     printBasicInfo(geomGisWKB,
                    "Construct Geometry from WKB GIS source");
 
-
     GisGeometry geomGisIsisCube("$ISISTESTDATA/isis/src/messenger/unitTestData/EW0213634118G.lev1.cub",
                                 GisGeometry::IsisCube);
     printBasicInfo(geomGisIsisCube,
@@ -109,6 +113,19 @@ int main() {
 //???    geomDefault.setGeometry(geos); // SEGFAULT
 //???    printBasicInfo(geomDefault, "Set Default Geometry from GEOSGeometry");
 
+    // polygon with self-intersecting geometry that lies
+    // within the boundaries of EW0211286081G.lev1.cub
+    QString wktSelfIntersect
+        = QString::fromStdString("POLYGON ((286.0 51.0, 291.5 53.0, 295.0 49.8, 289.5 47.0, 286.6 51.5, 286.0 51.0))");
+    GisGeometry geomGisWKTSelfIntersect(wktSelfIntersect, GisGeometry::WKT);
+    printBasicInfo(geomGisWKTSelfIntersect,
+                   "Construct Self-Intersecting Geometry from WKT GIS source");
+
+    // repair the self-intersecting geometry with buffer(0)
+    GisGeometry *repairedSelfIntersect = geomGisWKTSelfIntersect.buffer(0);
+    printBasicInfo(*repairedSelfIntersect,
+                   "Repaired Self-Intersecting Geometry from WKT GIS source");
+
     GisGeometry geomCopy(geomCube);
     printBasicInfo(geomCopy, "Construct Copy Geometry from GisGeometry from Cube");
 
@@ -136,7 +153,10 @@ int main() {
     printTargetInfo(geomGisWKT, geomDefault,
                     "Source: WKT Geometry, Target: Invalid Geometry");
 
-    // overlaping geometries
+    // overlapping geometries
+    printTargetInfo(*repairedSelfIntersect, geomGisIsisCube,
+                    "Source: Repaired Self-Intersecting WKT Geometry, Target: GeomGisIsisCube Geometry");
+
     printTargetInfo(geomGisIsisCube, geomGisWKT,
                     "Source: GisIsisCube Geometry, Target: WKT Geometry");
 
@@ -186,11 +206,13 @@ int main() {
     printBasicInfo(*intersectionInvalidTargetGeometry,
                    "Intersection Geometry of WKT Geometry with Invalid Geometry as target");
 
-
     GisGeometry *intersectionGeom = geomGisIsisCube.intersection(geomGisWKT);
     printBasicInfo(*intersectionGeom,
                    "Intersection Geometry of GisIsisCube Geometry with WKT Geometry");
 
+    GisGeometry *intersectCubeAndRepairedGeom = geomCube.intersection(*repairedSelfIntersect);
+    printBasicInfo(*intersectCubeAndRepairedGeom,
+                   "Intersection Geometry of GeomCube and Repaired Self-Intersecting WKT Geometries");
 
     // These two tests below should output empty geometries (union w/ invalid -> invalid)
     GisGeometry *unionInvalidSourceGeom = geomDefault.g_union(geomGisWKT);

diff --git a/isis/src/base/objs/Strategy/Strategy.cpp b/isis/src/base/objs/Strategy/Strategy.cpp
@@ -728,6 +728,7 @@ namespace Isis {
 
         // Get decision keys
         bool repairGeom = toBool(keys.get("RepairInvalidGeometry", "true"));
+
         QString geomAction = keys.get("InvalidGeometryAction", "disable").toLower();
         if ( !QString("disable error continue").contains(geomAction) ) {
             if ( isDebug() ) {
@@ -786,7 +787,7 @@ namespace Isis {
             geosgeom.reset( geosgeom->buffer(0) );
             if ( isDebug() ) {
               if (geosgeom.isNull() || !geosgeom->isValid() ) {
-                 cout << "  Geomtry could not be repaired!\n";
+                 cout << "  Geometry could not be repaired!\n";
               }
               else {
                 cout << "  Geometry was successfully repaired!\n";
@@ -808,7 +809,7 @@ namespace Isis {
             }
 
             // Throw an error
-            QString mess = resource->name() + " failed to construct geomtry - Error: " +
+            QString mess = resource->name() + " failed to construct geometry - Error: " +
                             geomError;
             throw IException(IException::Programmer, mess, _FILEINFO_);
           }

diff --git a/isis/src/base/objs/Strategy/Strategy.h b/isis/src/base/objs/Strategy/Strategy.h
@@ -56,7 +56,7 @@ namespace Isis {
    * @internal 
    *   @history 2012-07-15 Kris Becker - Original version.
    *   @history 2015-03-18 Jeannie Backer - Brought class files closer to ISIS coding standards.
-   *   @history 2015-04-07 Kristin Berry - Modified applytoIntserectedGeometry to deal
+   *   @history 2015-04-07 Kristin Berry - Modified applytoIntersectedGeometry to deal
    *   @history 2015-03-26 Jeannie Backer - Updated documentation.
    *   @history 2015-03-28 Kris Becker - Added the composite(Resource, Resource)
    *                          method

diff --git a/isis/src/base/objs/Strategy/Strategy.truth b/isis/src/base/objs/Strategy/Strategy.truth
@@ -478,11 +478,47 @@ Testing composite(...):
 
 
 importGeometry = 
-true 
+true
 
 
 
 
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% 
+
+%             repairInvalidGeometry            %
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% 
+
+
+
+
+
+importPolygonGeometry (Repair self-intersection on) = true 
+
+importPolygonGeometry (Repair self-intersection off) = false
+
+
+
+
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% 
+
+%             invalidGeometryAction            %
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% 
+
+
+
+
+
+  invalidGeometryAction = disable 
+invalidGeometryDisabled = true 
+
+  invalidGeometryAction = error 
+"**PROGRAMMER ERROR** Polygon 1 failed to construct geometry - Error: Self-intersection[16.6666666666667 0]." 
+
+  invalidGeometryAction = continue 
+importPolygonGeometry (Repair off; Action continue) = false
+invalidGeometryDisabled = false 
+
+
 
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%