feat(svg-builder): build SVGs after Suzuki-Abe topology extraction

[Krasner]

What was changed & why

Reworks SVG contour export. With the Graph and Node structure, currently contours are computed per node using the Suzuki-Abe method. While these contours are correct, neighboring regions will have non-overlapping contours - they will be off by 1 pixel. For SVG this is a problem - neighboring contours will show up doubled up.

We need a method to merge neighboring contours into one line.

Fixes:

Changes

1. Move contour management to Graph - it will call nodes to compute their own contours, then will manage merging.
   https://github.com/Krasner/Img2Num/blob/41ce003750ce8902473ec547df3b577754a3c30f/src/wasm/modules/image/src/kmeans_graph.cpp#L221

2. After the nodes compute their contours, they are laid out on a binary image where the double borders will be apparent. Use skeletonization to thin all borders to 1 pixel thickness
   https://github.com/Krasner/Img2Num/blob/41ce003750ce8902473ec547df3b577754a3c30f/src/wasm/modules/image/src/graph.cpp#L165

3. For each node, we must adjust its contour to match these thinned boundaries. We have to determine which contour pixels got removed and/or shifted, and so we must search for the nearest skeletonized pixel
   https://github.com/Krasner/Img2Num/blob/41ce003750ce8902473ec547df3b577754a3c30f/src/wasm/modules/image/src/graph.cpp#L179

3a. To do this we take the binary representation of each Node (n->create_binary_image) and we figure out how to add more pixels to its boundary so that the resultant contour will fall along the skeletonized borders.
https://github.com/Krasner/Img2Num/blob/41ce003750ce8902473ec547df3b577754a3c30f/src/wasm/modules/image/src/graph.cpp#L261

3b. once the node binaries are updated, we recompute contours - these should be VERY close to the skeletonized borders.
https://github.com/Krasner/Img2Num/blob/41ce003750ce8902473ec547df3b577754a3c30f/src/wasm/modules/image/src/graph.cpp#L384

What we have done at this point is force neighboring regions to produce contour lines that overlap on the same pixels. For SVG this will remove the double border problem.

4. However some holes still exist! We correct this as a packing problem. Envision all regions are in a 2D box. We want to maximally inflate each region to tightly press along neighboring regions. We can additionally apply contour smoothing here. To prevent holes, neighboring contours MUST follow each other EXACTLY!
   https://github.com/Krasner/Img2Num/blob/41ce003750ce8902473ec547df3b577754a3c30f/src/wasm/modules/image/src/graph.cpp#L424

5. We update node contours with these new smoothed and packed contours
   https://github.com/Krasner/Img2Num/blob/41ce003750ce8902473ec547df3b577754a3c30f/src/wasm/modules/image/src/graph.cpp#L435

6. We export these contours to the SVG exporter
   https://github.com/Krasner/Img2Num/blob/41ce003750ce8902473ec547df3b577754a3c30f/src/wasm/modules/image/src/kmeans_graph.cpp#L224

Testing & Verification

Additional Resources

[vercel]

@Krasner is attempting to deploy a commit to the Ryan Millard's projects Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

• 🔍 Trigger a full review

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Krasner]

cow_contours.txt

Look at the attached file. For the cow test image, this is the binary plot of the node contours before and after skeletonization

[Ryan-Millard]

Thank you for this. I'm having a look at it now, but it might take a day or two because I don't have as much spare time as I used to.

How did you generate that txt file? It shows the difference very clearly.

My interpretation of the problem you fixed

Important

Please let me know if this is correct.

_{For the purpose of this explanation, I will use straight lines because they are less complex than other types}

