Skip to content

Line drawing, tests and circles improvement #378

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Aug 16, 2025
Merged

Line drawing, tests and circles improvement #378

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
59fad02
pbio/image: Implement line drawing.
schodet Jul 8, 2025
4768bf3
pbio/image: Add Image test.
schodet Aug 2, 2025
d27d6a3
pbio/image: Add more tests.
schodet Aug 4, 2025
8d9b7cd
pbio/image: Improve circle and disc shapes.
schodet Aug 16, 2025
700dbb5
pbio/image: Use [in] instead of [in,out] for image parameter.
schodet Aug 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/pbio/platform/test/pbioconfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#define PBIO_CONFIG_DCMOTOR (1)
#define PBIO_CONFIG_DCMOTOR_NUM_DEV (6)
#define PBIO_CONFIG_DRIVEBASE_SPIKE (0)
#define PBIO_CONFIG_IMAGE (1)
#define PBIO_CONFIG_IMU (0)
#define PBIO_CONFIG_LIGHT (1)
#define PBIO_CONFIG_LOGGER (1)
Expand Down
217 changes: 208 additions & 9 deletions lib/pbio/src/image/image.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,142 @@ void pbio_image_draw_vline(pbio_image_t *image, int x, int y, int l,
}
}

/**
* Draw a line with a flat slope (less or equal to 1).
* @param [in,out] image Image to draw into.
Copy link
Member

@dlech dlech Aug 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param [in,out] image Image to draw into.
* @param [in] image Image to draw into.

[in,out] would require pbio_image_t **image and would return a different pointer than the one that was passed in.

Copy link
Contributor Author

@schodet schodet Aug 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not agree, the image is read and written, so it’s both in and out.

Random people on the internet seems to agree: https://stackoverflow.com/questions/47732665/is-that-an-in-or-in-out-parameter-doxygen-c 🙂

BTW, I find those in/out redundant for C, it’s usually clear from the function prototype.

Copy link
Member

@dlech dlech Aug 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, there is room for interpretation. I learned C# before C, so I've always used the C# semantics for in/out, which are well defined.

BTW, I find those in/out redundant for C, it’s usually clear from the function prototype.

This is exactly why I don't consider [in,out] correct here. It is only useful for something like:

int read(void* data, size_t *size);

Where the precondition is that *size is set to the size of data in bytes and the post-condition is that *size is set to the number of bytes actually read, which may be less than the input size. Just going by the signature alone, one would normally expect size_t *size to be an out parameter so [in,out] makes it a bit more obvious that something unusual is going on.

Anyway, I'm not going to make a big deal about it. I don't think we've used this 100% consistently throughout the code anyway. But, in the majority of cases, we have only used [in] for cases where a struct is modified by a function. So it would be more consistent to stick with just [in].

Copy link
Member

@dlech dlech Aug 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, there is room for interpretation. I learned C# before C, so I've always used the C# semantics for in/out, which are well defined.

I guess for C# structs though, we would need to call it [in,out] like you are doing here since otherwise C# would pass the struct by value. 😄 It's just that normally something like the image type would be an object in C# rather than a struct. I consider passing a struct by reference in C like passing an object in C#. So, yeah, I get your point.

Copy link
Contributor Author

@schodet schodet Aug 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, made the change for the whole file.

* @param [in] x1 X coordinate of the first end.
* @param [in] y1 Y coordinate of the first end.
* @param [in] x2 X coordinate of the second end.
* @param [in] y2 Y coordinate of the second end.
* @param [in] value Pixel value.
*
* This is an internal function, x2 must be greater or equal to x1.
*
* Clipping: drawing is clipped to image dimensions.
*/
static void pbio_image_draw_line_flat(pbio_image_t *image, int x1, int y1,
int x2, int y2, uint8_t value) {
int dx = x2 - x1;
int dy = y2 - y1;
int ydir = 1;
int x, y, err;

// Fall back to horizontal line, much faster.
if (dy == 0) {
pbio_image_draw_hline(image, x1, y1, dx + 1, value);
return;
}

// Clipping, X out of image.
if (x1 >= image->width || x2 < 0) {
return;
}

// Check Y direction.
if (dy < 0) {
dy = -dy;
ydir = -1;
}

// Error is scaled by 2 * dx, offset with one half to look at mid point.
err = -dx;
y = y1;

// Skip pixels left of image.
if (x1 < 0) {
err += -x1 * dy * 2;
int yskip = (err + dx * 2) / (dx * 2);
err -= yskip * dx * 2;
y += yskip * ydir;
x1 = 0;
}

// Skip pixels right of image.
if (x2 >= image->width) {
x2 = image->width - 1;
}

// Draw.
x = x1;
do {
pbio_image_draw_pixel(image, x, y, value);
err += dy * 2;
if (err >= 0) {
err -= dx * 2;
y += ydir;
}
x++;
} while (x <= x2);
}

