From 772cd3e73f810c142ff3f7b45c3bad73146098e0 Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Mon, 22 Mar 2021 10:56:51 +0900 Subject: [PATCH 01/13] Improve stack usage for flood filling * Make flood filling logic iterative (vs recursive) I basically tried one-to-one conversions here to avoid mistakes. probably it has a room for later optimizations. * Use explicit malloc (vs variables on stack) to allocate the work area. * Estimate the amount of memory for the work area dynamically from the image size, instead of using a constant FLOOD_FILL_MAX_DEPTH, which is too big in the most cases. --- lib/identify.c | 117 ++++++++++++++++++++++++++++++------------- lib/quirc.c | 23 +++++++++ lib/quirc_internal.h | 14 ++++++ 3 files changed, 119 insertions(+), 35 deletions(-) diff --git a/lib/identify.c b/lib/identify.c index 3cf2d28..00c4dd4 100644 --- a/lib/identify.c +++ b/lib/identify.c @@ -122,52 +122,99 @@ static void perspective_unmap(const double *c, * Span-based floodfill routine */ -#define FLOOD_FILL_MAX_DEPTH 4096 - typedef void (*span_func_t)(void *user_data, int y, int left, int right); -static void flood_fill_seed(struct quirc *q, int x, int y, int from, int to, - span_func_t func, void *user_data, - int depth) +static void flood_fill_seed(struct quirc *q, + int x0, int y0, + int from, int to, + span_func_t func, void *user_data) { - int left = x; - int right = x; + struct quirc_flood_fill_vars *const stack = q->flood_fill_vars; + const size_t stack_size = q->num_flood_fill_vars; + const struct quirc_flood_fill_vars *const last_vars = + &stack[stack_size - 1]; + + struct quirc_flood_fill_vars *vars; + struct quirc_flood_fill_vars *next_vars; int i; - quirc_pixel_t *row = q->pixels + y * q->w; + quirc_pixel_t *row; - if (depth >= FLOOD_FILL_MAX_DEPTH) - return; + /* Set up the first context */ + next_vars = stack; + next_vars->x = x0; + next_vars->y = y0; - while (left > 0 && row[left - 1] == from) - left--; +call: + vars = next_vars; + vars->left = vars->x; + vars->right = vars->x; - while (right < q->w - 1 && row[right + 1] == from) - right++; + row = q->pixels + vars->y * q->w; + + while (vars->left > 0 && row[vars->left - 1] == from) + vars->left--; + + while (vars->right < q->w - 1 && row[vars->right + 1] == from) + vars->right++; /* Fill the extent */ - for (i = left; i <= right; i++) + for (i = vars->left; i <= vars->right; i++) row[i] = to; if (func) - func(user_data, y, left, right); + func(user_data, vars->y, vars->left, vars->right); - /* Seed new flood-fills */ - if (y > 0) { - row = q->pixels + (y - 1) * q->w; - - for (i = left; i <= right; i++) - if (row[i] == from) - flood_fill_seed(q, i, y - 1, from, to, - func, user_data, depth + 1); + if (vars == last_vars) { + return; } - if (y < q->h - 1) { - row = q->pixels + (y + 1) * q->w; + /* Seed new flood-fills */ + if (vars->y > 0) { + row = q->pixels + (vars->y - 1) * q->w; - for (i = left; i <= right; i++) - if (row[i] == from) - flood_fill_seed(q, i, y + 1, from, to, - func, user_data, depth + 1); + for (i = vars->left; i <= vars->right; i++) + if (row[i] == from) { + /* Save the current context */ + vars->i = i; + vars->pc = 1; + + /* Set up the next context */ + next_vars = vars + 1; + next_vars->x = i; + next_vars->y = vars->y - 1; + goto call; +return_from_call1: ; + } + } + + if (vars->y < q->h - 1) { + row = q->pixels + (vars->y + 1) * q->w; + + for (i = vars->left; i <= vars->right; i++) + if (row[i] == from) { + /* Save the current context */ + vars->i = i; + vars->pc = 2; + + /* Set up the next context */ + next_vars = vars + 1; + next_vars->x = i; + next_vars->y = vars->y + 1; + goto call; +return_from_call2: ; + } + } + + if (vars > stack) { + /* Restore the previous context */ + vars--; + i = vars->i; + if (vars->pc == 1) { + row = q->pixels + (vars->y - 1) * q->w; + goto return_from_call1; + } + row = q->pixels + (vars->y + 1) * q->w; + goto return_from_call2; } } @@ -260,7 +307,7 @@ static int region_code(struct quirc *q, int x, int y) box->seed.y = y; box->capstone = -1; - flood_fill_seed(q, x, y, pixel, region, area_count, box, 0); + flood_fill_seed(q, x, y, pixel, region, area_count, box); return region; } @@ -330,7 +377,7 @@ static void find_region_corners(struct quirc *q, psd.scores[0] = -1; flood_fill_seed(q, region->seed.x, region->seed.y, rcode, QUIRC_PIXEL_BLACK, - find_one_corner, &psd, 0); + find_one_corner, &psd); psd.ref.x = psd.corners[0].x - psd.ref.x; psd.ref.y = psd.corners[0].y - psd.ref.y; @@ -348,7 +395,7 @@ static void find_region_corners(struct quirc *q, flood_fill_seed(q, region->seed.x, region->seed.y, QUIRC_PIXEL_BLACK, rcode, - find_other_corners, &psd, 0); + find_other_corners, &psd); } static void record_capstone(struct quirc *q, int ring, int stone) @@ -966,10 +1013,10 @@ static void record_qr_grid(struct quirc *q, int a, int b, int c) flood_fill_seed(q, reg->seed.x, reg->seed.y, qr->align_region, QUIRC_PIXEL_BLACK, - NULL, NULL, 0); + NULL, NULL); flood_fill_seed(q, reg->seed.x, reg->seed.y, QUIRC_PIXEL_BLACK, qr->align_region, - find_leftmost_to_line, &psd, 0); + find_leftmost_to_line, &psd); } } diff --git a/lib/quirc.c b/lib/quirc.c index bbe1fe9..6108cfb 100644 --- a/lib/quirc.c +++ b/lib/quirc.c @@ -41,6 +41,7 @@ void quirc_destroy(struct quirc *q) same size, so we need to be careful here to avoid a double free */ if (!QUIRC_PIXEL_ALIAS_IMAGE) free(q->pixels); + free(q->flood_fill_vars); free(q); } @@ -48,6 +49,8 @@ int quirc_resize(struct quirc *q, int w, int h) { uint8_t *image = NULL; quirc_pixel_t *pixels = NULL; + size_t num_vars; + struct quirc_flood_fill_vars *vars = NULL; /* * XXX: w and h should be size_t (or at least unsigned) as negatives @@ -86,6 +89,22 @@ int quirc_resize(struct quirc *q, int w, int h) goto fail; } + /* + * alloc the work area for the flood filling logic. + * + * the size was chosen with the following assumptions and observations: + * + * - rings are the regions which requires the biggest work area. + * - they consumes the most when they are rotated by about 45 degree. + * in that case, the necessary depth is about (2 * height_of_the_ring). + * - the maximum height of rings would be about 1/3 of the image height. + */ + + num_vars = h * 2 / 3; + vars = malloc(sizeof(*vars) * num_vars); + if (!vars) + goto fail; + /* alloc succeeded, update `q` with the new size and buffers */ q->w = w; q->h = h; @@ -95,12 +114,16 @@ int quirc_resize(struct quirc *q, int w, int h) free(q->pixels); q->pixels = pixels; } + free(q->flood_fill_vars); + q->flood_fill_vars = vars; + q->num_flood_fill_vars = num_vars; return 0; /* NOTREACHED */ fail: free(image); free(pixels); + free(vars); return -1; } diff --git a/lib/quirc_internal.h b/lib/quirc_internal.h index 371572f..a94d8e9 100644 --- a/lib/quirc_internal.h +++ b/lib/quirc_internal.h @@ -17,6 +17,8 @@ #ifndef QUIRC_INTERNAL_H_ #define QUIRC_INTERNAL_H_ +#include + #include "quirc.h" #define QUIRC_PIXEL_WHITE 0 @@ -75,6 +77,15 @@ struct quirc_grid { double c[QUIRC_PERSPECTIVE_PARAMS]; }; +struct quirc_flood_fill_vars { + int x; + int y; + int right; + int left; + int i; + int pc; /* caller id */ +}; + struct quirc { uint8_t *image; quirc_pixel_t *pixels; @@ -89,6 +100,9 @@ struct quirc { int num_grids; struct quirc_grid grids[QUIRC_MAX_GRIDS]; + + size_t num_flood_fill_vars; + struct quirc_flood_fill_vars *flood_fill_vars; }; /************************************************************************ From f1dd37fbdb2a8c18c896f79aa48010d516b6cd1c Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Thu, 6 May 2021 09:54:00 +0900 Subject: [PATCH 02/13] quirc_resize: Make this a bit more careful about integer overflows Also, avoid malloc(0), which is not too portable. --- lib/quirc.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/quirc.c b/lib/quirc.c index 6108cfb..3cf75b9 100644 --- a/lib/quirc.c +++ b/lib/quirc.c @@ -50,6 +50,7 @@ int quirc_resize(struct quirc *q, int w, int h) uint8_t *image = NULL; quirc_pixel_t *pixels = NULL; size_t num_vars; + size_t vars_byte_size; struct quirc_flood_fill_vars *vars = NULL; /* @@ -100,8 +101,19 @@ int quirc_resize(struct quirc *q, int w, int h) * - the maximum height of rings would be about 1/3 of the image height. */ - num_vars = h * 2 / 3; - vars = malloc(sizeof(*vars) * num_vars); + if ((size_t)h * 2 / 2 != h) { + goto fail; /* size_t overflow */ + } + num_vars = (size_t)h * 2 / 3; + if (num_vars == 0) { + num_vars = 1; + } + + vars_byte_size = sizeof(*vars) * num_vars; + if (vars_byte_size / sizeof(*vars) != num_vars) { + goto fail; /* size_t overflow */ + } + vars = malloc(vars_byte_size); if (!vars) goto fail; From aad3fc63b6f7653f610462b8a35c2a69235be07d Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Tue, 11 May 2021 12:53:15 +0900 Subject: [PATCH 03/13] flood_fill_seed: separate a logic to fill a line --- lib/identify.c | 43 +++++++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/lib/identify.c b/lib/identify.c index 00c4dd4..d3631d0 100644 --- a/lib/identify.c +++ b/lib/identify.c @@ -124,6 +124,35 @@ static void perspective_unmap(const double *c, typedef void (*span_func_t)(void *user_data, int y, int left, int right); +static void flood_fill_line(struct quirc *q, int x, int y, + int from, int to, + int *leftp, int *rightp) +{ + quirc_pixel_t *row; + int left; + int right; + int i; + + left = x; + right = x; + + row = q->pixels + y * q->w; + + while (left > 0 && row[left - 1] == from) + left--; + + while (right < q->w - 1 && row[right + 1] == from) + right++; + + /* Fill the extent */ + for (i = left; i <= right; i++) + row[i] = to; + + /* Return the processed range */ + *leftp = left; + *rightp = right; +} + static void flood_fill_seed(struct quirc *q, int x0, int y0, int from, int to, @@ -146,20 +175,10 @@ static void flood_fill_seed(struct quirc *q, call: vars = next_vars; - vars->left = vars->x; - vars->right = vars->x; - - row = q->pixels + vars->y * q->w; - - while (vars->left > 0 && row[vars->left - 1] == from) - vars->left--; - - while (vars->right < q->w - 1 && row[vars->right + 1] == from) - vars->right++; /* Fill the extent */ - for (i = vars->left; i <= vars->right; i++) - row[i] = to; + flood_fill_line(q, vars->x, vars->y, from, to, &vars->left, + &vars->right); if (func) func(user_data, vars->y, vars->left, vars->right); From 40def012e77516e12b0fe685b441834bcdca1353 Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Tue, 11 May 2021 13:03:10 +0900 Subject: [PATCH 04/13] flood_fill_seed: Unify "left" and "x" vars This reduces the memory usage a bit. --- lib/identify.c | 8 ++++---- lib/quirc_internal.h | 1 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/identify.c b/lib/identify.c index d3631d0..90f41ff 100644 --- a/lib/identify.c +++ b/lib/identify.c @@ -170,14 +170,14 @@ static void flood_fill_seed(struct quirc *q, /* Set up the first context */ next_vars = stack; - next_vars->x = x0; + next_vars->left = x0; next_vars->y = y0; call: vars = next_vars; /* Fill the extent */ - flood_fill_line(q, vars->x, vars->y, from, to, &vars->left, + flood_fill_line(q, vars->left, vars->y, from, to, &vars->left, &vars->right); if (func) @@ -199,7 +199,7 @@ call: /* Set up the next context */ next_vars = vars + 1; - next_vars->x = i; + next_vars->left = i; next_vars->y = vars->y - 1; goto call; return_from_call1: ; @@ -217,7 +217,7 @@ return_from_call1: ; /* Set up the next context */ next_vars = vars + 1; - next_vars->x = i; + next_vars->left = i; next_vars->y = vars->y + 1; goto call; return_from_call2: ; diff --git a/lib/quirc_internal.h b/lib/quirc_internal.h index a94d8e9..ed23319 100644 --- a/lib/quirc_internal.h +++ b/lib/quirc_internal.h @@ -78,7 +78,6 @@ struct quirc_grid { }; struct quirc_flood_fill_vars { - int x; int y; int right; int left; From dd6c64cafec02aa9ebedf98b931487283575a938 Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Tue, 11 May 2021 13:22:54 +0900 Subject: [PATCH 05/13] Add QUIRC_ASSERT --- lib/quirc_internal.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/quirc_internal.h b/lib/quirc_internal.h index ed23319..9eec32a 100644 --- a/lib/quirc_internal.h +++ b/lib/quirc_internal.h @@ -17,10 +17,13 @@ #ifndef QUIRC_INTERNAL_H_ #define QUIRC_INTERNAL_H_ +#include #include #include "quirc.h" +#define QUIRC_ASSERT(a) assert(a) + #define QUIRC_PIXEL_WHITE 0 #define QUIRC_PIXEL_BLACK 1 #define QUIRC_PIXEL_REGION 2 From 1fa9b0c10931c5015fca73cdbe05799c3709f366 Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Tue, 11 May 2021 13:12:35 +0900 Subject: [PATCH 06/13] flood_fill_seed: Add assertions --- lib/identify.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/identify.c b/lib/identify.c index 90f41ff..41b87dd 100644 --- a/lib/identify.c +++ b/lib/identify.c @@ -133,11 +133,12 @@ static void flood_fill_line(struct quirc *q, int x, int y, int right; int i; + row = q->pixels + y * q->w; + QUIRC_ASSERT(row[x] == from); + left = x; right = x; - row = q->pixels + y * q->w; - while (left > 0 && row[left - 1] == from) left--; @@ -163,6 +164,9 @@ static void flood_fill_seed(struct quirc *q, const struct quirc_flood_fill_vars *const last_vars = &stack[stack_size - 1]; + QUIRC_ASSERT(from != to); + QUIRC_ASSERT(q->pixels[y0 * q->w + x0] == from); + struct quirc_flood_fill_vars *vars; struct quirc_flood_fill_vars *next_vars; int i; From 38f882b3d6bb33e6973a5506a40772d715a65a7f Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Tue, 11 May 2021 13:46:31 +0900 Subject: [PATCH 07/13] flood_fill_seed: Simplify the flow by having a single "return point". --- lib/identify.c | 51 +++++++++++++++++++++----------------------- lib/quirc_internal.h | 5 ++--- 2 files changed, 26 insertions(+), 30 deletions(-) diff --git a/lib/identify.c b/lib/identify.c index 41b87dd..f42fbea 100644 --- a/lib/identify.c +++ b/lib/identify.c @@ -169,75 +169,72 @@ static void flood_fill_seed(struct quirc *q, struct quirc_flood_fill_vars *vars; struct quirc_flood_fill_vars *next_vars; - int i; quirc_pixel_t *row; /* Set up the first context */ next_vars = stack; - next_vars->left = x0; + next_vars->left_up = x0; next_vars->y = y0; call: vars = next_vars; + /* + * Note about inputs: + * + * - vars->left_up contains x value to look at. + * - vars->left_down is still a garbage here. + */ + /* Fill the extent */ - flood_fill_line(q, vars->left, vars->y, from, to, &vars->left, + flood_fill_line(q, vars->left_up, vars->y, from, to, &vars->left_up, &vars->right); if (func) - func(user_data, vars->y, vars->left, vars->right); + func(user_data, vars->y, vars->left_up, vars->right); if (vars == last_vars) { return; } + vars->left_down = vars->left_up; + /* Seed new flood-fills */ +return_from_call: if (vars->y > 0) { row = q->pixels + (vars->y - 1) * q->w; - for (i = vars->left; i <= vars->right; i++) - if (row[i] == from) { - /* Save the current context */ - vars->i = i; - vars->pc = 1; - + while (vars->left_up <= vars->right) { + if (row[vars->left_up] == from) { /* Set up the next context */ next_vars = vars + 1; - next_vars->left = i; + next_vars->left_up = vars->left_up; next_vars->y = vars->y - 1; goto call; -return_from_call1: ; } + vars->left_up++; + } } if (vars->y < q->h - 1) { row = q->pixels + (vars->y + 1) * q->w; - for (i = vars->left; i <= vars->right; i++) - if (row[i] == from) { - /* Save the current context */ - vars->i = i; - vars->pc = 2; - + while (vars->left_down <= vars->right) { + if (row[vars->left_down] == from) { /* Set up the next context */ next_vars = vars + 1; - next_vars->left = i; + next_vars->left_up = vars->left_down; next_vars->y = vars->y + 1; goto call; -return_from_call2: ; } + vars->left_down++; + } } if (vars > stack) { /* Restore the previous context */ vars--; - i = vars->i; - if (vars->pc == 1) { - row = q->pixels + (vars->y - 1) * q->w; - goto return_from_call1; - } - row = q->pixels + (vars->y + 1) * q->w; - goto return_from_call2; + goto return_from_call; } } diff --git a/lib/quirc_internal.h b/lib/quirc_internal.h index 9eec32a..0d8a229 100644 --- a/lib/quirc_internal.h +++ b/lib/quirc_internal.h @@ -83,9 +83,8 @@ struct quirc_grid { struct quirc_flood_fill_vars { int y; int right; - int left; - int i; - int pc; /* caller id */ + int left_up; + int left_down; }; struct quirc { From 0b699408764f0d048b0d74c8f21c68bec9b4603c Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Tue, 11 May 2021 14:05:02 +0900 Subject: [PATCH 08/13] flood_fill_line: call user callback as well --- lib/identify.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/identify.c b/lib/identify.c index f42fbea..2377296 100644 --- a/lib/identify.c +++ b/lib/identify.c @@ -126,6 +126,7 @@ typedef void (*span_func_t)(void *user_data, int y, int left, int right); static void flood_fill_line(struct quirc *q, int x, int y, int from, int to, + span_func_t func, void *user_data, int *leftp, int *rightp) { quirc_pixel_t *row; @@ -152,6 +153,9 @@ static void flood_fill_line(struct quirc *q, int x, int y, /* Return the processed range */ *leftp = left; *rightp = right; + + if (func) + func(user_data, y, left, right); } static void flood_fill_seed(struct quirc *q, @@ -187,11 +191,9 @@ call: */ /* Fill the extent */ - flood_fill_line(q, vars->left_up, vars->y, from, to, &vars->left_up, - &vars->right); - - if (func) - func(user_data, vars->y, vars->left_up, vars->right); + flood_fill_line(q, vars->left_up, vars->y, from, to, + func, user_data, + &vars->left_up, &vars->right); if (vars == last_vars) { return; From eadbc2caca1bd0eb817d5fb2ce211a9ee15c63ec Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Tue, 11 May 2021 14:09:24 +0900 Subject: [PATCH 09/13] flood_fill_seed: Move flood_fill_line to the "caller" This restructure allows us to unify "call" and "return_from_call". --- lib/identify.c | 44 ++++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/lib/identify.c b/lib/identify.c index 2377296..fa2b9f5 100644 --- a/lib/identify.c +++ b/lib/identify.c @@ -180,29 +180,21 @@ static void flood_fill_seed(struct quirc *q, next_vars->left_up = x0; next_vars->y = y0; -call: - vars = next_vars; - - /* - * Note about inputs: - * - * - vars->left_up contains x value to look at. - * - vars->left_down is still a garbage here. - */ - /* Fill the extent */ - flood_fill_line(q, vars->left_up, vars->y, from, to, + flood_fill_line(q, next_vars->left_up, next_vars->y, from, to, func, user_data, - &vars->left_up, &vars->right); + &next_vars->left_up, &next_vars->right); + next_vars->left_down = next_vars->left_up; + +call: +return_from_call: + vars = next_vars; if (vars == last_vars) { return; } - vars->left_down = vars->left_up; - /* Seed new flood-fills */ -return_from_call: if (vars->y > 0) { row = q->pixels + (vars->y - 1) * q->w; @@ -212,6 +204,16 @@ return_from_call: next_vars = vars + 1; next_vars->left_up = vars->left_up; next_vars->y = vars->y - 1; + + /* Fill the extent */ + flood_fill_line(q, + next_vars->left_up, + next_vars->y, from, to, + func, user_data, + &next_vars->left_up, + &next_vars->right); + next_vars->left_down = next_vars->left_up; + goto call; } vars->left_up++; @@ -227,6 +229,16 @@ return_from_call: next_vars = vars + 1; next_vars->left_up = vars->left_down; next_vars->y = vars->y + 1; + + /* Fill the extent */ + flood_fill_line(q, + next_vars->left_up, + next_vars->y, from, to, + func, user_data, + &next_vars->left_up, + &next_vars->right); + next_vars->left_down = next_vars->left_up; + goto call; } vars->left_down++; @@ -235,7 +247,7 @@ return_from_call: if (vars > stack) { /* Restore the previous context */ - vars--; + next_vars = vars - 1; goto return_from_call; } } From a436fde8a08fa0ec6b0d0462e85436df7e076826 Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Tue, 11 May 2021 14:41:01 +0900 Subject: [PATCH 10/13] flood_fill_seed: Reduce code duplicatino This is also a preparation to eliminate the uses of goto. --- lib/identify.c | 92 +++++++++++++++++++++++++++++--------------------- 1 file changed, 54 insertions(+), 38 deletions(-) diff --git a/lib/identify.c b/lib/identify.c index fa2b9f5..29150b4 100644 --- a/lib/identify.c +++ b/lib/identify.c @@ -158,6 +158,48 @@ static void flood_fill_line(struct quirc *q, int x, int y, func(user_data, y, left, right); } +static struct quirc_flood_fill_vars *flood_fill_call_next( + struct quirc *q, + quirc_pixel_t *row, + int from, int to, + span_func_t func, void *user_data, + struct quirc_flood_fill_vars *vars, + int direction) +{ + int *leftp; + + if (direction < 0) { + leftp = &vars->left_up; + } else { + leftp = &vars->left_down; + } + + while (*leftp <= vars->right) { + if (row[*leftp] == from) { + struct quirc_flood_fill_vars *next_vars; + + /* Set up the next context */ + next_vars = vars + 1; + next_vars->left_up = *leftp; + next_vars->y = vars->y + direction; + + /* Fill the extent */ + flood_fill_line(q, + next_vars->left_up, + next_vars->y, + from, to, + func, user_data, + &next_vars->left_up, + &next_vars->right); + next_vars->left_down = next_vars->left_up; + + return next_vars; + } + (*leftp)++; + } + return NULL; +} + static void flood_fill_seed(struct quirc *q, int x0, int y0, int from, int to, @@ -198,50 +240,24 @@ return_from_call: if (vars->y > 0) { row = q->pixels + (vars->y - 1) * q->w; - while (vars->left_up <= vars->right) { - if (row[vars->left_up] == from) { - /* Set up the next context */ - next_vars = vars + 1; - next_vars->left_up = vars->left_up; - next_vars->y = vars->y - 1; - - /* Fill the extent */ - flood_fill_line(q, - next_vars->left_up, - next_vars->y, from, to, - func, user_data, - &next_vars->left_up, - &next_vars->right); - next_vars->left_down = next_vars->left_up; - - goto call; - } - vars->left_up++; + next_vars = flood_fill_call_next(q, row, + from, to, + func, user_data, + vars, -1); + if (next_vars != NULL) { + goto call; } } if (vars->y < q->h - 1) { row = q->pixels + (vars->y + 1) * q->w; - while (vars->left_down <= vars->right) { - if (row[vars->left_down] == from) { - /* Set up the next context */ - next_vars = vars + 1; - next_vars->left_up = vars->left_down; - next_vars->y = vars->y + 1; - - /* Fill the extent */ - flood_fill_line(q, - next_vars->left_up, - next_vars->y, from, to, - func, user_data, - &next_vars->left_up, - &next_vars->right); - next_vars->left_down = next_vars->left_up; - - goto call; - } - vars->left_down++; + next_vars = flood_fill_call_next(q, row, + from, to, + func, user_data, + vars, 1); + if (next_vars != NULL) { + goto call; } } From 87ca2b6c2db7061fdbd1d7e07978b0d8d6f6e46d Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Tue, 11 May 2021 14:48:55 +0900 Subject: [PATCH 11/13] flood_fill_seed: Switch from goto to while --- lib/identify.c | 68 ++++++++++++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/lib/identify.c b/lib/identify.c index 29150b4..3b69d36 100644 --- a/lib/identify.c +++ b/lib/identify.c @@ -16,6 +16,7 @@ #include #include +#include #include #include #include "quirc_internal.h" @@ -213,9 +214,7 @@ static void flood_fill_seed(struct quirc *q, QUIRC_ASSERT(from != to); QUIRC_ASSERT(q->pixels[y0 * q->w + x0] == from); - struct quirc_flood_fill_vars *vars; struct quirc_flood_fill_vars *next_vars; - quirc_pixel_t *row; /* Set up the first context */ next_vars = stack; @@ -228,43 +227,46 @@ static void flood_fill_seed(struct quirc *q, &next_vars->left_up, &next_vars->right); next_vars->left_down = next_vars->left_up; -call: -return_from_call: - vars = next_vars; + while (true) { + struct quirc_flood_fill_vars * const vars = next_vars; + quirc_pixel_t *row; - if (vars == last_vars) { - return; - } - - /* Seed new flood-fills */ - if (vars->y > 0) { - row = q->pixels + (vars->y - 1) * q->w; - - next_vars = flood_fill_call_next(q, row, - from, to, - func, user_data, - vars, -1); - if (next_vars != NULL) { - goto call; + if (vars == last_vars) { + break; } - } - if (vars->y < q->h - 1) { - row = q->pixels + (vars->y + 1) * q->w; + /* Seed new flood-fills */ + if (vars->y > 0) { + row = q->pixels + (vars->y - 1) * q->w; - next_vars = flood_fill_call_next(q, row, - from, to, - func, user_data, - vars, 1); - if (next_vars != NULL) { - goto call; + next_vars = flood_fill_call_next(q, row, + from, to, + func, user_data, + vars, -1); + if (next_vars != NULL) { + continue; + } } - } - if (vars > stack) { - /* Restore the previous context */ - next_vars = vars - 1; - goto return_from_call; + if (vars->y < q->h - 1) { + row = q->pixels + (vars->y + 1) * q->w; + + next_vars = flood_fill_call_next(q, row, + from, to, + func, user_data, + vars, 1); + if (next_vars != NULL) { + continue; + } + } + + if (vars > stack) { + /* Restore the previous context */ + next_vars = vars - 1; + continue; + } + + break; } } From 22269c9a43c93221bfd8f2fadbb6c13f84e1b60a Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Tue, 11 May 2021 14:53:23 +0900 Subject: [PATCH 12/13] flood_fill_seed: initialize left_up and left_down together Just for readability. --- lib/identify.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/identify.c b/lib/identify.c index 3b69d36..b6993f7 100644 --- a/lib/identify.c +++ b/lib/identify.c @@ -178,21 +178,22 @@ static struct quirc_flood_fill_vars *flood_fill_call_next( while (*leftp <= vars->right) { if (row[*leftp] == from) { struct quirc_flood_fill_vars *next_vars; + int next_left; /* Set up the next context */ next_vars = vars + 1; - next_vars->left_up = *leftp; next_vars->y = vars->y + direction; /* Fill the extent */ flood_fill_line(q, - next_vars->left_up, + *leftp, next_vars->y, from, to, func, user_data, - &next_vars->left_up, + &next_left, &next_vars->right); - next_vars->left_down = next_vars->left_up; + next_vars->left_down = next_left; + next_vars->left_up = next_left; return next_vars; } @@ -215,17 +216,18 @@ static void flood_fill_seed(struct quirc *q, QUIRC_ASSERT(q->pixels[y0 * q->w + x0] == from); struct quirc_flood_fill_vars *next_vars; + int next_left; /* Set up the first context */ next_vars = stack; - next_vars->left_up = x0; next_vars->y = y0; /* Fill the extent */ - flood_fill_line(q, next_vars->left_up, next_vars->y, from, to, + flood_fill_line(q, x0, next_vars->y, from, to, func, user_data, - &next_vars->left_up, &next_vars->right); - next_vars->left_down = next_vars->left_up; + &next_left, &next_vars->right); + next_vars->left_down = next_left; + next_vars->left_up = next_left; while (true) { struct quirc_flood_fill_vars * const vars = next_vars; From 6b3575cd59b54fca37408fff564bfbcfed42556c Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Tue, 11 May 2021 15:03:09 +0900 Subject: [PATCH 13/13] flood_fill_seed: Add comments on exit conditions --- lib/identify.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/identify.c b/lib/identify.c index b6993f7..44fa48d 100644 --- a/lib/identify.c +++ b/lib/identify.c @@ -234,6 +234,13 @@ static void flood_fill_seed(struct quirc *q, quirc_pixel_t *row; if (vars == last_vars) { + /* + * "Stack overflow". + * Just stop and return. + * This can be caused by very complex shapes in + * the image, which is not likely a part of + * a valid QR code anyway. + */ break; } @@ -268,6 +275,7 @@ static void flood_fill_seed(struct quirc *q, continue; } + /* We've done. */ break; } }