From 4f270e458c4ede9374895310f31462cb7541bdf3 Mon Sep 17 00:00:00 2001 From: nimaib2 Date: Mon, 27 Oct 2025 15:49:43 -0700 Subject: [PATCH 1/4] Replaced maps with vectors, bounds stored as indices instead of Vec2 --- src/common/pipeline/pipelines.hpp | 2 +- src/common/style.hpp | 15 +- src/datafile/encoding.hpp | 22 +- src/distance/edge.cpp | 206 +++++++++++++----- test/command-line/parsing/parser-test.cpp | 5 +- .../edge-test/connected-components-test.cpp | 117 +++++----- test/example/example-test.cpp | 2 +- 7 files changed, 249 insertions(+), 120 deletions(-) diff --git a/src/common/pipeline/pipelines.hpp b/src/common/pipeline/pipelines.hpp index 0313f79c..0ac5aea9 100644 --- a/src/common/pipeline/pipelines.hpp +++ b/src/common/pipeline/pipelines.hpp @@ -57,7 +57,7 @@ class Pipeline : public FunctionStage { * indicates whether storage is present or not for * the output. */ - virtual Output Run(const Input &input) = 0; + Output Run(const Input &input) override = 0; protected: /// The stages of this diff --git a/src/common/style.hpp b/src/common/style.hpp index 3605eca4..83ce1a6c 100644 --- a/src/common/style.hpp +++ b/src/common/style.hpp @@ -14,7 +14,12 @@ namespace found { /// Alias for very precise floating point numbers. -typedef __float128 PreciseDecimal; +#if defined(__aarch64__) || defined(__arm64__) + // ARM/Apple Silicon doesn't support __float128 + typedef long double PreciseDecimal; +#else + typedef __float128 PreciseDecimal; +#endif /// The output for Edge Detection Algorithms (edge.hpp/cpp). Currently set /// to a vector of 2D points on the image, according to image coordinate systems @@ -76,10 +81,10 @@ typedef std::vector Edges; struct Component { /// The points in this component std::unordered_set points; - /// The lowest point (left upper edge) - Vec2 upperLeft; - /// The highest point (right lower edge) - Vec2 lowerRight; + /// The pixel index of the upper left bound + uint64_t upperLeftIndex; + /// The pixel index of the lower right bound + uint64_t lowerRightIndex; }; /// A collection of Image Pixels diff --git a/src/datafile/encoding.hpp b/src/datafile/encoding.hpp index 2d94010b..3596ae23 100644 --- a/src/datafile/encoding.hpp +++ b/src/datafile/encoding.hpp @@ -1,11 +1,31 @@ #ifndef SRC_DATAFILE_ENCODING_HPP_ #define SRC_DATAFILE_ENCODING_HPP_ -#include #include +#ifdef __APPLE__ + #include +#else + #include +#endif #include "common/decimal.hpp" +// On macOS, undefine the system macros before defining our functions +#ifdef __APPLE__ +#ifdef htonl +#undef htonl +#endif +#ifdef ntohl +#undef ntohl +#endif +#ifdef htons +#undef htons +#endif +#ifdef ntohs +#undef ntohs +#endif +#endif + #ifdef __BYTE_ORDER__ #define ENDIANESS __BYTE_ORDER__ #else diff --git a/src/distance/edge.cpp b/src/distance/edge.cpp index d5068960..46af0584 100644 --- a/src/distance/edge.cpp +++ b/src/distance/edge.cpp @@ -1,6 +1,5 @@ #include "distance/edge.hpp" -#include #include #include #include @@ -31,16 +30,27 @@ Points SimpleEdgeDetectionAlgorithm::Run(const Image &image) { for (auto &component : spaces) { // Basically, if the component touches the border, and its the biggest one, // we assume it is space - if ((component.upperLeft.x < this->borderLength_ || - component.upperLeft.y < this->borderLength_ || - component.lowerRight.x >= image.width - this->borderLength_ || - component.lowerRight.y >= image.height - this->borderLength_)) { + uint64_t upperLeftX = component.upperLeftIndex % image.width; + uint64_t upperLeftY = component.upperLeftIndex / image.width; + uint64_t lowerRightX = component.lowerRightIndex % image.width; + uint64_t lowerRightY = component.lowerRightIndex / image.width; + + if ((upperLeftX < static_cast(this->borderLength_) || + upperLeftY < static_cast(this->borderLength_) || + lowerRightX >= static_cast(image.width - this->borderLength_) || + lowerRightY >= static_cast(image.height - this->borderLength_))) { if (!space || component.points.size() > space->points.size()) space = &component; } } if (space == nullptr || space->points.size() == imageSize) return Points(); std::unordered_set &points = space->points; + + // Extract x, y coordinates from indices + uint64_t spaceUpperLeftX = space->upperLeftIndex % image.width; + uint64_t spaceUpperLeftY = space->upperLeftIndex / image.width; + uint64_t spaceLowerRightX = space->lowerRightIndex % image.width; + uint64_t spaceLowerRightY = space->lowerRightIndex / image.width; // Step 2: Identify the edge as the edge of space @@ -80,19 +90,19 @@ Points SimpleEdgeDetectionAlgorithm::Run(const Image &image) { if (itrDirection.y < 0) { // Iterate up, and start at the bottom left corner update = -image.width; - start = static_cast(space->lowerRight.y * image.width + space->upperLeft.x); + start = spaceLowerRightY * image.width + spaceUpperLeftX; offset = -this->offset_; edge_condition = image.height - 1; } else { // Iterate down, and start at the top left corner update = image.width; - start = static_cast(space->upperLeft.y * image.width + space->upperLeft.x); + start = spaceUpperLeftY * image.width + spaceUpperLeftX; offset = this->offset_; edge_condition = 0; } // Step 2c: Get all edge points along the edge, identifying the edge // as the first point that is found inside space - for (int col = space->upperLeft.x; col <= space->lowerRight.x; col++) { + for (uint64_t col = spaceUpperLeftX; col <= spaceLowerRightX; col++) { // because index is uint64_t, going below zero will overflow, and it will indeed be past the dimensions uint64_t index = start; while (points.find(index) == points.end()) index += update; @@ -112,19 +122,19 @@ Points SimpleEdgeDetectionAlgorithm::Run(const Image &image) { if (itrDirection.x < 0) { // Iterate left, and start at the top right corner update = -1; - start = static_cast(space->upperLeft.y * image.width + space->lowerRight.x); + start = spaceUpperLeftY * image.width + spaceLowerRightX; offset = -this->offset_; edge_condition = image.width - 1; } else { // Iterate right, and start at the top left corner update = 1; - start = static_cast(space->upperLeft.y * image.width + space->upperLeft.x); + start = spaceUpperLeftY * image.width + spaceUpperLeftX; offset = this->offset_; edge_condition = 0; } // Step 2c: Get all edge points along the edge, identifying the edge // as the first point that is found inside space - for (int row = space->upperLeft.y; row <= space->lowerRight.y; row++) { + for (uint64_t row = spaceUpperLeftY; row <= spaceLowerRightY; row++) { uint64_t index = start; while (points.find(index) == points.end()) index += update; if (index % image.width != edge_condition) { @@ -166,17 +176,57 @@ inline bool LabelPresent(int label, int *adjacentLabels, int size) { * * @param component The component to update * @param index The index to add - * @param pixel The pixel to add + * @param imageWidth The width of the image (for computing x, y coordinates) * * @pre Must be called in order of increasing index */ -inline void UpdateComponent(Component &component, uint64_t index, Vec2 &pixel) { +inline void UpdateComponent(Component &component, uint64_t index, int imageWidth) { component.points.insert(index); - if (component.upperLeft.x > pixel.x) component.upperLeft.x = pixel.x; - else if (component.lowerRight.x < pixel.x) component.lowerRight.x = pixel.x; - // We skip this statement, since its impossible: - // if (component.upperLeft.y > pixel.y) component.upperLeft.y = pixel.y; - if (component.lowerRight.y < pixel.y) component.lowerRight.y = pixel.y; + + // Get x, y coordinates from index + uint64_t x = index % imageWidth; + uint64_t y = index / imageWidth; + + // Get current bounding box coordinates + uint64_t upperLeftIdx = component.upperLeftIndex; + uint64_t lowerRightIdx = component.lowerRightIndex; + + uint64_t upperLeftX = upperLeftIdx % imageWidth; + uint64_t upperLeftY = upperLeftIdx / imageWidth; + + uint64_t lowerRightX = lowerRightIdx % imageWidth; + uint64_t lowerRightY = lowerRightIdx / imageWidth; + + // Update bounding box - track min and max independently + bool needUpdate = false; + + // Update minX + if (x < upperLeftX) { + upperLeftX = x; + needUpdate = true; + } + // Update minY + if (y < upperLeftY) { + upperLeftY = y; + needUpdate = true; + } + if (needUpdate) { + component.upperLeftIndex = upperLeftY * imageWidth + upperLeftX; + } + + // Update maxX and maxY for lowerRight + needUpdate = false; + if (x > lowerRightX) { + lowerRightX = x; + needUpdate = true; + } + if (y > lowerRightY) { + lowerRightY = y; + needUpdate = true; + } + if (needUpdate) { + component.lowerRightIndex = lowerRightY * imageWidth + lowerRightX; + } } /** @@ -187,8 +237,9 @@ inline void UpdateComponent(Component &component, uint64_t index, Vec2 &pixel) { * @param L The current label * @param adjacentLabels The labels of the adjacent components * @param size The number of adjacent labels - * @param components The components that are part of the image - * @param equivalencies The labels that are equivalent to each other + * @param components The components that are part of the image (array) + * @param equivalencies The labels that are equivalent to each other (array) + * @param componentExists Track which components exist (array) * * @return The label of the component point that was added * @@ -199,23 +250,25 @@ inline int NWayEquivalenceAdd(const Image &image, int &L, int adjacentLabels[4], int size, - std::unordered_map &components, - std::unordered_map &equivalencies) { - Vec2 pixel = {DECIMAL(index % image.width), DECIMAL(index / image.width)}; + std::vector &components, + std::vector &equivalencies, + std::vector &componentExists) { if (size == 0) { // No adjacent labels - components.insert({++L, {{index}, pixel, pixel}}); + L++; + components[L] = {{index}, index, index}; // upperLeftIndex and lowerRightIndex both start at index + componentExists[L] = true; return L; } else if (size == 1) { // One adjacent label - UpdateComponent(components[adjacentLabels[0]], index, pixel); + UpdateComponent(components[adjacentLabels[0]], index, image.width); return adjacentLabels[0]; } else if (size == 2) { // Added for optimization if (adjacentLabels[0] < adjacentLabels[1]) { // Two adjacent labels, first is smaller - UpdateComponent(components[adjacentLabels[0]], index, pixel); - if (equivalencies.find(adjacentLabels[1]) == equivalencies.end()) { - equivalencies.try_emplace(adjacentLabels[1], adjacentLabels[0]); + UpdateComponent(components[adjacentLabels[0]], index, image.width); + if (equivalencies[adjacentLabels[1]] == 0) { + equivalencies[adjacentLabels[1]] = adjacentLabels[0]; } else { equivalencies[adjacentLabels[1]] = std::min(equivalencies[adjacentLabels[1]], adjacentLabels[0]); } @@ -223,9 +276,9 @@ inline int NWayEquivalenceAdd(const Image &image, } // Two adjacent labels, second is smaller - UpdateComponent(components[adjacentLabels[1]], index, pixel); - if (equivalencies.find(adjacentLabels[0]) == equivalencies.end()) { - equivalencies.try_emplace(adjacentLabels[0], adjacentLabels[1]); + UpdateComponent(components[adjacentLabels[1]], index, image.width); + if (equivalencies[adjacentLabels[0]] == 0) { + equivalencies[adjacentLabels[0]] = adjacentLabels[1]; } else { equivalencies[adjacentLabels[0]] = std::min(equivalencies[adjacentLabels[0]], adjacentLabels[1]); } @@ -237,11 +290,11 @@ inline int NWayEquivalenceAdd(const Image &image, minLabel = adjacentLabels[i]; } } - UpdateComponent(components[minLabel], index, pixel); + UpdateComponent(components[minLabel], index, image.width); for (int i = 0; i < size; i++) { if (adjacentLabels[i] != minLabel) { - if (equivalencies.find(adjacentLabels[i]) == equivalencies.end()) { - equivalencies.try_emplace(adjacentLabels[i], minLabel); + if (equivalencies[adjacentLabels[i]] == 0) { + equivalencies[adjacentLabels[i]] = minLabel; } else { equivalencies[adjacentLabels[i]] = std::min(equivalencies[adjacentLabels[i]], minLabel); } @@ -252,9 +305,22 @@ inline int NWayEquivalenceAdd(const Image &image, Components ConnectedComponentsAlgorithm(const Image &image, std::function Criteria) { // Step 0: Setup the Problem - std::unordered_map components; - std::unordered_map equivalencies; - std::unique_ptr componentPoints(new int[image.width * image.height]{}); // Faster than using a hashset + // Early return for empty images (width or height = 0) + if (image.width == 0 || image.height == 0) { + return Components(); + } + if (image.width < 0 || image.height < 0) { + throw std::invalid_argument("Image dimensions must be positive"); + } + if (image.width > 100000 || image.height > 100000) { + throw std::invalid_argument("Image dimensions too large"); + } + // Calculate maximum capacity: ⌈wh/4⌉ = (wh + 3) / 4 + uint64_t maxComponents = (static_cast(image.width) * image.height + 3) / 4; + std::vector components(maxComponents + 1); // +1 because labels start at 1 + std::vector equivalencies(maxComponents + 1, 0); + std::vector componentExists(maxComponents + 1, false); + std::unique_ptr componentPoints(new int[image.width * image.height]{}); int L = 0; int adjacentLabels[4]; @@ -265,7 +331,9 @@ Components ConnectedComponentsAlgorithm(const Image &image, std::function= 0; i--) { - auto it = equivalencies.find(i); - if (it == equivalencies.end()) continue; + for (int i = L; i > 0; i--) { + if (equivalencies[i] == 0) continue; // Guarenteed to be the lowest label - int lowestLabel = it->second; + int lowestLabel = equivalencies[i]; - // Merge the components - auto compIt = components.find(i); - // compIt is guarenteed to exist, so we do not perform a check here - auto &compToMerge = compIt->second; + // Merge the components if this component exists + if (!componentExists[i]) continue; + auto &compToMerge = components[i]; auto &lowestComp = components[lowestLabel]; lowestComp.points.insert(compToMerge.points.begin(), compToMerge.points.end()); - if (compToMerge.upperLeft.x < lowestComp.upperLeft.x) lowestComp.upperLeft.x = compToMerge.upperLeft.x; - if (compToMerge.lowerRight.x > lowestComp.lowerRight.x) lowestComp.lowerRight.x = compToMerge.lowerRight.x; - // We skip this statement, because its impossible (a higher component is level or lower than a lower component): - // if (compToMerge.upperLeft.y < lowestComp.upperLeft.y) lowestComp.upperLeft.y = compToMerge.upperLeft.y; - if (compToMerge.lowerRight.y > lowestComp.lowerRight.y) lowestComp.lowerRight.y = compToMerge.lowerRight.y; + + // Update bounds by taking min/max independently + uint64_t mergeULIdx = compToMerge.upperLeftIndex; + uint64_t lowestULIdx = lowestComp.upperLeftIndex; + + uint64_t mergeULX = mergeULIdx % image.width; + uint64_t mergeULY = mergeULIdx / image.width; + + uint64_t lowestULX = lowestULIdx % image.width; + uint64_t lowestULY = lowestULIdx / image.width; + + // Update minX and minY + uint64_t newMinX = std::min(mergeULX, lowestULX); + uint64_t newMinY = std::min(mergeULY, lowestULY); + if (newMinX != lowestULX || newMinY != lowestULY) { + lowestComp.upperLeftIndex = newMinY * image.width + newMinX; + } + + uint64_t mergeLRIdx = compToMerge.lowerRightIndex; + uint64_t lowestLRIdx = lowestComp.lowerRightIndex; + + uint64_t mergeLRX = mergeLRIdx % image.width; + uint64_t mergeLRY = mergeLRIdx / image.width; + + uint64_t lowestLRX = lowestLRIdx % image.width; + uint64_t lowestLRY = lowestLRIdx / image.width; + + // Update maxX and maxY + uint64_t newMaxX = std::max(mergeLRX, lowestLRX); + uint64_t newMaxY = std::max(mergeLRY, lowestLRY); + if (newMaxX != lowestLRX || newMaxY != lowestLRY) { + lowestComp.lowerRightIndex = newMaxY * image.width + newMaxX; + } - components.erase(compIt); + componentExists[i] = false; } // Step 3: Return the components Components result; - for (const auto &[label, component] : components) result.push_back(component); + for (int i = 1; i <= L; i++) { + if (componentExists[i]) { + result.push_back(components[i]); + } + } return result; } diff --git a/test/command-line/parsing/parser-test.cpp b/test/command-line/parsing/parser-test.cpp index 4d9edaba..c5cea361 100644 --- a/test/command-line/parsing/parser-test.cpp +++ b/test/command-line/parsing/parser-test.cpp @@ -143,8 +143,9 @@ TEST_F(ParserTest, DistanceParserNoRefAsOriValue) { "--calibration-data", "test/common/assets/empty-df.found"}; DistanceOptions options = ParseDistanceOptions(argc, const_cast(argv)); Image expectedImage = strtoimage("test/common/assets/example_image.jpg"); - EulerAngles expectedRefOrientation(1.1, 1.2, 1.3); - EulerAngles expectedRelOrientation(1.4, 1.5, 1.6); + // These variables are declared but not currently used in assertions + // EulerAngles expectedRefOrientation(1.1, 1.2, 1.3); + // EulerAngles expectedRelOrientation(1.4, 1.5, 1.6); DataFile expectedDataFile = strtodf("test/common/assets/empty-df.found"); ASSERT_IMAGE_EQ(expectedImage, options.image); diff --git a/test/distance/edge-test/connected-components-test.cpp b/test/distance/edge-test/connected-components-test.cpp index d83e8bfc..dd973520 100644 --- a/test/distance/edge-test/connected-components-test.cpp +++ b/test/distance/edge-test/connected-components-test.cpp @@ -17,10 +17,15 @@ std::function criteria = [](uint64_t index, const return image.image[index] > 0; }; +// Helper function to convert (x, y) coordinates to index +inline uint64_t xyToIndex(int width, int x, int y) { + return y * width + x; +} + MATCHER_P(ComponentEqual, expected, "") { return expected.points == arg.points && - vectorEqual(arg.upperLeft, expected.upperLeft) && - vectorEqual(arg.lowerRight, expected.lowerRight); + arg.upperLeftIndex == expected.upperLeftIndex && + arg.lowerRightIndex == expected.lowerRightIndex; } TEST(ConnectedComponentsTest, TestInvalidImage) { @@ -75,8 +80,8 @@ TEST(ConnectedComponentsTest, TestOnePixelBase) { Components expected = { { {0}, - {0, 0}, - {0, 0} + 0, // upperLeftIndex: y=0, x=0 -> index = 0*2 + 0 = 0 + 0 // lowerRightIndex: y=0, x=0 -> index = 0*2 + 0 = 0 } }; @@ -107,8 +112,8 @@ TEST(ConnectedComponentsTest, TestOnePixelCorner) { Components expected = { { {1}, - {1, 0}, - {1, 0} + 1, // upperLeftIndex: y=0, x=1 -> index = 0*2 + 1 = 1 + 1 // lowerRightIndex: y=0, x=1 -> index = 0*2 + 1 = 1 } }; @@ -139,8 +144,8 @@ TEST(ConnectedComponentsTest, TestTwoPixels) { Components expected = { { {0, 1}, - {0, 0}, - {1, 0} + 0, // upperLeftIndex: y=0, x=0 -> index = 0*2 + 0 = 0 + 1 // lowerRightIndex: y=0, x=1 -> index = 0*2 + 1 = 1 } }; @@ -170,8 +175,8 @@ TEST(ConnectedComponentsTest, TestTwoPixelsDiagonalNormal) { Components expected = { { {0, 3}, - {0, 0}, - {1, 1} + xyToIndex(2, 0, 0), // (0, 0) + xyToIndex(2, 1, 1) // (1, 1) }, }; @@ -201,8 +206,8 @@ TEST(ConnectedComponentsTest, TestTwoPixelsDiagonalReverse) { Components expected = { { {1, 2}, - {0, 0}, - {1, 1} + xyToIndex(2, 0, 0), // (0, 0) + xyToIndex(2, 1, 1) // (1, 1) }, }; @@ -233,8 +238,8 @@ TEST(ConnectedComponentsTest, TestDoubleDiagonal) { Components expected = { { {0, 2, 4, 6, 8}, - {0, 0}, - {2, 2} + xyToIndex(3, 0, 0), // (0, 0) + xyToIndex(3, 2, 2) // (2, 2) }, }; @@ -265,8 +270,8 @@ TEST(ConnectedComponentsTest, TestLineDiagonalVertical) { Components expected = { { {0, 2, 3, 4, 6}, - {0, 0}, - {2, 2} + xyToIndex(3, 0, 0), // (0, 0) + xyToIndex(3, 2, 2) // (2, 2) }, }; @@ -297,8 +302,8 @@ TEST(ConnectedComponentsTest, TestLineDiagonalHorizontal1) { Components expected = { { {0, 1, 2, 4, 8}, - {0, 0}, - {2, 2} + xyToIndex(3, 0, 0), // (0, 0) + xyToIndex(3, 2, 2) // (2, 2) }, }; @@ -329,8 +334,8 @@ TEST(ConnectedComponentsTest, TestLineDiagonalHorizontal2) { Components expected = { { {0, 4, 6, 7, 8}, - {0, 0}, - {2, 2} + xyToIndex(3, 0, 0), // (0, 0) + xyToIndex(3, 2, 2) // (2, 2) }, }; @@ -363,8 +368,8 @@ TEST(ConnectedComponentsTest, Test2ConvergingLines1) { Components expected = { { {9, 10, 11, 12, 13, 17, 18, 19, 23, 24}, - {0, 1}, - {4, 4} + xyToIndex(5, 0, 1), // (0, 1) + xyToIndex(5, 4, 4) // (4, 4) }, }; @@ -401,8 +406,8 @@ TEST(ConnectedComponentsTest, Test2ConvergingLines2) { Components expected = { { {4, 5, 7, 9, 10, 12, 13, 15, 16, 17, 18}, - {0, 0}, - {4, 3} + xyToIndex(5, 0, 0), // (0, 0) + xyToIndex(5, 4, 3) // (4, 3) }, }; @@ -439,8 +444,8 @@ TEST(ConnectedComponentsTest, Test2ConvergingLines3) { Components expected = { { {9, 10, 11, 12, 13, 15}, - {0, 1}, - {4, 3} + xyToIndex(5, 0, 1), // (0, 1) + xyToIndex(5, 4, 3) // (4, 3) }, }; @@ -477,8 +482,8 @@ TEST(ConnectedComponentsTest, Test3ConvergingLines1) { Components expected = { { {0, 2, 4, 5, 7, 9, 10, 12, 14, 15, 17, 19, 20, 21, 22, 23, 24}, - {0, 0}, - {4, 4} + xyToIndex(5, 0, 0), // (0, 0) + xyToIndex(5, 4, 4) // (4, 4) }, }; @@ -515,8 +520,8 @@ TEST(ConnectedComponentsTest, Test3ConvergingLines2) { Components expected = { { {4, 7, 9, 10, 12, 14, 15, 17, 19, 20, 21, 22, 23, 24}, - {0, 0}, - {4, 4} + xyToIndex(5, 0, 0), // (0, 0) + xyToIndex(5, 4, 4) // (4, 4) }, }; @@ -551,8 +556,8 @@ TEST(ConnectedComponentsTest, Test2AdjacentPixelsGeneral) { Components expected = { { {2, 3, 4}, - {0, 0}, - {2, 1} + xyToIndex(3, 0, 0), // (0, 0) + xyToIndex(3, 2, 1) // (2, 1) }, }; @@ -584,8 +589,8 @@ TEST(ConnectedComponentsTest, TestConvergingBlob) { Components expected = { { {0, 4, 5, 7, 9, 11, 12, 13, 16, 17, 18}, - {0, 0}, - {4, 3} + xyToIndex(5, 0, 0), // (0, 0) + xyToIndex(5, 4, 3) // (4, 3) }, }; @@ -616,13 +621,13 @@ TEST(ConnectedComponentsTest, Test2BlobsSimple) { Components expected = { { {2}, - {2, 0}, - {2, 0} + xyToIndex(3, 2, 0), // (2, 0) + xyToIndex(3, 2, 0) // (2, 0) }, { {6, 7}, - {0, 2}, - {1, 2} + xyToIndex(3, 0, 2), // (0, 2) + xyToIndex(3, 1, 2) // (1, 2) } }; @@ -655,13 +660,13 @@ TEST(ConnectedComponentsTest, Test2BlobsGeneral) { Components expected = { { {4, 8, 9, 14}, - {3, 0}, - {4, 2} + xyToIndex(5, 3, 0), // (3, 0) + xyToIndex(5, 4, 2) // (4, 2) }, { {10, 15, 17, 20, 21, 22, 23, 24}, - {0, 2}, - {4, 4} + xyToIndex(5, 0, 2), // (0, 2) + xyToIndex(5, 4, 4) // (4, 4) } }; @@ -697,18 +702,18 @@ TEST(ConnectedComponentsTest, Test3BlobsGeneral) { Components expected = { { {2, 6, 7, 14, 15}, - {0, 0}, - {3, 2} + xyToIndex(6, 0, 0), // (0, 0) + xyToIndex(6, 3, 2) // (3, 2) }, { {4, 5, 11}, - {4, 0}, - {5, 1} + xyToIndex(6, 4, 0), // (4, 0) + xyToIndex(6, 5, 1) // (5, 1) }, { {23, 25, 27, 29, 31, 32, 33, 34, 35}, - {1, 3}, - {5, 5} + xyToIndex(6, 1, 3), // (1, 3) + xyToIndex(6, 5, 5) // (5, 5) }, }; @@ -743,23 +748,23 @@ TEST(ConnectedComponentsTest, Test4BlobsGeneral) { Components expected = { { {2, 5, 6, 10}, - {0, 0}, - {2, 2} + xyToIndex(5, 0, 0), // (0, 0) + xyToIndex(5, 2, 2) // (2, 2) }, { {4, 9}, - {4, 0}, - {4, 1} + xyToIndex(5, 4, 0), // (4, 0) + xyToIndex(5, 4, 1) // (4, 1) }, { {20}, - {0, 4}, - {0, 4} + xyToIndex(5, 0, 4), // (0, 4) + xyToIndex(5, 0, 4) // (0, 4) }, { {17, 19, 23, 24}, - {2, 3}, - {4, 4} + xyToIndex(5, 2, 3), // (2, 3) + xyToIndex(5, 4, 4) // (4, 4) } }; diff --git a/test/example/example-test.cpp b/test/example/example-test.cpp index 00eeedbd..c2cbb822 100644 --- a/test/example/example-test.cpp +++ b/test/example/example-test.cpp @@ -54,7 +54,7 @@ class ExampleTest : public testing::Test { * test cases, and can be used to destroy any heap objects. * Here, we delete our mock. */ - virtual void TearDown() { + void TearDown() override { delete eda; } }; From e28cc348cb99777b8c34c320f23572e81e200eff Mon Sep 17 00:00:00 2001 From: nimaib2 Date: Thu, 6 Nov 2025 15:39:25 -0800 Subject: [PATCH 2/4] Improved test coverage (sorry this took so long I got very busy these last two weeks) --- src/distance/edge.cpp | 39 +-- .../edge-test/connected-components-test.cpp | 226 +++++++++++++++++- 2 files changed, 245 insertions(+), 20 deletions(-) diff --git a/src/distance/edge.cpp b/src/distance/edge.cpp index 46af0584..ba74b10a 100644 --- a/src/distance/edge.cpp +++ b/src/distance/edge.cpp @@ -34,7 +34,7 @@ Points SimpleEdgeDetectionAlgorithm::Run(const Image &image) { uint64_t upperLeftY = component.upperLeftIndex / image.width; uint64_t lowerRightX = component.lowerRightIndex % image.width; uint64_t lowerRightY = component.lowerRightIndex / image.width; - + if ((upperLeftX < static_cast(this->borderLength_) || upperLeftY < static_cast(this->borderLength_) || lowerRightX >= static_cast(image.width - this->borderLength_) || @@ -45,7 +45,7 @@ Points SimpleEdgeDetectionAlgorithm::Run(const Image &image) { } if (space == nullptr || space->points.size() == imageSize) return Points(); std::unordered_set &points = space->points; - + // Extract x, y coordinates from indices uint64_t spaceUpperLeftX = space->upperLeftIndex % image.width; uint64_t spaceUpperLeftY = space->upperLeftIndex / image.width; @@ -182,30 +182,30 @@ inline bool LabelPresent(int label, int *adjacentLabels, int size) { */ inline void UpdateComponent(Component &component, uint64_t index, int imageWidth) { component.points.insert(index); - + // Get x, y coordinates from index uint64_t x = index % imageWidth; uint64_t y = index / imageWidth; - + // Get current bounding box coordinates uint64_t upperLeftIdx = component.upperLeftIndex; uint64_t lowerRightIdx = component.lowerRightIndex; - + uint64_t upperLeftX = upperLeftIdx % imageWidth; uint64_t upperLeftY = upperLeftIdx / imageWidth; - + uint64_t lowerRightX = lowerRightIdx % imageWidth; uint64_t lowerRightY = lowerRightIdx / imageWidth; - + // Update bounding box - track min and max independently bool needUpdate = false; - + // Update minX if (x < upperLeftX) { upperLeftX = x; needUpdate = true; } - // Update minY + // Update minY if (y < upperLeftY) { upperLeftY = y; needUpdate = true; @@ -213,7 +213,7 @@ inline void UpdateComponent(Component &component, uint64_t index, int imageWidth if (needUpdate) { component.upperLeftIndex = upperLeftY * imageWidth + upperLeftX; } - + // Update maxX and maxY for lowerRight needUpdate = false; if (x > lowerRightX) { @@ -398,7 +398,8 @@ Components ConnectedComponentsAlgorithm(const Image &image, std::function(0), actual.size()); } +TEST(ConnectedComponentsTest, TestEmptyImageWidth) { + // Test edge case where width is 0 but height is not + unsigned char imageData[1] = {0}; + + Image image = { + 0, + 10, + 1, + imageData, + }; + + Components actual = ConnectedComponentsAlgorithm(image, criteria); + ASSERT_EQ(static_cast(0), actual.size()); +} + +TEST(ConnectedComponentsTest, TestEmptyImageHeight) { + // Test edge case where height is 0 but width is not + unsigned char imageData[1] = {0}; + + Image image = { + 10, + 0, + 1, + imageData, + }; + + Components actual = ConnectedComponentsAlgorithm(image, criteria); + ASSERT_EQ(static_cast(0), actual.size()); +} + +TEST(ConnectedComponentsTest, TestImageTooLarge) { + // Test the maximum dimension check + unsigned char imageData[1] = {0}; + + Image image = { + 100001, // Exceeds maximum width + 100, + 1, + imageData, + }; + + ASSERT_ANY_THROW(ConnectedComponentsAlgorithm(image, criteria)); + + Image image2 = { + 100, + 100001, // Exceeds maximum height + 1, + imageData, + }; + + ASSERT_ANY_THROW(ConnectedComponentsAlgorithm(image2, criteria)); +} + TEST(ConnectedComponentsTest, TestOnePixelBase) { // Setup Dependencies unsigned char imageData[4] = {1, 0, @@ -781,4 +848,161 @@ TEST(ConnectedComponentsTest, Test4BlobsGeneral) { ASSERT_THAT(actual, testing::UnorderedElementsAreArray(matchers)); } +TEST(ConnectedComponentsTest, TestBoundsNoUpdateWhenPixelInside) { + // Test case where adding a pixel doesn't change bounds (pixel is inside bounding box) + // This covers the needUpdate = false branches in UpdateComponent + unsigned char imageData[9] = {1, 1, 1, + 1, 0, 1, + 1, 1, 1}; + + Image image = { + 3, + 3, + 1, + imageData, + }; + + Components actual = ConnectedComponentsAlgorithm(image, criteria); + + // Should have 1 component with all 8 pixels + ASSERT_EQ(static_cast(1), actual.size()); + ASSERT_EQ(static_cast(8), actual[0].points.size()); + // Bounds should be from (0,0) to (2,2) - covering the entire 3x3 area + ASSERT_EQ(xyToIndex(3, 0, 0), actual[0].upperLeftIndex); + ASSERT_EQ(xyToIndex(3, 2, 2), actual[0].lowerRightIndex); +} + +TEST(ConnectedComponentsTest, TestMergeBoundsNoChange) { + // Test case where merging components doesn't change bounds + // This covers the branches where newMinX == lowestULX && newMinY == lowestULY + // and newMaxX == lowestLRX && newMaxY == lowestLRY + // Create a pattern where components merge but one's bounds are contained in the other + unsigned char imageData[25] = {1, 0, 0, 0, 1, + 0, 1, 1, 1, 0, + 0, 1, 0, 1, 0, + 0, 1, 1, 1, 0, + 1, 0, 0, 0, 1}; + + Image image = { + 5, + 5, + 1, + imageData, + }; + + Components actual = ConnectedComponentsAlgorithm(image, criteria); + + // Should have 1 component (all pixels connect) + ASSERT_EQ(static_cast(1), actual.size()); + // Bounds should cover entire area + ASSERT_EQ(xyToIndex(5, 0, 0), actual[0].upperLeftIndex); + ASSERT_EQ(xyToIndex(5, 4, 4), actual[0].lowerRightIndex); +} + +TEST(ConnectedComponentsTest, TestBoundsUpdateOnlyX) { + // Test case where only X coordinate changes (not Y) + // Pixels are connected horizontally + unsigned char imageData[6] = {1, 1, 1, + 0, 0, 0}; + + Image image = { + 3, + 2, + 1, + imageData, + }; + + Components actual = ConnectedComponentsAlgorithm(image, criteria); + + ASSERT_EQ(static_cast(1), actual.size()); + ASSERT_EQ(static_cast(3), actual[0].points.size()); + // Bounds should be from (0,0) to (2,0) + ASSERT_EQ(xyToIndex(3, 0, 0), actual[0].upperLeftIndex); + ASSERT_EQ(xyToIndex(3, 2, 0), actual[0].lowerRightIndex); +} + +TEST(ConnectedComponentsTest, TestBoundsUpdateOnlyY) { + // Test case where only Y coordinate changes (not X) + // Pixels are connected vertically + unsigned char imageData[6] = {1, 0, 0, + 1, 0, 0}; + + Image image = { + 3, + 2, + 1, + imageData, + }; + + Components actual = ConnectedComponentsAlgorithm(image, criteria); + + ASSERT_EQ(static_cast(1), actual.size()); + ASSERT_EQ(static_cast(2), actual[0].points.size()); + // Bounds should be from (0,0) to (0,1) + ASSERT_EQ(xyToIndex(3, 0, 0), actual[0].upperLeftIndex); + ASSERT_EQ(xyToIndex(3, 0, 1), actual[0].lowerRightIndex); +} + +TEST(ConnectedComponentsTest, TestBoundsUpdateMinYWhenMerging) { + // Test case to cover y < upperLeftY branch in UpdateComponent + // Since we process top-to-bottom, this branch is only reachable if + // a component's upperLeftY gets set incorrectly, or if we manually + // construct a scenario. However, with sequential processing, this + // branch may be unreachable in practice. + // + // This test creates a pattern where pixels connect in a way that + // might trigger the branch, though it may not be reachable. + unsigned char imageData[12] = {1, 0, 0, 0, + 0, 1, 0, 0, + 0, 0, 1, 0}; + + Image image = { + 4, + 3, + 1, + imageData, + }; + + Components actual = ConnectedComponentsAlgorithm(image, criteria); + + // Should have 1 component (diagonal connection) + ASSERT_EQ(static_cast(1), actual.size()); + ASSERT_EQ(xyToIndex(4, 0, 0), actual[0].upperLeftIndex); + ASSERT_EQ(xyToIndex(4, 2, 2), actual[0].lowerRightIndex); +} + +TEST(ConnectedComponentsTest, TestBoundsUpdateMinXWhenMerging) { + // Test case to cover x < upperLeftX branch in UpdateComponent + // Similar to TestBoundsUpdateMinYWhenMerging, this may be unreachable + // with sequential processing, but we test it anyway. + // Note: With 4-connectivity, diagonal pixels don't connect, so we need + // a pattern where pixels connect horizontally/vertically. + unsigned char imageData[12] = {0, 0, 1, 1, + 0, 1, 1, 0, + 1, 1, 0, 0}; + + Image image = { + 4, + 3, + 1, + imageData, + }; + + Components actual = ConnectedComponentsAlgorithm(image, criteria); + + // Should have 1 component (connected via horizontal/vertical) + ASSERT_EQ(static_cast(1), actual.size()); + // Verify it has all 6 pixels + ASSERT_EQ(static_cast(6), actual[0].points.size()); + // Bounds should cover all pixels - verify they're correct + uint64_t upperLeftX = actual[0].upperLeftIndex % 4; + uint64_t upperLeftY = actual[0].upperLeftIndex / 4; + uint64_t lowerRightX = actual[0].lowerRightIndex % 4; + uint64_t lowerRightY = actual[0].lowerRightIndex / 4; + ASSERT_EQ(0u, upperLeftX); + ASSERT_EQ(0u, upperLeftY); + ASSERT_EQ(3u, lowerRightX); + ASSERT_EQ(2u, lowerRightY); +} + } // namespace found From 9e1305516fc8fea0a386d7d09c0c7fa9e3508617 Mon Sep 17 00:00:00 2001 From: nimaib2 Date: Thu, 6 Nov 2025 16:03:51 -0800 Subject: [PATCH 3/4] Trying to extend coverage to last couple of lines and branches, removed unreachable branches --- src/distance/edge.cpp | 12 ++++++------ .../distance/edge-test/connected-components-test.cpp | 9 ++++----- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/distance/edge.cpp b/src/distance/edge.cpp index ba74b10a..55791ba1 100644 --- a/src/distance/edge.cpp +++ b/src/distance/edge.cpp @@ -198,18 +198,18 @@ inline void UpdateComponent(Component &component, uint64_t index, int imageWidth uint64_t lowerRightY = lowerRightIdx / imageWidth; // Update bounding box - track min and max independently + // Note: Since pixels are processed sequentially top-to-bottom, left-to-right, + // we can only expand bounds rightward and downward, not leftward or upward. + // Therefore, y < upperLeftY will never be true during sequential processing. + // The x < upperLeftX case can occur when adding a pixel to the left in a lower row. bool needUpdate = false; - // Update minX + // Update minX (only reachable when merging components) if (x < upperLeftX) { upperLeftX = x; needUpdate = true; } - // Update minY - if (y < upperLeftY) { - upperLeftY = y; - needUpdate = true; - } + if (needUpdate) { component.upperLeftIndex = upperLeftY * imageWidth + upperLeftX; } diff --git a/test/distance/edge-test/connected-components-test.cpp b/test/distance/edge-test/connected-components-test.cpp index 8c225209..6467e811 100644 --- a/test/distance/edge-test/connected-components-test.cpp +++ b/test/distance/edge-test/connected-components-test.cpp @@ -863,7 +863,7 @@ TEST(ConnectedComponentsTest, TestBoundsNoUpdateWhenPixelInside) { }; Components actual = ConnectedComponentsAlgorithm(image, criteria); - + // Should have 1 component with all 8 pixels ASSERT_EQ(static_cast(1), actual.size()); ASSERT_EQ(static_cast(8), actual[0].points.size()); @@ -891,7 +891,7 @@ TEST(ConnectedComponentsTest, TestMergeBoundsNoChange) { }; Components actual = ConnectedComponentsAlgorithm(image, criteria); - + // Should have 1 component (all pixels connect) ASSERT_EQ(static_cast(1), actual.size()); // Bounds should cover entire area @@ -913,7 +913,7 @@ TEST(ConnectedComponentsTest, TestBoundsUpdateOnlyX) { }; Components actual = ConnectedComponentsAlgorithm(image, criteria); - + ASSERT_EQ(static_cast(1), actual.size()); ASSERT_EQ(static_cast(3), actual[0].points.size()); // Bounds should be from (0,0) to (2,0) @@ -949,7 +949,7 @@ TEST(ConnectedComponentsTest, TestBoundsUpdateMinYWhenMerging) { // a component's upperLeftY gets set incorrectly, or if we manually // construct a scenario. However, with sequential processing, this // branch may be unreachable in practice. - // + // This test creates a pattern where pixels connect in a way that // might trigger the branch, though it may not be reachable. unsigned char imageData[12] = {1, 0, 0, 0, @@ -1004,5 +1004,4 @@ TEST(ConnectedComponentsTest, TestBoundsUpdateMinXWhenMerging) { ASSERT_EQ(3u, lowerRightX); ASSERT_EQ(2u, lowerRightY); } - } // namespace found From 360c762518105a85b47a264789091542fb289898 Mon Sep 17 00:00:00 2001 From: nimaib2 Date: Thu, 6 Nov 2025 16:28:56 -0800 Subject: [PATCH 4/4] Testing where components are created in a spatial order/merging --- .../edge-test/connected-components-test.cpp | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/test/distance/edge-test/connected-components-test.cpp b/test/distance/edge-test/connected-components-test.cpp index 6467e811..282a5c14 100644 --- a/test/distance/edge-test/connected-components-test.cpp +++ b/test/distance/edge-test/connected-components-test.cpp @@ -1004,4 +1004,58 @@ TEST(ConnectedComponentsTest, TestBoundsUpdateMinXWhenMerging) { ASSERT_EQ(3u, lowerRightX); ASSERT_EQ(2u, lowerRightY); } + +TEST(ConnectedComponentsTest, TestMergeBoundsMinXSmaller) { + // Test case where merged component's minX is smaller than lowest component's minX + // This covers the std::min branch at line 432 where mergeULX < lowestULX + // Pattern: Component at (4,0) created first (label 1, minX=4), then component at (0,2) created (label 2, minX=0) + // They connect via a continuous bridge, causing label 2 to merge into label 1 + // When merging, mergeULX=0 < lowestULX=4, triggering the branch + unsigned char imageData[20] = {0, 0, 0, 0, 1, + 0, 0, 1, 1, 1, + 1, 1, 1, 0, 0, + 0, 0, 0, 0, 0}; + + Image image = { + 5, + 4, + 1, + imageData, + }; + + Components actual = ConnectedComponentsAlgorithm(image, criteria); + + // Should have 1 component (all pixels connect via continuous bridge) + ASSERT_EQ(static_cast(1), actual.size()); + // Bounds should cover from (0,0) to (4,2) - minX comes from merged component + ASSERT_EQ(xyToIndex(5, 0, 0), actual[0].upperLeftIndex); + ASSERT_EQ(xyToIndex(5, 4, 2), actual[0].lowerRightIndex); +} + +TEST(ConnectedComponentsTest, TestMergeBoundsMaxXLarger) { + // Test case where merged component's maxX is larger than lowest component's maxX + // This covers the std::max branch at line 448 where mergeLRX > lowestLRX + // Pattern: Component at (0,0) created first (label 1, maxX=0), then component at (4,2) created (label 2, maxX=4) + // They connect via a continuous bridge, causing label 2 to merge into label 1 + // When merging, mergeLRX=4 > lowestLRX=0, triggering the branch + unsigned char imageData[20] = {1, 1, 1, 0, 0, + 0, 1, 1, 1, 0, + 0, 0, 0, 0, 1, + 0, 0, 0, 0, 0}; + + Image image = { + 5, + 4, + 1, + imageData, + }; + + Components actual = ConnectedComponentsAlgorithm(image, criteria); + + // Should have 1 component (all pixels connect via continuous bridge) + ASSERT_EQ(static_cast(1), actual.size()); + // Bounds should cover from (0,0) to (4,2) - maxX comes from merged component + ASSERT_EQ(xyToIndex(5, 0, 0), actual[0].upperLeftIndex); + ASSERT_EQ(xyToIndex(5, 4, 2), actual[0].lowerRightIndex); +} } // namespace found