Repository navigation
fix(core)!: prevent holes during SVG generation #429
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 9 commits
72a6a7a
aac9407
7e5ac60
37309f0
b37807e
761fd24
7cbded3
51348fc
b4ee348
9efab7c
02cc692
0e973cb
8e35991
680922e
6da4d6f
7aa21d7
b3dc95c
0017754
b728615
8160c24
1cb1f87
b9864ce
c2bc23b
df07f98
b3e33ef
d6f29a1
9360efa
b3d016e
7dcf8aa
b2a8c27
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ static img2num::ImageToSvgConfig to_cpp(const img2num_ImageToSvgConfig& c) { | |
| }, | ||
|
|
||
| .min_cluster_area = c.min_cluster_area, | ||
| .min_thickness = c.min_thickness, | ||
| .color_space = c.color_space | ||
| }; | ||
| // clang-format on | ||
|
|
@@ -38,6 +39,7 @@ static img2num_ImageToSvgConfig to_c(const img2num::ImageToSvgConfig& cpp) { | |
| .max_iter = cpp.kmeans.max_iter | ||
| }, | ||
| .min_cluster_area = cpp.min_cluster_area, | ||
| .min_thickness = cpp.min_thickness, | ||
| .color_space = cpp.color_space | ||
| }; | ||
| // clang-format on | ||
|
|
@@ -90,22 +92,19 @@ void img2num_bilateral_filter( | |
| ); | ||
| } | ||
|
|
||
| char* img2num_labels_to_svg( | ||
| const uint8_t* data, const int32_t* labels, const int width, const int height, | ||
| const int min_area | ||
| ) { | ||
| char* result {nullptr}; | ||
| char *img2num_labels_to_svg(const uint8_t *data, const int32_t *labels, const int width, | ||
| const int height, const int min_area, const int min_thickness) { | ||
| char *result{nullptr}; | ||
| img2num::clear_last_error_and_catch( | ||
| [&](const uint8_t* d, const int32_t* l, const int w, const int h, const int min_a) { | ||
| std::string svg {img2num::labels_to_svg(d, l, w, h, min_a)}; | ||
| result = static_cast<char*>(std::malloc(svg.size() + 1)); | ||
| [&](const uint8_t *d, const int32_t *l, const int w, const int h, const int min_a, const int min_t) { | ||
| std::string svg{img2num::labels_to_svg(d, l, w, h, min_a, min_t)}; | ||
| result = static_cast<char *>(std::malloc(svg.size() + 1)); | ||
| if (!result) { | ||
| return; // Allocation failed | ||
| } | ||
| std::memcpy(result, svg.c_str(), svg.size() + 1); | ||
| }, | ||
| data, labels, width, height, min_area | ||
| ); | ||
| data, labels, width, height, min_area, min_thickness); | ||
| return result; | ||
| } | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What are your thoughts on us removing this function entirely in the future (not now) to make it easier to maintain the library: we only expose one interface for converting to SVGs.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd still keep it... it's not difficult to maintain |
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| #ifndef DOUGLAS_PEUCKER_H | ||
| #define DOUGLAS_PEUCKER_H | ||
|
|
||
| #include <cstdint> | ||
| #include <vector> | ||
|
|
||
| #include "internal/contours.h" | ||
|
|
||
| // Douglas-Peucker point reduction. Reduces each contour's point count directly | ||
| // (the bezier least-squares fit's only real purpose here was shrinking the SVG). | ||
| // | ||
| // `fixed[i][k]!=0` marks junctions/corners that must NOT be removed: the chain is | ||
| // split at them so every one survives as an exact, shared endpoint -- neighbours | ||
| // that share a junction therefore split at the SAME point. Simplification is | ||
| // bounded so it never retracts a boundary inward past the inter-region overlap | ||
| // margin, so reducing one region never eats its overlap with another and opens a | ||
| // gap. Kept points are emitted as straight-line quads, so the existing QuadBezier | ||
| // SVG path emission (and its closing 'Z') is unchanged. | ||
| // | ||
| // `eps` : overall deviation tolerance (px). Env override IMG2NUM_DP_EPS. | ||
| // retract_eps : max inward (region-shrinking) move allowed when dropping a | ||
| // vertex; defaults to min(eps, 0.5px). Env override | ||
| // IMG2NUM_DP_RETRACT. | ||
|
Ryan-Millard marked this conversation as resolved.
Outdated
|
||
| void dp_curve_reduction(const std::vector<std::vector<Point>> &chains, | ||
| const std::vector<std::vector<uint8_t>> &fixed, | ||
| std::vector<std::vector<QuadBezier>> &results, float eps); | ||
|
Krasner marked this conversation as resolved.
Outdated
|
||
|
|
||
| #endif | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,11 +39,16 @@ class Graph { | |
| void hash_node_ids(void); | ||
| void process_overlapping_edges(); | ||
|
|
||
| public: | ||
| inline Graph(std::unique_ptr<std::vector<Node_ptr>>& nodes, int width, int height) | ||
| : m_nodes(std::move(nodes)) | ||
| , m_width(width) | ||
| , m_height(height) { | ||
| inline uint8_t getPixel(const std::vector<uint8_t>& img, int w, int h, int x, int y) { | ||
| if (x < 0 || x >= w || y < 0 || y >= h) return 0; // Boundary check | ||
| return img[y * w + x]; | ||
| } | ||
|
Comment on lines
+42
to
+56
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably schedule a refactor to make the library use the image class wherever possible because we repeat this pattern a lot. |
||
|
|
||
| std::vector<uint8_t> analyzeJunctions(const std::vector<uint8_t>& skel, int w, int h); | ||
|
Ryan-Millard marked this conversation as resolved.
|
||
|
|
||
| public: | ||
| inline Graph(std::unique_ptr<std::vector<Node_ptr>> &nodes, int width, int height) | ||
| : m_nodes(std::move(nodes)), m_width(width), m_height(height) { | ||
| hash_node_ids(); | ||
| } | ||
|
|
||
|
|
@@ -68,10 +73,9 @@ class Graph { | |
| return m_nodes->size(); | ||
| } | ||
|
|
||
| void discover_edges( | ||
| const std::vector<int32_t>& region_labels, const int32_t width, const int32_t height | ||
| ); | ||
| void merge_small_area_nodes(const int32_t min_area); | ||
| void discover_edges(const std::vector<int32_t> ®ion_labels, const int32_t width, | ||
| const int32_t height); | ||
| void merge_small_area_nodes(const int32_t min_area, const int32_t min_thickness = 0); | ||
| void compute_contours(); | ||
| }; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| #ifndef SHARED_CONTOURS_H | ||
| #define SHARED_CONTOURS_H | ||
|
|
||
| #include <cstdint> | ||
| #include <unordered_map> | ||
| #include <vector> | ||
|
|
||
| #include "internal/Point.h" | ||
| #include "internal/contours.h" // QuadBezier | ||
|
|
||
| // Crack-based shared-edge contour builder. | ||
| // | ||
| // Builds region boundaries on the pixel-corner ("crack") grid rather than on | ||
| // pixel centres. The boundary between two regions is extracted ONCE as a | ||
| // canonical edge (a chain of cracks between two junction corners), simplified | ||
| // once, and then reused by BOTH adjacent regions. Neighbouring contours are | ||
| // therefore exactly coincident along every shared edge -- no overlap band and no | ||
| // gaps, by construction. | ||
| // | ||
| // `labels` is the per-pixel region id (w*h, row major); any value is treated as a | ||
| // distinct region, out-of-bounds is the image exterior. Returns, per region id, | ||
| // its list of closed boundary loops, each a sequence of quadratic beziers in | ||
| // corner coordinates (0..w, 0..h). Each shared edge is fitted ONCE and reused by | ||
| // both adjacent regions, so neighbouring loops are exactly coincident. | ||
| // | ||
| // `eps` is the curve-fit tolerance applied to each canonical edge. | ||
| std::unordered_map<int32_t, std::vector<std::vector<QuadBezier>>> build_shared_loops( | ||
| const std::vector<int32_t> &labels, int w, int h, float eps); | ||
|
Krasner marked this conversation as resolved.
Outdated
|
||
|
|
||
| #endif | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -154,3 +154,37 @@ void fit_curve_reduction( | |
| results.push_back(result); | ||
| } | ||
| } | ||
|
|
||
| // --- Junction-aware wrapper --- | ||
| // Splits each chain at its fixed (junction) points and fits the pieces | ||
| // separately. Because fitRecursive always keeps a segment's first and last point | ||
| // exactly, every junction becomes a pinned on-curve point the fit cannot move. | ||
| void fit_curve_reduction(const std::vector<std::vector<Point>> &chains, | ||
| const std::vector<std::vector<uint8_t>> &fixed, | ||
| std::vector<std::vector<QuadBezier>> &results, float tolerance) { | ||
| for (size_t i = 0; i < chains.size(); ++i) { | ||
| const std::vector<Point> &chain = chains[i]; | ||
| const int n = static_cast<int>(chain.size()); | ||
| std::vector<QuadBezier> result; | ||
| if (n < 2) { | ||
| results.push_back(result); | ||
| continue; | ||
| } | ||
|
|
||
| // Segment boundaries: chain ends plus every interior junction point. | ||
| std::vector<int> bounds; | ||
| bounds.push_back(0); | ||
| for (int k = 1; k < n - 1; ++k) | ||
| if (k < static_cast<int>(fixed[i].size()) && fixed[i][k]) bounds.push_back(k); | ||
| bounds.push_back(n - 1); | ||
|
|
||
| // Fit each [bounds[s], bounds[s+1]] piece; consecutive pieces share the | ||
| // junction point, so the curve stays continuous and pinned there. | ||
| for (size_t s = 0; s + 1 < bounds.size(); ++s) { | ||
| const int a = bounds[s], b = bounds[s + 1]; | ||
| std::vector<Point> seg(chain.begin() + a, chain.begin() + b + 1); | ||
| fitRecursive(seg, tolerance, result); | ||
| } | ||
| results.push_back(result); | ||
| } | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate that The function assumes 🛡️ Proposed validation void fit_curve_reduction(const std::vector<std::vector<Point>> &chains,
const std::vector<std::vector<uint8_t>> &fixed,
std::vector<std::vector<QuadBezier>> &results, float tolerance) {
+ if (fixed.size() != chains.size()) {
+ throw std::invalid_argument("fixed and chains must have the same outer dimension");
+ }
for (size_t i = 0; i < chains.size(); ++i) {
const std::vector<Point> &chain = chains[i];
const int n = static_cast<int>(chain.size());
+ if (i < fixed.size() && fixed[i].size() != static_cast<size_t>(n)) {
+ throw std::invalid_argument("fixed[i] and chains[i] must have the same size");
+ }
std::vector<QuadBezier> result;🤖 Prompt for AI Agents
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Krasner please check this out. I'm not sure if this is even likely.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
Uh oh!
There was an error while loading. Please reload this page.