From 2492d7d1d12416dbf767c667a596fba1b1247118 Mon Sep 17 00:00:00 2001
From: Li-Ta Lo <ollie@lanl.gov>
Date: Thu, 20 Aug 2020 15:08:55 -0600
Subject: [PATCH] add test case from Valentine, more comments in ImageGraft

---
 .../connectivities/ImageConnectivity.h        | 12 ++---
 .../testing/UnitTestImageConnectivity.cxx     | 45 +++++++++++++++++++
 2 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/vtkm/worklet/connectivities/ImageConnectivity.h b/vtkm/worklet/connectivities/ImageConnectivity.h
index f6f4f30bb..9d2f94bcc 100644
--- a/vtkm/worklet/connectivities/ImageConnectivity.h
+++ b/vtkm/worklet/connectivities/ImageConnectivity.h
@@ -61,7 +61,7 @@ public:
     auto minComp = myComp;
     auto myColor = neighborColor.Get(0, 0, 0);
 
-    // FIXME: we are doing this "local conneivity finding" at each call of this
+    // FIXME: we are doing this "local connectivity finding" at each call of this
     // worklet. This creates a large demand on the memory bandwidth.
     // Is this necessary? It looks like we only need a local, partial spanning
     // tree at the beginning. Is it true?
@@ -83,17 +83,18 @@ public:
     /// The minComp in one invocation might hold old value that was modified by the
     /// following compOut.Set() in some other invocations. The question is if this
     /// data race is harmful or not.
-    /// FIXME: this might be bogus as well. This detach the node from its current
-    /// tree and reattach it to its neighbor
+    /// FIXME: this line might be bogus as well. This detach the node from its current
+    /// tree and reattach it to its neighbor. Later in the algorithm, the reset of the
+    /// component will join this new component via Union.
     //compOut.Set(index, minComp);
 
-    // I don't just only want to update the component label of this pixel, I actually
+    // I don't want to update the component label of this pixel, I actually
     // want to Union(FindRoot(myComponent), FindRoot(minComp)) and then Flatten the
     // result.
     auto myRoot = findRoot(compOut, myComp);
     auto newRoot = findRoot(compOut, minComp);
 
-    // This "linking by index" as in SV Jayanti et.al. with less than as the total
+    // This is "linking by index" as in SV Jayanti et.al. with less than as the total
     // order.
     // TODO: should be change to Compare and Swap, according to SV Janati et. al.
     if (myRoot < newRoot)
@@ -103,6 +104,7 @@ public:
     // else, no need to do anything when they are the same set.
 
     // FIXME: is this the right termination condition?
+    // FIXME: should the Get()/Set() be replace with a CompareAnsSwap()?
     // mark an update occurred if no other worklets have done so yet
     if (myComp != minComp && updated.Get(0) == 0)
     {
diff --git a/vtkm/worklet/testing/UnitTestImageConnectivity.cxx b/vtkm/worklet/testing/UnitTestImageConnectivity.cxx
index 6f1327ae0..7cdd9f425 100644
--- a/vtkm/worklet/testing/UnitTestImageConnectivity.cxx
+++ b/vtkm/worklet/testing/UnitTestImageConnectivity.cxx
@@ -11,6 +11,7 @@
 #include <vtkm/cont/testing/Testing.h>
 #include <vtkm/filter/Contour.h>
 
+#include <iomanip>
 #include <vtkm/worklet/connectivities/ImageConnectivity.h>
 
 class TestImageConnectivity
@@ -22,6 +23,7 @@ public:
   {
     CCL_CUDA8x4();
     CCL_CUDA8x8();
+    Valentine();
   }
 
   void CCL_CUDA8x4() const
@@ -88,6 +90,49 @@ public:
                        "Components has unexpected value.");
     }
   }
+
+  void Valentine() const
+  {
+    // Sample image by VALENTINE PELTIER
+
+    // clang-format off
+    auto pixels = vtkm::cont::make_ArrayHandle<vtkm::UInt8>( {
+      1, 1, 0, 1, 0, 0,
+      0, 0, 0, 1, 1, 0,
+      1, 1, 0, 1, 0, 1,
+      1, 0, 1, 0, 0, 0,
+      0, 1, 0, 1, 1, 1,
+      1, 1, 0, 0, 1, 0,
+    });
+    // clang-format on
+
+    vtkm::cont::DataSetBuilderUniform builder;
+    vtkm::cont::DataSet data = builder.Create(vtkm::Id3(6, 6, 1));
+
+    auto colorField = vtkm::cont::make_FieldPoint("color", pixels);
+    data.AddField(colorField);
+
+    vtkm::cont::ArrayHandle<vtkm::Id> component;
+    vtkm::worklet::connectivity::ImageConnectivity().Run(
+      data.GetCellSet().Cast<vtkm::cont::CellSetStructured<2>>(), colorField.GetData(), component);
+
+    // clang-format off
+    std::vector<vtkm::UInt8> componentExpected = {
+      0,  0,  1,  2,  1,  1,
+      1,  1,  1,  2,  2,  1,
+      2,  2,  1,  2,  1,  2,
+      2,  1,  2,  1,  1,  1,
+      1,  2,  1,  2,  2,  2,
+      2,  2,  1,  1,  2,  3
+    };
+    // clang-format on
+
+    for (vtkm::Id i = 0; i < component.GetNumberOfValues(); ++i)
+    {
+      VTKM_TEST_ASSERT(component.ReadPortal().Get(i) == componentExpected[size_t(i)],
+                       "Components has unexpected value.");
+    }
+  }
 };