The parallel lines below show the types of lines that are yielded after contour tracing using Suzuki-Abe. Since the contour / border tracing maps the outermost pixels of a contiguous region (a K-means cluster in Img2Num's context), shared border sections are effectively traced twice - once for region A and the once for region B, where one border section will typically take a concave shape and the other a convex shape.

xychart-beta
    x-axis "x"
    y-axis "y"
    line "A" [5, 12]
    line "B" [5.1, 12.1]

Loading

As a result of this duplication (which is still logically sound), mapping the contours to SVG paths results in gaps between the neighbor regions because the borders in that context differ to those in raster contexts. In other terms, raster borders are not actually shared like they are in SVG images - each touching neighbor's border has different coordinates (offset by 1px in x / y direction) in raster space but SVG space expects them to be exactly identical to render them in the same way. The SVGs below show my understanding of it:

Using raster borders

View code

<svg viewBox="0 0 99 10" xmlns="http://www.w3.org/2000/svg">
  <!-- Background -->
  <rect x="0" y="0" width="99" height="10" fill="#eee"/>

  <!-- First rectangle, top-left -->
  <path d="M 0 0 L 49 0 L 49 10 L 0 10 Z" fill="yellow"/>
  
  <!-- Second rectangle, with 1px gap -->
  <path d="M 50 0 L 99 0 L 99 10 L 50 10 Z" fill="blue"/>

  <!-- Plot the corners & coordinates -->
  <circle cx="0" cy="0" r="0.8" fill="red"/>
  <circle cx="49" cy="0" r="0.8" fill="red"/>
  <circle cx="49" cy="10" r="0.8" fill="red"/>
  <circle cx="0" cy="10" r="0.8" fill="red"/>
  
  <circle cx="50" cy="0" r="0.8" fill="green"/>
  <circle cx="99" cy="0" r="0.8" fill="green"/>
  <circle cx="99" cy="10" r="0.8" fill="green"/>
  <circle cx="50" cy="10" r="0.8" fill="green"/>

  <text x="1" y="3" font-size="2" fill="black">(0,0)</text>
  <text x="43" y="3" font-size="2" fill="black">(49,0)</text>
  <text x="42" y="9" font-size="2" fill="black">(49,10)</text>
  <text x="1" y="9" font-size="2" fill="black">(0,10)</text>

  <text x="51" y="3" font-size="2" fill="black">(50,0)</text>
  <text x="93" y="3" font-size="2" fill="black">(99,0)</text>
  <text x="92" y="9" font-size="2" fill="black">(99,10)</text>
  <text x="51" y="9" font-size="2" fill="black">(50,10)</text>
</svg>

Using SVG shared borders

View code

<svg viewBox="0 0 99 10" xmlns="http://www.w3.org/2000/svg">
  <!-- Background -->
  <rect x="0" y="0" width="99" height="10" fill="#eee"/>

  <!-- First rectangle, top-left -->
  <path d="M 0 0 L 49 0 L 49 10 L 0 10 Z" fill="yellow"/>
  
  <!-- Second rectangle, touching the first -->
  <path d="M 49 0 L 99 0 L 99 10 L 49 10 Z" fill="blue"/>

  <!-- Plot the corners & coordinates -->
  <circle cx="0" cy="0" r="0.8" fill="red"/>
  <circle cx="0" cy="10" r="0.8" fill="red"/>

  <circle cx="49" cy="0" r="0.8" fill="brown"/>
  <circle cx="49" cy="10" r="0.8" fill="brown"/>
  
  <circle cx="99" cy="0" r="0.8" fill="green"/>
  <circle cx="99" cy="10" r="0.8" fill="green"/>

  <!-- Coordinate labels inside the corners -->
  <text x="1" y="3" font-size="2" fill="black">(0,0)</text>
  <text x="43" y="3" font-size="2" fill="black">(49,0)</text>
  <text x="42" y="9" font-size="2" fill="black">(49,10)</text>
  <text x="1" y="9" font-size="2" fill="black">(0,10)</text>

  <text x="50" y="3" font-size="2" fill="black">(49,0)</text>
  <text x="93" y="3" font-size="2" fill="black">(99,0)</text>
  <text x="92" y="9" font-size="2" fill="black">(99,10)</text>
  <text x="50" y="9" font-size="2" fill="black">(49,10)</text>
</svg>

[Krasner]

yes your understanding is correct.

another way to think about it, we present contours as (x,y) points. but pixels are actually squares, so the points we are interested in are going through the middle of the squares. that means there is a half-pixel distance from the contour to the pixel edge.

therefore for two regions that touch the actual contour points are 1 pixel away. SVG is points and images are pixels.

[Krasner]

for testing... locally I added a folder src/wasm/modules/image/test/
there I create small c++ programs to test functions and then compile with g++

g++ -o test ./test.cpp

that's where I generated the contours plots

[Ryan-Millard]

for testing... locally I added a folder src/wasm/modules/image/test/
there I create small c++ programs to test functions and then compile with g++

g++ -o test ./test.cpp

that's where I generated the contours plots

I haven't used another compiler with this code yet, so it's good to know that there weren't any problems. I thought Emscripten was too permissive at times, but I was luckily wrong about that.

We need to add a full testing suite for this library once we have a basics complete.

[Ryan-Millard]

yes your understanding is correct.

another way to think about it, we present contours as (x,y) points. but pixels are actually squares, so the points we are interested in are going through the middle of the squares. that means there is a half-pixel distance from the contour to the pixel edge.

therefore for two regions that touch the actual contour points are 1 pixel away. SVG is points and images are pixels.

Thank you.

How are you managing the shared border? Are you choosing the leftmost/topmost one or are you using something smarter?

[Krasner]

yes your understanding is correct.
another way to think about it, we present contours as (x,y) points. but pixels are actually squares, so the points we are interested in are going through the middle of the squares. that means there is a half-pixel distance from the contour to the pixel edge.
therefore for two regions that touch the actual contour points are 1 pixel away. SVG is points and images are pixels.

Thank you.

How are you managing the shared border? Are you choosing the leftmost/topmost one or are you using something smarter?

That's the skeletonization step. It will chose some optimal pixel to remove based on other pixels nearby

[Krasner]

this is a nice SVG resource https://www.joshwcomeau.com/svg/interactive-guide-to-paths/
going to see if our path based svgs can be simplified with bezier curves instead

[Ryan-Millard]

@Krasner, I don't know so much about this... This seems to be very lossy and the output is quite far from the original - it looks like we ran SLIC on it. Do you know why it wants to create round borders so much?

[Krasner]

yes it's because we are asking KMeans for more colors than there are present in this image. If you ask for 2 colors I suspect you'll get better results.

To this end KMeans should run a better statistical analysis on the image colors (histogram maybe) to determine what the optimal number of colors should be... but this isn't easy either

[Ryan-Millard]

I'm not sure if this is the right approach because it creates tiny regions that defeats the purpose of the merging we have just before contour tracing:

[Ryan-Millard]

yes it's because we are asking KMeans for more colors than there are present in this image. If you ask for 2 colors I suspect you'll get better results.

To this end KMeans should run a better statistical analysis on the image colors (histogram maybe) to determine what the optimal number of colors should be... but this isn't easy either

I'll try that now.

[Krasner]

I'm not sure if this is the right approach because it creates tiny regions that defeats the purpose of the merging we have just before contour tracing:

yeah i've noticed that too... thinking about an approach to remove these weird holes but I think what i have now is in the correct direction

[Krasner]

2 colors

[Ryan-Millard]

2 colors

That looks good, but 4 colors would be closer to that image and 4 colors looks strange. Additionally, how would users know how many colors to use?

[Krasner]

very interesting... will take a look at what's causing these issues.
for kmeans maybe provide a slider where to user can see how the number of distinct colors affects results.

[Krasner]

if I remove the packWithBoundaryConstraints step then the results are less strange (4 colors) but still slightly unsatisfying

[Ryan-Millard]

That looks good. What does packWithBoundaryConstraints do exactly?

[Ryan-Millard]

@Krasner, I just noticed that the CIELAB implementation of K-Means on main doesn't work.

I'm busy with the monorepo refactor and noticed that the K-Means function only ever return 0,0,0255 for each pixel when 0 (CIELAB) is passed for color_space. I think those are the default values assigned to the pixels.

If you want to test it on my branch, run the below:

emcmake cmake -B build-wasm .
cmake --build build-wasm
cd example-apps/react-js
# whatever yarn's dev command is

You'll want to import kmeans from img2num and use it inside of example-apps/react-js/src/components/WasmImageProcessor.jsx.

[Krasner]

I just noticed that the CIELAB implementation of K-Means on main doesn't work.

I've been using CIELAB Kmeans on my branch and it works fine. Let me pull the main branch and see if anything changed

[Ryan-Millard]

I just noticed that the CIELAB implementation of K-Means on main doesn't work.

I've been using CIELAB Kmeans on my branch and it works fine. Let me pull the main branch and see if anything changed

Okay. I don't think it's possible for it to be a problem with my branch because it's 1 commit behind main and it isn't a C++-related commit.

[Krasner]

@Ryan-Millard just tested the main branch and it works too.
can't get the refactor/monorepo branch running yet

[Krasner]

```shell
cd example-apps/react-js
# whatever yarn's dev command is

what do i do after this? yarn run dev:debug doesn't work

npm run dev works but i get this error:

[plugin:vite:import-analysis] Failed to resolve import "img2num" from "src/components/WasmImageProcessor.jsx". Does the file exist?

/home/ubuntu/Img2Num/example-apps/react-js/src/components/WasmImageProcessor.jsx:4:32

4  |  import { Upload } from "lucide-react";
5  |  import { loadImageToUint8Array } from "@utils/image-utils"; // keep your existing loader
6  |  import { bilateralFilter } from "img2num";
   |                                   ^
7  |  import GlassCard from "@components/GlassCard";
8  |  import styles from "./WasmImageProcessor.module.css";

[Krasner]

anyways I dont think the kmeans issue is on my end

[Ryan-Millard]

```shell
cd example-apps/react-js
# whatever yarn's dev command is

what do i do after this? yarn run dev:debug doesn't work

npm run dev works but i get this error:

[plugin:vite:import-analysis] Failed to resolve import "img2num" from "src/components/WasmImageProcessor.jsx". Does the file exist?

/home/ubuntu/Img2Num/example-apps/react-js/src/components/WasmImageProcessor.jsx:4:32

4  |  import { Upload } from "lucide-react";
5  |  import { loadImageToUint8Array } from "@utils/image-utils"; // keep your existing loader
6  |  import { bilateralFilter } from "img2num";
   |                                   ^
7  |  import GlassCard from "@components/GlassCard";
8  |  import styles from "./WasmImageProcessor.module.css";

I forgot that you need to do yarn link ../../packages/js and probably yarn install

[Ryan-Millard]

anyways I dont think the kmeans issue is on my end

Damn. I wonder what I broke. I'll have to look deeper into it.

[Krasner]

@Ryan-Millard There is an error you introduced. I thought it was mine but on my branch it's not. Issue is in cielab_impl.h

on my branch (and on your main branch)

template <typename Tin, typename Tout>
void lab_to_rgb(const ImageLib::LABAPixel<Tin> &laba,
                ImageLib::RGBAPixel<Tout> &rgba) {

  lab_to_rgb<Tin, Tout>(laba.l, laba.a, laba.b, rgba.red, rgba.green,
                        rgba.blue);
  rgba.alpha = static_cast<Tout>(laba.alpha);
}

but on your branch (refactor/monorepo):

Fix in cielab_impl.h

template <typename Tin, typename Tout>
void lab_to_rgb(const ImageLib::LABAPixel<Tin> &laba,
                ImageLib::RGBAPixel<Tout> &rgba) {
  Tout r{rgba.red};
  Tout g{rgba.green};
  Tout b{rgba.blue};

  lab_to_rgb<Tin, Tout>(laba.l, laba.a, laba.b, r, g, b);
  # fix here!
  rgba.red = r;
  rgba.green = g;
  rgba.blue = b;
  
  rgba.alpha = static_cast<Tout>(laba.alpha);
}

looking at git blame this is something you added

[Ryan-Millard]

@Ryan-Millard There is an error you introduced. I thought it was mine but on my branch it's not. Issue is in cielab_impl.h

on my branch (and on your main branch)

template <typename Tin, typename Tout>
void lab_to_rgb(const ImageLib::LABAPixel<Tin> &laba,
                ImageLib::RGBAPixel<Tout> &rgba) {

  lab_to_rgb<Tin, Tout>(laba.l, laba.a, laba.b, rgba.red, rgba.green,
                        rgba.blue);
  rgba.alpha = static_cast<Tout>(laba.alpha);
}

but on your branch (refactor/monorepo):

Fix in cielab_impl.h

template <typename Tin, typename Tout>
void lab_to_rgb(const ImageLib::LABAPixel<Tin> &laba,
                ImageLib::RGBAPixel<Tout> &rgba) {
  Tout r{rgba.red};
  Tout g{rgba.green};
  Tout b{rgba.blue};

  lab_to_rgb<Tin, Tout>(laba.l, laba.a, laba.b, r, g, b);
  # fix here!
  rgba.red = r;
  rgba.green = g;
  rgba.blue = b;
  
  rgba.alpha = static_cast<Tout>(laba.alpha);
}

looking at git blame this is something you added

Thank you. I had no idea I even made that change.

[Krasner]

I think this PR is basically ready. I'm still not fully satisfied in the smoothness of curves, but that will have to be done in a separate PR at this point.

[Krasner]

i do have updates for smoother SVGs but they should probably be in a new PR.... this one is already too big

[Ryan-Millard]

I think this PR is basically ready. I'm still not fully satisfied in the smoothness of curves, but that will have to be done in a separate PR at this point.

It isn't quite ready yet. The documentation needs to be fixed.

[1] [webpackbar] ✔ Client: Compiled with some errors in 1.05m
[1] Error: MDX compilation failed for file "/usr/src/app/docs/docs/reference/wasm/modules/image/graph/overview.md"
[1] Cause: Markdown image with URL `./diagrams/diagram1.svg` in source file "docs/reference/wasm/modules/image/graph/overview.md" (11:1) couldn't be resolved to an existing local image file.
[1] To ignore this error, use the `siteConfig.markdown.hooks.onBrokenMarkdownImages` option, or apply the `pathname://` protocol to the broken image URLs.
[1] Details:
[1] Error: Markdown image with URL `./diagrams/diagram1.svg` in source file "docs/reference/wasm/modules/image/graph/overview.md" (11:1) couldn't be resolved to an existing local image file.
[1] To ignore this error, use the `siteConfig.markdown.hooks.onBrokenMarkdownImages` option, or apply the `pathname://` protocol to the broken image URLs.
[1]     at async Promise.all (index 0)
[1] client (webpack 5.103.0) compiled with 1 error

I'm going to apply the fixes now and do a last once-over.

[Ryan-Millard]

Are you interested in learning how to write better Docusaurus documentation? There is some work that needs to be done on the documentation site that I keep wanting to do because it probably blocks newcomers from onboarding easily, but I just never have the time for it.

[Krasner]

Are you interested in learning how to write better Docusaurus documentation?

yeah, do u have an easy explanation

[Ryan-Millard]

I don't like the overall styling of the C++ in this repository because some files don't conform. We need to discuss a good convention then stick to it no matter what. Over time, the code will grow and it may become difficult to manage because the code doesn't follow our own convention. That isn't a concern for this PR, though - I just wanted to mention it beforehand.

Thanks for the great work so far!

I just want to discuss the LABPixel struct before we merge this.

[Ryan-Millard]

Isn't this better for LABPixel because it ensures that NumberT is a floating point number and also ensures that colorDistance always returns the correct value?

Suggested change

	#ifndef LABPIXEL_H
	#define LABPIXEL_H
	#include "Pixel.h"
	#include <cmath>
	/*
	can support signed data types
	preferrably float or double
	*/
	namespace ImageLib {
	template <typename NumberT> struct LABPixel : public Pixel<NumberT> {
	// ----- Members -----
	NumberT l, a, b;
	constexpr LABPixel(NumberT l = 0, NumberT a = 0, NumberT b = 0)
	: l(l), a(a), b(b) {}
	// ----- Modifiers -----
	[[nodiscard]] inline bool operator==(const LABPixel &other) const {
	return l == other.l && a == other.a && b == other.b;
	}
	[[nodiscard]] inline bool operator!=(const LABPixel &other) const {
	return !(*this == other);
	}
	// ----- Utilities -----
	inline void setGray(NumberT new_luma) {
	l = new_luma;
	a = b = 0;
	}
	static inline float colorDistance(const LABPixel<NumberT> &a,
	const LABPixel<NumberT> &b) {
	LABPixel<float> af{static_cast<float>(a.l), static_cast<float>(a.a),
	static_cast<float>(a.b)};
	LABPixel<float> bf{static_cast<float>(b.l), static_cast<float>(b.a),
	static_cast<float>(b.b)};
	return std::sqrt((a.l - b.l) * (a.l - b.l) + (a.a - b.a) * (a.a - b.a) +
	(a.b - b.b) * (a.b - b.b));
	}
	} __attribute__((packed));
	} // namespace ImageLib
	#endif // LABPIXEL_H
	#ifndef LABPIXEL_H
	#define LABPIXEL_H
	#include "Pixel.h"
	#include <cmath>
	#include <type_traits>
	/*
	can support signed data types
	preferrably float or double
	*/
	namespace ImageLib {
	template <typename NumberT> struct LABPixel : public Pixel<NumberT> {
	static_assert(std::is_floating_point_v<NumberT>, "LABPixel requires floating point");
	// ----- Members -----
	NumberT l, a, b;
	constexpr LABPixel(NumberT l = 0, NumberT a = 0, NumberT b = 0)
	: l(l), a(a), b(b) {}
	// ----- Modifiers -----
	[[nodiscard]] inline bool operator==(const LABPixel &other) const {
	return l == other.l && a == other.a && b == other.b;
	}
	[[nodiscard]] inline bool operator!=(const LABPixel &other) const {
	return !(*this == other);
	}
	// ----- Utilities -----
	inline void setGray(NumberT new_luma) {
	l = new_luma;
	a = b = 0;
	}
	static NumberT colorDistance(const LABPixel& a, const LABPixel& b) {
	return std::sqrt(
	(a.l - b.l) * (a.l - b.l) +
	(a.a - b.a) * (a.a - b.a) +
	(a.b - b.b) * (a.b - b.b)
	);
	}
	} __attribute__((packed));
	} // namespace ImageLib
	#endif // LABPIXEL_H

[Krasner]

yes that makes sense. i suppose it's specific for LAB. For RGB for example no matter what the datatype is, uint8_t most likely, the colorDistance should be float so that output type is hardcoded.

[Ryan-Millard]

For RGB, we shouldn't ensure it is of a specific type because some functions rely on strange types (like K-Means that uses float).

If colorDistance always returns float in LABPixel, there will be a problem if NumberT is a double.

[Ryan-Millard]

Your work is absolutely brilliant.

[Krasner]

great image 😂

[Ryan-Millard]

Are you interested in learning how to write better Docusaurus documentation?

yeah, do u have an easy explanation

#214 could be a good one to get started on if you're interested in it. The Getting Started page should probably be excluded, though, because #250 is about to change everything that applies in that section.

A lot of people have mentioned that it is difficult to read through and know where to go. Some of the pages, like the usage page and the commit style page should just be entirely deleted because they're a waste of energy. For example, I usually squash PRs, which means that the person's commit messages don't matter at all.

Additionally, some people have said that the sidebar goes over the fold, which isn't great because it hides away more important documentation.