Skip to content
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

Miscellaneous refactoring #12

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Miscellaneous refactoring #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 6 additions & 6 deletions src/bmpwrite.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,22 +337,22 @@ void generate_BMP_RGB_data (struct context * context, unsigned char * offset_poi
if ((context -> source -> color_format & PLUM_COLOR_MASK) == PLUM_COLOR_32)
data = context -> source -> data;
else {
data = ctxmalloc(context, sizeof *data * context -> source -> width * context -> source -> height);
plum_convert_colors(data, context -> source -> data, (size_t) context -> source -> width * context -> source -> height,
PLUM_COLOR_32, context -> source -> color_format);
size_t size = (size_t) context -> source -> width * context -> source -> height;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Factoring out a common (size_t) context -> source -> width * context -> source -> height expression, as is done almost everywhere else.

data = ctxmalloc(context, sizeof *data * size);
plum_convert_colors(data, context -> source -> data, size, PLUM_COLOR_32, context -> source -> color_format);
}
size_t rowsize = (size_t) context -> source -> width * 3, padding = 0;
if (rowsize & 3) {
padding = 4 - (rowsize & 3);
rowsize += padding;
}
unsigned char * out = append_output_node(context, rowsize * context -> source -> height);
unsigned char * output = append_output_node(context, rowsize * context -> source -> height);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

output is a much more common name for this kind of variable elsewhere in the codebase.

uint_fast32_t row = context -> source -> height - 1;
do {
size_t pos = (size_t) row * context -> source -> width;
for (uint_fast32_t remaining = context -> source -> width; remaining; pos ++, remaining --)
out += byteappend(out, data[pos] >> 16, data[pos] >> 8, data[pos]);
for (uint_fast32_t p = 0; p < padding; p ++) *(out ++) = 0;
output += byteappend(output, data[pos] >> 16, data[pos] >> 8, data[pos]);
for (uint_fast32_t p = 0; p < padding; p ++) *(output ++) = 0;
} while (row --);
if (data != context -> source -> data) ctxfree(context, data);
}
44 changes: 21 additions & 23 deletions src/color.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,26 @@
#include "proto.h"

#define formatpair(from, to) (((from) << 2) & (PLUM_COLOR_MASK << 2) | (to) & PLUM_COLOR_MASK)

