cbgfx: make the code more descriptive

This change makes the code in graphics.c more descriptive and readable.
Especially, it makes expressions for scale calculation look what they
are meant to do. It also includes:

- Rename variables (struct fraction, dim_org, etc.) for more consistency
- Add more input validation (div-by-zero, etc.)

BUG=chromium:502066
BRANCH=master
TEST=Tested on Samus
CQ-DEPEND=CL:304860

Change-Id: I2694912bb7b6017d5655de2fd655b95432addb22
Signed-off-by: Patrick Georgi <pgeorgi@google.com>
Original-Commit-Id: 0863dc3ee925d3a05c83c66397b19a57f5478ef3
Original-Change-Id: Id8e349b8e09082fb84c3e1a984617f916e16c518
Original-Signed-off-by: Daisuke Nojiri <dnojiri@chromium.org>
Original-Reviewed-on: https://chromium-review.googlesource.com/304861
Original-Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Reviewed-on: http://review.coreboot.org/11928
Tested-by: build bot (Jenkins)
This commit is contained in:
Daisuke Nojiri 2015-10-08 14:03:56 -07:00 committed by Patrick Georgi
parent a11e3ff160
commit 9aed1465d7
2 changed files with 162 additions and 157 deletions

View File

@ -45,18 +45,26 @@ static void add_vectors(struct vector *out,
out->y = v1->y + v2->y; out->y = v1->y + v2->y;
} }
static int is_valid_fraction(const struct fraction *f)
{
return f->d != 0;
}
/* /*
* Transform a vector: * Transform a vector:
* x' = x * a_x + offset_x * x' = x * a_x + offset_x
* y' = y * a_y + offset_y * y' = y * a_y + offset_y
*/ */
static void transform_vector(struct vector *out, static int transform_vector(struct vector *out,
const struct vector *in, const struct vector *in,
const struct scale *a, const struct scale *a,
const struct vector *offset) const struct vector *offset)
{ {
out->x = a->x.nume * in->x / a->x.deno + offset->x; if (!is_valid_fraction(&a->x) || !is_valid_fraction(&a->y))
out->y = a->y.nume * in->y / a->y.deno + offset->y; return CBGFX_ERROR_INVALID_PARAMETER;
out->x = a->x.n * in->x / a->x.d + offset->x;
out->y = a->y.n * in->y / a->y.d + offset->y;
return CBGFX_SUCCESS;
} }
/* /*
@ -162,12 +170,12 @@ int draw_box(const struct rect *box, const struct rgb_color *rgb)
struct vector p, t; struct vector p, t;
const uint32_t color = calculate_color(rgb); const uint32_t color = calculate_color(rgb);
const struct scale top_left_s = { const struct scale top_left_s = {
.x = { .nume = box->offset.x, .deno = CANVAS_SCALE, }, .x = { .n = box->offset.x, .d = CANVAS_SCALE, },
.y = { .nume = box->offset.y, .deno = CANVAS_SCALE, } .y = { .n = box->offset.y, .d = CANVAS_SCALE, }
}; };
const struct scale size_s = { const struct scale size_s = {
.x = { .nume = box->size.x, .deno = CANVAS_SCALE, }, .x = { .n = box->size.x, .d = CANVAS_SCALE, },
.y = { .nume = box->size.y, .deno = CANVAS_SCALE, } .y = { .n = box->size.y, .d = CANVAS_SCALE, }
}; };
if (cbgfx_init()) if (cbgfx_init())
@ -220,33 +228,25 @@ int clear_screen(const struct rgb_color *rgb)
return CBGFX_SUCCESS; return CBGFX_SUCCESS;
} }
static int check_pixel_boundary(const struct vector *image, /*
const struct bitmap_header_v3 *header, * Bi-linear Interpolation
const struct scale *scale) *
{ * It estimates the value of a middle point (tx, ty) using the values from four
struct vector p = { * adjacent points (q00, q01, q10, q11).
.x = (image->width - 1) * scale->x.deno / scale->x.nume, */
.y = (image->height -1) * scale->y.deno / scale->y.nume,
};
struct rect bound = {
.offset = vzero,
.size = { .width = header->width, .height = header->height, },
};
return within_box(&p, &bound) < 0;
}
static uint32_t bli(uint32_t q00, uint32_t q10, uint32_t q01, uint32_t q11, static uint32_t bli(uint32_t q00, uint32_t q10, uint32_t q01, uint32_t q11,
struct fraction *tx, struct fraction *ty) struct fraction *tx, struct fraction *ty)
{ {
uint32_t r0 = (tx->nume * q10 + (tx->deno - tx->nume) * q00) / tx->deno; uint32_t r0 = (tx->n * q10 + (tx->d - tx->n) * q00) / tx->d;
uint32_t r1 = (tx->nume * q11 + (tx->deno - tx->nume) * q01) / tx->deno; uint32_t r1 = (tx->n * q11 + (tx->d - tx->n) * q01) / tx->d;
uint32_t p = (ty->nume * r1 + (ty->deno - ty->nume) * r0) / ty->deno; uint32_t p = (ty->n * r1 + (ty->d - ty->n) * r0) / ty->d;
return p; return p;
} }
static int draw_bitmap_v3(const struct vector *top_left, static int draw_bitmap_v3(const struct vector *top_left,
const struct scale *scale, const struct scale *scale,
const struct vector *image, const struct vector *dim,
const struct vector *dim_org,
const struct bitmap_header_v3 *header, const struct bitmap_header_v3 *header,
const struct bitmap_palette_element_v3 *pal, const struct bitmap_palette_element_v3 *pal,
const uint8_t *pixel_array) const uint8_t *pixel_array)
@ -267,16 +267,12 @@ static int draw_bitmap_v3(const struct vector *top_left,
LOG("Unsupported bits per pixel: %d\n", bpp); LOG("Unsupported bits per pixel: %d\n", bpp);
return CBGFX_ERROR_BITMAP_FORMAT; return CBGFX_ERROR_BITMAP_FORMAT;
} }
if (scale->x.nume == 0 || scale->y.nume == 0) { if (scale->x.n == 0 || scale->y.n == 0) {
LOG("Scaling out of range\n"); LOG("Scaling out of range\n");
return CBGFX_ERROR_SCALE_OUT_OF_RANGE; return CBGFX_ERROR_SCALE_OUT_OF_RANGE;
} }
/* This check guarantees we will not try to read outside pixel data. */ const int32_t y_stride = ROUNDUP(dim_org->width * bpp / 8, 4);
if (check_pixel_boundary(image, header, scale))
return CBGFX_ERROR_SCALE_OUT_OF_RANGE;
const int32_t y_stride = ROUNDUP(header->width * bpp / 8, 4);
/* /*
* header->height can be positive or negative. * header->height can be positive or negative.
* *
@ -290,33 +286,39 @@ static int draw_bitmap_v3(const struct vector *top_left,
if (header->height < 0) { if (header->height < 0) {
dir = 1; dir = 1;
} else { } else {
p.y += image->height - 1; p.y += dim->height - 1;
dir = -1; dir = -1;
} }
/* /*
* Plot pixels scaled by the bilinear interpolation. We scan * Plot pixels scaled by the bilinear interpolation. We scan over the
* over the image on canvas (using d) and find the corresponding pixel * image on canvas (using d) and find the corresponding pixel in the
* in the bitmap data (using s). * bitmap data (using s0, s1).
*
* When d hits the right bottom corner, s0 also hits the right bottom
* corner of the pixel array because that's how scale->x and scale->y
* have been set. Since the pixel array size is already validated in
* parse_bitmap_header_v3, s0 is guranteed not to exceed pixel array
* boundary.
*/ */
struct vector s0, s1, d; struct vector s0, s1, d;
struct fraction tx, ty; struct fraction tx, ty;
for (d.y = 0; d.y < image->height; d.y++, p.y += dir) { for (d.y = 0; d.y < dim->height; d.y++, p.y += dir) {
s0.y = d.y * scale->y.deno / scale->y.nume; s0.y = d.y * scale->y.d / scale->y.n;
s1.y = s0.y; s1.y = s0.y;
if (s0.y + 1 < ABS(header->height)) if (s0.y + 1 < dim_org->height)
s1.y++; s1.y++;
ty.deno = scale->y.nume; ty.d = scale->y.n;
ty.nume = (d.y * scale->y.deno) % scale->y.nume; ty.n = (d.y * scale->y.d) % scale->y.n;
const uint8_t *data0 = pixel_array + s0.y * y_stride; const uint8_t *data0 = pixel_array + s0.y * y_stride;
const uint8_t *data1 = pixel_array + s1.y * y_stride; const uint8_t *data1 = pixel_array + s1.y * y_stride;
p.x = top_left->x; p.x = top_left->x;
for (d.x = 0; d.x < image->width; d.x++, p.x++) { for (d.x = 0; d.x < dim->width; d.x++, p.x++) {
s0.x = d.x * scale->x.deno / scale->x.nume; s0.x = d.x * scale->x.d / scale->x.n;
s1.x = s0.x; s1.x = s0.x;
if (s1.x + 1 < header->width) if (s1.x + 1 < dim_org->width)
s1.x++; s1.x++;
tx.deno = scale->x.nume; tx.d = scale->x.n;
tx.nume = (d.x * scale->x.deno) % scale->x.nume; tx.n = (d.x * scale->x.d) % scale->x.n;
uint8_t c00 = data0[s0.x]; uint8_t c00 = data0[s0.x];
uint8_t c10 = data0[s1.x]; uint8_t c10 = data0[s1.x];
uint8_t c01 = data1[s0.x]; uint8_t c01 = data1[s0.x];
@ -370,18 +372,27 @@ static int get_bitmap_file_header(const void *bitmap, size_t size,
return CBGFX_SUCCESS; return CBGFX_SUCCESS;
} }
static int parse_bitmap_header_v3(const uint8_t *bitmap, static int parse_bitmap_header_v3(
const struct bitmap_file_header *file_header, const uint8_t *bitmap,
size_t size,
/* ^--- IN / OUT ---v */ /* ^--- IN / OUT ---v */
struct bitmap_header_v3 *header, struct bitmap_header_v3 *header,
const struct bitmap_palette_element_v3 **palette, const struct bitmap_palette_element_v3 **palette,
const uint8_t **pixel_array) const uint8_t **pixel_array,
struct vector *dim_org)
{ {
struct bitmap_file_header file_header;
struct bitmap_header_v3 *h; struct bitmap_header_v3 *h;
int rv;
rv = get_bitmap_file_header(bitmap, size, &file_header);
if (rv)
return rv;
size_t header_offset = sizeof(struct bitmap_file_header); size_t header_offset = sizeof(struct bitmap_file_header);
size_t header_size = sizeof(struct bitmap_header_v3); size_t header_size = sizeof(struct bitmap_header_v3);
size_t palette_offset = header_offset + header_size; size_t palette_offset = header_offset + header_size;
size_t file_size = file_header->file_size; size_t file_size = file_header.file_size;
h = (struct bitmap_header_v3 *)(bitmap + header_offset); h = (struct bitmap_header_v3 *)(bitmap + header_offset);
header->header_size = le32toh(h->header_size); header->header_size = le32toh(h->header_size);
@ -389,15 +400,23 @@ static int parse_bitmap_header_v3(const uint8_t *bitmap,
LOG("Unsupported bitmap format\n"); LOG("Unsupported bitmap format\n");
return CBGFX_ERROR_BITMAP_FORMAT; return CBGFX_ERROR_BITMAP_FORMAT;
} }
header->width = le32toh(h->width); header->width = le32toh(h->width);
header->height = le32toh(h->height); header->height = le32toh(h->height);
if (header->width == 0 || header->height == 0) {
LOG("Invalid image width or height\n");
return CBGFX_ERROR_BITMAP_DATA;
}
dim_org->width = header->width;
dim_org->height = ABS(header->height);
header->bits_per_pixel = le16toh(h->bits_per_pixel); header->bits_per_pixel = le16toh(h->bits_per_pixel);
header->compression = le32toh(h->compression); header->compression = le32toh(h->compression);
header->size = le32toh(h->size); header->size = le32toh(h->size);
header->colors_used = le32toh(h->colors_used); header->colors_used = le32toh(h->colors_used);
size_t palette_size = header->colors_used size_t palette_size = header->colors_used
* sizeof(struct bitmap_palette_element_v3); * sizeof(struct bitmap_palette_element_v3);
size_t pixel_offset = file_header->bitmap_offset; size_t pixel_offset = file_header.bitmap_offset;
if (pixel_offset > file_size) { if (pixel_offset > file_size) {
LOG("Bitmap pixel data exceeds buffer boundary\n"); LOG("Bitmap pixel data exceeds buffer boundary\n");
return CBGFX_ERROR_BITMAP_DATA; return CBGFX_ERROR_BITMAP_DATA;
@ -410,8 +429,8 @@ static int parse_bitmap_header_v3(const uint8_t *bitmap,
palette_offset); palette_offset);
size_t pixel_size = header->size; size_t pixel_size = header->size;
if (pixel_size != header->height * if (pixel_size != dim_org->height *
ROUNDUP(header->width * header->bits_per_pixel / 8, 4)) { ROUNDUP(dim_org->width * header->bits_per_pixel / 8, 4)) {
LOG("Bitmap pixel array size does not match expected size\n"); LOG("Bitmap pixel array size does not match expected size\n");
return CBGFX_ERROR_BITMAP_DATA; return CBGFX_ERROR_BITMAP_DATA;
} }
@ -424,58 +443,51 @@ static int parse_bitmap_header_v3(const uint8_t *bitmap,
return CBGFX_SUCCESS; return CBGFX_SUCCESS;
} }
static int calculate_scale(const struct bitmap_header_v3 *header, /*
* This calculates the dimension of the image projected on the canvas from the
* dimension relative to the canvas size. If either width or height is zero, it
* is derived from the other (non-zero) value to keep the aspect ratio.
*/
static int calculate_dimension(const struct vector *dim_org,
const struct scale *dim_rel, const struct scale *dim_rel,
struct scale *scale)
{
if (dim_rel->x.nume == 0 && dim_rel->y.nume == 0)
return CBGFX_ERROR_INVALID_PARAMETER;
if (dim_rel->x.nume > dim_rel->x.deno
|| dim_rel->y.nume > dim_rel->y.deno)
return CBGFX_ERROR_INVALID_PARAMETER;
if (dim_rel->x.nume > 0) {
scale->x.nume = dim_rel->x.nume * canvas.size.width;
scale->x.deno = header->width * dim_rel->x.deno;
}
if (dim_rel->y.nume > 0) {
scale->y.nume = dim_rel->y.nume * canvas.size.height;
scale->y.deno = header->height * dim_rel->y.deno;
}
if (dim_rel->y.nume == 0) {
scale->y.nume = scale->x.nume;
scale->y.deno = scale->x.deno;
}
if (dim_rel->x.nume == 0) {
scale->x.nume = scale->y.nume;
scale->x.deno = scale->y.deno;
}
if (scale->x.deno == 0 || scale->y.deno == 0)
return CBGFX_ERROR_INVALID_PARAMETER;
return CBGFX_SUCCESS;
}
static void calculate_dimention(const struct bitmap_header_v3 *header,
const struct scale *scale,
struct vector *dim) struct vector *dim)
{ {
dim->width = header->width; if (dim_rel->x.n == 0 && dim_rel->y.n == 0)
dim->height = ABS(header->height); return CBGFX_ERROR_INVALID_PARAMETER;
transform_vector(dim, dim, scale, &vzero);
if (dim_rel->x.n > dim_rel->x.d || dim_rel->y.n > dim_rel->y.d)
return CBGFX_ERROR_INVALID_PARAMETER;
if (dim_rel->x.n > 0) {
if (!is_valid_fraction(&dim_rel->x))
return CBGFX_ERROR_INVALID_PARAMETER;
dim->width = canvas.size.width * dim_rel->x.n / dim_rel->x.d;
}
if (dim_rel->y.n > 0) {
if (!is_valid_fraction(&dim_rel->y))
return CBGFX_ERROR_INVALID_PARAMETER;
dim->height = canvas.size.height * dim_rel->y.n / dim_rel->y.d;
}
/* Derive height from width using aspect ratio */
if (dim_rel->y.n == 0)
dim->height = dim->width * dim_org->height / dim_org->width;
/* Derive width from height using aspect ratio */
if (dim_rel->x.n == 0)
dim->width = dim->height * dim_org->width / dim_org->height;
return CBGFX_SUCCESS;
} }
static int caclcuate_position(const struct vector *dim, static int caclcuate_position(const struct vector *dim,
const struct scale *pos_rel, uint8_t pivot, const struct scale *pos_rel, uint8_t pivot,
struct vector *top_left) struct vector *top_left)
{ {
if (pos_rel->x.deno == 0 || pos_rel->y.deno == 0) int rv;
return CBGFX_ERROR_INVALID_PARAMETER;
transform_vector(top_left, &canvas.size, pos_rel, &canvas.offset); rv = transform_vector(top_left, &canvas.size, pos_rel, &canvas.offset);
if (rv)
return rv;
switch (pivot & PIVOT_H_MASK) { switch (pivot & PIVOT_H_MASK) {
case PIVOT_H_LEFT: case PIVOT_H_LEFT:
@ -506,28 +518,6 @@ static int caclcuate_position(const struct vector *dim,
return CBGFX_SUCCESS; return CBGFX_SUCCESS;
} }
static int setup_image(const struct bitmap_header_v3 *header,
const struct scale *dim_rel,
const struct scale *pos_rel, uint8_t pivot,
/* ^--- IN / OUT ---v */
struct scale *scale,
struct vector *dim,
struct vector *top_left)
{
int rv;
/* Calculate self-scale (scale relative to image size) */
rv = calculate_scale(header, dim_rel, scale);
if (rv)
return rv;
/* Calculate height and width of the image */
calculate_dimention(header, scale, dim);
/* Calculate coordinate */
return caclcuate_position(dim, pos_rel, pivot, top_left);
}
static int check_boundary(const struct vector *top_left, static int check_boundary(const struct vector *top_left,
const struct vector *dim, const struct vector *dim,
const struct rect *bound) const struct rect *bound)
@ -541,67 +531,83 @@ static int check_boundary(const struct vector *top_left,
return CBGFX_SUCCESS; return CBGFX_SUCCESS;
} }
static int do_draw_bitmap(const void *bitmap, size_t size, int draw_bitmap(const void *bitmap, size_t size,
const struct scale *pos_rel, const uint8_t pivot, const struct scale *pos_rel, uint8_t pivot,
const struct vector *position,
const struct scale *dim_rel) const struct scale *dim_rel)
{ {
struct bitmap_file_header file_header;
struct bitmap_header_v3 header; struct bitmap_header_v3 header;
const struct bitmap_palette_element_v3 *palette; const struct bitmap_palette_element_v3 *palette;
const uint8_t *pixel_array; const uint8_t *pixel_array;
struct vector top_left, dim; struct vector top_left, dim, dim_org;
struct scale scale; struct scale scale;
int rv; int rv;
if (cbgfx_init()) if (cbgfx_init())
return CBGFX_ERROR_INIT; return CBGFX_ERROR_INIT;
rv = get_bitmap_file_header(bitmap, size, &file_header);
if (rv)
return rv;
/* only v3 is supported now */ /* only v3 is supported now */
rv = parse_bitmap_header_v3(bitmap, &file_header, rv = parse_bitmap_header_v3(bitmap, size,
&header, &palette, &pixel_array); &header, &palette, &pixel_array, &dim_org);
if (rv) if (rv)
return rv; return rv;
/* Calculate image scale, dimention, and position */ /* Calculate height and width of the image */
if (position) { rv = calculate_dimension(&dim_org, dim_rel, &dim);
scale.x.nume = 1;
scale.x.deno = 1;
scale.y.nume = 1;
scale.y.deno = 1;
calculate_dimention(&header, &scale, &dim);
add_vectors(&top_left, position, &vzero);
rv = check_boundary(&top_left, &dim, &screen);
} else {
rv = setup_image(&header, dim_rel, pos_rel, pivot,
&scale, &dim, &top_left);
if (rv) if (rv)
return rv; return rv;
/* Calculate self scale */
scale.x.n = dim.width;
scale.x.d = dim_org.width;
scale.y.n = dim.height;
scale.y.d = dim_org.height;
/* Calculate coordinate */
rv = caclcuate_position(&dim, pos_rel, pivot, &top_left);
if (rv)
return rv;
rv = check_boundary(&top_left, &dim, &canvas); rv = check_boundary(&top_left, &dim, &canvas);
}
if (rv) { if (rv) {
LOG("Bitmap image exceeds screen or canvas boundary\n"); LOG("Bitmap image exceeds canvas boundary\n");
return rv; return rv;
} }
return draw_bitmap_v3(&top_left, &scale, &dim, return draw_bitmap_v3(&top_left, &scale, &dim, &dim_org,
&header, palette, pixel_array); &header, palette, pixel_array);
} }
int draw_bitmap(const void *bitmap, size_t size,
const struct scale *pos_rel, uint8_t pivot,
const struct scale *dim_rel)
{
return do_draw_bitmap(bitmap, size, pos_rel, pivot, NULL, dim_rel);
}
int draw_bitmap_direct(const void *bitmap, size_t size, int draw_bitmap_direct(const void *bitmap, size_t size,
const struct vector *position) const struct vector *top_left)
{ {
return do_draw_bitmap(bitmap, size, NULL, 0, position, NULL); struct bitmap_header_v3 header;
} const struct bitmap_palette_element_v3 *palette;
const uint8_t *pixel_array;
struct vector dim;
struct scale scale;
int rv;
if (cbgfx_init())
return CBGFX_ERROR_INIT;
/* only v3 is supported now */
rv = parse_bitmap_header_v3(bitmap, size,
&header, &palette, &pixel_array, &dim);
if (rv)
return rv;
/* Calculate self scale */
scale.x.n = 1;
scale.x.d = 1;
scale.y.n = 1;
scale.y.d = 1;
rv = check_boundary(top_left, &dim, &screen);
if (rv) {
LOG("Bitmap image exceeds screen boundary\n");
return rv;
}
return draw_bitmap_v3(top_left, &scale, &dim, &dim,
&header, palette, pixel_array);
}

View File

@ -30,8 +30,8 @@
#define CBGFX_ERROR_SCALE_OUT_OF_RANGE 0x13 #define CBGFX_ERROR_SCALE_OUT_OF_RANGE 0x13
struct fraction { struct fraction {
int32_t nume; int32_t n;
int32_t deno; int32_t d;
}; };
struct scale { struct scale {
@ -147,5 +147,4 @@ int draw_bitmap(const void *bitmap, size_t size,
* @return CBGFX_* error codes * @return CBGFX_* error codes
*/ */
int draw_bitmap_direct(const void *bitmap, size_t size, int draw_bitmap_direct(const void *bitmap, size_t size,
const struct vector *pos); const struct vector *top_left);