From c881616cfbbd8c20d67f469d363254b2c56a12b7 Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Wed, 27 Apr 2022 13:46:04 -0700 Subject: til_fb: til_fb_fragment_t.{pitch,stride} uint32_t units Originally it seemed sensible to make these units of bytes, for flexibility reasons. But it's advantageous for everything to be able to assume pixels are always 4-byte/32-bit aligned. Having the stride/pitch be in bytes of units made it theoretically possible to produce unaligned rows of pixels, which would break that assumption. I don't think anything was ever actually producing such things, and I've added some asserts to the {sdl,drm}_fb.c page acquisition code to go fatal on such pages. This change required going through all the modules and get rid of their uint32_t vs. void* dances and other such 1-byte vs. 4-byte scaling arithmetic. Code is simpler now, and probably faster in some cases. And now allows future work to just assume things cna always occur 4-bytes at a time without concern for unaligned accesses. --- src/drm_fb.c | 10 ++++++++-- src/libs/ray/ray_render.c | 2 +- src/modules/blinds/blinds.c | 4 ++-- src/modules/julia/julia.c | 2 +- src/modules/montage/montage.c | 4 ++-- src/modules/plasma/plasma.c | 2 +- src/modules/roto/roto.c | 3 +-- src/sdl_fb.c | 7 +++++-- src/til_fb.c | 7 +++---- src/til_fb.h | 10 +++++----- 10 files changed, 29 insertions(+), 22 deletions(-) (limited to 'src') diff --git a/src/drm_fb.c b/src/drm_fb.c index 328c92c..927f46c 100644 --- a/src/drm_fb.c +++ b/src/drm_fb.c @@ -476,6 +476,12 @@ static void * drm_fb_page_alloc(til_fb_t *fb, void *context, til_fb_page_t *res_ pexit_if(drmModeAddFB(c->drm_fd, c->mode->hdisplay, c->mode->vdisplay, 24, 32, create_dumb.pitch, create_dumb.handle, &fb_id) < 0, "unable to add dumb buffer"); + /* prevent unaligned pitches, we're just simplifying everything in rototiller that wants + * to do word-at-a-time operations without concern for arches that get angry when that happens + * on unaligned addresses. + */ + assert(!(create_dumb.pitch & 0x3)); + p->mmap = map; p->mmap_size = create_dumb.size; p->drm_dumb_handle = map_dumb.handle; @@ -486,8 +492,8 @@ static void * drm_fb_page_alloc(til_fb_t *fb, void *context, til_fb_page_t *res_ res_page->fragment.frame_width = c->mode->hdisplay; res_page->fragment.height = c->mode->vdisplay; res_page->fragment.frame_height = c->mode->vdisplay; - res_page->fragment.stride = create_dumb.pitch - (c->mode->hdisplay * 4); - res_page->fragment.pitch = create_dumb.pitch; + res_page->fragment.pitch = create_dumb.pitch >> 2; + res_page->fragment.stride = res_page->fragment.pitch - (c->mode->hdisplay); return p; } diff --git a/src/libs/ray/ray_render.c b/src/libs/ray/ray_render.c index 535b71a..9b36e6c 100644 --- a/src/libs/ray/ray_render.c +++ b/src/libs/ray/ray_render.c @@ -213,7 +213,7 @@ void ray_render_trace_fragment(ray_render_t *render, til_fb_fragment_t *fb_fragm buf++; } while (ray_camera_fragment_x_step(&fragment)); - buf = ((void *)buf) + fb_fragment->stride; + buf += fb_fragment->stride; } while (ray_camera_fragment_y_step(&fragment)); } diff --git a/src/modules/blinds/blinds.c b/src/modules/blinds/blinds.c index c0ae079..9996d8a 100644 --- a/src/modules/blinds/blinds.c +++ b/src/modules/blinds/blinds.c @@ -67,7 +67,7 @@ static inline void draw_blind_horizontal(til_fb_fragment_t *fragment, unsigned r unsigned height = roundf(t * (float)row_height); for (unsigned y = 0; y < height; y++) - memset(fragment->buf + ((row * row_height) + y ) * (fragment->pitch >> 2), 0xff, fragment->width * 4); + memset(fragment->buf + ((row * row_height) + y ) * fragment->pitch, 0xff, fragment->width * 4); } @@ -78,7 +78,7 @@ static inline void draw_blind_vertical(til_fb_fragment_t *fragment, unsigned col unsigned width = roundf(t * (float)column_width); for (unsigned y = 0; y < fragment->height; y++) - memset(fragment->buf + y * (fragment->pitch >> 2) + column * column_width, 0xff, width * 4); + memset(fragment->buf + y * fragment->pitch + column * column_width, 0xff, width * 4); } diff --git a/src/modules/julia/julia.c b/src/modules/julia/julia.c index fb3bdae..4b3cd73 100644 --- a/src/modules/julia/julia.c +++ b/src/modules/julia/julia.c @@ -155,7 +155,7 @@ static void julia_render_fragment(void *context, unsigned ticks, unsigned cpu, t *buf = colors[julia_iter(real, imag, ctxt->creal, ctxt->cimag, sizeof(colors) / sizeof(*colors), ctxt->threshold)]; } - buf = ((void *)buf) + fragment->stride; + buf += fragment->stride; } } diff --git a/src/modules/montage/montage.c b/src/modules/montage/montage.c index 13c8a36..e6f6ef1 100644 --- a/src/modules/montage/montage.c +++ b/src/modules/montage/montage.c @@ -144,14 +144,14 @@ static int montage_fragment_tile(const til_fb_fragment_t *fragment, unsigned til xoff = x * tile_width; yoff = y * tile_height; - res_fragment->buf = (void *)fragment->buf + (yoff * fragment->pitch) + (xoff * 4); + res_fragment->buf = fragment->buf + (yoff * fragment->pitch) + xoff; res_fragment->x = 0; /* fragment is a new frame */ res_fragment->y = 0; /* fragment is a new frame */ res_fragment->width = MIN(fragment->width - xoff, tile_width); res_fragment->height = MIN(fragment->height - yoff, tile_height); res_fragment->frame_width = res_fragment->width; /* fragment is a new frame */ res_fragment->frame_height = res_fragment->height; /* fragment is a new frame */ - res_fragment->stride = fragment->stride + ((fragment->width - res_fragment->width) * 4); + res_fragment->stride = fragment->stride + (fragment->width - res_fragment->width); res_fragment->pitch = fragment->pitch; res_fragment->number = number; res_fragment->cleared = fragment->cleared; diff --git a/src/modules/plasma/plasma.c b/src/modules/plasma/plasma.c index 81540c5..0a6b1e5 100644 --- a/src/modules/plasma/plasma.c +++ b/src/modules/plasma/plasma.c @@ -156,7 +156,7 @@ static void plasma_render_fragment(void *context, unsigned ticks, unsigned cpu, *buf = color2pixel(&c); } - buf = ((void *)buf) + fragment->stride; + buf += fragment->stride; } } diff --git a/src/modules/roto/roto.c b/src/modules/roto/roto.c index 5409970..65b3400 100644 --- a/src/modules/roto/roto.c +++ b/src/modules/roto/roto.c @@ -240,11 +240,10 @@ static void roto_render_fragment(void *context, unsigned ticks, unsigned cpu, ti x_sin_r += sin_r; } - buf = ((void *)buf) + fragment->stride; + buf += fragment->stride; y_cos_r += cos_r; y_sin_r += sin_r; } - } diff --git a/src/sdl_fb.c b/src/sdl_fb.c index 2605d6c..249d746 100644 --- a/src/sdl_fb.c +++ b/src/sdl_fb.c @@ -199,13 +199,16 @@ static void * sdl_fb_page_alloc(til_fb_t *fb, void *context, til_fb_page_t *res_ p->surface = SDL_CreateRGBSurface(0, c->width, c->height, 32, 0, 0, 0, 0); + /* rototiller wants to assume all pixels to be 32-bit aligned, so prevent unaligning pitches */ + assert(!(p->surface->pitch & 0x3)); + res_page->fragment.buf = p->surface->pixels; res_page->fragment.width = c->width; res_page->fragment.frame_width = c->width; res_page->fragment.height = c->height; res_page->fragment.frame_height = c->height; - res_page->fragment.stride = p->surface->pitch - (c->width * 4); - res_page->fragment.pitch = p->surface->pitch; + res_page->fragment.pitch = p->surface->pitch >> 2; + res_page->fragment.stride = res_page->fragment.pitch - c->width; return p; } diff --git a/src/til_fb.c b/src/til_fb.c index 5a6a956..eb14e27 100644 --- a/src/til_fb.c +++ b/src/til_fb.c @@ -395,12 +395,11 @@ int til_fb_fragment_slice_single(const til_fb_fragment_t *fragment, unsigned n_f { unsigned slice = fragment->height / n_fragments; unsigned yoff = slice * number; - unsigned pitch; if (yoff >= fragment->height) return 0; - res_fragment->buf = ((void *)fragment->buf) + yoff * fragment->pitch; + res_fragment->buf = fragment->buf + yoff * fragment->pitch; res_fragment->x = fragment->x; res_fragment->y = yoff; res_fragment->width = fragment->width; @@ -436,14 +435,14 @@ int til_fb_fragment_tile_single(const til_fb_fragment_t *fragment, unsigned tile xoff = x * tile_size; yoff = y * tile_size; - res_fragment->buf = (void *)fragment->buf + (yoff * fragment->pitch) + (xoff * 4); + res_fragment->buf = fragment->buf + (yoff * fragment->pitch) + (xoff); res_fragment->x = fragment->x + xoff; res_fragment->y = fragment->y + yoff; res_fragment->width = MIN(fragment->width - xoff, tile_size); res_fragment->height = MIN(fragment->height - yoff, tile_size); res_fragment->frame_width = fragment->frame_width; res_fragment->frame_height = fragment->frame_height; - res_fragment->stride = fragment->stride + ((fragment->width - res_fragment->width) * 4); + res_fragment->stride = fragment->stride + (fragment->width - res_fragment->width); res_fragment->pitch = fragment->pitch; res_fragment->number = number; res_fragment->cleared = fragment->cleared; diff --git a/src/til_fb.h b/src/til_fb.h index 6dd0975..55e9b6c 100644 --- a/src/til_fb.h +++ b/src/til_fb.h @@ -17,8 +17,8 @@ typedef struct til_fb_fragment_t { unsigned width, height; /* width and height of this fragment */ unsigned frame_width; /* width of the frame this fragment is part of */ unsigned frame_height; /* height of the frame this fragment is part of */ - unsigned stride; /* number of bytes from the end of one row to the start of the next */ - unsigned pitch; /* number of bytes separating y from y + 1, including any padding */ + unsigned stride; /* number of 32-bit words from the end of one row to the start of the next */ + unsigned pitch; /* number of 32-bit words separating y from y + 1, including any padding */ unsigned number; /* this fragment's number as produced by fragmenting */ unsigned cleared:1; /* if this fragment has been cleared since last flip */ } til_fb_fragment_t; @@ -73,7 +73,7 @@ static inline int til_fb_fragment_contains(til_fb_fragment_t *fragment, int x, i /* puts a pixel into the fragment, no bounds checking is performed. */ static inline void til_fb_fragment_put_pixel_unchecked(til_fb_fragment_t *fragment, int x, int y, uint32_t pixel) { - uint32_t *pixels = ((void *)fragment->buf) + (y - fragment->y) * fragment->pitch; + uint32_t *pixels = fragment->buf + (y - fragment->y) * fragment->pitch; pixels[x - fragment->x] = pixel; } @@ -94,13 +94,13 @@ static inline int til_fb_fragment_put_pixel_checked(til_fb_fragment_t *fragment, /* fill a fragment with an arbitrary pixel */ static inline void til_fb_fragment_fill(til_fb_fragment_t *fragment, uint32_t pixel) { - void *buf = fragment->buf; + uint32_t *buf = fragment->buf; /* TODO: there should be a fast-path for non-divided fragments where there's no stride to skip */ for (int y = 0; y < fragment->height; y++, buf += fragment->pitch) { /* TODO: this should use something memset-like for perf */ for (int x = 0; x < fragment->width; x++) - ((uint32_t *)buf)[x] = pixel; + buf[x] = pixel; } } -- cgit v1.2.1