void plum_convert_colors (void * restrict destination, const void * restrict source, size_t count, unsigned long to, unsigned long from) {
if (!(source && destination && count)) return;
if ((from & (PLUM_COLOR_MASK | PLUM_ALPHA_INVERT)) == (to & (PLUM_COLOR_MASK | PLUM_ALPHA_INVERT))) {
memcpy(destination, source, plum_color_buffer_size(count, to));
return;
}
#define convert(sp) do \
if ((to & PLUM_COLOR_MASK) == PLUM_COLOR_16) \
for (uint16_t * dp = destination; count; count --) *(dp ++) = plum_convert_color(*(sp ++), from, to); \
else if ((to & PLUM_COLOR_MASK) == PLUM_COLOR_64) \
for (uint64_t * dp = destination; count; count --) *(dp ++) = plum_convert_color(*(sp ++), from, to); \
else \
for (uint32_t * dp = destination; count; count --) *(dp ++) = plum_convert_color(*(sp ++), from, to); \
while (false)
if ((from & PLUM_COLOR_MASK) == PLUM_COLOR_16) {
const uint16_t * sp = source;
convert(sp);
} else if ((from & PLUM_COLOR_MASK) == PLUM_COLOR_64) {
const uint64_t * sp = source;
convert(sp);
} else {
const uint32_t * sp = source;
convert(sp);
#define convert(frombits, tobits) do { \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You already had the formatpair macro below, may as well use it here and save a couple of lines + make the to-from paired logic clearer.

const uint ## frombits ## _t * sp = source; \
uint ## tobits ## _t * dp = destination; \
while (count --) *(dp ++) = plum_convert_color(*(sp ++), from, to); \
} while (false)
switch (formatpair(from, to)) {
case formatpair(PLUM_COLOR_16, PLUM_COLOR_64): convert(16, 64); break;
case formatpair(PLUM_COLOR_64, PLUM_COLOR_16): convert(64, 16); break;
case formatpair(PLUM_COLOR_16, PLUM_COLOR_32X):
case formatpair(PLUM_COLOR_16, PLUM_COLOR_32): convert(16, 32); break;
case formatpair(PLUM_COLOR_64, PLUM_COLOR_32):
case formatpair(PLUM_COLOR_64, PLUM_COLOR_32X): convert(64, 32); break;
case formatpair(PLUM_COLOR_32, PLUM_COLOR_16):
case formatpair(PLUM_COLOR_32X, PLUM_COLOR_16): convert(32, 16); break;
case formatpair(PLUM_COLOR_32, PLUM_COLOR_64):
case formatpair(PLUM_COLOR_32X, PLUM_COLOR_64): convert(32, 64); break;
default: memcpy(destination, source, plum_color_buffer_size(count, to));
}
#undef convert
}
Expand All @@ -34,7 +32,6 @@ uint64_t plum_convert_color (uint64_t color, unsigned long from, unsigned long t
from &= 0xffffu;
else if ((from & PLUM_COLOR_MASK) != PLUM_COLOR_64)
from &= 0xffffffffu;
#define formatpair(from, to) (((from) << 2) & (PLUM_COLOR_MASK << 2) | (to) & PLUM_COLOR_MASK)
switch (formatpair(from, to)) {
case formatpair(PLUM_COLOR_32, PLUM_COLOR_32):
case formatpair(PLUM_COLOR_64, PLUM_COLOR_64):
Expand Down Expand Up @@ -82,11 +79,12 @@ uint64_t plum_convert_color (uint64_t color, unsigned long from, unsigned long t
case formatpair(PLUM_COLOR_32X, PLUM_COLOR_16):
result = ((color >> 5) & 0x1f) | ((color >> 10) & 0x3e0) | ((color >> 15) & 0x7c00) | ((color >> 16) & 0x8000u);
}
#undef formatpair
if ((to ^ from) & PLUM_ALPHA_INVERT) result ^= alpha_component_masks[to & PLUM_COLOR_MASK];
return result;
}

#undef formatpair

void plum_remove_alpha (struct plum_image * image) {
if (!(image && image -> data && plum_check_valid_image_size(image -> width, image -> height, image -> frames))) return;
void * colordata;
Expand Down
67 changes: 26 additions & 41 deletions src/framebounds.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,26 @@ struct plum_rectangle * get_frame_boundaries (struct context * context, bool anc

void adjust_frame_boundaries (const struct plum_image * image, struct plum_rectangle * restrict boundaries) {
uint64_t empty_color = get_empty_color(image);
#define checkframe(type) do \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here the palette code was so similar to the 16/32/64 color code that I wanted to use the existing checkframe macro for the palette as well.

for (uint_fast32_t frame = 0; frame < image -> frames; frame ++) { \
for (size_t remaining = (size_t) boundaries[frame].top * image -> width; remaining; remaining --) \
if (notempty(type)) goto done ## type; \
if (boundaries[frame].left || boundaries[frame].width != image -> width) \
for (uint_fast32_t row = 0; row < boundaries[frame].height; row ++) { \
for (uint_fast32_t col = 0; col < boundaries[frame].left; col ++) if (notempty(type)) goto done ## type; \
index += boundaries[frame].width; \
for (uint_fast32_t col = boundaries[frame].left + boundaries[frame].width; col < image -> width; col ++) \
if (notempty(type)) goto done ## type; \
} \
else \
index += (size_t) boundaries[frame].height * image -> width; \
for (size_t remaining = (size_t) (image -> height - boundaries[frame].top - boundaries[frame].height) * image -> width; remaining; remaining --) \
if (notempty(type)) goto done ## type; \
continue; \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This continue is an alternative to a manually-checked bool adjust flag.

This control flow:

for (...) {
  bool adjust = true;
  if (various stuff) goto done;
  adjust = false;
  done:
  if (adjust) finish up;
}

was refactored to this:

for (...) {
  if (various stuff) goto done;
  continue;
  done:
  finish up;
}

done ## type: \
boundaries[frame] = (struct plum_rectangle) {.left = 0, .top = 0, .width = image -> width, .height = image -> height}; \
} \
while (false)
if (image -> palette) {
bool empty[256];
switch (image -> color_format & PLUM_COLOR_MASK) {
Expand All @@ -34,55 +54,20 @@ void adjust_frame_boundaries (const struct plum_image * image, struct plum_recta
for (size_t p = 0; p <= image -> max_palette_index; p ++) empty[p] = image -> palette32[p] == empty_color;
}
size_t index = 0;
for (uint_fast32_t frame = 0; frame < image -> frames; frame ++) {
bool adjust = true;
for (size_t remaining = (size_t) boundaries[frame].top * image -> width; remaining; remaining --)
if (!empty[image -> data8[index ++]]) goto paldone;
if (boundaries[frame].left || boundaries[frame].width != image -> width)
for (uint_fast32_t row = 0; row < boundaries[frame].height; row ++) {
for (uint_fast32_t col = 0; col < boundaries[frame].left; col ++) if (!empty[image -> data8[index ++]]) goto paldone;
index += boundaries[frame].width;
for (uint_fast32_t col = boundaries[frame].left + boundaries[frame].width; col < image -> width; col ++)
if (!empty[image -> data8[index ++]]) goto paldone;
}
else
index += (size_t) boundaries[frame].height * image -> width;
for (size_t remaining = (size_t) (image -> height - boundaries[frame].top - boundaries[frame].height) * image -> width; remaining; remaining --)
if (!empty[image -> data8[index ++]]) goto paldone;
adjust = false;
paldone:
if (adjust) boundaries[frame] = (struct plum_rectangle) {.left = 0, .top = 0, .width = image -> width, .height = image -> height};
}
#define notempty(...) (!empty[image -> data8[index ++]])
checkframe(pal);
} else {
size_t index = 0;
#define checkframe(bits) do \
for (uint_fast32_t frame = 0; frame < image -> frames; frame ++) { \
bool adjust = true; \
for (size_t remaining = (size_t) boundaries[frame].top * image -> width; remaining; remaining --) \
if (image -> data ## bits[index ++] != empty_color) goto done ## bits; \
if (boundaries[frame].left || boundaries[frame].width != image -> width) \
for (uint_fast32_t row = 0; row < boundaries[frame].height; row ++) { \
for (uint_fast32_t col = 0; col < boundaries[frame].left; col ++) if (image -> data ## bits[index ++] != empty_color) goto done ## bits; \
index += boundaries[frame].width; \
for (uint_fast32_t col = boundaries[frame].left + boundaries[frame].width; col < image -> width; col ++) \
if (image -> data ## bits[index ++] != empty_color) goto done ## bits; \
} \
else \
index += (size_t) boundaries[frame].height * image -> width; \
for (size_t remaining = (size_t) (image -> height - boundaries[frame].top - boundaries[frame].height) * image -> width; remaining; remaining --) \
if (image -> data ## bits[index ++] != empty_color) goto done ## bits; \
adjust = false; \
done ## bits: \
if (adjust) boundaries[frame] = (struct plum_rectangle) {.left = 0, .top = 0, .width = image -> width, .height = image -> height}; \
} \
while (false)
#undef notempty
#define notempty(bits) (image -> data ## bits[index ++] != empty_color)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

rgbds has already been converted to C++; next up, libplum, with templates and anonymous closures replacing all these macros. >:3

switch (image -> color_format & PLUM_COLOR_MASK) {
case PLUM_COLOR_16: checkframe(16); break;
case PLUM_COLOR_64: checkframe(64); break;
default: checkframe(32);
}
#undef checkframe
}
#undef notempty
#undef checkframe
}

bool image_rectangles_have_transparency (const struct plum_image * image, const struct plum_rectangle * rectangles) {
Expand Down
Loading