/**
* Draw a line with a steep slope (more than 1).
* @param [in,out] image Image to draw into.
Copy link
Member

@dlech dlech Aug 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param [in,out] image Image to draw into.
* @param [in] image Image to draw into.

And everywhere else too.

Copy link
Contributor Author

@schodet schodet Aug 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

* @param [in] x1 X coordinate of the first end.
* @param [in] y1 Y coordinate of the first end.
* @param [in] x2 X coordinate of the second end.
* @param [in] y2 Y coordinate of the second end.
* @param [in] value Pixel value.
*
* This is an internal function, y2 must be greater or equal to y1.
*
* Clipping: drawing is clipped to image dimensions.
*/
static void pbio_image_draw_line_steep(pbio_image_t *image, int x1, int y1,
int x2, int y2, uint8_t value) {
int dx = x2 - x1;
int dy = y2 - y1;
int xdir = 1;
int x, y, err;

// Fall back to vertical line, much faster.
if (dx == 0) {
pbio_image_draw_vline(image, x1, y1, dy + 1, value);
return;
}

// Clipping, Y out of image.
if (y1 >= image->height || y2 < 0) {
return;
}

// Check X direction.
if (dx < 0) {
dx = -dx;
xdir = -1;
}

// Error is scaled by 2 * dy, offset with one half to look at mid point.
err = -dy;
x = x1;

// Skip pixels above image.
if (y1 < 0) {
err += -y1 * dx * 2;
int xskip = (err + dy * 2) / (dy * 2);
err -= xskip * dy * 2;
x += xskip * xdir;
y1 = 0;
}

// Skip pixels bellow image.
if (y2 >= image->height) {
y2 = image->height - 1;
}

// Draw.
y = y1;
do {
pbio_image_draw_pixel(image, x, y, value);
err += dx * 2;
if (err >= 0) {
err -= dy * 2;
x += xdir;
}
y++;
} while (y <= y2);
}

/**
* Draw a line.
* @param [in,out] image Image to draw into.
Expand All @@ -275,7 +411,26 @@ void pbio_image_draw_vline(pbio_image_t *image, int x, int y, int l,
*/
void pbio_image_draw_line(pbio_image_t *image, int x1, int y1, int x2, int y2,
uint8_t value) {
// TODO
int dx = x2 - x1;
int dy = y2 - y1;
int abs_dx = dx < 0 ? -dx : dx;
int abs_dy = dy < 0 ? -dy : dy;

if (abs_dx >= abs_dy) {
// Flat slope, X always increasing.
if (dx > 0) {
pbio_image_draw_line_flat(image, x1, y1, x2, y2, value);
} else {
pbio_image_draw_line_flat(image, x2, y2, x1, y1, value);
}
} else {
// Steep slope, Y always increasing.
if (dy > 0) {
pbio_image_draw_line_steep(image, x1, y1, x2, y2, value);
} else {
pbio_image_draw_line_steep(image, x2, y2, x1, y1, value);
}
}
}

/**
Expand All @@ -289,7 +444,7 @@ void pbio_image_draw_line(pbio_image_t *image, int x1, int y1, int x2, int y2,
* @param [in] value Pixel value.
*
* When line thickness is odd, pixels are centered on the line. When even,
* line is thicker on one side.
* line is thicker on left side, when looking from first end to second end.
*
* Clipping: drawing is clipped to image dimensions.
*/
Expand All @@ -299,7 +454,47 @@ void pbio_image_draw_thick_line(pbio_image_t *image, int x1, int y1, int x2,
if (thickness <= 0) {
return;
}
// TODO

// Fall back to regular line.
if (thickness <= 1) {
pbio_image_draw_line(image, x1, y1, x2, y2, value);
return;
}

int dx = x2 - x1;
int dy = y2 - y1;
int abs_dx = dx < 0 ? -dx : dx;
int abs_dy = dy < 0 ? -dy : dy;
int i;
int offset = thickness / 2;

if (abs_dx >= abs_dy) {
// Flat slope, X always increasing.
if (dx > 0) {
for (i = 0; i < thickness; i++) {
pbio_image_draw_line_flat(image, x1, y1 + i - offset,
x2, y2 + i - offset, value);
}
} else {
for (i = 0; i < thickness; i++) {
pbio_image_draw_line_flat(image, x2, y2 - i + offset,
x1, y1 - i + offset, value);
}
}
} else {
// Steep slope, Y always increasing.
if (dy > 0) {
for (i = 0; i < thickness; i++) {
pbio_image_draw_line_steep(image, x1 - i + offset, y1,
x2 - i + offset, y2, value);
}
} else {
for (i = 0; i < thickness; i++) {
pbio_image_draw_line_steep(image, x2 + i - offset, y2,
x1 + i - offset, y1, value);
}
}
}
}

/**
Expand Down Expand Up @@ -498,7 +693,7 @@ void pbio_image_draw_circle(pbio_image_t *image, int x, int y, int r,

// Draw.
int dx = 0, dy = r;
int r2 = r * r;
int err = 5 - r * 4;
while (dx <= dy) {
pbio_image_draw_pixel(image, x + dx, y + dy, value);
pbio_image_draw_pixel(image, x - dx, y + dy, value);
Expand All @@ -508,10 +703,12 @@ void pbio_image_draw_circle(pbio_image_t *image, int x, int y, int r,
pbio_image_draw_pixel(image, x - dy, y + dx, value);
pbio_image_draw_pixel(image, x + dy, y - dx, value);
pbio_image_draw_pixel(image, x - dy, y - dx, value);
dx++;
if (dx * dx + dy * dy > r2) {
if (err > 0) {
dy--;
err -= dy * 8;
}
dx++;
err += 4 + dx * 8;
}
}

Expand All @@ -534,7 +731,7 @@ void pbio_image_fill_circle(pbio_image_t *image, int x, int y, int r,

// Draw.
int dx = 0, dy = r;
int r2 = r * r;
int err = 5 - r * 4;
while (dx <= dy) {
// Optimization opportunity: when dy is not moving, the same pixels
// are drawn again and again.
Expand All @@ -544,10 +741,12 @@ void pbio_image_fill_circle(pbio_image_t *image, int x, int y, int r,
pbio_image_draw_hline(image, x - dy, y + dx, 1 + 2 * dy, value);
pbio_image_draw_hline(image, x - dy, y - dx, 1 + 2 * dy, value);
}
dx++;
if (dx * dx + dy * dy > r2) {
if (err > 0) {
dy--;
err -= dy * 8;
}
dx++;
err += 4 + dx * 8;
}
}

Expand Down
Loading